Simplify WKTR's view options
Created attachment 258519 [details] Patch
Created attachment 258523 [details] Patch
Committed r188157: <http://trac.webkit.org/changeset/188157>
Comment on attachment 258523 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=258523&action=review > Tools/WebKitTestRunner/mac/PlatformWebViewMac.mm:247 > - WKRetainPtr<WKStringRef> useThreadedScrollingKey(AdoptWK, WKStringCreateWithUTF8CString("ThreadedScrolling")); > - WKTypeRef useThreadedScrollingValue = WKDictionaryGetItemForKey(options, useThreadedScrollingKey.get()); > - bool useThreadedScrolling = useThreadedScrollingValue && WKBooleanGetValue(static_cast<WKBooleanRef>(useThreadedScrollingValue)); > + if (m_options.useThreadedScrolling != options.useThreadedScrolling) > + return false; > > - return useThreadedScrolling == [(TestRunnerWKView *)m_view useThreadedScrolling]; > + return true; This changed the behavior, because the options structure doesn't have optional values any more, but it's used as if it did. See e.g. here: void TestController::updateLayoutTypeForTest(const TestInvocation& test) { ViewOptions viewOptions; viewOptions.useFixedLayout = shouldUseFixedLayout(test); ensureViewSupportsOptions(viewOptions); } This results in WebKitTestRunner re-creating its view multiple times when running tests in tiled-drawing directory (which makes attaching to the process to debug super hard), ad likely in other misbehaviors. This code turned out to be super fragile. Another example (pre-existing) is that TestController::configureViewForTest() indirectly calls ensureViewSupportsOptions multiple times, and if any of those decides to re-create the view, then any prior adjustments are lost. While I agree with the premise of this patch, it seems more appropriate to start fresh than to further mutate from current state. I'll see if this can be easily reverted now.
Bummer, the rollout doesn't apply cleanly any more.
Anders, could you please take a look at this? The known symptom is that when running any tiled-drawing test, we end up re-creating the view in TestController::ensureViewSupportsOptions. There may be other consequences, too.