Bug 195747 – [PSON] Make sure the WebProcessCache is leveraged when relaunching a process after termination

• Status: RESOLVED
• Resolution: FIXED
• Component: WebKit2

[Chris Dumez]

Make sure the WebProcessCache is leverage when relaunching a process after termination (e.g. crash).

[Chris Dumez]

Created attachment 364662 [details]
Patch

[Geoffrey Garen]

Comment on attachment 364662 [details]
Patch

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

r=me

> Source/WebKit/UIProcess/WebPageProxy.cpp:737
> +void WebPageProxy::reattachToWebProcess(const RegistrableDomain& registrableDomain)

At some point we should probably rename this family of functions to downplay the idea that there used to be a web process. Maybe this should be "attachToWebProcess" or "launchWebProcess".

> Source/WebKit/UIProcess/WebProcessPool.cpp:1145
> +WebProcessProxy& WebProcessPool::getNewWebProcessForNavigation(WebsiteDataStore& websiteDataStore, WebPageProxy* page, const RegistrableDomain& registrableDomain)

I would call this "webProcessForNavigation".

We usually use the "get" prefix for out parameters.

Also, we might return an old web process that's in cache.

[Chris Dumez]

Comment on attachment 364662 [details]
Patch

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

>> Source/WebKit/UIProcess/WebProcessPool.cpp:1145
>> +WebProcessProxy& WebProcessPool::getNewWebProcessForNavigation(WebsiteDataStore& websiteDataStore, WebPageProxy* page, const RegistrableDomain& registrableDomain)
> 
> I would call this "webProcessForNavigation".
> 
> We usually use the "get" prefix for out parameters.
> 
> Also, we might return an old web process that's in cache.

The issue is that there is the pre-existing WebProcessPool::processForNavigation() which is called on navigation for PSON. WebProcessPool::processForNavigation() then calls this method when it wants to swap. So while I agree we need a better name, I think "webProcessForNavigation" would not work.

[Geoffrey Garen]

Comment on attachment 364662 [details]
Patch

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

>>> Source/WebKit/UIProcess/WebProcessPool.cpp:1145
>>> +WebProcessProxy& WebProcessPool::getNewWebProcessForNavigation(WebsiteDataStore& websiteDataStore, WebPageProxy* page, const RegistrableDomain& registrableDomain)
>> 
>> I would call this "webProcessForNavigation".
>> 
>> We usually use the "get" prefix for out parameters.
>> 
>> Also, we might return an old web process that's in cache.
> 
> The issue is that there is the pre-existing WebProcessPool::processForNavigation() which is called on navigation for PSON. WebProcessPool::processForNavigation() then calls this method when it wants to swap. So while I agree we need a better name, I think "webProcessForNavigation" would not work.

Maybe just call this webProcess, or webProcessForDomain?

[Chris Dumez]

Created attachment 364669 [details]
Patch

[Chris Dumez]

Comment on attachment 364669 [details]
Patch

Clearing flags on attachment: 364669

Committed r242952: <https://trac.webkit.org/changeset/242952>

[Chris Dumez]

All reviewed patches have been landed.  Closing bug.

[Radar WebKit Bug Importer]

<rdar://problem/48896266>

[Shawn Roberts]

Rolled out in https://trac.webkit.org/changeset/243003/webkit

Causing API failures on iOS Simulator

TestWebKitAPI.ProcessSwap.UseWebProcessCacheAfterTermination
        Received data during response processing, queuing it.
        Received data during response processing, queuing it.
        Received data during response processing, queuing it.
        
        /Volumes/Data/slave/ios-simulator-12-debug/build/Tools/TestWebKitAPI/Tests/WebKitCocoa/ProcessSwapOnNavigation.mm:3238
        Expected equality of these values:
          1U
            Which is: 1
          [processPool _webProcessCountIgnoringPrewarmed]
            Which is: 0
        
        
        /Volumes/Data/slave/ios-simulator-12-debug/build/Tools/TestWebKitAPI/Tests/WebKitCocoa/ProcessSwapOnNavigation.mm:3251
        Expected equality of these values:
          2U
            Which is: 2
          [processPool _webProcessCountIgnoringPrewarmed]
            Which is: 1
        
        
        /Volumes/Data/slave/ios-simulator-12-debug/build/Tools/TestWebKitAPI/Tests/WebKitCocoa/ProcessSwapOnNavigation.mm:3269
        Expected equality of these values:
          webkitPID
            Which is: 4276
          [webView _webProcessIdentifier]
            Which is: 4279

[Chris Dumez]

Committed r243009: <https://trac.webkit.org/changeset/243009>