[GTK] Graphics corruption when entering/leaving AC mode quickly

• Component: WebKitGTK
• Status: RESOLVED FIXED

[Mario Sanchez Prada]

Created attachment 263467 [details]
Reduced test case

DESCRIPTION

Starting with WebKit2GTK+ 2.8, there seems to be an issue that happens sometimes when Accelerated Compositing mode is exited and then re-activated again very quickly, which sometimes can lead to some weird "flickering effects", where the webview would show wrong contents for a split second.

More specifically, in my case I could see at least 3 manifestations of this issue, which I think it could be a sign of a memory corruption issue in the backing store:
  A. The webview displays a previous rendering state for a split second, then quickly switches to its current state again
  B. The webview refuses to load the contents of an image, which could be never shown in the end if belonging to an animation (where the image would be shown only for a moment)
  C. The webview shows some corrupted pixels at the top for a moment

I've debugged this issue for a while already and found that in WebKit2GTK+ 2.6 and previous versions the PageClientImpl::enterAcceleratedCompositingMode() and PageClientImpl::exitAcceleratedCompositingMode() where not implemented, while now they are implemented to cause a resize of both the XRedirectedCompositeWindow used for AC, and the drawing area (only on entering AC), so I believe that's the crucial difference that exposed the problem now, and why I could not see it in 2.6 at all.

Now, trying to come up with a reliable test case to prove this issue I wrote a simple test HTML which I can use to reproduce at least (B) and (C) in my laptop, using WebKit upstream (r191124) and MiniBrowser, which I'm attaching right now. (A) is harder to reproduce as a simplified test case, so forgive me for not providing a reliable test for it, but the other two problems I can reproduce them with this test case relatively easy (as in 3/10) in my laptop.


STEPS TO REPRODUCE

  1. Uncompress the contents of the attached file and load index.html in MiniBrowser
  2. You should see some small text together with a black box underneath for a split second, then just some text without any black box around
  3. Press Ctrl+R to refresh and repeat step 2
  4. Repeat steps 2-3 a bunch of times (e.g. 20 times or so)


EXPECTED OUTCOME:

Whenever you refresh, you should ALWAYS see a black box underneath the text that shows up briefly text, and then no black box at all when the second text is shown    


ACTUAL OUTCOME:

Sometimes the black box is not shown together with the initial text when refreshing, sometimes some corrupted graphics are rendered at the top of the web view. Sometimes it works ok, too!


OTHER COMMENTS:

Notice that the problems mentioned above do NOT HAPPEN AT ALL if Accelerated Compositing mode stayed ON ALL THE TIME while the test is run (instead of exitting/re-entering quickly, which is what that test forces).

In order to try this out yourself, simply uncomment lines 22-24 in the attached test case and repear the STEPS above. In this case, you should never see the problems and the EXPECTED OUTCOME should be the actual time, everytime you refresh.

[Carlos Garcia Campos]

Created attachment 263476 [details]
Patch

The ExitAcceleratedCompositingMode message that the web process sends to the UI process, includes a backing store update to avoid this flickering. The DrawingArea first calls exitAcceleratedCompositingMode() and then incorporateUpdate() to render the given update. Since we are resizing the redirected window immediately to clear its resources, it's happening before the incorporateUpdate(). So, the same way we wait for 2 seconds to discard the non AC backing store, we can do the same with the redirected window. This ensure that when the window is resized, the new update has already rendered and also that switching quickly between AC and non Ac mode doesn't resize the redirected window unnecessarily. Hopefully this removes the flickering and rendering artifacts.

[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

[Mario Sanchez Prada]

While I believe this patch made the reduced test case attached to fail less often, I've tested in the machine where I originally found the issue and the problem is still there.

So, it's possible that this is a race condition depending on how fast the machine is, or just that this patch did not really fix the issue and I'm just a victim of a placebo effect :)

[Carlos Garcia Campos]

(In reply to comment #3)
> While I believe this patch made the reduced test case attached to fail less
> often, I've tested in the machine where I originally found the issue and the
> problem is still there.
> 
> So, it's possible that this is a race condition depending on how fast the
> machine is, or just that this patch did not really fix the issue and I'm
> just a victim of a placebo effect :)

I think the patch fixes half of the problem, the flickering when leaving AC mode,  but not the one when entering. I'll submit a new patch to hopefully fix the other half too.

[Carlos Garcia Campos]

Created attachment 263558 [details]
Updated patch

Try to fix the other half of the problem :-)

[Mario Sanchez Prada]

Comment on attachment 263558 [details]
Updated patch

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

I've tried this patch locally and I can confirm it fixes all the glitches I could see so far, not only with the provided test case but also with the original problem that prompted me to report the issue.

I've also reviewed and it looks good to me, I only have a suggestion related to code readability, but overall I think this is good to land. Thanks Carlos!

> Source/WebKit2/UIProcess/API/gtk/WebKitWebViewBasePrivate.h:75
> +void webkitWebViewBaseWillEnterAcceleratedCompositingMode(WebKitWebViewBase*);

I think I'd declare this method before webkitWebViewBaseEnterAcceleratedCompositingMode(), so that it's clear that these 3 callbacks are normally executed in that order.

[Carlos Garcia Campos]

Committed r191340: <http://trac.webkit.org/changeset/191340>