Summary: | Make it possible to test non-stable-state scrolling on iOS | ||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Simon Fraser (smfr) <simon.fraser> | ||||||||
Component: | Tools / Tests | Assignee: | Simon Fraser (smfr) <simon.fraser> | ||||||||
Status: | RESOLVED FIXED | ||||||||||
Severity: | Normal | CC: | koivisto, lforschler, ossy, simon.fraser, thorton | ||||||||
Priority: | P2 | ||||||||||
Version: | WebKit Nightly Build | ||||||||||
Hardware: | Unspecified | ||||||||||
OS: | Unspecified | ||||||||||
Bug Depends on: | 164967 | ||||||||||
Bug Blocks: | 164855 | ||||||||||
Attachments: |
|
Description
Simon Fraser (smfr)
2016-11-17 19:49:45 PST
I have a patch but the associated layer tree dump, with visible and coverage rects, is flakey. The source of the flakiness is that sometimes TileController::adjustTileCoverageRect() has CoverageForVisibleArea and sometimes CoverageForVerticalScrolling. That's affected by FrameView::speculativeTilingEnabled() which is in turn affected by FrameView::wasScrolledByUser(). FrameView:: wasScrolledByUser() is being set via EventHandler::sendScrollEvent() (why?). This is called from FrameView::scrollPositionChanged(), which uses the eventThrottlingDelay() from WebKit2. There are also timers at various points in this chain. It's all very hard to get stable for testing. In my particular case m_estimatedLatency is 16ms so we decide to send a scroll event, setting FrameView's scrolledByUser, which then triggers additional tile coverage. Created attachment 295133 [details]
Patch
Created attachment 295135 [details]
Patch
Need to redo parts of this patch after bug 164980. Created attachment 295264 [details]
Patch
Comment on attachment 295264 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=295264&action=review > Source/WebKit2/ChangeLog:11 > + scrolling and layer trees in these transients states. “transients”? > Source/WebKit2/UIProcess/API/Cocoa/WKWebViewPrivate.h:293 > +@property (nonatomic, setter=_setStableStateOverride:) NSNumber *_stableStateOverride WK_API_AVAILABLE(ios(WK_IOS_TBA)); Can be readonly in this class, with subclasses redeclaring as readwrite. > Tools/WebKitTestRunner/cocoa/TestRunnerWKWebView.mm:50 > +@property (nonatomic, copy) NSNumber *testRunnerStableStateOverride; Strange to add this level of indirection. Can just declare a RetainPtr<NSNumber> ivar to back the existing property. > Tools/WebKitTestRunner/ios/UIScriptControllerIOS.mm:63 > + [webView _doAfterNextPresentationUpdate:^ { Extra space after the ^, and I thought we preferred C++ lambdas with explicit capture over Objective-C blocks. > Tools/WebKitTestRunner/ios/UIScriptControllerIOS.mm:423 > + if (overrideValue) > + webView._stableStateOverride = @(overrideValue.value()); > + else > + webView._stableStateOverride = nil; Can be written more succinctly with a ternary operator. (In reply to comment #9) > https://trac.webkit.org/changeset/208926 It broke the Apple Mac build on all buildbots, see build.webkit.org for details. Fixing. |