Bug 87783

Oops, will investigate.

I'm not able to reproduce this when building/running with Chromium's DRT. Does this affect any other bots besides the WebKit2 one? I did add the resetting logic to InternalSettings::restoreTo (http://trac.webkit.org/changeset/118599/trunk/Source/WebCore/testing/InternalSettings.cpp), just like all the other settings.

Yes, this seems specific to WebKitTestRunner as far as I can tell.

I'm not familiar with how WebKitTestRunner is set up, but it seems to be missing a call to WebCoreTestSupport::resetInternalsObject. Where is its equivalent to resetWebViewToConsistentStateBeforeTesting?

TestController::resetStateToConsistentValues is the function. Yes, it doesn't appear to call resetInternalsObject (whoa!)

(In reply to comment #5) > TestController::resetStateToConsistentValues is the function. Yes, it doesn't appear to call resetInternalsObject (whoa!) As far as I can tell, the call wouldn't be right there, since this needs to happen in the web process. However, that does send a "reset" message to the process, which eventually ends up in InjectBundlePage::reset. Doing the reset there seems to make sense, but it crashes with: Exception Type: EXC_BAD_ACCESS (SIGSEGV) Exception Codes: KERN_INVALID_ADDRESS at 0x0000000000000000 Crashed Thread: 0 Dispatch queue: com.apple.main-thread Thread 0 Crashed: Dispatch queue: com.apple.main-thread 0 libWebCoreTestSupport.dylib 0x0000000106704b07 WebCore::toInternals(JSC::JSValue) + 23 (JSCell.h:195) 1 libWebCoreTestSupport.dylib 0x0000000106712a89 WebCoreTestSupport::resetInternalsObject(OpaqueJSContext const*) + 265 (WebCoreTestSupport.cpp:55) 2 WebKitTestRunnerInjectedBundle 0x000000010655f103 WTR::InjectedBundlePage::reset() + 185 3 WebKitTestRunnerInjectedBundle 0x000000010655d637 WTR::InjectedBundle::beginTesting(OpaqueWKDictionary const*) + 491 (InjectedBundle.cpp:235) 4 WebKitTestRunnerInjectedBundle 0x000000010655d719 WTR::InjectedBundle::didReceiveMessage(OpaqueWKString const*, void const*) + 193 (WKRetainPtr.h:85) 5 com.apple.WebKit2 0x000000010022082c WebKit::InjectedBundleClient::didReceiveMessage(WebKit::InjectedBundle*, WTF::String const&, WebKit::APIObject*) + 78 (PassRefPtr.h:51)

The actual resetting code that I'm adding is: WKBundleFrameRef frame = WKBundlePageGetMainFrame(m_page); JSGlobalContextRef context = WKBundleFrameGetJavaScriptContext(frame); WebCoreTestSupport::resetInternalsObject(context);

Created attachment 144867 [details] Patch

The patch doesn't fix the underlying issue (WebKitTestRunner not resetting internals correctly), since I'm not where to begin. However, it does fix the flakiness caused by the test that I added.

Can someone comment on what it would take to make WebKitTestRunner reset internals state? It's very surprising that it hasn't been implemented ages ago.

Also CC'ing Dominic, who implemented window.internals for WebKit2 in <http://trac.webkit.org/changeset/89530>, and Mark who reviewed the patch.

window.internals is an extremely poor mechanism for changing settings: 1. Changing a setting in one window.internals.settings object affects all frames in a page, but does not work across secondary windows. This is inconsistent. 2. If a test navigates to a different URL (with waitUntilDone/notifyDone), settings are restored from last document's InternalSettings instead of first one's, so changes made in first one just leak to future tests. 3. As seen in this bug, it's just completely broken in WebKit2.

Created attachment 144965 [details] reset internals in WTR Just fixes the WTR issue, not the cluster of Internals bugs. The reason why Mihai's code was crashing is that on first iteration, reset() is called before there is an internals object injected. We just need to do that after running a test, not before.

Comment on attachment 144965 [details] reset internals in WTR View in context: https://bugs.webkit.org/attachment.cgi?id=144965&action=review > Tools/WebKitTestRunner/InjectedBundle/InjectedBundlePage.cpp:354 > + WebCoreTestSupport::resetInternalsObject(context); FWIW, the Qt port would appear to need a DumpRenderTreeSupportQ::resetInternalsObject call, based on the injectInternalsObject callsite.

Committed <http://trac.webkit.org/changeset/119127>. I'll file a new bug for the more general Internals issues. Mihai, there are good changes in your patch, would you be willing to file a new bug for those, minus test change?

It made 2 tests crash on Qt WK2. Could you check it, please? The new bug report is https://bugs.webkit.org/show_bug.cgi?id=88064

(In reply to comment #15) > Mihai, there are good changes in your patch, would you be willing to file a new bug for those, minus test change? Filed bug 88032 and added the patch.