Bug 178749 - When navigating back to a page, compositing layers may not use accelerated drawing
Summary: When navigating back to a page, compositing layers may not use accelerated dr...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Layout and Rendering (show other bugs)
Version: WebKit Local Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Simon Fraser (smfr)
URL:
Keywords: InRadar
Depends on: 179026
Blocks:
  Show dependency treegraph
 
Reported: 2017-10-24 14:09 PDT by Simon Fraser (smfr)
Modified: 2017-11-13 20:38 PST (History)
9 users (show)

See Also:


Attachments
Testcase (999 bytes, text/html)
2017-10-24 14:09 PDT, Simon Fraser (smfr)
no flags Details
Patch (18.11 KB, patch)
2017-10-24 18:45 PDT, Simon Fraser (smfr)
no flags Details | Formatted Diff | Diff
Patch (18.98 KB, patch)
2017-10-24 18:51 PDT, Simon Fraser (smfr)
dino: review+
buildbot: commit-queue-
Details | Formatted Diff | Diff
Archive of layout-test-results from ews102 for mac-elcapitan (999.88 KB, application/zip)
2017-10-24 20:01 PDT, Build Bot
no flags Details
Archive of layout-test-results from ews114 for mac-elcapitan (1.79 MB, application/zip)
2017-10-24 20:10 PDT, Build Bot
no flags Details
Archive of layout-test-results from ews121 for ios-simulator-wk2 (10.38 MB, application/zip)
2017-10-24 20:25 PDT, Build Bot
no flags Details
Archive of layout-test-results from ews105 for mac-elcapitan-wk2 (1.48 MB, application/zip)
2017-10-24 21:57 PDT, Build Bot
no flags Details
Patch (26.64 KB, patch)
2017-11-13 16:47 PST, Simon Fraser (smfr)
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Simon Fraser (smfr) 2017-10-24 14:09:43 PDT
When going back to a simple page like the attached testcase, the compositing layer is created without accelerated drawing enabled and never updated thereafter.
Comment 1 Simon Fraser (smfr) 2017-10-24 14:09:59 PDT
Created attachment 324716 [details]
Testcase
Comment 2 Radar WebKit Bug Importer 2017-10-24 14:10:27 PDT
<rdar://problem/35158946>
Comment 3 Simon Fraser (smfr) 2017-10-24 14:11:37 PDT
There are two things going on here. First, in GraphicsLayerCA::commitLayerChangesBeforeSublayers() we call updateTiles() before updateAcceleratesDrawing(), so it's possible for tiles to not get accelerated.

