Bug 209410

Summary:

WebKitTestRunner should enable ResourceLoadStatistics also for non-Cocoa ports

Product:

WebKit

Reporter:

Fujii Hironori <fujii.hironori>

Component:

WebKit2

Assignee:

Fujii Hironori <fujii.hironori>

Status:

RESOLVED FIXED

Severity:

Normal

CC:

bfulgham, don.olmstead, dpino, ross.kirsling, webkit-bug-importer, wilander, youennf

Priority:

Keywords:

InRadar

Version:

WebKit Nightly Build

Hardware:

Unspecified

OS:

Unspecified

See Also:

https://bugs.webkit.org/show_bug.cgi?id=209644

Attachments:

Description	Flags
WIP patch	none
Patch	none
Patch	youennf: review+

Fujii Hironori

Reported 2020-03-23 00:09:05 PDT

[WinCairo][WebKitTestRunner] Failing ASSERT(m_resourceLoadStatistics) in NetworkSession::setThirdPartyCookieBlockingMode WinCairo debug builds always failing to start WebKitTestRunner. > ASSERTION FAILED: m_resourceLoadStatistics > ..\..\Source\WebKit\NetworkProcess/NetworkSession.cpp(238) : setThirdPartyCookieBlockingMode > 1 00007FFB58071429 WTFCrash > 2 00007FFB29931F70 WTFCrashWithInfo > 3 00007FFB2D7AC65F WebKit::NetworkSession::setThirdPartyCookieBlockingMode > 4 00007FFB2D70C368 WebKit::NetworkProcess::setShouldBlockThirdPartyCookiesForTesting > 5 00007FFB2D385104 IPC::callMemberFunctionImpl<WebKit::NetworkProcess,void (WebKit::NetworkProcess::*)(PAL::SessionID, WebCore::ThirdPartyCookieBlockingMode, WTF::CompletionHandler<void ()> &&),void (),std::tuple<PAL::SessionID,WebCore::ThirdPartyCookieBlockingMode>,0,1> > 6 00007FFB2D383444 IPC::callMemberFunction<WebKit::NetworkProcess,void (WebKit::NetworkProcess::*)(PAL::SessionID, WebCore::ThirdPartyCookieBlockingMode, WTF::CompletionHandler<void ()> &&),void (),std::tuple<PAL::SessionID,WebCore::ThirdPartyCookieBlockingMode>,std::integer_sequence<unsigned long long,0,1> > > 7 00007FFB2D31DB53 IPC::handleMessageAsync<Messages::NetworkProcess::SetShouldBlockThirdPartyCookiesForTesting,WebKit::NetworkProcess,void (WebKit::NetworkProcess::*)(PAL::SessionID, WebCore::ThirdPartyCookieBlockingMode, WTF::CompletionHandler<void ()> &&)> > 8 00007FFB2D311F61 WebKit::NetworkProcess::didReceiveNetworkProcessMessage > 9 00007FFB2D70525C WebKit::NetworkProcess::didReceiveMessage > 10 00007FFB2D936E56 IPC::Connection::dispatchMessage > 11 00007FFB2D937547 IPC::Connection::dispatchMessage > 12 00007FFB2D937BEB IPC::Connection::dispatchOneIncomingMessage > 13 00007FFB2D93AB2F IPC::Connection::enqueueIncomingMessage::<unnamed-tag>::operator() > 14 00007FFB2D93AA6C WTF::Detail::CallableWrapper<`lambda at ..\..\Source\WebKit\Platform\IPC\Connection.cpp:981:30',void>::call > 15 00007FFB580850A0 WTF::Function<void ()>::operator() > 16 00007FFB580E1B36 WTF::RunLoop::performWork > 17 00007FFB581ACC97 WTF::RunLoop::wndProc > 18 00007FFB581ACBB9 WTF::RunLoop::RunLoopWndProc > 19 00007FFB87F65C0D CallWindowProcW > 20 00007FFB87F65602 DispatchMessageW > 21 00007FFB581ACE93 WTF::RunLoop::run > 22 00007FFB2D1C931B WebKit::AuxiliaryProcessMain<WebKit::NetworkProcess,WebKit::NetworkProcessMainCurl> > 23 00007FFB2D1C90AB WebKit::NetworkProcessMain > 24 00007FF6FDDD101C main > 25 00007FF6FDDD123C __scrt_common_main_seh > 26 00007FFB87EA7BD4 BaseThreadInitThunk > 27 00007FFB884CCED1 RtlUserThreadStart

Attachments
WIP patch (2.23 KB, patch) 2020-03-23 01:28 PDT, Fujii Hironori	no flags	Details Formatted Diff Diff
Patch (8.06 KB, patch) 2020-03-23 13:37 PDT, Fujii Hironori	no flags	Details Formatted Diff Diff
Patch (7.45 KB, patch) 2020-03-24 14:18 PDT, Fujii Hironori	youennf: review+	Details Formatted Diff Diff
Show Obsolete (2) View All Add attachment proposed patch, testcase, etc.

Fujii Hironori

Comment 1 2020-03-23 00:29:09 PDT

Here is the callstack of UI process to send SetShouldBlockThirdPartyCookiesForTesting to Network process. > WebKit2.dll!WebKit::NetworkProcessProxy::setShouldBlockThirdPartyCookiesForTesting(PAL::SessionID sessionID={...}, WebCore::ThirdPartyCookieBlockingMode blockingMode=OnlyAccordingToPerDomainPolicy, WTF::CompletionHandler<void ()> && completionHandler={...}) Line 1156 C++ > WebKit2.dll!WebKit::WebsiteDataStore::setResourceLoadStatisticsShouldBlockThirdPartyCookiesForTesting(bool enabled=false, bool onlyOnSitesWithoutUserInteraction=false, WTF::CompletionHandler<void ()> && completionHandler={...}) Line 1846 C++ > WebKit2.dll!WKWebsiteDataStoreStatisticsResetToConsistentState(const OpaqueWKWebsiteDataStore * dataStoreRef=0x00000269129822e0, void * context=0x00000082018fe058, void(*)(void *) completionHandler=0x00007ffb4edbe440) Line 597 C++ > WebKitTestRunnerLib.dll!WTR::TestController::statisticsResetToConsistentState() Line 3711 C++ > WebKitTestRunnerLib.dll!WTR::TestController::resetStateToConsistentValues(const WTR::TestOptions & options={...}, WTR::TestController::ResetStage resetStage=BeforeTest) Line 1131 C++ > WebKitTestRunnerLib.dll!WTR::TestController::ensureViewSupportsOptionsForTest(const WTR::TestInvocation & test={...}) Line 822 C++ > WebKitTestRunnerLib.dll!WTR::TestController::configureViewForTest(const WTR::TestInvocation & test={...}) Line 1561 C++ > WebKitTestRunnerLib.dll!WTR::TestInvocation::invoke() Line 161 C++ > WebKitTestRunnerLib.dll!WTR::TestController::runTest(const char * inputLine=0x00000082018fe750) Line 1780 C++ > WebKitTestRunnerLib.dll!WTR::TestController::runTestingServerLoop() Line 1826 C++ > WebKitTestRunnerLib.dll!WTR::TestController::run() Line 1834 C++ > WebKitTestRunnerLib.dll!WTR::TestController::TestController(int argc=2, const char * * argv=0x000002691292e8c0) Line 168 C++ > WebKitTestRunnerLib.dll!dllLauncherEntryPoint(int argc=2, const char * * argv=0x000002691292e8c0) Line 34 C++ > WebKitTestRunner.exe!main(int argc=2, const char * * argv=0x000002691292e8c0) Line 230 C++ > [External Code]

Fujii Hironori

Comment 2 2020-03-23 01:15:30 PDT

It seems that Cocoa WebKitTestRunner always enables ResourceLoadStatistics. https://trac.webkit.org/browser/webkit/trunk/Tools/WebKitTestRunner/cocoa/TestControllerCocoa.mm?rev=258562#L75 > [globalWebViewConfiguration.websiteDataStore _setResourceLoadStatisticsEnabled:YES];

Fujii Hironori

Comment 3 2020-03-23 01:28:49 PDT

Created attachment 394244 [details] WIP patch

Fujii Hironori

Comment 4 2020-03-23 13:37:46 PDT

Created attachment 394295 [details] Patch

John Wilander

Comment 5 2020-03-23 13:53:00 PDT

Comment on attachment 394295 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=394295&action=review > Source/WebKit/ChangeLog:3 > + [WinCairo][WebKitTestRunner] Failing ASSERT(m_resourceLoadStatistics) in NetworkSession::setThirdPartyCookieBlockingMode I don't think WinCairo and WebKitTestRunner are good prefixes on a patch that changes WebKit code across platforms. Also the "Failing ASSERT …" description does not convey what's being changed. Please rephrase. > Tools/WebKitTestRunner/TestController.cpp:3115 > + WKWebsiteDataStoreSetResourceLoadStatisticsEnabled(WKContextGetWebsiteDataStore(context), true); I remember someone making changes here recently. Or was it a specific platform?

Fujii Hironori

Comment 6 2020-03-23 14:10:51 PDT

Comment on attachment 394295 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=394295&action=review Thank you for the review. >> Source/WebKit/ChangeLog:3 >> + [WinCairo][WebKitTestRunner] Failing ASSERT(m_resourceLoadStatistics) in NetworkSession::setThirdPartyCookieBlockingMode > > I don't think WinCairo and WebKitTestRunner are good prefixes on a patch that changes WebKit code across platforms. Also the "Failing ASSERT …" description does not convey what's being changed. Please rephrase. Agreed. Will fix. >> Tools/WebKitTestRunner/TestController.cpp:3115 >> + WKWebsiteDataStoreSetResourceLoadStatisticsEnabled(WKContextGetWebsiteDataStore(context), true); > > I remember someone making changes here recently. Or was it a specific platform? I can't find it. which bug?

John Wilander

Comment 7 2020-03-23 14:38:07 PDT

(In reply to Fujii Hironori from comment #6) > Comment on attachment 394295 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=394295&action=review > > Thank you for the review. > > >> Source/WebKit/ChangeLog:3 > >> + [WinCairo][WebKitTestRunner] Failing ASSERT(m_resourceLoadStatistics) in NetworkSession::setThirdPartyCookieBlockingMode > > > > I don't think WinCairo and WebKitTestRunner are good prefixes on a patch that changes WebKit code across platforms. Also the "Failing ASSERT …" description does not convey what's being changed. Please rephrase. > > Agreed. Will fix. > > >> Tools/WebKitTestRunner/TestController.cpp:3115 > >> + WKWebsiteDataStoreSetResourceLoadStatisticsEnabled(WKContextGetWebsiteDataStore(context), true); > > > > I remember someone making changes here recently. Or was it a specific platform? > > I can't find it. which bug? Found it: https://bugs.webkit.org/show_bug.cgi?id=208506

Fujii Hironori

Comment 8 2020-03-24 14:18:08 PDT

Created attachment 394410 [details] Patch

youenn fablet

Comment 9 2020-03-26 05:30:48 PDT

Comment on attachment 394410 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=394410&action=review > Tools/WebKitTestRunner/TestController.cpp:3115 > + WKWebsiteDataStoreSetResourceLoadStatisticsEnabled(WKContextGetWebsiteDataStore(context), true); This probably makes the call to _setResourceLoadStatisticsEnabled:YES in TestControlerCocoa.mm unnecessary.

Fujii Hironori

Comment 10 2020-03-26 13:32:08 PDT

Comment on attachment 394410 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=394410&action=review Thank you for the review. >> Tools/WebKitTestRunner/TestController.cpp:3115 >> + WKWebsiteDataStoreSetResourceLoadStatisticsEnabled(WKContextGetWebsiteDataStore(context), true); > > This probably makes the call to _setResourceLoadStatisticsEnabled:YES in TestControlerCocoa.mm unnecessary. No. Cocoa ports don't use this code. https://trac.webkit.org/browser/webkit/trunk/Tools/WebKitTestRunner/TestController.cpp?rev=258562#L3098

Fujii Hironori

Comment 11 2020-03-26 13:59:22 PDT

Committed r259076: <https://trac.webkit.org/changeset/259076>

Radar WebKit Bug Importer

Comment 12 2020-03-26 14:00:16 PDT

<rdar://problem/60935096>

Note You need to log in before you can comment on or make changes to this bug.