[webkit-reviews] review denied: [Bug 187108] implement process pre-warming and allow for configuration : [Attachment 345050] Patch

[bugzilla-daemon at webkit.org]

Brady Eidson <beidson at apple.com> has denied Ben Richards
<benton_richards at apple.com>'s request for review:
Bug 187108: implement process pre-warming and allow for configuration
https://bugs.webkit.org/show_bug.cgi?id=187108

Attachment 345050: Patch

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




--- Comment #19 from Brady Eidson <beidson at apple.com> ---
Comment on attachment 345050
  --> https://bugs.webkit.org/attachment.cgi?id=345050
Patch

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

In general, I do not see the value of making this an entirely different
experimental feature from process swapping.

We're not interesting in pre-warming more than one process with current WebKit
behavior, and we also know that we always want prewarming *with* process
swapping.

Let's dump all of the configuration related to enabling/disabling this
separately.

> Source/WebKit/UIProcess/API/C/WKContext.cpp:508
> -void WKContextWarmInitialProcess(WKContextRef contextRef)
> +void WKContextWarmNewProcess(WKContextRef contextRef)
>  {
> -    toImpl(contextRef)->warmInitialProcess();
> +    toImpl(contextRef)->warmNewProcess();

This change will break Safari.

> Source/WebKit/UIProcess/API/Cocoa/WKProcessPool.mm:395
> -- (void)_warmInitialProcess
> +- (void)_warmNewProcess
>  {
> -    _processPool->warmInitialProcess();
> +    _processPool->warmNewProcess();

This may-or-may-not break Safari.

I don't think we should change it.

> Source/WebKit/UIProcess/WebProcessPool.h:159
> +    void setMaximumNumberOfPrewarmedProcesses(unsigned);

AFAICT nobody actually calls this method.