[webkit-reviews] review granted: [Bug 58485] Convert WebProcess over to using disableTermination/enableTermination : [Attachment 89470] Patch

[bugzilla-daemon at webkit.org]

Adam Roben (:aroben) <aroben at apple.com> has granted Anders Carlsson
<andersca at apple.com>'s request for review:
Bug 58485: Convert WebProcess over to using
disableTermination/enableTermination
https://bugs.webkit.org/show_bug.cgi?id=58485

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

------- Additional Comments from Adam Roben (:aroben) <aroben at apple.com>
View in context: https://bugs.webkit.org/attachment.cgi?id=89470&action=review

It's a little confusing that all the terminateIfPossible calls have turned into
a temporary disabling of termination.

> Source/WebKit2/WebProcess/WebProcess.cpp:505
>  void WebProcess::removeWebPage(uint64_t pageID)
>  {
>      m_pageMap.remove(pageID);
> -    terminateIfPossible();
> +
> +    enableTermination();

Do we need to verify that pageID was in m_pageMap?

> Source/WebKit2/WebProcess/WebProcess.cpp:733
> +    WebProcess::LocalTerminationDisabler
terminationDisabler(WebProcess::shared());

No need for either of the "WebProcess::"s.

> Source/WebKit2/WebProcess/WebProcess.cpp:755
> +    WebProcess::LocalTerminationDisabler
terminationDisabler(WebProcess::shared());

Or here.

> Source/WebKit2/WebProcess/WebCoreSupport/WebDatabaseManager.cpp:158
> +    ChildProcess::LocalTerminationDisabler
terminationDisabler(WebProcess::shared());

Why ChildProcess and not WebProcess?