Bug 195879

Summary:

Assertion failure !isInAcceleratedCompositingMode() in DrawingAreaProxyCoordinatedGraphics::incorporateUpdate when forceCompositingMode is turned on

Product:

WebKit

Reporter:

Tomoki Imai <tomoki.imai>

Component:

WebKitGTK

Assignee:

Nobody <webkit-unassigned>

Status:

RESOLVED FIXED

Severity:

Normal

CC:

aperez, bugs-noreply, cgarcia, commit-queue, don.olmstead, ews-watchlist, magomez, mcatanzaro, zan

Priority:

Version:

WebKit Nightly Build

Hardware:

All

OS:

Linux

See Also:

https://bugs.webkit.org/show_bug.cgi?id=233455

Attachments:

Description	Flags
Patch	none
Patch	cgarcia: review+, cgarcia: commit-queue-
Patch, fixes reviewed point	ews-watchlist: commit-queue-
Archive of layout-test-results from ews200 for win-future	none
Patch, fixes reviewed point	none

Tomoki Imai

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

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
Show Obsolete (3) View All Add attachment proposed patch, testcase, etc.

Tomoki Imai

Comment 1 2019-03-18 03:48:40 PDT

Created attachment 365007 [details] Patch

Tomoki Imai

Comment 2 2019-03-18 03:55:50 PDT

Created attachment 365008 [details] Patch Replace tabs in ChangeLog

Don Olmstead

Comment 3 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.

Carlos Garcia Campos

Comment 4 2019-03-21 23:44:58 PDT

I'll review it, it was already in my TODO.

Carlos Garcia Campos

Comment 5 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.

Tomoki Imai

Comment 6 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.

Tomoki Imai

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

Tomoki Imai

Comment 8 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.

EWS Watchlist

Comment 9 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

EWS Watchlist

Comment 10 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

Tomoki Imai

Comment 11 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.

WebKit Commit Bot

Comment 12 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>

WebKit Commit Bot

Comment 13 2019-03-26 10:58:05 PDT

All reviewed patches have been landed. Closing bug.

Note You need to log in before you can comment on or make changes to this bug.