Closed Bug 1013552 Opened 8 years ago Closed 8 years ago

[GTK3] Electrolysis - nsWindow::StartRemoteDrawing() needs a cairo surface


(Core :: Widget: Gtk, defect)

31 Branch
Not set





(Reporter: kxra, Assigned: stransky)




(1 file, 1 obsolete file)

User Agent: Mozilla/5.0 (X11; Linux x86_64; rv:31.0) Gecko/20100101 Firefox/31.0 (Beta/Release)
Build ID: 20140428123133

Steps to reproduce:

In Firefox-GTK3 versions 28 or 29, go to about:config and set the following to 'true'


Restarting the browser will show a completely empty window (even tab bar isn't shown)

In Firefox-GTK3 version 30 and 31, I set the following to true as well for the same result: 


IF I do not set autostart to true, and instead only leave 

the original 'browser.tabs.remote' set to 'true', and then go to File -> New e10s window, then I get an error indicating that I must enable the following: 


After doing so, I still get an empty window, although the first time I was able to at least see the tab bar (and not one time since then). 

Actual results:

I got an empty window (obviously unusable)

Expected results:

I should've gotten a normally-functioning, although perhaps less stable window.
Blocks: gtk3
I should mention that this has been the case for the aferomentioned versions of Firefox-GTK3 on Fedora 20 and 21 (Rawhide), both 64bit
Does enabling |layers.offmainthreadcomposition.enabled| without either browser.tabs.remove pref still cause a blank window?
(In reply to John Schoenick [:johns] from comment #2)
> Does enabling |layers.offmainthreadcomposition.enabled| without either
> browser.tabs.remove pref still cause a blank window?

I am no longer running one of the older versions of Firefox-GTK3 in which browser.tabs.remote affects the main browser window, so I believe the short answer is no. 

In Firefox-GTK3 30 & 31, I either have to set browser.tabs.remote.autostart to 'true' and restart the browser OR specifically go to File -> New e10s window to get a blank window.
I can reproduce it on latest trunk & gtk3 build. Looks like a gfx issue - a webpage is loaded but not rendered.
It's because nsWindow::StartRemoteDrawing() does not provide a cairo surface for drawing - the cairo surface is provided by expose event in gtk3. So we may need to draw to some buffer and update actual window surface on the expose event. (It also affects Bug 991272).
Ever confirmed: true
Component: Untriaged → Widget: Gtk
Product: Firefox → Core
Summary: Electrolysis (separate processes per tab) breaks Firefox-GTK3 (empty window) → [GTK3] Electrolysis - nsWindow::StartRemoteDrawing() needs a cairo surface
Attached patch patch (obsolete) — Splinter Review
What about this one? It creates a xlib surface when cr is not passed to nsWindow::GetThebesSurface().
Attachment #8427791 - Flags: feedback?(karlt)
Blocks: e10s
Duplicate of this bug: 991272
Comment on attachment 8427791 [details] [diff] [review]

> gfxASurface*
> #if (MOZ_WIDGET_GTK == 2)
> nsWindow::GetThebesSurface()
> #else
>+    return GetThebesSurface(nullptr);
> nsWindow::GetThebesSurface(cairo_t *cr)
> #endif
> {

The two nsWindow::GetThebesSurface() lines can be merged and so there would just be a "#if (MOZ_WIDGET_GTK > 2)" preprocessor conditional block.

>@@ -6092,18 +6084,31 @@ nsWindow::GetThebesSurface(cairo_t *cr)
>     mThebesSurface = new gfxXlibSurface
>         (GDK_WINDOW_XDISPLAY(mGdkWindow),
>          gdk_x11_window_get_xid(mGdkWindow),
>          visual,
>          size);
> #else
> #error "cairo-gtk3 target must be built with --enable-system-cairo"
>-    mThebesSurface = gfxASurface::Wrap(surf);
>+    if (cr) {
>+        cairo_surface_t *surf = cairo_get_target(cr);
>+        if (cairo_surface_status(surf) != CAIRO_STATUS_SUCCESS) {
>+          NS_NOTREACHED("Missing cairo target?");
>+          return nullptr;
>+        }
>+        mThebesSurface = gfxASurface::Wrap(surf);
>+    } else {
>+        mThebesSurface = new gfxXlibSurface
>+            (GDK_WINDOW_XDISPLAY(mGdkWindow),
>+             gdk_x11_window_get_xid(mGdkWindow),
>+             visual,
>+             size);    
>+    }

Here also, the mThebesSurface = new gfxXlibSurface code is equivalent for both
GTK versions.  The (MOZ_WIDGET_GTK == 2) block can be removed by changing its
#else to "#if (MOZ_WIDGET_GTK > 2)" and ending that block after the "} else"
after the "if (cr)" block.

>-#if (MOZ_WIDGET_GTK == 2)
>     gfxASurface       *GetThebesSurface();
>+#if (MOZ_WIDGET_GTK == 2)
>     static already_AddRefed<gfxASurface> GetSurfaceForGdkDrawable(GdkDrawable* aDrawable,
>                                                                   const nsIntSize& aSize);
> #else
>     gfxASurface       *GetThebesSurface(cairo_t *cr);

If you are able to make the first declaration

    virtual gfxASurface *GetThebesSurface() MOZ_OVERRIDE;

it would make it clearer that it is the public method.
Actually, GetThebesSurface(cairo_t *cr) should move to the private
declarations too.
Attachment #8427791 - Flags: review+
Attachment #8427791 - Flags: feedback?(karlt)
Attachment #8427791 - Flags: feedback?
Attachment #8427791 - Flags: feedback?
No longer blocks: e10s-m1
No longer blocks: old-e10s-m2
Attached patch patch, v.2Splinter Review
An updated one, Thanks!
Assignee: nobody → stransky
Attachment #8432421 - Flags: review?(karlt)
Comment on attachment 8432421 [details] [diff] [review]
patch, v.2

Thank you!
Attachment #8432421 - Flags: review?(karlt) → review+
Duplicate of this bug: 1015211
Attachment #8427791 - Attachment is obsolete: true
Hey Martin, do you have a link to a recent Try run for this? We're requiring that for all checkin-needed bugs to avoid bustage pileups. Feel free to ping me for assistance if you don't have access to Try. Otherwise, the link below has some suggestions for what to run.
Keywords: checkin-needed
Duplicate of this bug: 1022135
Closed: 8 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla32
Blocks: 1034064
No longer blocks: 1034064
You need to log in before you can comment on or make changes to this bug.