[webkit-reviews] review denied: [Bug 198797] Ensure ITP state is relayed to Network Process on restart : [Attachment 372004] Patch

[bugzilla-daemon at webkit.org]

Sam Weinig <sam at webkit.org> has denied Brent Fulgham <bfulgham at webkit.org>'s
request for review:
Bug 198797: Ensure ITP state is relayed to Network Process on restart
https://bugs.webkit.org/show_bug.cgi?id=198797

Attachment 372004: Patch

https://bugs.webkit.org/attachment.cgi?id=372004&action=review




--- Comment #5 from Sam Weinig <sam at webkit.org> ---
Comment on attachment 372004
  --> https://bugs.webkit.org/attachment.cgi?id=372004
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=372004&action=review

> Source/WebKit/UIProcess/WebProcessPool.cpp:637
> +	      
m_networkProcess->send(Messages::NetworkProcess::SetResourceLoadStatisticsEnabl
ed(true), 0);

To avoid the unnecessary extra IPC, can this be added to
NetworkProcessCreationParameters? What is the purpose of predicating this on
!withWebsiteDataStore && !m_websiteDataStore?

> Source/WebKit/UIProcess/WebProcessPool.cpp:682
> +    // Make sure the newly-spawned NetworkProcess is in the right ITP state.
> +    if (m_itpIsEnabled && !m_websiteDataStore)
> +	  
newNetworkProcess.send(Messages::NetworkProcess::SetResourceLoadStatisticsEnabl
ed(true), 0);

This seems like duplicate work given you also call this in
ensureNetworkProcess().

> Source/WebKit/UIProcess/WebProcessPool.h:730
> +    bool m_itpIsEnabled { false };

It seems like this might make more sense as a property of the WebSiteDataStore,
rather than the network process itself.