Second, we create GraphicsLayers under style resolution, which happens before RenderLayerCompositor::cacheAcceleratedCompositingFlags(), so m_acceleratedDrawingEnabled is still false there.
Comment 4 Simon Fraser (smfr) 2017-10-24 14:13:34 PDT
Backtrace for creating compositing layers under style change:

  * frame #0: 0x00000007ea56b01a WebCore`WebCore::RenderLayerBacking::createPrimaryGraphicsLayer(this=0x00000007fb9fb630) at RenderLayerBacking.cpp:391
    frame #1: 0x00000007ea56ae54 WebCore`WebCore::RenderLayerBacking::RenderLayerBacking(this=0x00000007fb9fb630, layer=0x00000007fb9975c8) at RenderLayerBacking.cpp:216
    frame #2: 0x00000007ea56b9dd WebCore`WebCore::RenderLayerBacking::RenderLayerBacking(this=0x00000007fb9fb630, layer=0x00000007fb9975c8) at RenderLayerBacking.cpp:210
    frame #3: 0x00000007ea5557db WebCore`WebCore::RenderLayer::ensureBacking() [inlined] std::__1::__unique_if<WebCore::RenderLayerBacking>::__unique_single std::__1::make_unique<WebCore::RenderLayerBacking, WebCore::RenderLayer&>(__args=0x00000007fb9975c8) at memory:3006
    frame #4: 0x00000007ea5557b2 WebCore`WebCore::RenderLayer::ensureBacking(this=0x00000007fb9975c8) at RenderLayer.cpp:5911
    frame #5: 0x00000007ea5936d8 WebCore`WebCore::RenderLayerCompositor::updateBacking(this=0x00000007fb9b1b10, layer=0x00000007fb9975c8, shouldRepaint=CompositingChangeRepaintNow, backingRequired=Yes) at RenderLayerCompositor.cpp:1020
    frame #6: 0x00000007ea592f38 WebCore`WebCore::RenderLayerCompositor::updateLayerCompositingState(this=0x00000007fb9b1b10, layer=0x00000007fb9975c8, shouldRepaint=CompositingChangeRepaintNow) at RenderLayerCompositor.cpp:1109
    frame #7: 0x00000007ea592e42 WebCore`WebCore::RenderLayerCompositor::layerStyleChanged(this=0x00000007fb9b1b10, diff=StyleDifferenceNewStyle, layer=0x00000007fb9975c8, oldStyle=0x0000000000000000) at RenderLayerCompositor.cpp:933
    frame #8: 0x00000007ea55904d WebCore`WebCore::RenderLayer::styleChanged(this=0x00000007fb9975c8, diff=StyleDifferenceNewStyle, oldStyle=0x0000000000000000) at RenderLayer.cpp:6635
    frame #9: 0x00000007ea5aeec6 WebCore`WebCore::RenderLayerModelObject::styleDidChange(this=0x00000007fe4786c0, diff=StyleDifferenceNewStyle, oldStyle=0x0000000000000000) at RenderLayerModelObject.cpp:193
    frame #10: 0x00000007ea45ac22 WebCore`WebCore::RenderBox::styleDidChange(this=0x00000007fe4786c0, diff=StyleDifferenceNewStyle, oldStyle=0x0000000000000000) at RenderBox.cpp:302
    frame #11: 0x00000007ea3d3602 WebCore`WebCore::RenderBlock::styleDidChange(this=0x00000007fe4786c0, diff=StyleDifferenceNewStyle, oldStyle=0x0000000000000000) at RenderBlock.cpp:432
    frame #12: 0x00000007ea4245c3 WebCore`WebCore::RenderBlockFlow::styleDidChange(this=0x00000007fe4786c0, diff=StyleDifferenceNewStyle, oldStyle=0x0000000000000000) at RenderBlockFlow.cpp:2011
    frame #13: 0x00000007ea4bed34 WebCore`WebCore::RenderElement::initializeStyle(this=0x00000007fe4786c0) at RenderElement.cpp:380
    frame #14: 0x00000007ea73ea0c WebCore`WebCore::RenderTreeUpdater::createRenderer(this=0x00007ffee2db2360, element=0x00000007fe44d0d0, style=0x00007ffee2daf5d0) at RenderTreeUpdater.cpp:373
    frame #15: 0x00000007ea73df18 WebCore`WebCore::RenderTreeUpdater::updateElementRenderer(this=0x00007ffee2db2360, element=0x00000007fe44d0d0, update=0x00000007fbbda950) at RenderTreeUpdater.cpp:326
    frame #16: 0x00000007ea73d611 WebCore`WebCore::RenderTreeUpdater::updateRenderTree(this=0x00007ffee2db2360, root=0x00000007fb956000) at RenderTreeUpdater.cpp:198
    frame #17: 0x00000007ea73cf59 WebCore`WebCore::RenderTreeUpdater::commit(this=0x00007ffee2db2360, styleUpdate=unique_ptr<const WebCore::Style::Update, std::__1::default_delete<const WebCore::Style::Update> > @ 0x00007ffee2db2358) at RenderTreeUpdater.cpp:134
    frame #18: 0x00000007e87a7632 WebCore`WebCore::Document::resolveStyle(this=0x00000007fb956000, type=Rebuild) at Document.cpp:1825
    frame #19: 0x00000007e87ab1be WebCore`WebCore::Document::createRenderTree(this=0x00000007fb956000) at Document.cpp:2172
    frame #20: 0x00000007e87ab375 WebCore`WebCore::Document::didBecomeCurrentDocumentInFrame(this=0x00000007fb956000) at Document.cpp:2183
