[webkit-reviews] review denied: [Bug 100997] Add application occlusion criterion to enable/disable process suppression on Mac : [Attachment 172433] Patch

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Mon Nov 5 16:43:51 PST 2012


Alexey Proskuryakov <ap at webkit.org> has denied Kiran Muppala
<cmuppala at apple.com>'s request for review:
Bug 100997: Add application occlusion criterion to enable/disable process
suppression on Mac
https://bugs.webkit.org/show_bug.cgi?id=100997

Attachment 172433: Patch
https://bugs.webkit.org/attachment.cgi?id=172433&action=review

------- Additional Comments from Alexey Proskuryakov <ap at webkit.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=172433&action=review


> This is only there because NetworkProcessManager does not expose the pointer
to the NetworkProcess itself.  Since PluginProcessManager and
SharedWorkerProcessManager also didn't expose pointers to their underlying
processes, I didn't add a getter to NetworkProcessManager either.

I see. I suggest following the existing model then, and adding a function that
sends the message internally, instead of exposing a "sendToProcess" function.
For example, it may well make sense to send a blunt SIGSTOP to some of the
processes instead of enabling automatic suspension. Knowledge about this should
be encapsulated in process manager.

> Source/WebKit2/Platform/mac/ProcessUtilitiesMac.mm:34
> +    // The following call assumes that AutoTerminationSupport is not enabled
for ChildProcesses.

This is the text I suggested, but mentioning ChildProcess does not make sense
in ProcessUtilities. This applies to any process where this is called, whether
it is a child process or not.

> Source/WebKit2/UIProcess/API/C/WKContext.cpp:332
> +void WKContextSetProcessSuppressionSupportEnabled(bool enable)
> +{
> +    WebContext::setProcessSuppressionSupportEnabled(enable);
> +}

As long as the function name starts with "WKContext", it should operate on
WKContexts. It's just a naming scheme.

But this doesn't really feel like a per-context API.

I'm sorry that I cannot suggest what to do here. Perhaps you should consider
how the feature will evolve in near future, and make an API for that, even if
not fully implemented.


More information about the webkit-reviews mailing list