The criticals are unrelated bugs that I've been meaning to fix for a while. This part looks bad:

> (epiphany-gmail-eabdf94e13996f57a124d22f8d5b1fda50156307:15521): Gdk-WARNING
> **: Error 71 (Protocol error) dispatching to Wayland display.
> 
> This version of epiphany was built with WebKit 2.13.91. I rebuilt against
> WebKit 2.13.92 but it didn't fix the bug.
> 
> Notably, epiphany itself runs fine; it's just app mode that isn't working.

No idea why app mode would affect this. I'll see if I can reproduce over the weekend.

One tricky issue is that the xdg-shell extension version was recently bumped, so I had to build GTK+ 3.20 in jhbuild to test this to be usable with my system mutter, else the X11 backend gets used. It works for me either way, though. But if the issue only occurs with newer GTK+, then I wouldn't notice....

CCing Carlos and Carlos in case they have any ideas what might be up here.

I can reproduce, WAYLAND_DEBUG reveals:

[1472413.950]  -> zxdg_toplevel_v6@30.set_min_size(280, -5)
...
[1472701.594] wl_display@1.error(zxdg_toplevel_v6@30, 4, "invalid negative min size requested 280 x -5")

given ephy doesn't seem to set any explicit geometry hints, I'd say this is gtk+, and related to the xdg_shell update. Moving there.

CCing Jonas. IMHO we should be just warning and clamping min/max sizes to 0 if we get dumb values, there's indeed something else going on if we end up getting negative sizes from gtk+ code paths, but this seems like could be easily triggered from application code, an async protocol error seems harsh to me in this case.

FWIW, Some gdb debugging:

(gdb) bt

+ Trace 236675

$4 = {min_width = 405, min_height = 47, max_width = 0, max_height = 0, base_width = 0, base_height = 0, width_inc = 0, height_inc = 0, min_aspect = 0, max_aspect = 0, 
  win_gravity = GDK_GRAVITY_NORTH_WEST}
(gdb) p (GdkWindowImplWayland)*window->impl
$5 = {parent_instance = {parent = {g_type_instance = {g_class = 0x6fd5d0}, ref_count = 30, qdata = 0x0}}, wrapper = 0x6fe330, display_server = {outputs = 0x0, wl_surface = 0xc17710, 
    xdg_surface = 0x11b9d70, xdg_toplevel = 0x11b9dc0, xdg_popup = 0x0, gtk_surface = 0x11b8430, wl_subsurface = 0x0, egl_window = 0x0, dummy_egl_window = 0x0, xdg_exported = 0x0}, 
  egl_surface = 0x0, dummy_egl_surface = 0x0, initial_configure_received = 0, mapped = 1, use_custom_surface = 0, pending_buffer_attached = 0, pending_commit = 0, awaiting_frame = 0, 
  hint = GDK_WINDOW_TYPE_HINT_NORMAL, transient_for = 0x0, popup_parent = 0x0, position_method = POSITION_METHOD_NONE, staging_cairo_surface = 0x0, committed_cairo_surface = 0x0, 
  backfill_cairo_surface = 0x0, pending_buffer_offset_x = 0, pending_buffer_offset_y = 0, title = 0x11c6da0 "Blank page", application = {was_set = 1, 
    application_id = 0x11f6400 "org.gnome.Epiphany", app_menu_path = 0x11f6710 "/org/gnome/Epiphany/menus/appmenu", menubar_path = 0x0, 
    window_object_path = 0x11f6740 "/org/gnome/Epiphany/window/1", application_object_path = 0x11f6770 "/org/gnome/Epiphany", unique_bus_name = 0x11f6790 ":1.113"}, geometry_hints = {
    min_width = 457, min_height = 99, max_width = 0, max_height = 0, base_width = 0, base_height = 0, width_inc = 0, height_inc = 0, min_aspect = 0, max_aspect = 0, 
    win_gravity = GDK_GRAVITY_NORTH_WEST}, geometry_mask = (GDK_HINT_MIN_SIZE | GDK_HINT_BASE_SIZE | GDK_HINT_WIN_GRAVITY), grab_input_seat = 0x0, pending_frame_counter = 0, scale = 1, 
  margin_left = 26, margin_right = 26, margin_top = 23, margin_bottom = 29, margin_dirty = 0, initial_fullscreen_monitor = -1, opaque_region = 0x1192550, opaque_region_dirty = 1, 
  input_region = 0x0, input_region_dirty = 1, staged_updates_region = 0x0, saved_width = 600, saved_height = 547, parent_surface_committed_handler = 0, pending_move_to_rect = {rect = {x = 0, 
      y = 0, width = 0, height = 0}, rect_anchor = 0, window_anchor = 0, anchor_hints = 0, rect_anchor_dx = 0, rect_anchor_dy = 0}, pending = {width = 0, height = 0, state = (unknown: 0)}, 
  exported = {callback = 0x0, user_data = 0x0, destroy_func = 0x0}, imported_transient_for = 0x0}

So min_height is 47, and impl->margin_top+bottom are 23+29=52, this results in the -5 size.

Yea, looks like an incorrect min size, and yes, makes sense to g_warn and clamp to 0.

Created attachment 335777 [details] [review]
wayland: Correct min/max size calculation

The geometry pased to _set_geometry_hints doesn't take shadow margins
into account so don't decrease the width/height with the margins.

Fixes applications getting a protocol error from the wayland compositor
if their minimal width/height are less then the respective margins.

Signed-off-by: Sjoerd Simons <sjoerd@luon.net>

