Bug 172861

Summary: ResourceLoadStatistics are not using unique paths during test runs
Product: WebKit Reporter: Brent Fulgham <bfulgham>
Component: WebKit2Assignee: Chris Dumez <cdumez>
Status: RESOLVED FIXED    
Severity: Normal CC: aestes, beidson, bfulgham, cdumez, commit-queue, darin, webkit-bug-importer, wilander
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch none

Brent Fulgham
Reported 2017-06-02 09:53:34 PDT
The ResourceLoadStatistics data store is not being set to a unique path when running tests, causing multiple processes to interact with the same file on disk. This is causing crashes in the test system.
Attachments
Patch (10.33 KB, patch)
2017-06-02 14:41 PDT, Chris Dumez 		no flags
DetailsFormatted DiffDiff
View All Add attachment proposed patch, testcase, etc.
Chris Dumez
Comment 1 2017-06-02 13:53:25 PDT
<rdar://problem/32442251>
Chris Dumez
Comment 2 2017-06-02 14:41:38 PDT
Created attachment 311863 [details] Patch
Brent Fulgham
Comment 3 2017-06-02 14:59:09 PDT
Comment on attachment 311863 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=311863&action=review > Source/WebKit2/UIProcess/API/APIProcessPoolConfiguration.cpp:71 > + configuration->m_resourceLoadStatisticsDirectory = legacyConfiguration.resourceLoadStatisticsDirectory; Do we need this same change in ProcessPoolConfiguration::createWithLegacyOptions?
Chris Dumez
Comment 4 2017-06-02 15:11:15 PDT
Comment on attachment 311863 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=311863&action=review >> Source/WebKit2/UIProcess/API/APIProcessPoolConfiguration.cpp:71 >> + configuration->m_resourceLoadStatisticsDirectory = legacyConfiguration.resourceLoadStatisticsDirectory; > > Do we need this same change in ProcessPoolConfiguration::createWithLegacyOptions? ProcessPoolConfiguration::createWithLegacyOptions() calls ProcessPoolConfiguration::create(), which will call the default constructor of ProcessPoolConfiguration which initializes m_resourceLoadStatisticsDirectory to WebsiteDataStore::defaultResourceLoadStatisticsDirectory() (in my patch). So it looks fine to me. Am I missing something? I don't think we have a legacy path for resource statistics so I assume using the default one is fine?
WebKit Commit Bot
Comment 5 2017-06-02 16:48:54 PDT
Comment on attachment 311863 [details] Patch Clearing flags on attachment: 311863 Committed r217743: <http://trac.webkit.org/changeset/217743>
WebKit Commit Bot
Comment 6 2017-06-02 16:48:55 PDT
All reviewed patches have been landed. Closing bug.
Brent Fulgham
Comment 7 2017-06-02 17:20:41 PDT
Comment on attachment 311863 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=311863&action=review >>> Source/WebKit2/UIProcess/API/APIProcessPoolConfiguration.cpp:71 >>> + configuration->m_resourceLoadStatisticsDirectory = legacyConfiguration.resourceLoadStatisticsDirectory; >> >> Do we need this same change in ProcessPoolConfiguration::createWithLegacyOptions? > > ProcessPoolConfiguration::createWithLegacyOptions() calls ProcessPoolConfiguration::create(), which will call the default constructor of ProcessPoolConfiguration which initializes m_resourceLoadStatisticsDirectory to WebsiteDataStore::defaultResourceLoadStatisticsDirectory() (in my patch). So it looks fine to me. Am I missing something? I don't think we have a legacy path for resource statistics so I assume using the default one is fine? You are right: we have no legacy data to worry about here.
Note You need to log in before you can comment on or make changes to this bug.