Bug 77743 - [GTK] WebKitWebView does a lot of work during size_allocate when not mapped
Summary: [GTK] WebKitWebView does a lot of work during size_allocate when not mapped
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebKitGTK (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Martin Robinson
URL:
Keywords:
: 77760 (view as bug list)
Depends on:
Blocks:
 
Reported: 2012-02-03 08:43 PST by Martin Robinson
Modified: 2012-02-06 09:59 PST (History)
5 users (show)

See Also:


Attachments
Patch (9.00 KB, patch)
2012-02-03 09:30 PST, Martin Robinson
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Martin Robinson 2012-02-03 08:43:34 PST
The way GTK+ 3.x works now, GtkNotebook will unmap its children, but still call size_allocate on them. This means that the cost of resizing a GtkNotebook full of WebKitWebView (a web browser) is the cost of resizing every single WebView there. This is quite expensive and leads to very sluggish resizing of Epiphany.
Comment 1 Martin Robinson 2012-02-03 09:30:10 PST
Created attachment 125351 [details]
Patch
Comment 2 WebKit Review Bot 2012-02-03 09:33:56 PST
Thanks for the patch. If this patch contains new public API please make sure it follows the guidelines for new WebKit2 GTK+ API. See http://trac.webkit.org/wiki/WebKitGTK/AddingNewWebKit2API
Comment 3 Kalev Lember 2012-02-03 13:55:34 PST
*** Bug 77760 has been marked as a duplicate of this bug. ***
Comment 4 Carlos Garcia Campos 2012-02-05 12:24:25 PST
(In reply to comment #0)
> The way GTK+ 3.x works now, GtkNotebook will unmap its children, but still call size_allocate on them. This means that the cost of resizing a GtkNotebook full of WebKitWebView (a web browser) is the cost of resizing every single WebView there. This is quite expensive and leads to very sluggish resizing of Epiphany.

I'm not sure what have changed between gtk 2 and 3 here, but gtk2 also maps/unmaps the children. It's done when switching tabs, gtk_widget_set_child_visible() is called with TRUE for the new tab and FALSE for the old one. GtkNotebook checks the visibility of the children before calling size_allocate for the child, like other containers. In this case, according to gtk_notebook_set_current_page() doc, children of a notebook should be visible, and that's why size_allocate is always called for all children. I think this patch is actually a workaround for the gtk issue (fixing a specific case of a WebView inside a GtkNotebook), and it should be fixed in GTK+, otherwise all gtk widgets should do the same just in case they are added to a GtkNotebook. I've filed a bug report to GTK+ and attached a patch there:

https://bugzilla.gnome.org/show_bug.cgi?id=669394
Comment 5 Martin Robinson 2012-02-05 16:54:27 PST
(In reply to comment #4)

> https://bugzilla.gnome.org/show_bug.cgi?id=669394

Thanks for the followup! Hopefully this bug is fixed upstream as well. I think we should still use the work-around, for old versions of GTK+ and also because it fixes issues on Windows.
Comment 6 Carlos Garcia Campos 2012-02-05 23:35:01 PST
What issues? If we land this, I would include a comment saying that it shouldn't be necessary, and it's a workaround for several issues in GTK+, so that if it's not needed in the future we can just remove it. The problem of this kind of workarounds is that they end up in the code forever and nobody remembers what the code is for and why it was added.
Comment 7 Martin Robinson 2012-02-06 00:14:33 PST
(In reply to comment #6)
> What issues? If we land this, I would include a comment saying that it shouldn't be necessary, and it's a workaround for several issues in GTK+, so that if it's not needed in the future we can just remove it. The problem of this kind of workarounds is that they end up in the code forever and nobody remembers what the code is for and why it was added.

This patch also fixes an issue on Windows (see the duplicated bug) where the backing store for the widget is created before it's mapped. Thus, the change cannot be reverted after the GTK+ bug is fixed, unless the backing store stuff changes.
Comment 8 Gustavo Noronha (kov) 2012-02-06 03:13:30 PST
(In reply to comment #6)
> What issues? If we land this, I would include a comment saying that it shouldn't be necessary, and it's a workaround for several issues in GTK+, so that if it's not needed in the future we can just remove it. The problem of this kind of workarounds is that they end up in the code forever and nobody remembers what the code is for and why it was added.

I would say our track record of know about the workarounds and removing them is pretty good, actually, but having a comment has always helped us =)
Comment 9 Gustavo Noronha (kov) 2012-02-06 03:18:58 PST
Comment on attachment 125351 [details]
Patch

Looks sensible to me.
Comment 10 WebKit Review Bot 2012-02-06 09:59:43 PST
Comment on attachment 125351 [details]
Patch

Clearing flags on attachment: 125351

Committed r106816: <http://trac.webkit.org/changeset/106816>
Comment 11 WebKit Review Bot 2012-02-06 09:59:49 PST
All reviewed patches have been landed.  Closing bug.