Review of attachment 335777 [details] [review]:

(In reply to Sjoerd Simons from comment #7)
> The geometry pased to _set_geometry_hints doesn't take shadow margins
> into account so don't decrease the width/height with the margins.

Actually, it does when using CSD (as in Wayland), so this patch is not correct, the correct approach imho is to clamp as suggested by Carlos.

CSD != shadows.

If your claim is correct (which i don't think it is, but i could be wrong) other code in gdkwindow-wayland is obviously wrong as gdk_wayland_window_get_window_geometry already removed the margins for the geometry, so calling _set_geometry_hints with that geometry will decrease with the margins twice.. 

clamping make sense to prevent protocol errors, but from what i can tell the problem here isn't the application given dumb values...

Created attachment 335846 [details]
Sample program

(In reply to Sjoerd Simons from comment #9)
> CSD != shadows.

Not sure what you mean by that sentence...

With CSD, the shadows are drawn by the client, i.e. they are within the toplevel GdkWindow size whereas with SSD the shadows are drawn by the window manager, ie outside of the GdkWindow.

That's precisely why you need _GTK_FRAME_EXTENTS for CSD on X11.
 
> If your claim is correct (which i don't think it is, but i could be wrong)
> other code in gdkwindow-wayland is obviously wrong as
> gdk_wayland_window_get_window_geometry already removed the margins for the
> geometry, so calling _set_geometry_hints with that geometry will decrease
> with the margins twice.. 
> 
> clamping make sense to prevent protocol errors, but from what i can tell the
> problem here isn't the application given dumb values...

To demonstrate why we need to remove the shadow sizes, please consider the attached sample program.

What this code does is trivially setting the min height of its window to 1 pixel usign the GtkWindow API (gtk_window_set_geometry_hints()).

Now, this needs to work in all 3 following cases:

1. X11 without CSD (i.e. with SSD)
2. X11 with CSD
3. Wayland

Which means that the geometry value passed to X11 must match what the program sets as-is, whereas the values given to CSD must add up the header bar and the shadow values.

This program works as expected in all 3 cases, ie the window *content* (ie not counting the shadows and hederbar) minimum is 1 in X11 with SSD, X11 with CSD and Wayland with CSD.

Applying your patch results in the min height being too large in Wayland as seen in WAYLAND_DEBUG=1 logs:

Without your patch:

[3747632.054]  -> zxdg_toplevel_v6@28.set_min_size(248, 37)
[3747632.062]  -> zxdg_toplevel_v6@28.set_max_size(248, 248)
[3747632.070]  -> zxdg_surface_v6@27.set_window_geometry(26, 23, 248, 239)

With your patch:

[3782500.453]  -> zxdg_toplevel_v6@28.set_min_size(300, 89)
[3782500.462]  -> zxdg_toplevel_v6@28.set_max_size(300, 300)
[3782500.472]  -> zxdg_surface_v6@27.set_window_geometry(26, 23, 248, 248)

And resizing to the min gives:

[4270822.924] zxdg_toplevel_v6@28.configure(248, 37, array)

And 37 is the height of the gtkheaderbar, matching the min height (without your patch) whereas your patch gives a min height of 89.

Created attachment 335854 [details]
Simple reproducer

Ok, trying to root cause this issue, I think I know what's going on...

Long story short, the toplevel epiphany window is switching to maximized state while realizing the window meaning that the shadows are 0, but the backend is not yet aware. That is definitely not a bug in epiphany, nor in gdk Wayland backend, but rather in gtk itself (ie higher in the stack)

Basically, what debugging in gdb shows is, initially, the min size set is 99 (in my case, the actual value doesn't matter, this may vary depending on the theme/and or DPI and/or font size, etc.)

Then, a few iterations later, it is set to 47, which leads to the min size being negative once you remove the shadow margins in the backend.

The initial value of 99 = 47 + 23 + 29, i.e. in gtk_window_get_preferred_height() minimum_size = title_min + window_border.top + window_border.bottom;

The last value of 47 is, well, 47 + 0 + 0 because gtk_container_get_border_width() returned (0,0,0,0), because the window has transitioned to the maximized state in between, but the Wayland backend still applies the "old" shadow margins...

Ths attached simple reproducer (similar program as before, just added a "gtk_window_maximize (GTK_WINDOW(window))" prior to showing the window shows the behavior.

So actually, I think we should not even clamp (although it would be safe), we should tahter fix the upper layers to keep the shadows margins in sync, if doable...

(In reply to Olivier Fourdan from comment #11)
> gtk_container_get_border_width() returned (0,0,0,0), because the window has
> [...]

Sorry, I meant get_shadow_width() here, not gtk_container_get_border_width() obviously.

Created attachment 335859 [details] [review]
[PATCH] gtkwindow: Update shadow size on state change

Otherwise, with CSD, we could have a discrepancy where gtk uses the right values for the shadows whereas the gdk backend still uses the old values, leading in some cases to invalid or negative min size being computed (which, in Wayland, leads a protocol error).

This fixes the issue at the upper, gtk level and avoids the invalid min size being set in Wayland (and consequently the protocol error).

You may still want to clamp() the values in gdk_wayland_window_set_geometry_hints() but that should not be needed.

Review of attachment 335859 [details] [review]:

ok

Comment on attachment 335859 [details] [review]
[PATCH] gtkwindow: Update shadow size on state change

attachment 335859 [details] [review] pished to git mater as commit 4cb1b96 - gtkwindow: Update shadow size on state change