Simplify WebResourceLoadObserver now that we have one WebProcess per session.
Created attachment 378865 [details] Patch
WinCairo failure appears to be a bot problem, unrelated to this patch.
Comment on attachment 378865 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=378865&action=review > Source/WebKit/ChangeLog:9 > + No need to pass sessionIDs around or stores statistics per sessionID. "or store" > Source/WebKit/WebProcess/WebCoreSupport/WebResourceLoadObserver.cpp:-214 > - return; Do we really not need this anymore? Even if we don't need to pass sessionID to the rest of the ITP system, surely preventing passing of observations when we should don't log for the current network session seems like a good idea? > Source/WebKit/WebProcess/WebCoreSupport/WebResourceLoadObserver.cpp:-238 > - return; Ditto. > Source/WebKit/WebProcess/WebProcess.cpp:456 > + if (parameters.resourceLoadStatisticsEnabled && !parameters.sessionID.isEphemeral()) While we don't log ITP data for ephemeral systems right now, we do want to apply these protections in the future. But I guess this is fine for now.
Comment on attachment 378865 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=378865&action=review >> Source/WebKit/WebProcess/WebCoreSupport/WebResourceLoadObserver.cpp:-214 >> - return; > > Do we really not need this anymore? Even if we don't need to pass sessionID to the rest of the ITP system, surely preventing passing of observations when we should don't log for the current network session seems like a good idea? If you look at WebProcess.cpp, you'll see that I do not even initialize the WebResourceLoadObserver if ITP is disabled or the sessionID is ephemeral. Actually, I should probably drop the shouldLogResourceLoadStatistics() method entirely, looks like I forgot.
(In reply to Chris Dumez from comment #4) > Comment on attachment 378865 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=378865&action=review > > >> Source/WebKit/WebProcess/WebCoreSupport/WebResourceLoadObserver.cpp:-214 > >> - return; > > > > Do we really not need this anymore? Even if we don't need to pass sessionID to the rest of the ITP system, surely preventing passing of observations when we should don't log for the current network session seems like a good idea? > > If you look at WebProcess.cpp, you'll see that I do not even initialize the > WebResourceLoadObserver if ITP is disabled or the sessionID is ephemeral. > > Actually, I should probably drop the shouldLogResourceLoadStatistics() > method entirely, looks like I forgot. The ITP system already has the sessionID without the WebContent process sending it to it since the NetworkConnectionToWebProcess knows the sessionID.
Comment on attachment 378865 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=378865&action=review > Source/WebKit/NetworkProcess/NetworkConnectionToWebProcess.cpp:696 > + if (auto* networkSession = this->networkSession()) { See here, it is getting the networkSession for the sessionID of the WebProcess (See implementation of networkSession()).
Comment on attachment 378865 [details] Patch Looks good. r=me.
Created attachment 378873 [details] Patch
Created attachment 378874 [details] Patch
Created attachment 378881 [details] Patch
Created attachment 378886 [details] Patch
Comment on attachment 378886 [details] Patch Clearing flags on attachment: 378886 Committed r249920: <https://trac.webkit.org/changeset/249920>
All reviewed patches have been landed. Closing bug.
<rdar://problem/55414296>