Created attachment 297818 [details] [review]
wayland-surface: Keep a reference to a popup's parent surface

This will allows us to access the parent while constructing the
MetaWindow.

Created attachment 297819 [details] [review]
window-wayland: Set transient and window type on manage() for popups

Doing this on manage() allows the common MetaWindow initialization to
do the right thing for popups like setting skip_taskbar and
skip_pager.

In particular this avoids gnome-shell's app tracker to create a new
ShellApp instance for every popup.

Review of attachment 297818 [details] [review]:

::: src/wayland/meta-wayland-surface.c
@@ +1056,3 @@
+  surface->popup.parent = NULL;
+
+  destroy_window (surface);

Don't really need to destroy the window here, as we just killed the client with the above wl_resource_post_error (which is a good catch on its own btw, we didn't detect when the top level surface was destroyed with active popups before) so everything will be cleaned up anyway.

Setting the transient / type *should* work at any time, so this shouldn't change behavior, just clean it up slightly.

(In reply to Jasper St. Pierre from comment #4)
> Setting the transient / type *should* work at any time, so this shouldn't
> change behavior, just clean it up slightly.

Yes, I never said it didn't work. This is an optimization, if you will, which avoids doing extra work in gnome-shell's app tracker (and possibly elsewhere?)

OK, I just wanted to make sure it didn't fix the bug you mentioned in the description. Both are fine in that case.

Ok, pushing with the destroy_window() since the
handle_popup_destroyed() below also calls it while arguably not
needing to either. I can't see how it could hurt anyway so let's keep
it consistent.

Attachment 297818 [details] pushed as 59f348e - wayland-surface: Keep a reference to a popup's parent surface
Attachment 297819 [details] pushed as 438410c - window-wayland: Set transient and window type on manage() for popups

(In reply to Rui Matos from comment #7)
> Ok, pushing with the destroy_window() since the
> handle_popup_destroyed() below also calls it while arguably not
> needing to either. I can't see how it could hurt anyway so let's keep
> it consistent.

In handle_popup_destroyed() it is actually needed because the compositor may dismiss a whole popup chain, and should not need to wait for wl_surface.destroy or xdg_popup.destroy to destroy the window.

> 
> Attachment 297818 [details] pushed as 59f348e - wayland-surface: Keep a
> reference to a popup's parent surface
> Attachment 297819 [details] pushed as 438410c - window-wayland: Set
> transient and window type on manage() for popups

*** Bug 738366 has been marked as a duplicate of this bug. ***