Bug 141682 – Viewport units should not dirty style just before we do layout

• Status: RESOLVED
• Resolution: FIXED
• Component: CSS

[Simon Fraser (smfr)]

The test fast/css-grid-layout/breadth-size-resolution-grid.html reveals some bad behavior where viewport units dirty style right before we do layout:

  * frame #1: 0x0000000104d020ba WebCore`WebCore::Document::scheduleStyleRecalc(this=0x0000000116043580) + 218 at Document.cpp:1693
    frame #2: 0x0000000105d1d53d WebCore`WebCore::Node::updateAncestorsForStyleRecalc(this=0x0000000117ff0618) + 349 at Node.cpp:671
    frame #3: 0x0000000105d15b4a WebCore`WebCore::Node::setNeedsStyleRecalc(this=0x0000000117ff0618, changeType=InlineStyleChange) + 170 at Node.cpp:685
    frame #4: 0x0000000104d098e0 WebCore`WebCore::Document::updateViewportUnitsOnResize(this=0x0000000116043580) + 176 at Document.cpp:3305
    frame #5: 0x00000001050920e0 WebCore`WebCore::FrameView::layout(this=0x0000000116016d00, allowSubtree=true) + 2976 at FrameView.cpp:1294
    frame #6: 0x0000000104cff357 WebCore`WebCore::Document::updateLayout(this=0x0000000116043580) + 343 at Document.cpp:1857
    frame #7: 0x0000000104d02daf WebCore`WebCore::Document::updateLayoutIgnorePendingStylesheets(this=0x0000000116043580, runPostLayoutTasks=Asynchronously) + 207 at Document.cpp:1889
    frame #8: 0x0000000104ea0e29 WebCore`WebCore::Element::offsetWidth(this=0x0000000117ff0a90) + 41 at Element.cpp:717
    frame #9: 0x00000001056cf6fc WebCore`WebCore::jsElementOffsetWidth(exec=0x00007fff5fbfc920, slotBase=0x0000000115fffda0, thisValue=4700567472, (null)=PropertyName at 0x00007fff5fbfc5b0) + 204 at JSElement.cpp:657
    frame #10: 0x000000010035d7e8 JavaScriptCore`JSC::PropertySlot::getValue(this=0x00007fff5fbfc760, exec=0x00007fff5fbfc920, propertyName=PropertyName at 0x00007fff5fbfc610) const + 184 at PropertySlot.h:256
    frame #11: 0x000000010038184b JavaScriptCore`JSC::JSValue::get(this=0x00007fff5fbfc798, exec=0x00007fff5fbfc920, propertyName=PropertyName at 0x00007fff5fbfc660, slot=0x00007fff5fbfc760) const + 91 at JSCJSValueInlines.h:703
    frame #12: 0x0000000100a699a1 JavaScriptCore`llint_slow_path_get_by_id(exec=0x00007fff5fbfc920, pc=0x000000011608a540) + 241 at LLIntSlowPaths.cpp:581
    frame #13: 0x0000000100a77567 JavaScriptCore`llint_entry + 11115
    frame #14: 0x0000000100a7acc7 JavaScriptCore`llint_entry + 25291
    frame #15: 0x0000000100a7acc7 JavaScriptCore`llint_entry + 25291
    frame #16: 0x0000000100a7acc7 JavaScriptCore`llint_entry + 25291
    frame #17: 0x0000000100a7ad3a JavaScriptCore`llint_entry + 25406
    frame #18: 0x0000000100a7ad3a JavaScriptCore`llint_entry + 25406
    frame #19: 0x0000000100a7ad3a JavaScriptCore`llint_entry + 25406
    frame #20: 0x0000000100a7ad3a JavaScriptCore`llint_entry + 25406
    frame #21: 0x00002c9e956019fa
    frame #22: 0x0000000100a747b9 JavaScriptCore`vmEntryToJavaScript + 361
    frame #23: 0x00000001008fda8c JavaScriptCore`JSC::JITCode::execute(this=0x0000000117f81660, vm=0x0000000116027380, protoCallFrame=0x00007fff5fbfcf48) + 252 at JITCode.cpp:77
    frame #24: 0x00000001008e2379 JavaScriptCore`JSC::Interpreter::executeCall(this=0x0000000117ff02d8, callFrame=0x00000001182bf4b0, function=0x00000001182f99f0, callType=CallTypeJS, callData=0x00007fff5fbfd418, thisValue=JSValue at 0x00007fff5fbfd020, args=0x00007fff5fbfd2e8) + 1465 at Interpreter.cpp:912
    frame #25: 0x00000001003e71de JavaScriptCore`JSC::call(exec=0x00000001182bf4b0, functionObject=JSValue at 0x00007fff5fbfd100, callType=CallTypeJS, callData=0x00007fff5fbfd418, thisValue=JSValue at 0x00007fff5fbfd0f8, args=0x00007fff5fbfd2e8) + 190 at CallData.cpp:39
    frame #26: 0x00000001003e7243 JavaScriptCore`JSC::call(exec=0x00000001182bf4b0, functionObject=JSValue at 0x00007fff5fbfd180, callType=CallTypeJS, callData=0x00007fff5fbfd418, thisValue=JSValue at 0x00007fff5fbfd178, args=0x00007fff5fbfd2e8, exception=0x00007fff5fbfd310) + 83 at CallData.cpp:44
    frame #27: 0x00000001055828eb WebCore`WebCore::JSMainThreadExecState::call(exec=0x00000001182bf4b0, functionObject=JSValue at 0x00007fff5fbfd200, callType=CallTypeJS, callData=0x00007fff5fbfd418, thisValue=JSValue at 0x00007fff5fbfd1f8, args=0x00007fff5fbfd2e8, exception=0x00007fff5fbfd310) + 107 at JSMainThreadExecState.h:56
    frame #28: 0x00000001056fdfbe WebCore`WebCore::JSEventListener::handleEvent(this=0x0000000117ff0548, scriptExecutionContext=0x0000000116043620, event=0x0000000117f8b500) + 1278 at JSEventListener.cpp:127
    frame #29: 0x0000000104efd9a4 WebCore`WebCore::EventTarget::fireEventListeners(this=0x00000001177ef180, event=0x0000000117f8b500, d=0x00000001177ef188, entry=0x0000000117f91c40) + 1444 at EventTarget.cpp:255
    frame #30: 0x0000000104efd1de WebCore`WebCore::EventTarget::fireEventListeners(this=0x00000001177ef180, event=0x0000000117f8b500) + 334 at EventTarget.cpp:207
    frame #31: 0x0000000104e27e3b WebCore`WebCore::DOMWindow::dispatchEvent(this=0x00000001177ef180, prpEvent=PassRefPtr<WebCore::Event> at 0x00007fff5fbfd8a0, prpTarget=(m_ptr = 0x0000000000000000)) + 539 at DOMWindow.cpp:1897
    frame #32: 0x0000000104e2f795 WebCore`WebCore::DOMWindow::dispatchLoadEvent(this=0x00000001177ef180) + 293 at DOMWindow.cpp:1855
    frame #33: 0x0000000104d0550d WebCore`WebCore::Document::dispatchWindowLoadEvent(this=0x0000000116043580) + 141 at Document.cpp:3833
    frame #34: 0x0000000104d0285d WebCore`WebCore::Document::implicitClose(this=0x0000000116043580) + 557 at Document.cpp:2477
    frame #35: 0x000000010505be1b WebCore`WebCore::FrameLoader::checkCallImplicitClose(this=0x00000001177f20a0) + 155 at FrameLoader.cpp:900
    frame #36: 0x000000010505baee WebCore`WebCore::FrameLoader::checkCompleted(this=0x00000001177f20a0) + 270 at FrameLoader.cpp:846
    frame #37: 0x000000010505bc65 WebCore`WebCore::FrameLoader::loadDone(this=0x00000001177f20a0) + 21 at FrameLoader.cpp:779
    frame #38: 0x00000001049c3ec9 WebCore`WebCore::CachedResourceLoader::loadDone(this=0x00000001177ee6c0, resource=0x00000001177df000, shouldPerformPostLoadActions=true) + 121 at CachedResourceLoader.cpp:841
    frame #39: 0x00000001064d5045 WebCore`WebCore::SubresourceLoader::notifyDone(this=0x0000000116016000) + 293 at SubresourceLoader.cpp:445
    frame #40: 0x00000001064d4c66 WebCore`WebCore::SubresourceLoader::didFinishLoading(this=0x0000000116016000, finishTime=0) + 662 at SubresourceLoader.cpp:371
    frame #41: 0x00000001061c5415 WebCore`WebCore::ResourceLoader::didFinishLoading(this=0x0000000116016000, (null)=0x0000000117fb70f0, finishTime=0) + 53 at ResourceLoader.cpp:542
    frame #42: 0x0000000106775faa WebCore`-[WebCoreResourceHandleAsDelegate connectionDidFinishLoading:](self=0x000000010c628b80, _cmd=0x00007fff8c2ad992, connection=0x000000010c6290a0) + 186 at WebCoreResourceHandleAsDelegate.mm:260
    frame #43: 0x00007fff9055620d CFNetwork`__65-[NSURLConnectionInternal _withConnectionAndDelegate:onlyActive:]_block_invoke + 69
    frame #44: 0x00007fff905561b1 CFNetwork`-[NSURLConnectionInternal _withConnectionAndDelegate:onlyActive:] + 232
    frame #45: 0x00007fff90556317 CFNetwork`-[NSURLConnectionInternal _withActiveConnectionAndDelegate:] + 48
    frame #46: 0x00007fff90426b54 CFNetwork`___ZN27URLConnectionClient_Classic26_delegate_didFinishLoadingEU13block_pointerFvvE_block_invoke + 104
    frame #47: 0x00007fff904f0283 CFNetwork`___ZN27URLConnectionClient_Classic18_withDelegateAsyncEPKcU13block_pointerFvP16_CFURLConnectionPK33CFURLConnectionClientCurrent_VMaxE_block_invoke_2 + 94
    frame #48: 0x00007fff9041233c CFNetwork`RunloopBlockContext::_invoke_block(void const*, void*) + 72
    frame #49: 0x00007fff8f3ab274 CoreFoundation`CFArrayApplyFunction + 68
    frame #50: 0x00007fff904121fd CFNetwork`RunloopBlockContext::perform() + 133
    frame #51: 0x00007fff9041209e CFNetwork`MultiplexerSource::perform() + 282
    frame #52: 0x00007fff90411ec0 CFNetwork`MultiplexerSource::_perform(void*) + 72
    frame #53: 0x00007fff8f3df681 CoreFoundation`__CFRUNLOOP_IS_CALLING_OUT_TO_A_SOURCE0_PERFORM_FUNCTION__ + 17
    frame #54: 0x00007fff8f3d180d CoreFoundation`__CFRunLoopDoSources0 + 269
    frame #55: 0x00007fff8f3d0e3f CoreFoundation`__CFRunLoopRun + 927
    frame #56: 0x00007fff8f3d0858 CoreFoundation`CFRunLoopRunSpecific + 296
    frame #57: 0x000000010001de9d DumpRenderTree`runTest(inputLine=0x00007fff5fbff790) + 5389 at DumpRenderTree.mm:1886
    frame #58: 0x000000010001c172 DumpRenderTree`dumpRenderTree(argc=2, argv=0x00007fff5fbff898) + 690 at DumpRenderTree.mm:1182
    frame #59: 0x000000010001e756 DumpRenderTree`DumpRenderTreeMain(argc=2, argv=0x00007fff5fbff898) + 102 at DumpRenderTree.mm:1295
    frame #60: 0x000000010006e412 DumpRenderTree`main(argc=2, argv=0x00007fff5fbff898) + 34 at DumpRenderTreeMain.mm:30

I think this means we'll do a second style recalc + layout after the first one. This may double the number of layouts on pages using viewport units.

And, anyway, they are *viewport* units. Why are we triggering style recalc when the document size changes?

[Simon Fraser (smfr)]

Created attachment 246725 [details]
proto patch

[Simon Fraser (smfr)]

Created attachment 247317 [details]
Patch

[zalan]

Comment on attachment 247317 [details]
Patch

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

> Source/WebCore/platform/ScrollView.cpp:371
> +
> +    if (platformWidget())
> +        return;

There's no other way to signal content size changed from Wk1? Checking against platformWidget() looks hackish.

[Simon Fraser (smfr)]

Note to self: make sure this doesn't break iOS.

[Simon Fraser (smfr)]

https://trac.webkit.org/r180848