WK1 is a single process which means the UI and the core functionalities are in the same process. If the page scrolls, it will needs update so it does scheduleRenderingUpdate(). scheduleRenderingUpdate() times calling the scheduleCompositingLayerFlush() via the DisplayRefreshMonitor. This timing on a single process makes the frame to be missed almost always. Therefore scrolling in WK1 is stuttering. Servicing rAF callbacks, WebAnimations, IntersectionObserver and ResizeObserver still have to time the next RenderingUpdate through DisplayRefreshMonitor as it is currently implemented.
Created attachment 371594 [details] Patch
<rdar://problem/50192194>
Comment on attachment 371594 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=371594&action=review > Source/WebCore/ChangeLog:22 > + -- RenderingUpdateScheduler::scheduleRenderingUpdate() is renamed to > + scheduleCompositingLayerFlush() because this is what it does when > + the DispayRefresMonitor fires. Page::updateRendering() is called > + from the flushLayers() methods. I think this is going in the wrong direction. The fact that rendering updates use composting is an implementation detail, so scheduleRenderingUpdate() is the correct name to use. > Source/WebCore/ChangeLog:24 > + -- RenderLayerCompositor::scheduleLayerFlush() will be calling will be calling -> now calls > Source/WebCore/page/RenderingUpdateScheduler.h:49 > + WEBCORE_EXPORT void scheduleCompositingLayerFlush(); Keep the old name. > Source/WebCore/page/RenderingUpdateScheduler.h:50 > + void scheduleCompositingLayerFlushImmediately(); Maybe this should be called scheduleImmediateRenderingUpdate? Or maybe scheduleRenderingUpdate () should take an enum RenderingUpdateType { Immediate, PerScreenUpdate } > Source/WebCore/rendering/RenderLayerCompositor.cpp:469 > + page().chrome().client().scheduleCompositingLayerFlush(); Why does this not go through renderingUpdateScheduler? > Tools/Tracing/SystemTracePoints.plist:-275 > - <string>Trigger rendering update</string> > - <key>Type</key> > - <string>Impulse</string> > - <key>Component</key> > - <string>47</string> > - <key>Code</key> > - <string>5029</string> > - </dict> > - <dict> > - <key>Name</key> > - <string>Rendering update</string> > - <key>Type</key> > - <string>Interval</string> > - <key>Component</key> > - <string>47</string> > - <key>CodeBegin</key> > - <string>5030</string> > - <key>CodeEnd</key> > - <string>5031</string> > - </dict> > - <dict> > - <key>Name</key> > - <string>Schedule rendering update</string> > - <key>Type</key> > - <string>Impulse</string> > - <key>Component</key> > - <string>47</string> > - <key>Code</key> > - <string>5028</string> > - </dict> > - <dict> > - <key>Name</key> > - <string>Trigger rendering update</string> Don't remove keys. This breaks backwards compatibility.
Created attachment 371603 [details] Patch
Comment on attachment 371594 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=371594&action=review >> Source/WebCore/ChangeLog:22 >> + from the flushLayers() methods. > > I think this is going in the wrong direction. The fact that rendering updates use composting is an implementation detail, so scheduleRenderingUpdate() is the correct name to use. Current name was kept. >> Source/WebCore/ChangeLog:24 >> + -- RenderLayerCompositor::scheduleLayerFlush() will be calling > > will be calling -> now calls Fixed. >> Source/WebCore/page/RenderingUpdateScheduler.h:49 >> + WEBCORE_EXPORT void scheduleCompositingLayerFlush(); > > Keep the old name. Done. >> Source/WebCore/page/RenderingUpdateScheduler.h:50 >> + void scheduleCompositingLayerFlushImmediately(); > > Maybe this should be called scheduleImmediateRenderingUpdate? Or maybe scheduleRenderingUpdate () should take an enum RenderingUpdateType { Immediate, PerScreenUpdate } Names was changed to scheduleImmediateRenderingUpdate(). >> Source/WebCore/rendering/RenderLayerCompositor.cpp:469 >> + page().chrome().client().scheduleCompositingLayerFlush(); > > Why does this not go through renderingUpdateScheduler? RenderingUpdateScheduler::schedulePerScreenRenderingUpdate was added and was called here. >> Tools/Tracing/SystemTracePoints.plist:-275 >> - <string>Trigger rendering update</string> > > Don't remove keys. This breaks backwards compatibility. I am removing repeated entries with the codes: 5028, 5029, 5030 and 5031. In the atrace these entries appear twice.
Comment on attachment 371603 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=371603&action=review > Source/WebKit/WebProcess/WebCoreSupport/WebChromeClient.cpp:927 > + if (Page* corePage = m_page.corePage()) > + corePage->renderingUpdateScheduler().scheduleRenderingUpdate(); This is backwards. ChromeClient is how WebCore talks to WebKit, but here WebChromeClient is going back and calling a function on Page, which is in WebCore. Callers of scheduleCompositingLayerFlush() should just change to call renderingUpdateScheduler().scheduleRenderingUpdate(). > Source/WebKitLegacy/mac/WebCoreSupport/WebChromeClient.h:179 > + void scheduleCompositingLayerFlushImmediately() final; scheduleImmediateCompositingLayerFlush
Created attachment 371626 [details] Patch
Attachment 371626 [details] did not pass style-queue: ERROR: Source/WebCore/page/ChromeClient.h:323: Inline functions should not be in classes annotated with WEBCORE_EXPORT. Remove the macro from the class and apply it to each appropriate method, or move the inline function definition out-of-line. [build/webcore_export] [4] Total errors found: 1 in 10 files If any of these errors are false positives, please file a bug against check-webkit-style.
Comment on attachment 371626 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=371626&action=review > Source/WebCore/page/RenderingUpdateScheduler.h:51 > void scheduleRenderingUpdate(); > - void scheduleCompositingLayerFlush(); > + void scheduleImmediateRenderingUpdate(); > + void schedulePerScreenRenderingUpdate(); Let's call these: void scheduleTimedRenderingUpdate(); void scheduleImmediateRenderingUpdate(); void scheduleRenderingUpdate();
Created attachment 371638 [details] Patch
Attachment 371638 [details] did not pass style-queue: ERROR: Source/WebCore/page/ChromeClient.h:323: Inline functions should not be in classes annotated with WEBCORE_EXPORT. Remove the macro from the class and apply it to each appropriate method, or move the inline function definition out-of-line. [build/webcore_export] [4] Total errors found: 1 in 17 files If any of these errors are false positives, please file a bug against check-webkit-style.
Comment on attachment 371638 [details] Patch Attachment 371638 [details] did not pass win-ews (win): Output: https://webkit-queues.webkit.org/results/12414717 Number of test failures exceeded the failure limit.
Created attachment 371642 [details] Archive of layout-test-results from ews213 for win-future The attached test failures were seen while running run-webkit-tests on the win-ews. Bot: ews213 Port: win-future Platform: CYGWIN_NT-10.0-17763-3.0.5-338.x86_64-x86_64-64bit
Created attachment 371644 [details] Patch
Attachment 371644 [details] did not pass style-queue: ERROR: Source/WebCore/page/ChromeClient.h:323: Inline functions should not be in classes annotated with WEBCORE_EXPORT. Remove the macro from the class and apply it to each appropriate method, or move the inline function definition out-of-line. [build/webcore_export] [4] Total errors found: 1 in 15 files If any of these errors are false positives, please file a bug against check-webkit-style.
Comment on attachment 371644 [details] Patch Clearing flags on attachment: 371644 Committed r246231: <https://trac.webkit.org/changeset/246231>
All reviewed patches have been landed. Closing bug.