Bug 209410 - WebKitTestRunner should enable ResourceLoadStatistics also for non-Cocoa ports
Summary: WebKitTestRunner should enable ResourceLoadStatistics also for non-Cocoa ports
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebKit2 (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Fujii Hironori
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2020-03-23 00:09 PDT by Fujii Hironori
Modified: 2020-03-27 01:02 PDT (History)
7 users (show)

See Also:


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

Note You need to log in before you can comment on or make changes to this bug.
Description Fujii Hironori 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
Comment 1 Fujii Hironori 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]
Comment 2 Fujii Hironori 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];
Comment 3 Fujii Hironori 2020-03-23 01:28:49 PDT
Created attachment 394244 [details]
WIP patch
Comment 4 Fujii Hironori 2020-03-23 13:37:46 PDT
Created attachment 394295 [details]
Patch
Comment 5 John Wilander 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?
Comment 6 Fujii Hironori 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?
Comment 7 John Wilander 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
Comment 8 Fujii Hironori 2020-03-24 14:18:08 PDT
Created attachment 394410 [details]
Patch
Comment 9 youenn fablet 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.
Comment 10 Fujii Hironori 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
Comment 11 Fujii Hironori 2020-03-26 13:59:22 PDT
Committed r259076: <https://trac.webkit.org/changeset/259076>
Comment 12 Radar WebKit Bug Importer 2020-03-26 14:00:16 PDT
<rdar://problem/60935096>