Build Identifier: http://hg.mozilla.org/mozilla-central/rev/48c4938eaf57 Mozilla/5.0 (X11; Linux i686; rv:18.0) Gecko/18 Firefox/18.0a1 ID:20120921030601 Ubuntu 12.04.1 At the time of a start of the browser window, a desktop is seen in the top left corner temporarily This problem happens regardless with HWA on/off I can reproduce only in Nightly18.0a1. Regression window Good: http://hg.mozilla.org/mozilla-central/rev/f89feda9d997 Mozilla/5.0 (X11; Linux i686; rv:17.0) Gecko/17 Firefox/17.0a1 ID:20120813030532 Bad: http://hg.mozilla.org/mozilla-central/rev/22288130fea2 Mozilla/5.0 (X11; Linux i686; rv:17.0) Gecko/17 Firefox/17.0a1 ID:20120814030521 Pushlog: http://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=f89feda9d997&tochange=22288130fea2 In local build, Last Good: ed614ea130c0 First Bad: 13d19788ad1d Graphics Adapter Description : VMware, Inc. -- Gallium 0.4 on SVGA3D; build: RELEASE; Device ID : Gallium 0.4 on SVGA3D; build: RELEASE; Driver Version : 2.1 Mesa 8.0.2 GPU Accelerated Windows : 0/1 Basic no information Vendor ID : VMware, Inc. WebGL Renderer : VMware, Inc. -- Gallium 0.4 on SVGA3D; build: RELEASE; -- 2.1 Mesa 8.0.2 AzureCanvasBackend : cairo AzureContentBackend : none AzureFallbackCanvasBackend : none

Much more obvious without a compositing window manager. viewmanager.refresh-driver-painting.enabled false restores the old behavior.

I don't know if this is useful information, but I don't see this problem when omtc is enabled. (For whatever reason.)

This seems to be some interaction with GDK applying shapes to child windows. Before GTK maps the toplevel window, it issues a ConfigureRequest to resize the window. Probably because this request may be redirected to and modified by the window manager, though I'm not sure it would be for an unmapped window, GDK does not record the new size on the window until it receives the ConfigureNotify reply. Immediately after the toplevel is mapped, GDK sets a shape on the child window. This is based on parent window sizes - probably some client-side window code that doesn't really make much sense for the server-side toplevel window size. GDK still considers the toplevel to have the size it had before the ConfigureRequest, which happens to be the default GtkWindow size of 200x200. The child window has a back pixmap of None, meaning it shows nothing until it is drawn, and so the background remains. The toplevel window has a back pixmap or background color based on the theme and so looks inconsistent. Some options I'm thinking about: 1) Change the default size of the window. This needs to be done before realizing the window, but the desired size is not provided at that point. A small size could be selected to limit the effect to 1 pixel. 2) Try sizing the window as soon as possible instead of waiting until Show. The event loop would need to run to process the ConfigureNotify, so this may not be helpful. 3) Look into setting the back pixmap of the toplevel window to None. 4) Don't set the back pixmap of child windows to None. 5) Find out why dlbi changes made this show up.