Comment 5 Simon Fraser (smfr) 2017-10-24 18:45:14 PDT
Created attachment 324770 [details]
Patch
Comment 6 Simon Fraser (smfr) 2017-10-24 18:51:48 PDT
Created attachment 324772 [details]
Patch
Comment 7 Build Bot 2017-10-24 20:01:22 PDT
Comment on attachment 324772 [details]
Patch

Attachment 324772 [details] did not pass mac-ews (mac):
Output: http://webkit-queues.webkit.org/results/4978945

New failing tests:
compositing/iframes/page-cache-layer-tree.html
Comment 8 Build Bot 2017-10-24 20:01:23 PDT
Created attachment 324781 [details]
Archive of layout-test-results from ews102 for mac-elcapitan

The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: ews102  Port: mac-elcapitan  Platform: Mac OS X 10.11.6
Comment 9 Build Bot 2017-10-24 20:10:11 PDT
Comment on attachment 324772 [details]
Patch

Attachment 324772 [details] did not pass mac-debug-ews (mac):
Output: http://webkit-queues.webkit.org/results/4978912

New failing tests:
compositing/iframes/page-cache-layer-tree.html
Comment 10 Build Bot 2017-10-24 20:10:13 PDT
Created attachment 324782 [details]
Archive of layout-test-results from ews114 for mac-elcapitan

The attached test failures were seen while running run-webkit-tests on the mac-debug-ews.
Bot: ews114  Port: mac-elcapitan  Platform: Mac OS X 10.11.6
Comment 11 Build Bot 2017-10-24 20:25:27 PDT
Comment on attachment 324772 [details]
Patch

Attachment 324772 [details] did not pass ios-sim-ews (ios-simulator-wk2):
Output: http://webkit-queues.webkit.org/results/4978954

New failing tests:
compositing/accelerated-layers-after-back.html
compositing/iframes/page-cache-layer-tree.html
Comment 12 Build Bot 2017-10-24 20:25:29 PDT
Created attachment 324785 [details]
Archive of layout-test-results from ews121 for ios-simulator-wk2

The attached test failures were seen while running run-webkit-tests on the ios-sim-ews.
Bot: ews121  Port: ios-simulator-wk2  Platform: Mac OS X 10.12.6
Comment 13 Build Bot 2017-10-24 21:57:39 PDT
Comment on attachment 324772 [details]
Patch

Attachment 324772 [details] did not pass mac-wk2-ews (mac-wk2):
Output: http://webkit-queues.webkit.org/results/4979670

New failing tests:
compositing/iframes/page-cache-layer-tree.html
Comment 14 Build Bot 2017-10-24 21:57:41 PDT
Created attachment 324794 [details]
Archive of layout-test-results from ews105 for mac-elcapitan-wk2

The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: ews105  Port: mac-elcapitan-wk2  Platform: Mac OS X 10.11.6
Comment 15 Simon Fraser (smfr) 2017-10-25 14:32:46 PDT
https://trac.webkit.org/r223984
Comment 16 Ryan Haddad 2017-10-26 11:40:43 PDT
This change caused LayoutTests to hit an assertion failure:

