Bug 140494 - REGRESSION(r177075): [GTK] Creating a second web view disables accelerated compositing in existing web view
Summary: REGRESSION(r177075): [GTK] Creating a second web view disables accelerated co...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebKitGTK (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Nobody
URL:
Keywords: Gtk, Regression
Depends on:
Blocks:
 
Reported: 2015-01-15 01:27 PST by Carlos Garcia Campos
Modified: 2015-01-15 05:07 PST (History)
7 users (show)

See Also:


Attachments
Patch (7.08 KB, patch)
2015-01-15 01:33 PST, Carlos Garcia Campos
gns: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Carlos Garcia Campos 2015-01-15 01:27:41 PST
This is very easy to reproduce with epiphany. 

1.- Open ephy and load a page with an AC animation (like poster circle)
2.- Create a new tab
3.- Go back to previous tab

The animation no longer works because AC has been disabled.

The problem is that r177075 moved the creation of the redirected XComposite window to realize method, but leaving the call to webkitWebViewBaseUpdatePreferences() in both places Realize() and CreateWebPage(). webkitWebViewBaseUpdatePreferences() only updates the accelerating compositing setting nowadays, depending on whether the redirected XComposited window could be created or not (something that depends on whether XRender, XComposite and XDamage extensions are available in the current display). So, when the first web view is created, webkitWebViewBaseUpdatePreferences() is called first from CreateWebPage(), and always disabling accelerated compositing because the redirected window hasn't been created yet, and then from Realize() right after the redirected window is created so that accelerated compositing is enabled. When the second web view is created the same happens, but since settings are shared among web views, the first call to webkitWebViewBaseUpdatePreferences() from CreateWebPage() disables accelerated compositing and the web page exists accelerated compositing mode and never enters it again unless the page is reloaded. I guess the web page should enter accelerated compositing mode again when the setting is enabled again from Realize(), but since the setting is global and doesn't depend on every web view, we should never disable it once it has been enabled in any case.
Comment 1 Carlos Garcia Campos 2015-01-15 01:33:56 PST
Created attachment 244683 [details]
Patch

We can save the IPC messages to change the setting, and simplify the code, by always enabling AC mode when the native surface handle is set in the web process.
Comment 2 WebKit Commit Bot 2015-01-15 01:35:32 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 Gustavo Noronha (kov) 2015-01-15 02:35:54 PST
Comment on attachment 244683 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=244683&action=review

> Source/WebKit2/WebProcess/WebPage/DrawingAreaImpl.cpp:691
> +    m_webPage.corePage()->settings().setAcceleratedCompositingEnabled(true);

Wouldn't it be better to enable this unconditionally in WebKitSettings's constructed? This looks a bit hackish.
Comment 4 Carlos Garcia Campos 2015-01-15 02:46:23 PST
(In reply to comment #3)
> Comment on attachment 244683 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=244683&action=review
> 
> > Source/WebKit2/WebProcess/WebPage/DrawingAreaImpl.cpp:691
> > +    m_webPage.corePage()->settings().setAcceleratedCompositingEnabled(true);
> 
> Wouldn't it be better to enable this unconditionally in WebKitSettings's
> constructed? This looks a bit hackish.

That would be a problem if the required X extensions are not present and SetNativeSurfaceHandleForCompositing message is not sent. The web process would enter AC mode but we wouldn't render anything. I think this is the safest approach, the setting is disabled by default, and should only be enabled when the web process has a native surface handle.
Comment 5 Gustavo Noronha (kov) 2015-01-15 03:11:30 PST
Comment on attachment 244683 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=244683&action=review

>>> Source/WebKit2/WebProcess/WebPage/DrawingAreaImpl.cpp:691
>>> +    m_webPage.corePage()->settings().setAcceleratedCompositingEnabled(true);
>> 
>> Wouldn't it be better to enable this unconditionally in WebKitSettings's constructed? This looks a bit hackish.
> 
> That would be a problem if the required X extensions are not present and SetNativeSurfaceHandleForCompositing message is not sent. The web process would enter AC mode but we wouldn't render anything. I think this is the safest approach, the setting is disabled by default, and should only be enabled when the web process has a native surface handle.

okidoki
Comment 6 Carlos Garcia Campos 2015-01-15 05:07:49 PST
Committed r178509: <http://trac.webkit.org/changeset/178509>