Bug 140494 – REGRESSION(r177075): [GTK] Creating a second web view disables accelerated compositing in existing web view

• Component: WebKitGTK
• Status: RESOLVED FIXED

[Carlos Garcia Campos]

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.

[Carlos Garcia Campos]

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.

[WebKit Commit Bot]

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

[Gustavo Noronha (kov)]

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.

[Carlos Garcia Campos]

(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.

[Gustavo Noronha (kov)]

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

[Carlos Garcia Campos]

Committed r178509: <http://trac.webkit.org/changeset/178509>