ASSERTION FAILED: !m_renderView.needsLayout()
/Volumes/Data/slave/ios-simulator-11-debug/build/Source/WebCore/rendering/RenderLayerCompositor.cpp(2356) : bool WebCore::RenderLayerCompositor::requiresCompositingForScrollableFrame() const
1   0x1169bd9dd WTFCrash
2   0x11b86be5a WebCore::RenderLayerCompositor::requiresCompositingForScrollableFrame() const
3   0x11b86ba40 WebCore::RenderLayerCompositor::cacheAcceleratedCompositingFlags()
4   0x11b86bed9 WebCore::RenderLayerCompositor::willRecalcStyle()
5   0x11b13b95a WebCore::FrameView::willRecalcStyle()
6   0x11a941585 WebCore::Document::resolveStyle(WebCore::Document::ResolveStyleType)
7   0x11a9431f4 WebCore::Document::updateStyleIfNeeded()
8   0x11b13f1b5 WebCore::FrameView::updateStyleForLayout()
9   0x11b13d80e WebCore::FrameView::layout()
10  0x11b964b95 WebCore::RenderWidget::updateWidgetPosition()
11  0x11b14083a WebCore::FrameView::updateWidgetPositions()
12  0x11b13818f WebCore::FrameView::performPostLayoutTasks()
13  0x11b140651 WebCore::FrameView::runOrSchedulePostLayoutTasks()
14  0x11b13e1b5 WebCore::FrameView::layout()
15  0x11b150114 WebCore::FrameView::updateLayoutAndStyleIfNeededRecursive()
16  0x10daf8ff5 WebKit::WebPage::layoutIfNeeded()
17  0x10d72871e WebKit::RemoteLayerTreeDrawingArea::flushLayers()
18  0x10d72ff21 WTF::Function<void ()>::CallableWrapper<std::__1::__bind<void (WebKit::RemoteLayerTreeDrawingArea::*&)(), WebKit::RemoteLayerTreeDrawingArea*> >::call()
19  0x10d2f189b WTF::Function<void ()>::operator()() const
20  0x10d4f4739 WebCore::Timer::fired()
21  0x11b2e5c34 WebCore::ThreadTimers::sharedTimerFiredInternal()
22  0x11b2eddd1 WebCore::ThreadTimers::setSharedTimer(WebCore::SharedTimer*)::$_7::operator()() const
23  0x11b2edd89 WTF::Function<void ()>::CallableWrapper<WebCore::ThreadTimers::setSharedTimer(WebCore::SharedTimer*)::$_7>::call()
24  0x118e94b5b WTF::Function<void ()>::operator()() const
25  0x11b2c7955 WebCore::MainThreadSharedTimer::fired()
26  0x11b35d579 WebCore::timerFired(__CFRunLoopTimer*, void*)
27  0x10ac65374 __CFRUNLOOP_IS_CALLING_OUT_TO_A_TIMER_CALLBACK_FUNCTION__
28  0x10ac65032 __CFRunLoopDoTimer
29  0x10ac64bea __CFRunLoopDoTimers
30  0x10ac5c604 __CFRunLoopRun
31  0x10ac5ba89 CFRunLoopRunSpecific
LEAK: 3 WebPageProxy

https://build.webkit.org/results/Apple%20iOS%2011%20Simulator%20Debug%20WK2%20(Tests)/r224023%20(645)/results.html
Comment 17 Ryan Haddad 2017-10-26 11:44:14 PDT
Reverted r223984 for reason:

Caused LayoutTest assertion failures.

Committed r224032: <https://trac.webkit.org/changeset/224032>
Comment 18 Simon Fraser (smfr) 2017-10-26 18:58:24 PDT
Re-landed with fixes:
https://trac.webkit.org/r224078
Comment 19 WebKit Commit Bot 2017-10-30 13:29:07 PDT
Re-opened since this is blocked by bug 179026
Comment 20 Simon Fraser (smfr) 2017-11-13 15:14:01 PST
The perf regression seems to happen because we're repainting all of the layers every frame now.
Comment 21 Simon Fraser (smfr) 2017-11-13 15:26:14 PST
And that seems to happen because we take filter blur radius into account when computing layer bounds (bug 81239).
Comment 22 Simon Fraser (smfr) 2017-11-13 15:36:55 PST
Ah, RenderLayerCompositor::cacheAcceleratedCompositingFlags() was calling setCompositingLayersNeedRebuild() every time because forceCompositingMode != m_forceCompositingMode
Comment 23 Simon Fraser (smfr) 2017-11-13 15:53:05 PST
m_forceCompositingMode is flip-flopping between cacheAcceleratedCompositingFlags and cacheAcceleratedCompositingFlagsAfterLayout.
Comment 24 Simon Fraser (smfr) 2017-11-13 16:47:32 PST
Created attachment 326824 [details]
Patch
Comment 25 WebKit Commit Bot 2017-11-13 18:01:44 PST
Comment on attachment 326824 [details]
Patch

Clearing flags on attachment: 326824

Committed r224796: <https://trac.webkit.org/changeset/224796>