Bug 177484

A simpler test, in case it helps: https://event-loop-tests.glitch.me/raf-timing-test.html.

<rdar://problem/36523583>

This was the cause of https://github.com/mapbox/mapbox-gl-js/issues/5390, which necessitated an elaborate workaround. Would appreciate a fix so that WebKit is spec-compliant.

Also committing the paint can happen multiple times between two requestAnimationFrames. See the attached test case.

Created attachment 356578 [details] Multiple paint commits between two requestAnimationFrames

Mass move bugs into the DOM component.

Created attachment 361620 [details] Patch

Comment on attachment 361620 [details] Patch Attachment 361620 [details] did not pass mac-ews (mac): Output: https://webkit-queues.webkit.org/results/11096358 Number of test failures exceeded the failure limit.

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

Comment on attachment 361620 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=361620&action=review > Source/WebCore/dom/Document.cpp:6296 > +void Document::serviceScriptedAnimations() I think we should rename everything that refers to "scripted aninations" to say "requestAnimationFrame" to make it easier to understand. > Source/WebCore/page/DisplayRefreshEventManager.cpp:46 > + for (Frame* frame = &m_page.mainFrame(); frame; frame = frame->tree().traverseNext()) { > + if (frame->document()) > + frame->document()->serviceScriptedAnimations(); > + } Why does the code here go through all the frames... > Source/WebCore/page/DisplayRefreshEventManager.cpp:49 > + m_page.updateIntersectionObservations(); But this is a single function on Page? > Source/WebCore/page/DisplayRefreshEventManager.h:35 > +class DisplayRefreshEventManager { We should try to avoid the name "manager" on anything: https://blog.codinghorror.com/i-shall-call-it-somethingmanager/ Does this really manage display refresh events? Why do we even need it? All the code could live in Page. > Source/WebCore/page/DisplayRefreshScheduler.h:36 > +class DisplayRefreshScheduler This doesn't schedule display refreshes (that's a hardware thing). It's really a paint (render) scheduler. > Source/WebCore/page/DisplayRefreshScheduler.h:49 > + void scheduleRefresh(); "refresh" is a bit vague. Should be "scheduleRender". > Source/WebCore/page/Page.h:866 > + std::unique_ptr<DisplayRefreshScheduler> m_refreshScheduler; Why not just have It by value? > Source/WebCore/page/Page.h:867 > + DisplayRefreshEventManager m_refreshEventManager { *this }; I think this should go away.

Comment on attachment 361620 [details] Patch Attachment 361620 [details] did not pass mac-debug-ews (mac): Output: https://webkit-queues.webkit.org/results/11096356 Number of test failures exceeded the failure limit.

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

Comment on attachment 361620 [details] Patch Attachment 361620 [details] did not pass mac-wk2-ews (mac-wk2): Output: https://webkit-queues.webkit.org/results/11096423 New failing tests: animations/animation-multiple-callbacks-timestamp.html legacy-animation-engine/animations/animation-multiple-callbacks-timestamp.html fast/animation/request-animation-frame-in-two-pages.html media/modern-media-controls/slider/slider-value.html imported/w3c/web-platform-tests/web-animations/timing-model/timelines/document-timelines.html webanimations/accelerated-transition-interrupted-on-composited-element.html

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

Created attachment 361718 [details] Patch

Created attachment 361727 [details] Patch

Comment on attachment 361727 [details] Patch Attachment 361727 [details] did not pass mac-ews (mac): Output: https://webkit-queues.webkit.org/results/11115312 Number of test failures exceeded the failure limit.

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

Comment on attachment 361727 [details] Patch Attachment 361727 [details] did not pass mac-wk2-ews (mac-wk2): Output: https://webkit-queues.webkit.org/results/11115742 New failing tests: accessibility/mac/slider-allows-title-ui-element.html legacy-animation-engine/animations/animation-multiple-callbacks-timestamp.html fast/animation/request-animation-frame-in-two-pages.html media/modern-media-controls/slider/slider-value.html imported/w3c/web-platform-tests/web-animations/timing-model/timelines/document-timelines.html scrollingcoordinator/scrolling-tree/toggle-coordinated-frame-scrolling.html webanimations/accelerated-transition-interrupted-on-composited-element.html animations/animation-multiple-callbacks-timestamp.html

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

Created attachment 361750 [details] Patch

Created attachment 361752 [details] Patch

Comment on attachment 361752 [details] Patch Attachment 361752 [details] did not pass mac-ews (mac): Output: https://webkit-queues.webkit.org/results/11116833 New failing tests: webanimations/accelerated-transition-interrupted-on-composited-element.html fast/animation/request-animation-frame-remove-client.html imported/w3c/web-platform-tests/web-animations/timing-model/timelines/update-and-send-events.html

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

Comment on attachment 361752 [details] Patch Attachment 361752 [details] did not pass mac-wk2-ews (mac-wk2): Output: https://webkit-queues.webkit.org/results/11117017 New failing tests: accessibility/mac/slider-allows-title-ui-element.html legacy-animation-engine/animations/animation-multiple-callbacks-timestamp.html fast/animation/request-animation-frame-in-two-pages.html media/modern-media-controls/slider/slider-value.html imported/w3c/web-platform-tests/web-animations/timing-model/timelines/document-timelines.html scrollingcoordinator/scrolling-tree/toggle-coordinated-frame-scrolling.html webanimations/accelerated-transition-interrupted-on-composited-element.html

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

Comment on attachment 361752 [details] Patch Attachment 361752 [details] did not pass ios-sim-ews (ios-simulator-wk2): Output: https://webkit-queues.webkit.org/results/11117138 New failing tests: scrollingcoordinator/scrolling-tree/toggle-coordinated-frame-scrolling.html animations/animation-multiple-callbacks-timestamp.html legacy-animation-engine/animations/animation-multiple-callbacks-timestamp.html imported/w3c/web-platform-tests/web-animations/timing-model/timelines/document-timelines.html

Created attachment 361764 [details] Archive of layout-test-results from ews125 for ios-simulator-wk2 The attached test failures were seen while running run-webkit-tests on the ios-sim-ews. Bot: ews125 Port: ios-simulator-wk2 Platform: Mac OS X 10.13.6

Created attachment 361769 [details] Patch

Comment on attachment 361752 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=361752&action=review > Source/WebCore/ChangeLog:10 > + 1. Servicing requestAnimationFrame callbacks happens when the DisplayLink I think it's clearer to write this in the past tense, since you're describing the old behavior: "happened". > Source/WebCore/ChangeLog:14 > + 2. Javascript may try to refresh the screen more than 60 FPS. WebCore I'd say "style changes and layout triggered by script" could trigger painting at more than 60fps. > Source/WebCore/ChangeLog:16 > + currently runs the layout and commits the layer although the graphical > + system will throttle to 60 FPS at the end. This could be more precise: "CoreAnimation commits could happen at more than 60fps, although WindowServer will throttle those, and only some will be shown on the screen." > Source/WebCore/ChangeLog:24 > + 1. DisplayMonitor callback will scheduleCompositingLayerFlush() instead > + of servicing requestAnimationFrame callbacks. > + > + 2. When the page is about to be displayed, requestAnimationFrame callbacks > + will be served. I think this needs a bit more explanation. Something like: "This change introduces a new paint scheduling model where painting is driven by a "RenderingScheduler", which only triggers paints once per 16.7ms frame. Code that previously scheduled a compositing layer flush now schedules a "render", and that render is driven by a DisplayRefreshMonitor callback. When the render happens, we service requestAnimationFrame callbacks, intersection observerations and Web Animations per the "Update the rendering" step of the HTML Event Loop specification <https://html.spec.whatwg.org/multipage/webappapis.html#update-the-rendering>. In the future, more rendering steps will be added to this code." > Source/WebCore/page/RenderScheduler.h:36 > +class RenderScheduler Let's call this RenderingScheduler. > Source/WebKit/WebProcess/WebPage/RemoteLayerTree/RemoteLayerTreeDrawingArea.mm:352 > + m_webPage.processPreLayoutActions(); > m_webPage.layoutIfNeeded(); > - m_webPage.willDisplayPage(); > + m_webPage.processPreRenderActions(); I think we should try to have one function on Page that does all these, since they are part of the HTML event loop. > Source/WebKit/WebProcess/WebPage/mac/TiledCoreAnimationDrawingArea.mm:471 > + m_webPage.processPreLayoutActions(); > m_webPage.layoutIfNeeded(); > m_webPage.flushPendingEditorStateUpdate(); > - m_webPage.willDisplayPage(); > + m_webPage.processPreRenderActions(); This should call the one function on Page. I'm guessing that the order of calling m_webPage.flushPendingEditorStateUpdate() probably doesn't matter (Wenson?). > Source/WebKitLegacy/mac/WebView/WebView.mm:9352 > + Page* page = m_webView->_private->page; > + page->processPreLayoutActions(); Where does processPreRenderActions() happen for WK1?

Comment on attachment 361769 [details] Patch Attachment 361769 [details] did not pass mac-wk2-ews (mac-wk2): Output: https://webkit-queues.webkit.org/results/11118531 New failing tests: scrollingcoordinator/scrolling-tree/toggle-coordinated-frame-scrolling.html legacy-animation-engine/animations/animation-multiple-callbacks-timestamp.html media/modern-media-controls/slider/slider-value.html

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

Comment on attachment 361769 [details] Patch Attachment 361769 [details] did not pass ios-sim-ews (ios-simulator-wk2): Output: https://webkit-queues.webkit.org/results/11118551 New failing tests: scrollingcoordinator/scrolling-tree/toggle-coordinated-frame-scrolling.html animations/animation-multiple-callbacks-timestamp.html legacy-animation-engine/animations/animation-multiple-callbacks-timestamp.html webanimations/accelerated-animation-suspension.html imported/w3c/web-platform-tests/web-animations/timing-model/timelines/document-timelines.html

Created attachment 361776 [details] Archive of layout-test-results from ews124 for ios-simulator-wk2 The attached test failures were seen while running run-webkit-tests on the ios-sim-ews. Bot: ews124 Port: ios-simulator-wk2 Platform: Mac OS X 10.13.6

Comment on attachment 361769 [details] Patch Attachment 361769 [details] did not pass mac-ews (mac): Output: https://webkit-queues.webkit.org/results/11119146 New failing tests: fast/animation/request-animation-frame-remove-client.html imported/w3c/web-platform-tests/web-animations/timing-model/timelines/update-and-send-events.html

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

Comment on attachment 361769 [details] Patch Attachment 361769 [details] did not pass mac-debug-ews (mac): Output: https://webkit-queues.webkit.org/results/11119139 New failing tests: fast/animation/request-animation-frame-remove-client.html compositing/backing/animate-into-view.html imported/w3c/web-platform-tests/web-animations/timing-model/timelines/document-timelines.html imported/w3c/web-platform-tests/web-animations/timing-model/timelines/update-and-send-events.html

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

Some of the layout test fail because of the following call stack: WebCore::ScriptedAnimationController::callRequestAnimationFrameCallbacks() WebCore::Document::callRequestAnimationFrameCallbacks() WebCore::Page::callRequestAnimationFrameCallbacks() WebCore::Page::processPreLayoutActions() WebKit::WebPage::processPreLayoutActions() WebKit::TiledCoreAnimationDrawingArea::flushLayers() WebKit::TiledCoreAnimationDrawingArea::forceRepaint() WebKit::WebPage::forceRepaintWithoutCallback() ::WKBundlePageForceRepaint(WKBundlePageRef) WTR::InjectedBundlePage::dump() WTR::TestRunner::notifyDone() WTR::JSTestRunner::notifyDone() long long JSC::APICallbackFunction::call<JSC::JSCallbackFunction>() 0x29473840102d () llint_entry llint_entry llint_entry vmEntryToJavaScript JSC::JITCode::execute() JSC::Interpreter::executeCall() JSC::call() JSC::call() JSC::profiledCall() WebCore::JSExecState::profiledCall() WebCore::JSCallbackData::invokeCallback() WebCore::JSCallbackDataStrong::invokeCallback() WebCore::JSRequestAnimationFrameCallback::handleEvent() WebCore::ScriptedAnimationController::callRequestAnimationFrameCallbacks() WebCore::Document::callRequestAnimationFrameCallbacks() WebCore::Page::callRequestAnimationFrameCallbacks() WebCore::Page::processPreLayoutActions() WebKit::WebPage::processPreLayoutActions() WebKit::TiledCoreAnimationDrawingArea::flushLayers() WebKit::TiledCoreAnimationDrawingArea::layerFlushRunLoopCallback() Now ScriptedAnimationController::callRequestAnimationFrameCallbacks() can end up calling itself only if a requestAnimationFrame callback forces paint. This can happen with if TestRunner::notifyDone() is called from the requestAnimationFrame callback.

Created attachment 361947 [details] Patch

Comment on attachment 361947 [details] Patch Attachment 361947 [details] did not pass mac-ews (mac): Output: https://webkit-queues.webkit.org/results/11139598 New failing tests: imported/w3c/web-platform-tests/web-animations/timing-model/timelines/update-and-send-events.html imported/w3c/web-platform-tests/web-animations/timing-model/timelines/document-timelines.html

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

Comment on attachment 361947 [details] Patch Attachment 361947 [details] did not pass mac-wk2-ews (mac-wk2): Output: https://webkit-queues.webkit.org/results/11139690 New failing tests: scrollingcoordinator/scrolling-tree/toggle-coordinated-frame-scrolling.html accessibility/mac/slider-allows-title-ui-element.html webanimations/accelerated-transition-interrupted-on-composited-element.html media/modern-media-controls/slider/slider-value.html imported/w3c/web-platform-tests/web-animations/timing-model/timelines/document-timelines.html

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

Comment on attachment 361947 [details] Patch Attachment 361947 [details] did not pass mac-debug-ews (mac): Output: https://webkit-queues.webkit.org/results/11139652 New failing tests: webanimations/accelerated-transition-interrupted-on-composited-element.html imported/w3c/web-platform-tests/web-animations/timing-model/timelines/update-and-send-events.html

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

Comment on attachment 361947 [details] Patch Attachment 361947 [details] did not pass ios-sim-ews (ios-simulator-wk2): Output: https://webkit-queues.webkit.org/results/11139647 New failing tests: scrollingcoordinator/scrolling-tree/toggle-coordinated-frame-scrolling.html webanimations/accelerated-animation-suspension.html

Created attachment 361974 [details] Archive of layout-test-results from ews122 for ios-simulator-wk2 The attached test failures were seen while running run-webkit-tests on the ios-sim-ews. Bot: ews122 Port: ios-simulator-wk2 Platform: Mac OS X 10.13.6

Created attachment 362845 [details] Patch

Comment on attachment 362845 [details] Patch Attachment 362845 [details] did not pass mac-ews (mac): Output: https://webkit-queues.webkit.org/results/11264756 New failing tests: legacy-animation-engine/animations/animation-direction-normal.html webanimations/accelerated-animation-suspension.html animations/animation-direction-reverse.html animations/no-style-recalc-during-accelerated-animation.html animations/play-state-in-shorthand.html webanimations/accelerated-animation-interruption-display-none.html legacy-animation-engine/animations/animation-direction-reverse.html animations/animation-direction-normal.html animations/dynamic-stylesheet-loading.html animations/play-state-paused.html

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

Comment on attachment 362845 [details] Patch Attachment 362845 [details] did not pass mac-wk2-ews (mac-wk2): Output: https://webkit-queues.webkit.org/results/11264785 New failing tests: legacy-animation-engine/animations/added-while-suspended.html compositing/video/video-clip-change-src.html animations/play-state-paused.html webanimations/accelerated-animation-suspension.html animations/animation-direction-reverse.html animations/no-style-recalc-during-accelerated-animation.html animations/added-while-suspended.html legacy-animation-engine/animations/animation-direction-normal.html transitions/start-transform-transition.html accessibility/mac/selection-notification-focus-change.html legacy-animation-engine/animations/animation-direction-reverse.html animations/animation-direction-normal.html animations/dynamic-stylesheet-loading.html webanimations/accelerated-animation-interruption-display-none.html

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

Comment on attachment 362845 [details] Patch Attachment 362845 [details] did not pass win-ews (win): Output: https://webkit-queues.webkit.org/results/11264835 Number of test failures exceeded the failure limit.

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

Comment on attachment 362845 [details] Patch Attachment 362845 [details] did not pass mac-debug-ews (mac): Output: https://webkit-queues.webkit.org/results/11264811 New failing tests: legacy-animation-engine/animations/animation-direction-normal.html webanimations/accelerated-animation-suspension.html animations/animation-direction-reverse.html animations/no-style-recalc-during-accelerated-animation.html animations/play-state-in-shorthand.html webanimations/accelerated-animation-interruption-display-none.html legacy-animation-engine/animations/animation-direction-reverse.html animations/animation-direction-normal.html animations/dynamic-stylesheet-loading.html animations/play-state-paused.html

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

Comment on attachment 362845 [details] Patch Attachment 362845 [details] did not pass ios-sim-ews (ios-simulator-wk2): Output: https://webkit-queues.webkit.org/results/11264858 New failing tests: animations/no-style-recalc-during-accelerated-animation.html webanimations/accelerated-animation-suspension.html legacy-animation-engine/animations/added-while-suspended.html animations/added-while-suspended.html webanimations/accelerated-animation-interruption-display-none.html animations/dynamic-stylesheet-loading.html

Created attachment 362851 [details] Archive of layout-test-results from ews124 for ios-simulator-wk2 The attached test failures were seen while running run-webkit-tests on the ios-sim-ews. Bot: ews124 Port: ios-simulator-wk2 Platform: Mac OS X 10.13.6

*** Bug 179293 has been marked as a duplicate of this bug. ***

The plan for this bug is to do the following: -- Schedule the page updateRendering every 16 ms. -- Service requestAnimationFrame callbacks, Web Animations and intersection observations per the "Update the rendering" step of the HTML Event Loop specification.

Created attachment 363268 [details] Patch

Created attachment 363274 [details] Patch

Comment on attachment 363274 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=363274&action=review This looks great, other than the renaming. > Source/WTF/wtf/SystemTracing.h:81 > + ScheduleUpdateRendering, > + TriggerUpdateRendering, > + UpdateRenderingStart, > + UpdateRenderingEnd, These should use the term "RenderingUpdate" > Source/WebCore/animation/DocumentTimeline.cpp:332 > +void DocumentTimeline::updateAnimationsAndSendEvents(double timestamp) double -> DOMHighResTimeStamp ? > Source/WebCore/animation/DocumentTimeline.cpp:338 > + cacheCurrentTime(Seconds(timestamp)); Would be better if cacheCurrentTime took a DOMHighResTimeStamp I think. > Source/WebCore/dom/Document.cpp:6259 > +void Document::updateAnimationsAndSendEvents(double timestamp) DOMHighResTimeStamp > Source/WebCore/dom/Document.cpp:6265 > +void Document::serviceRequestAnimationFrameCallbacks(double timestamp) DOMHighResTimeStamp > Source/WebCore/dom/ScriptedAnimationController.cpp:191 > +void ScriptedAnimationController::serviceRequestAnimationFrameCallbacks(double timestamp) DOMHighResTimeStamp > Source/WebCore/dom/ScriptedAnimationController.cpp:216 > + if (callback->m_useLegacyTimeBase) We should remove this legacy code now (in another patch) > Source/WebCore/page/Page.cpp:1267 > + // This function is not reentrant, e.g. a rAF callback may force repaint. The rAF-causing forced repaint is only an issue in our test harness, right? > Source/WebCore/page/Page.cpp:1283 > + double timestamp = document->domWindow()->nowTimestamp(); DOMHighResTimeStamp > Source/WebCore/page/Page.cpp:1292 > + document->updateIntersectionObservations(); Does this not take a timestamp? I think it should. > Source/WebCore/page/Page.h:272 > + UpdateRenderingScheduler& updateRenderingScheduler(); I think "renderingUpdateScheduler" is better. > Source/WebCore/page/Page.h:870 > + std::unique_ptr<UpdateRenderingScheduler> m_updateRenderingScheduler; m_renderingUpdateScheduler > Source/WebCore/page/Page.h:972 > + bool m_inUpdateRendering { false }; m_inRenderingUpdate > Source/WebCore/page/UpdateRenderingScheduler.h:36 > +class UpdateRenderingScheduler Let's call this RenderingUpdateScheduler > Source/WebCore/page/UpdateRenderingScheduler.h:48 > + UpdateRenderingScheduler(Page&); Make private. > Source/WebCore/page/UpdateRenderingScheduler.h:49 > + void scheduleUpdateRendering(); scheduleRenderingUpdate > Source/WebCore/page/UpdateRenderingScheduler.h:68 > + bool m_updateRenderingScheduled { false }; m_scheduled is probably enough. > Tools/Tracing/SystemTracePoints.plist:209 > + <string>Schedule display refresh</string> Schedule rendering update > Tools/Tracing/SystemTracePoints.plist:219 > + <string>Trigger display refresh</string> Trigger rendering update

Created attachment 363286 [details] Patch

Created attachment 363295 [details] Patch

Comment on attachment 363295 [details] Patch Attachment 363295 [details] did not pass mac-wk2-ews (mac-wk2): Output: https://webkit-queues.webkit.org/results/11327006 New failing tests: accessibility/mac/slider-allows-title-ui-element.html

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

Comment on attachment 363295 [details] Patch Attachment 363295 [details] did not pass win-ews (win): Output: https://webkit-queues.webkit.org/results/11327055 Number of test failures exceeded the failure limit.

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

Said, why do you think it's better to use DOMHighResTimeStamp instead of Seconds in a lot of places in DocumentTimeline now?

(In reply to Antoine Quint from comment #71) > Said, why do you think it's better to use DOMHighResTimeStamp instead of > Seconds in a lot of places in DocumentTimeline now? Using DOMHighResTimeStamp makes it more clear that this is related to performance()->now(), rather than being some arbitrary Seconds interval.

(In reply to Said Abou-Hallawa from comment #63) > Created attachment 363274 [details] This patch (363274) seems to break the following API Test: TestWebKitAPI.WKAttachmentTests.AddAttachmentToConnectedImageElement https://ews-build.webkit-uat.org/#/builders/19/builds/1379 If this is a false positive, please let us know.

(In reply to Aakash Jain from comment #73) > (In reply to Said Abou-Hallawa from comment #63) > > Created attachment 363274 [details] > > This patch (363274) seems to break the following API Test: > TestWebKitAPI.WKAttachmentTests.AddAttachmentToConnectedImageElement > > https://ews-build.webkit-uat.org/#/builders/19/builds/1379 > > If this is a false positive, please let us know. I do not think this patch can break this test. The test passed locally with the attached patch.

(In reply to Said Abou-Hallawa from comment #74) > (In reply to Aakash Jain from comment #73) > > (In reply to Said Abou-Hallawa from comment #63) > > > Created attachment 363274 [details] > > > > This patch (363274) seems to break the following API Test: > > TestWebKitAPI.WKAttachmentTests.AddAttachmentToConnectedImageElement > > > > https://ews-build.webkit-uat.org/#/builders/19/builds/1379 > > > > If this is a false positive, please let us know. > > I do not think this patch can break this test. The test passed locally with > the attached patch. Actually I take this back. This test is now flaky. Out of running it ten times, it failed once with the same error in the link above. I am not sure if my patch made it flaky or not. I will confirm and fix it this is the case.

Created attachment 363417 [details] Patch

Comment on attachment 363417 [details] Patch Attachment 363417 [details] did not pass mac-wk2-ews (mac-wk2): Output: https://webkit-queues.webkit.org/results/11341392 New failing tests: accessibility/mac/slider-allows-title-ui-element.html

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

Comment on attachment 363417 [details] Patch Attachment 363417 [details] did not pass win-ews (win): Output: https://webkit-queues.webkit.org/results/11341502 Number of test failures exceeded the failure limit.

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

Created attachment 363831 [details] Patch

Created attachment 363846 [details] Patch

Created attachment 363850 [details] Patch

Comment on attachment 363850 [details] Patch Attachment 363850 [details] did not pass win-ews (win): Output: https://webkit-queues.webkit.org/results/11411228 Number of test failures exceeded the failure limit.

Created attachment 363857 [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.10.0-0.325-5-3-x86_64-64bit

Comment on attachment 363850 [details] Patch Attachment 363850 [details] did not pass mac-wk2-ews (mac-wk2): Output: https://webkit-queues.webkit.org/results/11411644 New failing tests: accessibility/mac/slider-allows-title-ui-element.html

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

Created attachment 363940 [details] Patch

Comment on attachment 363940 [details] Patch Clearing flags on attachment: 363940 Committed r242624: <https://trac.webkit.org/changeset/242624>

All reviewed patches have been landed. Closing bug.

This patch broke the animation tests in the GTK port at least.

The changes in https://trac.webkit.org/changeset/242624/webkit have broken 5 layout tests on Mac Debug: css3/filters/composited-during-animation.html media/media-controls-accessibility.html compositing/layer-creation/fixed-position-out-of-view-scaled-scroll.html compositing/geometry/layer-due-to-layer-children-switch.html compositing/geometry/layer-due-to-layer-children-deep-switch.html History: https://webkit-test-results.webkit.org/dashboards/flakiness_dashboard.html#showAllRuns=true&tests=css3%2Ffilters%2Fcomposited-during-animation.html%20media%2Fmedia-controls-accessibility.html%20compositing%2Flayer-creation%2Ffixed-position-out-of-view-scaled-scroll.html%20compositing%2Fgeometry%2Flayer-due-to-layer-children-switch.html%20compositing%2Fgeometry%2Flayer-due-to-layer-children-deep-switch.html This is reproducible using command: run-webkit-tests css3/filters/composited-during-animation.html media/media-controls-accessibility.html compositing/layer-creation/fixed-position-out-of-view-scaled-scroll.html compositing/geometry/layer-due-to-layer-children-switch.html compositing/geometry/layer-due-to-layer-children-deep-switch.html --iterations 25 -f --debug on a checkout of 242624. These tests do not fail, crash, or timeout on 242623. This will need to be rolled back if it cant be fixed soon.

I see the following assertion failure on three tests: ASSERTION FAILED: !needsLayout() ./rendering/RenderView.cpp(361) : virtual void WebCore::RenderView::paint(WebCore::PaintInfo &, const WebCore::LayoutPoint &) 1 0x4725637b9 WTFCrash 2 0x46000dfdb WTFCrashWithInfo(int, char const*, char const*, int) 3 0x4638d4b5b WebCore::RenderView::paint(WebCore::PaintInfo&, WebCore::LayoutPoint const&) 4 0x4637963e3 WebCore::RenderLayer::paintBackgroundForFragments(WTF::Vector<WebCore::LayerFragment, 1ul, WTF::CrashOnOverflow, 16ul> const&, WebCore::GraphicsContext&, WebCore::GraphicsContext&, WebCore::LayoutRect const&, bool, WebCore::RenderLayer::LayerPaintingInfo const&, WTF::OptionSet<WebCore::PaintBehavior>, WebCore::RenderObject*) 5 0x46379239e WebCore::RenderLayer::paintLayerContents(WebCore::GraphicsContext&, WebCore::RenderLayer::LayerPaintingInfo const&, WTF::OptionSet<WebCore::RenderLayer::PaintLayerFlag>) 6 0x4637b2633 WebCore::RenderLayerBacking::paintIntoLayer(WebCore::GraphicsLayer const*, WebCore::GraphicsContext&, WebCore::IntRect const&, WTF::OptionSet<WebCore::PaintBehavior>, unsigned char) 7 0x4637b2bdf WebCore::RenderLayerBacking::paintContents(WebCore::GraphicsLayer const*, WebCore::GraphicsContext&, unsigned char, WebCore::FloatRect const&, unsigned int) 8 0x4632577e6 WebCore::GraphicsLayer::paintGraphicsLayerContents(WebCore::GraphicsContext&, WebCore::FloatRect const&, unsigned int) 9 0x4632ca9dc WebCore::GraphicsLayerCA::platformCALayerPaintContents(WebCore::PlatformCALayer*, WebCore::GraphicsContext&, WebCore::FloatRect const&, unsigned int) 10 0x4606c6bc5 WebCore::PlatformCALayer::drawLayerContents(CGContext*, WebCore::PlatformCALayer*, WTF::Vector<WebCore::FloatRect, 5ul, WTF::CrashOnOverflow, 16ul>&, unsigned int) 11 0x463307f31 WebCore::TileGrid::platformCALayerPaintContents(WebCore::PlatformCALayer*, WebCore::GraphicsContext&, WebCore::FloatRect const&, unsigned int) 12 0x46099b0b1 -[WebSimpleLayer drawInContext:] 13 0x7fff5a17baaf CABackingStoreUpdate_ 14 0x7fff5a1dd325 invocation function for block in CA::Layer::display_() 15 0x7fff5a17ac90 -[CALayer _display] 16 0x46099ae9b -[WebSimpleLayer display] 17 0x7fff5a17a1bc CA::Layer::display_if_needed(CA::Transaction*) 18 0x7fff5a168447 CA::Context::commit_transaction(CA::Transaction*) 19 0x7fff5a167d20 CA::Transaction::commit() 20 0x7fff5a167a2c CA::Transaction::observer_callback(__CFRunLoopObserver*, unsigned long, void*) 21 0x7fff4f224709 __CFRUNLOOP_IS_CALLING_OUT_TO_AN_OBSERVER_CALLBACK_FUNCTION__ 22 0x7fff4f22463e __CFRunLoopDoObservers 23 0x7fff4f1c6611 __CFRunLoopRun 24 0x7fff4f1c5dfa CFRunLoopRunSpecific 25 0x7fff515445da -[NSRunLoop(NSRunLoop) runMode:beforeDate:] 26 0x7fff515444af -[NSRunLoop(NSRunLoop) run] 27 0x7fff7c68aee6 _xpc_objc_main 28 0x7fff7c68a9e5 _xpc_copy_xpcservice_dictionary 29 0x106859a45 WebKit::XPCServiceMain(int, char const**) 30 0x1067772ab WKXPCServiceMain 31 0x106276f5e main LEAK: 1 WebPageProxy

These tests are also having consistent failures on WK2 due to the change: compositing/video/video-clip-change-src.html accessibility/mac/selection-notification-focus-change.html History: https://webkit-test-results.webkit.org/dashboards/flakiness_dashboard.html#showAllRuns=true&tests=compositing%2Fvideo%2Fvideo-clip-change-src.html%20accessibility%2Fmac%2Fselection-notification-focus-change.html

I put traces in the code and it what is happening is the following. We scheduleRenderingUpdate() but before the displayRefreshFired(), the CFRunLoop asks the TileGrid to platformCALayerPaintContents(). The ASSERT(!needsLayout()) in RenderView::paint() fires because we have not done it. Page::renderingUpdate() calls layoutIfNeeded() but it is only called from displayRefreshFired().

(In reply to Said Abou-Hallawa from comment #95) > I put traces in the code and it what is happening is the following. We > scheduleRenderingUpdate() but before the displayRefreshFired(), the > CFRunLoop asks the TileGrid to platformCALayerPaintContents(). What's the backtrace for this? Is it a CACommmit?

I see this call stack after scheduleRenderingUpdate() and before displayRefreshFired(). 1 0x576c1d97b -[WebSimpleLayer setNeedsDisplay] 2 0x576945590 WebCore::PlatformCALayerCocoa::setNeedsDisplay() 3 0x57954eb62 WebCore::GraphicsLayerCA::updateDrawsContent() 4 0x57954a843 WebCore::GraphicsLayerCA::commitLayerChangesBeforeSublayers(WebCore::GraphicsLayerCA::CommitState&, float, WebCore::FloatPoint const&, bool&) 5 0x579549b7f WebCore::GraphicsLayerCA::recursiveCommitChanges(WebCore::GraphicsLayerCA::CommitState&, WebCore::TransformState const&, float, WebCore::FloatPoint const&, bool) 6 0x579549df2 WebCore::GraphicsLayerCA::recursiveCommitChanges(WebCore::GraphicsLayerCA::CommitState&, WebCore::TransformState const&, float, WebCore::FloatPoint const&, bool) 7 0x579549df2 WebCore::GraphicsLayerCA::recursiveCommitChanges(WebCore::GraphicsLayerCA::CommitState&, WebCore::TransformState const&, float, WebCore::FloatPoint const&, bool) 8 0x579549df2 WebCore::GraphicsLayerCA::recursiveCommitChanges(WebCore::GraphicsLayerCA::CommitState&, WebCore::TransformState const&, float, WebCore::FloatPoint const&, bool) 9 0x579549df2 WebCore::GraphicsLayerCA::recursiveCommitChanges(WebCore::GraphicsLayerCA::CommitState&, WebCore::TransformState const&, float, WebCore::FloatPoint const&, bool) 10 0x579549df2 WebCore::GraphicsLayerCA::recursiveCommitChanges(WebCore::GraphicsLayerCA::CommitState&, WebCore::TransformState const&, float, WebCore::FloatPoint const&, bool) 11 0x579549df2 WebCore::GraphicsLayerCA::recursiveCommitChanges(WebCore::GraphicsLayerCA::CommitState&, WebCore::TransformState const&, float, WebCore::FloatPoint const&, bool) 12 0x579549df2 WebCore::GraphicsLayerCA::recursiveCommitChanges(WebCore::GraphicsLayerCA::CommitState&, WebCore::TransformState const&, float, WebCore::FloatPoint const&, bool) 13 0x5795492f3 WebCore::GraphicsLayerCA::flushCompositingState(WebCore::FloatRect const&) 14 0x579a3e8f8 WebCore::RenderLayerCompositor::flushPendingLayerChanges(bool) 15 0x579a473cb WebCore::RenderLayerCompositor::layerTreeAsText(unsigned int) 16 0x57918bcf8 WebCore::Frame::layerTreeAsText(unsigned int) const 17 0x590ce2dae WebCore::Internals::layerTreeAsText(WebCore::Document&, unsigned short) const 18 0x590dc8323 WebCore::jsInternalsPrototypeFunctionLayerTreeAsTextBody(JSC::ExecState*, WebCore::JSInternals*, JSC::ThrowScope&) 19 0x590d79320 long long WebCore::IDLOperation<WebCore::JSInternals>::call<&(WebCore::jsInternalsPrototypeFunctionLayerTreeAsTextBody(JSC::ExecState*, WebCore::JSInternals*, JSC::ThrowScope&)), (WebCore::CastedThisErrorBehavior)0>(JSC::ExecState&, char const*) 20 0x590d7900c WebCore::jsInternalsPrototypeFunctionLayerTreeAsText(JSC::ExecState*) 21 0x2daf44e0116b 22 0x588cf83d3 llint_entry 23 0x588ce50d2 vmEntryToJavaScript 24 0x58996147e JSC::JITCode::execute(JSC::VM*, JSC::ProtoCallFrame*) 25 0x589961aaf JSC::Interpreter::executeCall(JSC::ExecState*, JSC::JSObject*, JSC::CallType, JSC::CallData const&, JSC::JSValue, JSC::ArgList const&) 26 0x589c326fc JSC::call(JSC::ExecState*, JSC::JSValue, JSC::CallType, JSC::CallData const&, JSC::JSValue, JSC::ArgList const&) 27 0x589c327ea JSC::call(JSC::ExecState*, JSC::JSValue, JSC::CallType, JSC::CallData const&, JSC::JSValue, JSC::ArgList const&, WTF::NakedPtr<JSC::Exception>&) 28 0x589c32ade JSC::profiledCall(JSC::ExecState*, JSC::ProfilingReason, JSC::JSValue, JSC::CallType, JSC::CallData const&, JSC::JSValue, JSC::ArgList const&, WTF::NakedPtr<JSC::Exception>&) 29 0x57814b45b WebCore::JSExecState::profiledCall(JSC::ExecState*, JSC::ProfilingReason, JSC::JSValue, JSC::CallType, JSC::CallData const&, JSC::JSValue, JSC::ArgList const&, WTF::NakedPtr<JSC::Exception>&) 30 0x5781ce776 WebCore::ScheduledAction::executeFunctionInContext(JSC::JSGlobalObject*, JSC::JSValue, WebCore::ScriptExecutionContext&) 31 0x5781ce1d0 WebCore::ScheduledAction::execute(WebCore::Document&)

Changing RenderLayerCompositor::layerTreeAsText() such that it calls page().renderingUpdateScheduler().scheduleRenderingUpdate() Instead of calling flushPendingLayerChanges(true); fixes the following failures: compositing/layer-creation/fixed-position-out-of-view-scaled-scroll.html compositing/geometry/layer-due-to-layer-children-switch.html compositing/geometry/layer-due-to-layer-children-deep-switch.html accessibility/mac/selection-notification-focus-change.html

It looks like iOS Simulator Debug tests are exiting early with an assertion failure after this change, too: https://build.webkit.org/builders/Apple%20iOS%2012%20Simulator%20Debug%20WK2%20(Tests)/builds/2660 ASSERTION FAILED: m_inLayerFlush /Volumes/Data/slave/ios-simulator-12-debug/build/Source/WebKit/Shared/RemoteLayerTree/RemoteLayerBackingStoreCollection.mm(92) : bool WebKit::RemoteLayerBackingStoreCollection::backingStoreWillBeDisplayed(WebKit::RemoteLayerBackingStore &) 1 0x59c0dffb9 WTFCrash 2 0x1071699db WTFCrashWithInfo(int, char const*, char const*, int) 3 0x107660b3f WebKit::RemoteLayerBackingStoreCollection::backingStoreWillBeDisplayed(WebKit::RemoteLayerBackingStore&) 4 0x107e2986f WebKit::RemoteLayerTreeContext::backingStoreWillBeDisplayed(WebKit::RemoteLayerBackingStore&) 5 0x10765d931 WebKit::RemoteLayerBackingStore::display() 6 0x108166e3f WebKit::PlatformCALayerRemote::recursiveBuildTransaction(WebKit::RemoteLayerTreeContext&, WebKit::RemoteLayerTreeTransaction&) 7 0x108167174 WebKit::PlatformCALayerRemote::recursiveBuildTransaction(WebKit::RemoteLayerTreeContext&, WebKit::RemoteLayerTreeTransaction&) 8 0x108167174 WebKit::PlatformCALayerRemote::recursiveBuildTransaction(WebKit::RemoteLayerTreeContext&, WebKit::RemoteLayerTreeTransaction&) 9 0x108167174 WebKit::PlatformCALayerRemote::recursiveBuildTransaction(WebKit::RemoteLayerTreeContext&, WebKit::RemoteLayerTreeTransaction&) 10 0x108167174 WebKit::PlatformCALayerRemote::recursiveBuildTransaction(WebKit::RemoteLayerTreeContext&, WebKit::RemoteLayerTreeTransaction&) 11 0x107e29b5e WebKit::RemoteLayerTreeContext::buildTransaction(WebKit::RemoteLayerTreeTransaction&, WebCore::PlatformCALayer&) 12 0x1072b66b0 WebKit::RemoteLayerTreeDrawingArea::flushLayers() 13 0x1072bf5c7 WTF::Function<void ()>::CallableWrapper<std::__1::__bind<void (WebKit::RemoteLayerTreeDrawingArea::*&)(), WebKit::RemoteLayerTreeDrawingArea*> >::call() 14 0x10724d2ad WTF::Function<void ()>::operator()() const 15 0x1072bc2e9 WebCore::Timer::fired() 16 0x5a2f47607 WebCore::ThreadTimers::sharedTimerFiredInternal() 17 0x5a2f4e8d1 WebCore::ThreadTimers::setSharedTimer(WebCore::SharedTimer*)::$_0::operator()() const 18 0x5a2f4e889 WTF::Function<void ()>::CallableWrapper<WebCore::ThreadTimers::setSharedTimer(WebCore::SharedTimer*)::$_0>::call() 19 0x5a01b587d WTF::Function<void ()>::operator()() const 20 0x5a2f21527 WebCore::MainThreadSharedTimer::fired() 21 0x5a2fa17e9 WebCore::timerFired(__CFRunLoopTimer*, void*) 22 0x10d180f34 __CFRUNLOOP_IS_CALLING_OUT_TO_A_TIMER_CALLBACK_FUNCTION__ 23 0x10d180b32 __CFRunLoopDoTimer 24 0x10d18039a __CFRunLoopDoTimers 25 0x10d17aa1c __CFRunLoopRun 26 0x10d179e11 CFRunLoopRunSpecific 27 0x106be9322 -[NSRunLoop(NSRunLoop) runMode:beforeDate:] 28 0x106be9492 -[NSRunLoop(NSRunLoop) run] 29 0x10e969812 _xpc_objc_main 30 0x10e96bcbd xpc_main 31 0x107646807 WebKit::XPCServiceMain(int, char const**) LEAK: 1 WebPageProxy

The change to fix the GTK port is <https://trac.webkit.org/changeset/242643>.

Reverted change in https://trac.webkit.org/r242714

The change was reverted in https://trac.webkit.org/r242714.

Here is why some of the test fire assertions on the iOS simulator. notifyDone() is causing a reentrance in RemoteLayerTreeDrawingArea::flushLayers() which should not happen: ASSERTION FAILED: !m_inLayerFlush /Volumes/Data/WebKit/OpenSource/Source/WebKit/Shared/RemoteLayerTree/RemoteLayerBackingStoreCollection.mm(46) : void WebKit::RemoteLayerBackingStoreCollection::willFlushLayers() 1 0x159e2a529 WTFCrash 2 0x10be8e91b WTFCrashWithInfo(int, char const*, char const*, int) 3 0x10c46132b WebKit::RemoteLayerBackingStoreCollection::willFlushLayers() 4 0x10c093bc0 WebKit::RemoteLayerTreeDrawingArea::flushLayers() 5 0x10c09643e WebKit::RemoteLayerTreeDrawingArea::forceRepaint() 6 0x10d13c525 WebKit::WebPage::forceRepaintWithoutCallback() 7 0x10ce1c72d WKBundlePageForceRepaint 8 0x17b3237ee WTR::InjectedBundlePage::dump() 9 0x17b346c1d WTR::TestRunner::notifyDone() 10 0x17b3392b7 WTR::JSTestRunner::notifyDone(OpaqueJSContext const*, OpaqueJSValue*, OpaqueJSValue*, unsigned long, OpaqueJSValue const* const*, OpaqueJSValue const**) 11 0x15a26f011 long long JSC::APICallbackFunction::call<JSC::JSCallbackFunction>(JSC::ExecState*) 12 0x5761b6e01027 13 0x15a1f0051 llint_entry 14 0x15a1f0051 llint_entry 15 0x15a1f0051 llint_entry 16 0x15a1dcca0 vmEntryToJavaScript 17 0x15ab52c4e JSC::JITCode::execute(JSC::VM*, JSC::ProtoCallFrame*) 18 0x15ab5327f JSC::Interpreter::executeCall(JSC::ExecState*, JSC::JSObject*, JSC::CallType, JSC::CallData const&, JSC::JSValue, JSC::ArgList const&) 19 0x15ae2821c JSC::call(JSC::ExecState*, JSC::JSValue, JSC::CallType, JSC::CallData const&, JSC::JSValue, JSC::ArgList const&) 20 0x15ae2830a JSC::call(JSC::ExecState*, JSC::JSValue, JSC::CallType, JSC::CallData const&, JSC::JSValue, JSC::ArgList const&, WTF::NakedPtr<JSC::Exception>&) 21 0x15ae285fe JSC::profiledCall(JSC::ExecState*, JSC::ProfilingReason, JSC::JSValue, JSC::CallType, JSC::CallData const&, JSC::JSValue, JSC::ArgList const&, WTF::NakedPtr<JSC::Exception>&) 22 0x15fad452b WebCore::JSExecState::profiledCall(JSC::ExecState*, JSC::ProfilingReason, JSC::JSValue, JSC::CallType, JSC::CallData const&, JSC::JSValue, JSC::ArgList const&, WTF::NakedPtr<JSC::Exception>&) 23 0x15fad43df WebCore::JSCallbackData::invokeCallback(WebCore::JSDOMGlobalObject&, JSC::JSObject*, JSC::JSValue, JSC::MarkedArgumentBuffer&, WebCore::JSCallbackData::CallbackType, JSC::PropertyName, WTF::NakedPtr<JSC::Exception>&) 24 0x15e2fc6a2 WebCore::JSCallbackDataStrong::invokeCallback(JSC::JSValue, JSC::MarkedArgumentBuffer&, WebCore::JSCallbackData::CallbackType, JSC::PropertyName, WTF::NakedPtr<JSC::Exception>&) 25 0x15eeba959 WebCore::JSRequestAnimationFrameCallback::handleEvent(double) 26 0x1601ae0fe WebCore::ScriptedAnimationController::serviceRequestAnimationFrameCallbacks(double) 27 0x16000f696 WebCore::Document::serviceRequestAnimationFrameCallbacks(double) 28 0x160b510a2 WebCore::Page::renderingUpdate() 29 0x10d13d874 WebKit::WebPage::renderingUpdate() 30 0x10c093bd0 WebKit::RemoteLayerTreeDrawingArea::flushLayers() 31 0x10c09cf11 WTF::Function<void ()>::CallableWrapper<std::__1::__bind<void (WebKit::RemoteLayerTreeDrawingArea::*&)(), WebKit::RemoteLayerTreeDrawingArea*> >::call() LEAK: 1 WebPageProxy

Created attachment 366413 [details] Patch

Comment on attachment 366413 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=366413&action=review > Source/WebCore/animation/DocumentTimeline.cpp:272 > + if (!m_cachedCurrentTime) > + cacheCurrentTime(liveCurrentTime()); > + > return m_cachedCurrentTime.value() - m_originTime; Later (not in this patch) would should move cached time management to something that knows about the event loop. > Source/WebCore/page/Page.cpp:1262 > +void Page::renderingUpdate() This should be called updateRendering() > Source/WebCore/page/Page.cpp:1295 > +} This needs another layoutIfNeeded at the end. > Source/WebKit/WebProcess/WebPage/WebPage.cpp:3644 > +void WebPage::renderingUpdate() This should be called updateRendering() > Source/WebKit/WebProcess/WebPage/mac/TiledCoreAnimationDrawingArea.mm:460 > + TraceScope traceScope(RenderingUpdateStart, RenderingUpdateEnd); This scope should go inside page::updateRendering().

Comment on attachment 366413 [details] Patch Attachment 366413 [details] did not pass mac-ews (mac): Output: https://webkit-queues.webkit.org/results/11730627 Number of test failures exceeded the failure limit.

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

Comment on attachment 366413 [details] Patch Attachment 366413 [details] did not pass mac-wk2-ews (mac-wk2): Output: https://webkit-queues.webkit.org/results/11730631 Number of test failures exceeded the failure limit.

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

Comment on attachment 366413 [details] Patch Attachment 366413 [details] did not pass mac-debug-ews (mac): Output: https://webkit-queues.webkit.org/results/11730611 Number of test failures exceeded the failure limit.

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

Comment on attachment 366413 [details] Patch Attachment 366413 [details] did not pass win-ews (win): Output: https://webkit-queues.webkit.org/results/11731036 Number of test failures exceeded the failure limit.

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

Created attachment 366731 [details] Patch

Comment on attachment 366731 [details] Patch Attachment 366731 [details] did not pass mac-ews (mac): Output: https://webkit-queues.webkit.org/results/11766605 New failing tests: imported/w3c/web-platform-tests/resize-observer/notify.html imported/w3c/web-platform-tests/resize-observer/idlharness.window.html resize-observer/multi-frames.html resize-observer/observe-element-from-other-frame.html resize-observer/modify-frametree-in-callback.html imported/w3c/web-platform-tests/resize-observer/svg.html

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

Comment on attachment 366731 [details] Patch Attachment 366731 [details] did not pass mac-wk2-ews (mac-wk2): Output: https://webkit-queues.webkit.org/results/11766612 New failing tests: imported/w3c/web-platform-tests/resize-observer/idlharness.window.html accessibility/mac/selection-notification-focus-change.html

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

Comment on attachment 366731 [details] Patch Attachment 366731 [details] did not pass mac-debug-ews (mac): Output: https://webkit-queues.webkit.org/results/11766795 New failing tests: imported/w3c/web-platform-tests/resize-observer/notify.html imported/w3c/web-platform-tests/resize-observer/idlharness.window.html resize-observer/multi-frames.html resize-observer/observe-element-from-other-frame.html resize-observer/modify-frametree-in-callback.html imported/w3c/web-platform-tests/resize-observer/svg.html

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

Comment on attachment 366731 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=366731&action=review > Source/WebCore/page/FrameViewLayoutContext.cpp:-465 > - page->scheduleResizeObservations(); To share some experiences:) The ResizeObserver failures in WK1 might related to this. The layout test in WK1 might not trigger updateRendering() somehow.

Created attachment 367083 [details] Patch

Comment on attachment 367083 [details] Patch Attachment 367083 [details] did not pass mac-wk2-ews (mac-wk2): Output: https://webkit-queues.webkit.org/results/11824006 New failing tests: accessibility/mac/selection-notification-focus-change.html

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

Created attachment 367145 [details] Patch

Comment on attachment 367145 [details] Patch Attachment 367145 [details] did not pass mac-wk2-ews (mac-wk2): Output: https://webkit-queues.webkit.org/results/11831805 New failing tests: accessibility/mac/selection-notification-focus-change.html

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

Created attachment 367160 [details] Patch

Created attachment 367186 [details] Patch

Running TestWebKitAPI.WKAttachmentTests.AddAttachmentToConnectedImageElement locally passes with no failures.

Comment on attachment 367186 [details] Patch Clearing flags on attachment: 367186 Committed r244182: <https://trac.webkit.org/changeset/244182>

All reviewed patches have been landed. Closing bug.

After changes in https://trac.webkit.org/changeset/244182/webkit the iOS Simulator Debug tester is exiting early after 50+ failures I verified several crashes happen in the animations directory within ~30 tests , and they do not happen in the previous revision. Easiest way to reproduce: run-webkit-tests animations/ --ios-simulator --debug -f https://build.webkit.org/builders/Apple%20iOS%2012%20Simulator%20Debug%20WK2%20%28Tests%29/builds/3244 https://build.webkit.org/results/Apple%20iOS%2012%20Simulator%20Debug%20WK2%20(Tests)/r244182%20(3244)/results.html Assertion failure on the crashes: ASSERTION FAILED: layerTreeTransaction.transactionID() == m_lastVisibleTransactionID + 1 /Volumes/Data/slave/ios-simulator-12-debug/build/Source/WebKit/UIProcess/RemoteLayerTree/RemoteLayerTreeDrawingAreaProxy.mm(198) : void WebKit::RemoteLayerTreeDrawingAreaProxy::commitLayerTree(const WebKit::RemoteLayerTreeTransaction &, const WebKit::RemoteScrollingCoordinatorTransaction &) 1 0x108b03f19 WTFCrash 2 0x10daf805b WTFCrashWithInfo(int, char const*, char const*, int)

(In reply to Shawn Roberts from comment #133) > After changes in https://trac.webkit.org/changeset/244182/webkit the iOS > Simulator Debug tester is exiting early after 50+ failures > > I verified several crashes happen in the animations directory within ~30 > tests , and they do not happen in the previous revision. > > Easiest way to reproduce: > > run-webkit-tests animations/ --ios-simulator --debug -f > > https://build.webkit.org/builders/ > Apple%20iOS%2012%20Simulator%20Debug%20WK2%20%28Tests%29/builds/3244 > https://build.webkit.org/results/ > Apple%20iOS%2012%20Simulator%20Debug%20WK2%20(Tests)/r244182%20(3244)/ > results.html > > > Assertion failure on the crashes: > > ASSERTION FAILED: layerTreeTransaction.transactionID() == > m_lastVisibleTransactionID + 1 > /Volumes/Data/slave/ios-simulator-12-debug/build/Source/WebKit/UIProcess/ > RemoteLayerTree/RemoteLayerTreeDrawingAreaProxy.mm(198) : void > WebKit::RemoteLayerTreeDrawingAreaProxy::commitLayerTree(const > WebKit::RemoteLayerTreeTransaction &, const > WebKit::RemoteScrollingCoordinatorTransaction &) > 1 0x108b03f19 WTFCrash > 2 0x10daf805b WTFCrashWithInfo(int, char const*, char const*, int) I remember I saw this assertion while confirming the patch locally on iOS simulator. Then I fixed it but it looks like I added another change without reconfirming the patch. I will investigate it quickly and if I need more time, I will remove the assertion temporarily after making sure this is the only issue that will make the iOS simulator passes the animation tests.

I added some logging to know why the assertion ASSERT(layerTreeTransaction.transactionID() == m_lastVisibleTransactionID + 1); fires on iOS simulator. And here are some of the messages: In RemoteLayerTreeDrawingAreaProxy::commitLayerTre(): this = 0x600000278270, layerTreeTransaction.transactionID() = 29, m_lastVisibleTransactionID = 28 In RemoteLayerTreeDrawingAreaProxy::commitLayerTre(): this = 0x600000278270, layerTreeTransaction.transactionID() = 30, m_lastVisibleTransactionID = 29 In RemoteLayerTreeDrawingAreaProxy::commitLayerTre(): this = 0x600000278270, layerTreeTransaction.transactionID() = 31, m_lastVisibleTransactionID = 29 ASSERTION FAILED: layerTreeTransaction.transactionID() == m_lastVisibleTransactionID + 1 This happens on iOS only because changing m_lastVisibleTransactionID happens asynchronously after RemoteLayerTreeDrawingAreaProxy::commitLayerTree() finishes. It happens in RemoteLayerTreeDrawingAreaProxy::didRefreshDisplay() via the call: [displayLinkHandler() schedule];

Removing the assertion make the layout tests pass on the iOS simulator.

Here is the problem. RemoteLayerTreeDrawingArea::flushLayers() calls itself when TestRunner::notifyDone() forces repaint. 3 0x1033b3acd WebKit::RemoteLayerTreeDrawingArea::flushLayers() 4 0x1033b64be WebKit::RemoteLayerTreeDrawingArea::forceRepaint() 5 0x104462f85 WebKit::WebPage::forceRepaintWithoutCallback() 6 0x10413ddbd WKBundlePageForceRepaint 7 0x4ec12346e WTR::InjectedBundlePage::dump() 8 0x4ec146afd WTR::TestRunner::notifyDone() 9 0x4ec1390a7 WTR::JSTestRunner::notifyDone(OpaqueJSContext const*, OpaqueJSValue*, OpaqueJSValue*, unsigned long, OpaqueJSValue const* const*, OpaqueJSValue const**) 10 0x4cb554d51 long long JSC::APICallbackFunction::call<JSC::JSCallbackFunction>(JSC::ExecState*) 11 0x23a354601027 12 0x4cb4d18b1 llint_entry 13 0x4cb4d18b1 llint_entry 14 0x4cb4d18b1 llint_entry 15 0x4cb4be500 vmEntryToJavaScript 16 0x4cbe3dace JSC::JITCode::execute(JSC::VM*, JSC::ProtoCallFrame*) 17 0x4cbe3e0ff JSC::Interpreter::executeCall(JSC::ExecState*, JSC::JSObject*, JSC::CallType, JSC::CallData const&, JSC::JSValue, JSC::ArgList const&) 18 0x4cc115c4c JSC::call(JSC::ExecState*, JSC::JSValue, JSC::CallType, JSC::CallData const&, JSC::JSValue, JSC::ArgList const&) 19 0x4cc115d3a JSC::call(JSC::ExecState*, JSC::JSValue, JSC::CallType, JSC::CallData const&, JSC::JSValue, JSC::ArgList const&, WTF::NakedPtr<JSC::Exception>&) 20 0x4cc11602e JSC::profiledCall(JSC::ExecState*, JSC::ProfilingReason, JSC::JSValue, JSC::CallType, JSC::CallData const&, JSC::JSValue, JSC::ArgList const&, WTF::NakedPtr<JSC::Exception>&) 21 0x4d0ec3fdb WebCore::JSExecState::profiledCall(JSC::ExecState*, JSC::ProfilingReason, JSC::JSValue, JSC::CallType, JSC::CallData const&, JSC::JSValue, JSC::ArgList const&, WTF::NakedPtr<JSC::Exception>&) 22 0x4d0ec3e8f WebCore::JSCallbackData::invokeCallback(WebCore::JSDOMGlobalObject&, JSC::JSObject*, JSC::JSValue, JSC::MarkedArgumentBuffer&, WebCore::JSCallbackData::CallbackType, JSC::PropertyName, WTF::NakedPtr<JSC::Exception>&) 23 0x4cf65d332 WebCore::JSCallbackDataStrong::invokeCallback(JSC::JSValue, JSC::MarkedArgumentBuffer&, WebCore::JSCallbackData::CallbackType, JSC::PropertyName, WTF::NakedPtr<JSC::Exception>&) 24 0x4d020deb9 WebCore::JSRequestAnimationFrameCallback::handleEvent(double) 25 0x4d15d8344 WebCore::ScriptedAnimationController::serviceRequestAnimationFrameCallbacks(double) 26 0x4d14180d6 WebCore::Document::serviceRequestAnimationFrameCallbacks(double) 27 0x4d1f29387 WebCore::Page::updateRendering() 28 0x1044642d4 WebKit::WebPage::updateRendering() 29 0x1033b3ae9 WebKit::RemoteLayerTreeDrawingArea::flushLayers() 30 0x1033bcf91 WTF::Function<void ()>::CallableWrapper<std::__1::__bind<void (WebKit::RemoteLayerTreeDrawingArea::*&)(), WebKit::RemoteLayerTreeDrawingArea*> >::call() 31 0x1032f665d WTF::Function<void ()>::operator()() const

I filed https://bugs.webkit.org/show_bug.cgi?id=196825 to track this issue.

This appears to also be causing the WinCairo Debug test bot to time out on every test: https://build.webkit.org/builders/WinCairo%2064-bit%20WKL%20Debug%20%28Tests%29/builds/1760

After changes in https://trac.webkit.org/changeset/244182/webkit the following layout test is a flaky failure I verified locally, almost 60% failure rate on WK1 Debug builds. Test was flaky on WK2 Debug before, but not nearly this failure rate. Reproduced with: run-webkit-tests inspector/canvas/recording-webgl-snapshots.html --iterations 100 -f --debug -1 produced 59 out of 100 failures. Testing with prior revisions pass. Dashboard: https://webkit-test-results.webkit.org/dashboards/flakiness_dashboard.html#showAllRuns=true&tests=inspector%2Fcanvas%2Frecording-webgl-snapshots.html Diff: --- /Volumes/Data/slave/mojave-release-tests-wk2/build/layout-test-results/inspector/canvas/recording-webgl-snapshots-expected.txt +++ /Volumes/Data/slave/mojave-release-tests-wk2/build/layout-test-results/inspector/canvas/recording-webgl-snapshots-actual.txt @@ -3,6 +3,17 @@ == Running test suite: Canvas.recordingWebGL -- Running test case: Canvas.recordingWebGL.snapshots +!! EXCEPTION: No active recording for canvas +Stack Trace: #0: _dispatchResponseToPromise (TestCombined.js:7072:29) +#1: _dispatchResponse (TestCombined.js:7034:44) +#2: dispatch (TestCombined.js:6984:35) +#3: dispatchMessageFromTarget (TestCombined.js:43009:35) +#4: dispatchMessageFromTarget (TestCombined.js:9903:51) +#5: dispatchEvent (TestCombined.js:6751:42) +#6: _dispatchEvent (TestCombined.js:7109:32) +#7: dispatch (TestCombined.js:6986:32) +#8: dispatch (TestCombined.js:6589:52) +#9: dispatchNextQueuedMessageFromBackend (TestCombined.js:7500:34) initialState: attributes: width: 300

> run-webkit-tests inspector/canvas/recording-webgl-snapshots.html --iterations 100 -f --debug -1 Is there a new bug tracking that, or is that plan to roll back?

(In reply to Alexey Proskuryakov from comment #141) > > run-webkit-tests inspector/canvas/recording-webgl-snapshots.html --iterations 100 -f --debug -1 > > Is there a new bug tracking that, or is that plan to roll back? I filed https://bugs.webkit.org/show_bug.cgi?id=196875 to track this flakiness. Do I have to mark this test flaky for WK1 till it is fixed?

I am resolving this bug since all the issues were resolved except the last one. This last issue is now tracked by https://bugs.webkit.org/show_bug.cgi?id=196875.

(In reply to Ross Kirsling from comment #139) > This appears to also be causing the WinCairo Debug test bot to time out on > every test: > https://build.webkit.org/builders/WinCairo%2064- > bit%20WKL%20Debug%20%28Tests%29/builds/1760 Filed: Bug 211798 – [WinCairo] requestAnimationFrame doesn't work since r244182 in Accelerated Compositing mode