Bug 195879 - Assertion failure !isInAcceleratedCompositingMode() in DrawingAreaProxyCoordinatedGraphics::incorporateUpdate when forceCompositingMode is turned on
Summary: Assertion failure !isInAcceleratedCompositingMode() in DrawingAreaProxyCoordi...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebKitGTK (show other bugs)
Version: WebKit Nightly Build
Hardware: All Linux
: P2 Normal
Assignee: Nobody
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2019-03-18 03:31 PDT by Tomoki Imai
Modified: 2019-03-26 10:58 PDT (History)
9 users (show)

See Also:


Attachments
Patch (5.89 KB, patch)
2019-03-18 03:48 PDT, Tomoki Imai
no flags Details | Formatted Diff | Diff
Patch (5.92 KB, patch)
2019-03-18 03:55 PDT, Tomoki Imai
cgarcia: review+
cgarcia: commit-queue-
Details | Formatted Diff | Diff
Patch, fixes reviewed point (4.07 KB, patch)
2019-03-25 01:16 PDT, Tomoki Imai
ews-watchlist: commit-queue-
Details | Formatted Diff | Diff
Archive of layout-test-results from ews200 for win-future (12.88 MB, application/zip)
2019-03-25 02:59 PDT, EWS Watchlist
no flags Details
Patch, fixes reviewed point (4.07 KB, patch)
2019-03-26 00:51 PDT, Tomoki Imai
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Tomoki Imai 2019-03-18 03:31:13 PDT
How to reproduce:

1. Build WebKitGTK with --debug to enable ASSERT
2. Run GTK MiniBrowser
3. Disable accelerated compositing from settings (Hardware Acceleration Policy -> never)
4. Go to https://projects.lukehaas.me/css-loaders/ (any website with animation is enough)
5. Enable forceCompositingMode from settings (Hardware Acceleration Policy -> always)

Actual result:
ASSERTION FAILED: ASSERTION FAILED: !isInAcceleratedCompositingMode()
../../Source/WebKit/UIProcess/CoordinatedGraphics/DrawingAreaProxyCoordinatedGraphics.cpp(245) : void WebKit::DrawingAreaProxyCoordinatedGraphics::incorporateUpdate(const WebKit::UpdateInfo&)

Expected result:
No ASSERT error occurred.

The root cause is that DrawingAreaProxyCoordinatedGraphics::isInAcceleratedCompositingMode checks both of alwaysUseCompositing() and !m_layerTreeContext.isEmpty().
alwaysUseCompositing() refers preferences, which is written by the application (UIProcess).
On the other hand, m_layerTreeContext is changed when it receives enterAcceleratedCompositingMode/exitAcceleratedCompositingMode from WebProcess.

It results when we set forceCompositingMode and acceleratedCompositingEnabled to true, WebProcess and UIProcess is out of sync until WebProcess sends enterAcceleratedCompositingMode message.
In such situation, WebProcess sends incorporateUpdate to UIProcess because WebProcess is in non-AC mode, but isInAcceleratedCompositingMode becomes true in UIProcess side.

Note that turning on forceCompositingMode on https://www.webkitgtk.org/ results blank page, and resize window trigger redraws.
I guess it is a different problem from this bug.
Comment 1 Tomoki Imai 2019-03-18 03:48:40 PDT
Created attachment 365007 [details]
Patch
Comment 2 Tomoki Imai 2019-03-18 03:55:50 PDT
Created attachment 365008 [details]
Patch

Replace tabs in ChangeLog
Comment 3 Don Olmstead 2019-03-21 13:25:44 PDT
Igalia folks any particular issues with this patch? It LGTM but we wanted to make sure we aren't missing something here.
Comment 4 Carlos Garcia Campos 2019-03-21 23:44:58 PDT
I'll review it, it was already in my TODO.
Comment 5 Carlos Garcia Campos 2019-03-22 04:58:55 PDT
Comment on attachment 365008 [details]
Patch

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

> Source/WebKit/UIProcess/API/gtk/WebKitWebViewBase.cpp:545
> -    if (webViewBase->priv->acceleratedBackingStore && drawingArea->isInAcceleratedCompositingMode())
> +    if (webViewBase->priv->acceleratedBackingStore && (drawingArea->isInAcceleratedCompositingMode() || drawingArea->alwaysUseCompositing()))

I don't think we want to do this. If we are still transitioning, incorporateUpdate updated the backing store, so if at this point we haven't entered AC mode, we want to paint the new backing store, since the accelerated backing store won't have anything to paint yet.
Comment 6 Tomoki Imai 2019-03-22 05:20:00 PDT
Comment on attachment 365008 [details]
Patch

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

>> Source/WebKit/UIProcess/API/gtk/WebKitWebViewBase.cpp:545
>> +    if (webViewBase->priv->acceleratedBackingStore && (drawingArea->isInAcceleratedCompositingMode() || drawingArea->alwaysUseCompositing()))
> 
> I don't think we want to do this. If we are still transitioning, incorporateUpdate updated the backing store, so if at this point we haven't entered AC mode, we want to paint the new backing store, since the accelerated backing store won't have anything to paint yet.

Right, the reason I added `|| drawingArea->alwaysUseCompositing()` here is to stop painting new backing store right after `alwaysUseCompositing` is turned on, which is current behavior.
But if we don't have such usecase , we should check only `webViewBase->priv->acceleratedBackingStore && drawingArea->isInAcceleratedCompositingMode()` to continue using backing store until entering AC mode.
Comment 7 Tomoki Imai 2019-03-24 22:50:54 PDT
Comment on attachment 365008 [details]
Patch

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

> Source/WebKit/UIProcess/CoordinatedGraphics/DrawingAreaProxyCoordinatedGraphics.cpp:79
> +    if (isInAcceleratedCompositingMode() || alwaysUseCompositing())

I think we shouldn't have " || alwaysUseCompositing()" here for the same reason as webkitWebViewBaseDraw. If it's transitioning to AC mode, we should update BackingStore.
Comment 8 Tomoki Imai 2019-03-25 01:16:17 PDT
Created attachment 365854 [details]
Patch, fixes reviewed point

I removed alwaysUseCompositing() from webkitWebViewBaseDraw and DrawingAreaProxyCoordinatedGraphics::paint, because while transitioning to AC mode, we should render in non-AC mode.
Comment 9 EWS Watchlist 2019-03-25 02:59:19 PDT
Comment on attachment 365854 [details]
Patch, fixes reviewed point

Attachment 365854 [details] did not pass win-ews (win):
Output: https://webkit-queues.webkit.org/results/11658597

New failing tests:
animations/resume-after-page-cache.html
Comment 10 EWS Watchlist 2019-03-25 02:59:29 PDT
Created attachment 365856 [details]
Archive of layout-test-results from ews200 for win-future

The attached test failures were seen while running run-webkit-tests on the win-ews.
Bot: ews200  Port: win-future  Platform: CYGWIN_NT-6.1-2.9.0-0.318-5-3-x86_64-64bit
Comment 11 Tomoki Imai 2019-03-26 00:51:08 PDT
Created attachment 365950 [details]
Patch, fixes reviewed point

WinCairo build seems to be broken. Upload patch again to re-run EWS.
Comment 12 WebKit Commit Bot 2019-03-26 10:58:03 PDT
Comment on attachment 365950 [details]
Patch, fixes reviewed point

Clearing flags on attachment: 365950

Committed r243505: <https://trac.webkit.org/changeset/243505>
Comment 13 WebKit Commit Bot 2019-03-26 10:58:05 PDT
All reviewed patches have been landed.  Closing bug.