(In reply to Karl Tomlinson (:karlt) from comment #3) > Immediately after the toplevel is mapped, GDK sets a shape on the child > window. Immediately before, actually.

The reason this didn't happen before refresh-driver-painting was that GDK didn't create a native X Window for the child window until we requested it, and that didn't happen until the expose event, at which point I assume GDK knew the size of the toplevel window. What is different with refresh driver painting is that it calls GetThebesSurface() off the refresh driver to select a buffer format for the BasicThebesLayer buffer. This can result in the X Window for the child window being created before GDK receives the ConfigureNotify event. Perhaps we could come up with a different reference surface (the toplevel window surface would be fine in our controlled situation) but the problem would be harder to solve with GL layers where the GLContext is created for the native X window into which we'll draw. I don't think we can Show/Map the window, and then paint immediately because the Map request is redirected to the window manager and may not have been effected before we paint. And the child window will still have the incorrect shape, masking out drawing until the ConfigureNotify is received.

It looks like _NET_WM_SYNC_REQUEST with background None would be a way to postpone client and window manager drawing until the ConfigureNotify has been received. http://standards.freedesktop.org/wm-spec/1.5/ar01s06.html#id2761266 However, the support doesn't seem to be there in GTK or in window managers. It seems it is used mostly for window-manager-initiated resize drags, possibly because most apps don't have background None. gtk_window_show() does not increment configure_request_count when it resizes the window, and so gtk_window_configure_event() bails out early, replying to the _NET_WM_SYNC_REQUEST before the drawing has taken place. kwin 4.8.5 does not wait (any significant time at least) for the _NET_WM_SYNC_REQUEST reply before drawing the frame. It only sends the message for the resize before show when compositing is enabled. metacity 2.30.3 and 2.34.8 do not send a _NET_WM_SYNC_REQUEST for the resize before show, with or without compositing enabled. They do send these when dragging the frame border to resize. I haven't tested compiz or mutter which I would expect to use _NET_WM_SYNC_REQUEST at least for resize drags. Bug 378293 seems to suggest compiz is using _NET_WM_SYNC_REQUEST as an indicator of when it expects to have all Damage events for a window show.

Given the window manager is drawing the frame before we respond to the ConfigureNotify event, we might as well give in and let the background be drawn on Show. This would be quite the wrong thing for translucent windows, as the shape would be wrong until we paint. However, we only make override-redirect windows transparent and GDK handles override-redirect windows differently, resizing synchronously and temporarily setting the background to None during show to suppress display of the background. This patch is perhaps a simple quick fix, but we can do a better fix. Using ParentRelative makes child Windows look just like their parent Window's background until they are drawn. This seems to look a little better than setting background None on the toplevel, which leads to the window manager frame painting while there still seems to be a big gap in the middle. ParentRelative has the additional requirement that the parent window needs to have matching depth. Usually our child windows inherit the colormap/visual from the parent and so that is satisfied. That inheritance doesn't guarantee the same depth when windows are reparented. The only toplevel windows that we give different depths are popup windows, so that would only be a problem if we had windowed plugins in a popup. However, I'm not sure that styles/themes can't set different visuals on different widgets, which would be a problem to solve when reparenting to the GtkInvisible widget.

I started looking into drawing directly to the toplevel X Window and never doing anything to trigger creation of X Windows for child windows, but that seems to be relying too much on the implementation of GdkWindows. The best strategy would be to redesign MozContainer to be a GTK_NO_WINDOW widget and stop creating child nsWindows for each toplevel window. I'll upload patches for that.

This was interfering with focus change detection in SetFocus() when the nsWindow of the container receives focus, and so mIMModule->OnFocusWindow() was not getting called. (Currently the child window is the one receiving focus in SetFocus().)

When the widget calls WindowResized() on either of the listeners, nsDocShell::SetPositionAndSize() and DocumentViewerImpl::SetBounds() get called. This is a notification that the window has changed. When the viewer is not mAttachedToParent, it makes sense to resize the child window in response to toplevel change. Resizing the toplevel again doesn't make sense in this situation, and would lead to infinite recursion (unless broken in some way). When !mAttachedToParent the Resize() here would not resize the toplevel, so some other mechanism should be used if the intention is to resize the toplevel. https://tbpl.mozilla.org/?tree=Try&rev=5ab9d45888b3 https://tbpl.mozilla.org/?tree=Try&rev=6691a8e5486b

The following patch will change widget signal handlers somewhat. Removing the unused GtkWidget parameters makes it clear where the particular widget provided by the signal is not important.

Comment on attachment 673083 [details] [diff] [review] don't resize toplevel window in response to resized notification r=me if this is all tested in the cases when mAttachedToParent is true (Windows?)

Gtk has the concept of GTK_NO_WINDOW widgets. For these gtk_widget_get_window() returns the parent (or ancestor) widget's window. These widgets should not modify the parent widget's window, expect to draw. gdk_window_get_user_data(gdk_widget_get_window(widget), &data) will not return widget for these windows.

When logging was enabled, the gdk_x11_window_get_xid here for the child window was slightly concealing what has changed with dlbi because it caused GDK to generate a native X Window for the GdkWindow earlier. The child window XIDs are available from xwininfo anyway, so I've just included the parent window for child window logs to help tie things together. This also now output the single mGdkWindow as it is shared by both widgets.

This addresses a noticable difference in painting the new area of a window being resized by the window manager (through dragging the frame). With child windows, the resize notification here of parent window change caused a client request to resize the child window (through WindowResized). GDK invalidated the new part of the child window while doing this. Immediately after dispatching this size-allocate signal, GTK does a synchronous paint of invalid regions. Without a child window, the client-side invalidate doesn't happen and the X Expose event hasn't been received yet and so the repaint lags behind, leaving the background quite visible. The ForcedRepaint() code in the view system doesn't work as well as we might like. It invalidates the whole window, but with basic layers only the region in the native expose event is repainted. The whole invalidated rect is repainted later, but by that time it seems the window manager has already started compositing the new portion that was painted and includes the background of the new part of the window as well. I guess we could change this process to repaint freshly invalidated regions as well as the region of the expose signal, but I'll save that for a later date. This patch is essentially keeping the behavior that we have on child windows. I guess we could invalidate the whole window here, but we don't seem to need to. The key is to prevent the background flickering by painting the new part as fast as possible. It doesn't seem to matter that the painting of resized existing content lags a little. While modifying this code, I'm skipping the WindowResized if there is no change. If mBounds was changed in the Resize() methods, then WindowResized would have been called there.

One X window, without GDK black magic.

Comment on attachment 673088 [details] [diff] [review] don't use a separate GdkWindow for MozContainer (unless necessary) Review of attachment 673088 [details] [diff] [review]: ----------------------------------------------------------------- ::: widget/gtk2/mozcontainer.c @@ +196,5 @@ > tmp_list = tmp_list->next; > } > > + if (gtk_widget_get_has_window (widget)) > + gdk_window_show (gtk_widget_get_window(widget)); {} @@ +207,5 @@ > > gtk_widget_set_mapped(widget, FALSE); > > + if (gtk_widget_get_has_window (widget)) > + gdk_window_hide (gtk_widget_get_window(widget)); {} ::: widget/gtk2/nsWindow.cpp @@ +3718,5 @@ > g_signal_connect(mContainer, "drag_data_received", > G_CALLBACK(drag_data_received_event_cb), NULL); > > + GtkWidget *widgets[] = { GTK_WIDGET(mContainer), mShell, nullptr }; > + for (int i = 0; widgets[i]; ++i) { Slightly better to just use for (int i = 0; i < ArrayLength(widgets); ++i)

Comment on attachment 673117 [details] [diff] [review] attach DocumentViewer to top GTK widget Review of attachment 673117 [details] [diff] [review]: ----------------------------------------------------------------- Cool! On Windows this caused all kinds of breakage with third-party apps that try to monkey with Firefox's HWNDs. Hopefully we won't have that sort of trouble on X. ::: widget/gtk2/nsWindow.cpp @@ +458,5 @@ > + nsIWidgetListener *listeners[] = > + { mWidgetListener, mAttachedWidgetListener }; > + for (size_t i = 0; i < ArrayLength(listeners); ++i) { > + if (listeners[i]) > + listeners[i]->WindowResized(this, aWidth, aHeight); {} @@ +474,5 @@ > aStatus = nsEventStatus_eIgnore; > + nsIWidgetListener* listener = > + mAttachedWidgetListener ? mAttachedWidgetListener : mWidgetListener; > + if (listener) > + aStatus = listener->HandleEvent(aEvent, mUseAttachedEvents); {}

Thanks for the reviews. (In reply to Robert O'Callahan (:roc) (Mozilla Corporation) from comment #17) > > + GtkWidget *widgets[] = { GTK_WIDGET(mContainer), mShell, nullptr }; > > + for (int i = 0; widgets[i]; ++i) { > > Slightly better to just use for (int i = 0; i < ArrayLength(widgets); ++i) mShell may be null, so we'd also need if ( widgets[i] ) { } but maybe that's not so bad because it would be a little clearer.

Indeed!

https://tbpl.mozilla.org/?tree=Try&rev=babd1f5a47c7 https://hg.mozilla.org/integration/mozilla-inbound/rev/45b8a00043d5 https://hg.mozilla.org/integration/mozilla-inbound/rev/31df6929909f https://hg.mozilla.org/integration/mozilla-inbound/rev/ded8c7f9a0d2 https://hg.mozilla.org/integration/mozilla-inbound/rev/9541dbf6e020 https://hg.mozilla.org/integration/mozilla-inbound/rev/1c2eaa7e3edd https://hg.mozilla.org/integration/mozilla-inbound/rev/8da9c24417d0 https://hg.mozilla.org/integration/mozilla-inbound/rev/a0158d370785 These changes are probably too significant to consider for Aurora, when weighed against the significance of the glitch.

https://hg.mozilla.org/mozilla-central/rev/45b8a00043d5 https://hg.mozilla.org/mozilla-central/rev/31df6929909f https://hg.mozilla.org/mozilla-central/rev/ded8c7f9a0d2 https://hg.mozilla.org/mozilla-central/rev/9541dbf6e020 https://hg.mozilla.org/mozilla-central/rev/1c2eaa7e3edd https://hg.mozilla.org/mozilla-central/rev/8da9c24417d0 https://hg.mozilla.org/mozilla-central/rev/a0158d370785