[webkit-reviews] review granted: [Bug 194594] [PSON] Introduce a WebContent Process cache : [Attachment 361986] Patch

[bugzilla-daemon at webkit.org]

Geoffrey Garen <ggaren at apple.com> has granted Chris Dumez <cdumez at apple.com>'s
request for review:
Bug 194594: [PSON] Introduce a WebContent Process cache
https://bugs.webkit.org/show_bug.cgi?id=194594

Attachment 361986: Patch

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




--- Comment #5 from Geoffrey Garen <ggaren at apple.com> ---
Comment on attachment 361986
  --> https://bugs.webkit.org/attachment.cgi?id=361986
Patch

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

r=me

> Source/WebKit/UIProcess/WebProcessCache.cpp:121
> +void WebProcessCache::setApplicationIsActive(bool value)

value => isActive

> Source/WebKit/UIProcess/WebProcessCache.h:48
> +    unsigned maximumSize() { return m_maximumSize; }

We usually call this capacity.

> Source/WebKit/UIProcess/WebProcessPool.cpp:1337
>  void WebProcessPool::handleMemoryPressureWarning(Critical)
>  {
> +    RELEASE_LOG(PerformanceLogging, "%p -
WebProcessPool::handleMemoryPressureWarning", this);
> +
> +    m_webProcessCache->clear();

Does the process suspension code run the low memory warning in the web process?

Relatedly, maybe the web process should listen to the low memory warning, in
case it gets the warning before the UI process does.

> Source/WebKit/UIProcess/WebProcessPool.cpp:2301
> +    // FIXME: If the SuspendedPage is for this page, then we need to destroy
the suspended page as we do not support having
> +    // multiple WebPages with the same ID in a given WebProcess currently
(https://bugs.webkit.org/show_bug.cgi?id=191166).
> +    if (&(*it)->page() == &page)
> +	   m_suspendedPages.remove(it);
>  

Do we need the same check when we retrieve a process from the web process
cache?