Bug 183081

Comment on attachment 334524 [details] Patch (WIP) Attachment 334524 [details] did not pass mac-wk2-ews (mac-wk2): Output: http://webkit-queues.webkit.org/results/6639860 New failing tests: compositing/tiling/tiled-drawing-async-frame-scrolling.html

Created attachment 334529 [details] Archive of layout-test-results from ews107 for mac-sierra-wk2 The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews. Bot: ews107 Port: mac-sierra-wk2 Platform: Mac OS X 10.12.6

(In reply to Frédéric Wang (:fredw) from comment #0) > Maybe we also need to update scrolling tree and related stuff when > enabling/disabling async frame scrolling. Testing with the iOS frame scrolling patches, forcing a relayout seems enough for that.

Created attachment 334607 [details] Patch

Comment on attachment 334607 [details] Patch Attachment 334607 [details] did not pass mac-wk2-ews (mac-wk2): Output: http://webkit-queues.webkit.org/results/6674211 New failing tests: compositing/tiling/tiled-drawing-async-frame-scrolling.html fast/frames/flattening/frameset-flattening-simple.html

Created attachment 334613 [details] Archive of layout-test-results from ews104 for mac-sierra-wk2 The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews. Bot: ews104 Port: mac-sierra-wk2 Platform: Mac OS X 10.12.6

Comment on attachment 334607 [details] Patch Attachment 334607 [details] did not pass mac-debug-ews (mac): Output: http://webkit-queues.webkit.org/results/6674227 New failing tests: fast/frames/flattening/non-flattening-frame-inside-flattening-iframe-crash.html

Created attachment 334614 [details] Archive of layout-test-results from ews113 for mac-sierra The attached test failures were seen while running run-webkit-tests on the mac-debug-ews. Bot: ews113 Port: mac-sierra Platform: Mac OS X 10.12.6

Created attachment 334616 [details] Patch for EWS

Attachment 334616 [details] did not pass style-queue: ERROR: Source/WebCore/page/SettingsBase.cpp:278: Weird number of spaces at line-start. Are you using a 4-space indent? [whitespace/indent] [3] ERROR: Source/WebCore/page/SettingsBase.cpp:279: Weird number of spaces at line-start. Are you using a 4-space indent? [whitespace/indent] [3] ERROR: Source/WebCore/page/SettingsBase.cpp:280: Weird number of spaces at line-start. Are you using a 4-space indent? [whitespace/indent] [3] ERROR: Source/WebCore/page/SettingsBase.cpp:281: Weird number of spaces at line-start. Are you using a 4-space indent? [whitespace/indent] [3] ERROR: Source/WebCore/page/SettingsBase.cpp:283: Weird number of spaces at line-start. Are you using a 4-space indent? [whitespace/indent] [3] ERROR: Source/WebCore/page/SettingsBase.cpp:284: Weird number of spaces at line-start. Are you using a 4-space indent? [whitespace/indent] [3] Total errors found: 6 in 9 files If any of these errors are false positives, please file a bug against check-webkit-style.

Comment on attachment 334616 [details] Patch for EWS Attachment 334616 [details] did not pass mac-ews (mac): Output: http://webkit-queues.webkit.org/results/6675969 New failing tests: fast/frames/flattening/frameset-flattening-simple.html

Created attachment 334619 [details] Archive of layout-test-results from ews102 for mac-sierra The attached test failures were seen while running run-webkit-tests on the mac-ews. Bot: ews102 Port: mac-sierra Platform: Mac OS X 10.12.6

Comment on attachment 334616 [details] Patch for EWS Attachment 334616 [details] did not pass win-ews (win): Output: http://webkit-queues.webkit.org/results/6676215 New failing tests: fast/frames/flattening/frameset-flattening-simple.html

Created attachment 334622 [details] Archive of layout-test-results from ews202 for win-future The attached test failures were seen while running run-webkit-tests on the win-ews. Bot: ews202 Port: win-future Platform: CYGWIN_NT-6.1-2.9.0-0.318-5-3-x86_64-64bit

Comment on attachment 334616 [details] Patch for EWS Attachment 334616 [details] did not pass mac-debug-ews (mac): Output: http://webkit-queues.webkit.org/results/6676120 New failing tests: fast/frames/flattening/non-flattening-frame-inside-flattening-iframe-crash.html fast/frames/flattening/frameset-flattening-simple.html

Created attachment 334624 [details] Archive of layout-test-results from ews115 for mac-sierra The attached test failures were seen while running run-webkit-tests on the mac-debug-ews. Bot: ews115 Port: mac-sierra Platform: Mac OS X 10.12.6

Comment on attachment 334607 [details] Patch Attachment 334607 [details] did not pass win-ews (win): Output: http://webkit-queues.webkit.org/results/6676388 New failing tests: fast/frames/flattening/frameset-flattening-simple.html

Created attachment 334625 [details] Archive of layout-test-results from ews205 for win-future The attached test failures were seen while running run-webkit-tests on the win-ews. Bot: ews205 Port: win-future Platform: CYGWIN_NT-6.1-2.9.0-0.318-5-3-x86_64-64bit

Created attachment 334679 [details] Patch

Created attachment 334680 [details] Patch

Committed r229060: <https://trac.webkit.org/changeset/229060>

<rdar://problem/37940015>

@Don: The WinCairo bot is complaining about Settings::setFrameFlattening https://build.webkit.org/builders/WinCairo%2064-Bit%20Release/builds/11231/steps/compile-webkit/logs/stdio but the EWS was building correctly. I wonder whether it's because you need to force a clean rebuild on the bot as that happened in a previous bug? Can you please check? Thanks!

(In reply to Frédéric Wang (:fredw) from comment #23) > @Don: The WinCairo bot is complaining about Settings::setFrameFlattening > https://build.webkit.org/builders/WinCairo%2064-Bit%20Release/builds/11231/ > steps/compile-webkit/logs/stdio but the EWS was building correctly. I wonder > whether it's because you need to force a clean rebuild on the bot as that > happened in a previous bug? Can you please check? Thanks! Please ignore this. The bot is green again.

This made some tests assert more than before, notably: https://webkit-test-results.webkit.org/dashboards/flakiness_dashboard.html#showAllRuns=true&showExpectations=true&revision=229045&tests=imported%2Fmozilla%2Fsvg%2Fmask-transformed-01.svg https://webkit-test-results.webkit.org/dashboards/flakiness_dashboard.html#showAllRuns=true&tests=imported%2Fmozilla%2Fsvg%2FforeignObject-ancestor-style-change-01.svg

Thanks Alexey, I'll check this.

Re-opened since this is blocked by bug 183198

Created attachment 334734 [details] Patch

Comment on attachment 334680 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=334680&action=review > Source/WebCore/page/SettingsBase.cpp:285 > + frameView->layoutContext().scheduleLayout(); This was not needed since setNeedsLayout actually takes care of scheduling the layout. It was also causing the assert for the two SVG tests.

Comment on attachment 334734 [details] Patch r=me lets please follow on to make sure the svn failures were related to the trailing ScheduleLayout or something we were leaving over set between test runner instances, affecting the next tests.

(In reply to Antonio Gomes from comment #30) > Comment on attachment 334734 [details] > Patch > > r=me > > lets please follow on to make sure the svn failures were related to the > trailing ScheduleLayout or something we were leaving over set between test > runner instances, affecting the next tests. s/svn/svg/g

(In reply to Antonio Gomes from comment #30) > lets please follow on to make sure the svn failures were related to the > trailing ScheduleLayout or something we were leaving over set between test > runner instances, affecting the next tests. I had verified the ASSERT happened on these SVG tests, even when running only a single one so it's not something left between test runner instances. The foreignObject in these tests are subframes, so frame->ownerRenderer()->setNeedsLayoutAndPrefWidthsRecalc were called on them. Then it was wrong to force a relayout on the main frame myself: In LayoutContext::layout(), RenderTreeNeedsLayoutChecker was complaining that the subframes layout remained dirty. Instead, the calls to setNeedsLayoutAndPrefWidthsRecalc already takes care of scheduling the correct relayout. There is a separate issue, though: setNeedsRelayoutAllFrames is always called via the following stack: WebCore::SettingsBase::setNeedsRelayoutAllFrames() WebCore::Settings::setAsyncFrameScrollingEnabled(bool) WebKit::InjectedBundle::setAsyncFrameScrollingEnabled(WebKit::WebPageGroupProxy*, bool) WKBundleSetAsyncFrameScrollingEnabled WTR::InjectedBundle::beginTesting It's not clear to me why because async frame scrolling is disabled by default and WKBundleSetAsyncFrameScrollingEnabled forces it to be disable, so Settings::setAsyncFrameScrollingEnabled should just ignore the change. I'll try to check why later, maybe it's not specific to async frame scrolling.

Committed r229361: <https://trac.webkit.org/changeset/229361>