When the network process crashes, we need to notify the respawned process of the current knowledge of prevalent resources and other load statistics.
<rdar://problem/33171605>
Created attachment 314999 [details] Patch
Created attachment 315011 [details] Patch
Created attachment 315013 [details] Patch
Created attachment 315064 [details] Patch
Comment on attachment 315064 [details] Patch Attachment 315064 [details] did not pass mac-debug-ews (mac): Output: http://webkit-queues.webkit.org/results/4097651 New failing tests: storage/indexeddb/modern/new-database-after-user-delete.html
Created attachment 315071 [details] Archive of layout-test-results from ews112 for mac-elcapitan The attached test failures were seen while running run-webkit-tests on the mac-debug-ews. Bot: ews112 Port: mac-elcapitan Platform: Mac OS X 10.11.6
Created attachment 315074 [details] Patch (Rebaselined)
Comment on attachment 315074 [details] Patch (Rebaselined) View in context: https://bugs.webkit.org/attachment.cgi?id=315074&action=review Bots are red. Will do a proper review tomorrow. > Source/WebKit2/UIProcess/WebProcessPool.cpp:485 > +#if HAVE(CFNETWORK_STORAGE_PARTITIONING) The method we're calling is is not specific to partitioning (at least based on its name) and could be extended in the future to do other things. Therefore, I would move this ifdef to the implementation, where we actually call a partitioning-related method. > Source/WebKit2/UIProcess/WebResourceLoadStatisticsStore.cpp:634 > +void WebResourceLoadStatisticsStore::networkProcessDidCrash() I would call this scheduleCookiePartitioningStateReset. The "schedule" prefix is a pattern I have used for other methods in this class that dispatch to a background queue before doing something. > Source/WebKit2/UIProcess/WebResourceLoadStatisticsStore.h:154 > + void resetPartitionCookiesState(); Where is the implementation? Also, I would call this resetCookiePartitioningState. > Source/WebKit2/UIProcess/WebsiteData/WebsiteDataStore.cpp:1108 > +void WebsiteDataStore::notifyResourceLoadStatisticsNetworkProcessDidCrash() I would just call this one networkProcessDidCrash(). You could imagine it being useful for other things than load statistics.
Created attachment 315123 [details] Patch
Comment on attachment 315074 [details] Patch (Rebaselined) View in context: https://bugs.webkit.org/attachment.cgi?id=315074&action=review >> Source/WebKit2/UIProcess/WebProcessPool.cpp:485 >> +#if HAVE(CFNETWORK_STORAGE_PARTITIONING) > > The method we're calling is is not specific to partitioning (at least based on its name) and could be extended in the future to do other things. Therefore, I would move this ifdef to the implementation, where we actually call a partitioning-related method. Will do. >> Source/WebKit2/UIProcess/WebResourceLoadStatisticsStore.cpp:634 >> +void WebResourceLoadStatisticsStore::networkProcessDidCrash() > > I would call this scheduleCookiePartitioningStateReset. The "schedule" prefix is a pattern I have used for other methods in this class that dispatch to a background queue before doing something. Makes sense. >> Source/WebKit2/UIProcess/WebResourceLoadStatisticsStore.h:154 >> + void resetPartitionCookiesState(); > > Where is the implementation? Also, I would call this resetCookiePartitioningState. yes -- that's why the bots are red! Too many merges yesterday! :-) >> Source/WebKit2/UIProcess/WebsiteData/WebsiteDataStore.cpp:1108 >> +void WebsiteDataStore::notifyResourceLoadStatisticsNetworkProcessDidCrash() > > I would just call this one networkProcessDidCrash(). You could imagine it being useful for other things than load statistics. Will do.
Created attachment 315129 [details] Patch
Comment on attachment 315129 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=315129&action=review r=me if the bots are happy. > Source/WebKit2/UIProcess/WebProcessPool.cpp:485 > + m_websiteDataStore->websiteDataStore().networkProcessDidCrash(); Using websiteDataStore() instead of m_websiteDataStore would look safer as it returns a reference. > LayoutTests/http/tests/loading/resourceLoadStatistics/resources/get-cookies.php:13 > +if(!isset($_COOKIE[$_GET["name3"]])) { Could we also check that isset($_GET["name3"]) to avoid having to rebaseline other tests that do not have a name3?
Comment on attachment 315129 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=315129&action=review >> Source/WebKit2/UIProcess/WebProcessPool.cpp:485 >> + m_websiteDataStore->websiteDataStore().networkProcessDidCrash(); > > Using websiteDataStore() instead of m_websiteDataStore would look safer as it returns a reference. Will do. >> LayoutTests/http/tests/loading/resourceLoadStatistics/resources/get-cookies.php:13 >> +if(!isset($_COOKIE[$_GET["name3"]])) { > > Could we also check that isset($_GET["name3"]) to avoid having to rebaseline other tests that do not have a name3? Yep. I just tried this and it works properly. It also reduces the size of this change.
Committed r219352: <http://trac.webkit.org/changeset/219352>