Track pages preventing suppression in WebProcessProxy using RefCounter
authorbarraclough@apple.com <barraclough@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 12 Dec 2014 05:15:48 +0000 (05:15 +0000)
committerbarraclough@apple.com <barraclough@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 12 Dec 2014 05:15:48 +0000 (05:15 +0000)
https://bugs.webkit.org/show_bug.cgi?id=139108

Reviewed by Benjamin Poulain.

The networking process is allowed to app nap if all web pages are also currently in app nap.
In order to detect whether any page in any process currently requires the networking process
to be active we:
 - maintain hash sets in every WebProcessProxy of pages that are okay with suppression.
 - if anything changes, the WebContext iterates every WebProcessProxy to recompute state.

This is all crazy - all we actually need is a simple count of the number of pages that need
to prevent the networking process from entering app nap. This patch gets us half way there -
replace the HashSet with a RefCounter. Next step will be to hoist the RefCounters from the
process proxies up to the context to do away with the iteration.

* UIProcess/WebPageProxy.cpp:
(WebKit::WebPageProxy::WebPageProxy):
(WebKit::WebPageProxy::reattachToWebProcess):
    - make sure we prevent process suppression, per current viewstate & preferences.
(WebKit::WebPageProxy::dispatchViewStateChange):
    - make sure we prevent process suppression, per viewstate change.
(WebKit::WebPageProxy::updateProccessSuppressionState):
    - recompute whether we need to prevent process suppression.
(WebKit::WebPageProxy::preferencesDidChange):
    - when preferences change process supression may be disabled.
(WebKit::WebPageProxy::resetStateAfterProcessExited):
    - we need to drop the old ref, to allow a new one to be taken when we reattach.
(WebKit::WebPageProxy::isProcessSuppressible): Deleted.
    - moreged into updateProccessSuppressionState.
* UIProcess/WebPageProxy.h:
    - added m_preventProcessSuppression.
* UIProcess/WebProcessProxy.cpp:
(WebKit::WebProcessProxy::WebProcessProxy):
    - initialize m_pagesPreventingSuppression
(WebKit::WebProcessProxy::createWebPage):
    - moved to WebPageProxy::WebPageProxy
(WebKit::WebProcessProxy::addExistingWebPage):
    - moved to WebPageProxy::reattachToWebProcess
(WebKit::WebProcessProxy::removeWebPage):
    - now implicit; when the page is destroyed the RefPtr will release.
(WebKit::WebProcessProxy::pageSuppressibilityChanged): Deleted.
    - moved to WebPageProxy::dispatchViewStateChange.
(WebKit::WebProcessProxy::pagePreferencesChanged): Deleted.
    - moved to WebPageProxy::preferencesDidChange.
* UIProcess/WebProcessProxy.h:
(WebKit::WebProcessProxy::preventProcessSuppressionForPage):
    - reference count m_pagesPreventingSuppression
* UIProcess/mac/WebProcessProxyMac.mm:
(WebKit::WebProcessProxy::allPagesAreProcessSuppressible):
    - converted to use m_pagesPreventingSuppression.
    - removed guard that meant processes with no pages would keep the networking
      process from entering app nap, which made no sense.

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@177209 268f45cc-cd09-0410-ab3c-d52691b4dbfc

Source/WebKit2/ChangeLog
Source/WebKit2/UIProcess/WebPageProxy.cpp
Source/WebKit2/UIProcess/WebPageProxy.h
Source/WebKit2/UIProcess/WebProcessProxy.cpp
Source/WebKit2/UIProcess/WebProcessProxy.h
Source/WebKit2/UIProcess/mac/WebProcessProxyMac.mm

index 0041d0e..bbee842 100644 (file)
@@ -1,3 +1,59 @@
+2014-12-11  Gavin Barraclough  <barraclough@apple.com>
+
+        Track pages preventing suppression in WebProcessProxy using RefCounter
+        https://bugs.webkit.org/show_bug.cgi?id=139108
+
+        Reviewed by Benjamin Poulain.
+
+        The networking process is allowed to app nap if all web pages are also currently in app nap.
+        In order to detect whether any page in any process currently requires the networking process
+        to be active we:
+         - maintain hash sets in every WebProcessProxy of pages that are okay with suppression.
+         - if anything changes, the WebContext iterates every WebProcessProxy to recompute state.
+
+        This is all crazy - all we actually need is a simple count of the number of pages that need
+        to prevent the networking process from entering app nap. This patch gets us half way there -
+        replace the HashSet with a RefCounter. Next step will be to hoist the RefCounters from the
+        process proxies up to the context to do away with the iteration.
+
+        * UIProcess/WebPageProxy.cpp:
+        (WebKit::WebPageProxy::WebPageProxy):
+        (WebKit::WebPageProxy::reattachToWebProcess):
+            - make sure we prevent process suppression, per current viewstate & preferences.
+        (WebKit::WebPageProxy::dispatchViewStateChange):
+            - make sure we prevent process suppression, per viewstate change.
+        (WebKit::WebPageProxy::updateProccessSuppressionState):
+            - recompute whether we need to prevent process suppression.
+        (WebKit::WebPageProxy::preferencesDidChange):
+            - when preferences change process supression may be disabled.
+        (WebKit::WebPageProxy::resetStateAfterProcessExited):
+            - we need to drop the old ref, to allow a new one to be taken when we reattach.
+        (WebKit::WebPageProxy::isProcessSuppressible): Deleted.
+            - moreged into updateProccessSuppressionState.
+        * UIProcess/WebPageProxy.h:
+            - added m_preventProcessSuppression.
+        * UIProcess/WebProcessProxy.cpp:
+        (WebKit::WebProcessProxy::WebProcessProxy):
+            - initialize m_pagesPreventingSuppression
+        (WebKit::WebProcessProxy::createWebPage):
+            - moved to WebPageProxy::WebPageProxy
+        (WebKit::WebProcessProxy::addExistingWebPage):
+            - moved to WebPageProxy::reattachToWebProcess
+        (WebKit::WebProcessProxy::removeWebPage):
+            - now implicit; when the page is destroyed the RefPtr will release.
+        (WebKit::WebProcessProxy::pageSuppressibilityChanged): Deleted.
+            - moved to WebPageProxy::dispatchViewStateChange.
+        (WebKit::WebProcessProxy::pagePreferencesChanged): Deleted.
+            - moved to WebPageProxy::preferencesDidChange.
+        * UIProcess/WebProcessProxy.h:
+        (WebKit::WebProcessProxy::preventProcessSuppressionForPage):
+            - reference count m_pagesPreventingSuppression
+        * UIProcess/mac/WebProcessProxyMac.mm:
+        (WebKit::WebProcessProxy::allPagesAreProcessSuppressible):
+            - converted to use m_pagesPreventingSuppression.
+            - removed guard that meant processes with no pages would keep the networking
+              process from entering app nap, which made no sense.
+
 2014-12-11  Anders Carlsson  <andersca@apple.com>
 
         Temporarily use WebCore session storage for transient local storage
index 0f525c8..d09e47c 100644 (file)
@@ -388,6 +388,7 @@ WebPageProxy::WebPageProxy(PageClient& pageClient, WebProcessProxy& process, uin
 
     updateViewState();
     updateActivityToken();
+    updateProccessSuppressionState();
     
 #if HAVE(OUT_OF_PROCESS_LAYER_HOSTING)
     m_layerHostingMode = m_viewState & ViewState::IsInWindow ? m_pageClient.viewLayerHostingMode() : LayerHostingMode::OutOfProcess;
@@ -576,6 +577,7 @@ void WebPageProxy::reattachToWebProcess()
 
     updateViewState();
     updateActivityToken();
+    updateProccessSuppressionState();
 
 #if ENABLE(INSPECTOR)
     m_inspector = WebInspectorProxy::create(this);
@@ -659,11 +661,6 @@ void WebPageProxy::initializeWebPage()
 #endif
 }
 
-bool WebPageProxy::isProcessSuppressible() const
-{
-    return (m_viewState & ViewState::IsVisuallyIdle) && m_preferences->pageVisibilityBasedProcessSuppressionEnabled();
-}
-
 void WebPageProxy::close()
 {
     if (m_isClosed)
@@ -1245,9 +1242,7 @@ void WebPageProxy::dispatchViewStateChange()
 
     // This must happen after the SetViewState message is sent, to ensure the page visibility event can fire.
     updateActivityToken();
-
-    if (changed & ViewState::IsVisuallyIdle)
-        m_process->pageSuppressibilityChanged(this);
+    updateProccessSuppressionState();
 
     // If we've started the responsiveness timer as part of telling the web process to update the backing store
     // state, it might not send back a reply (since it won't paint anything if the web page is hidden) so we
@@ -1281,6 +1276,16 @@ void WebPageProxy::updateActivityToken()
 #endif
 }
 
+void WebPageProxy::updateProccessSuppressionState()
+{
+#if PLATFORM(COCOA)
+    if ((m_viewState & ViewState::IsVisuallyIdle) && m_preferences->pageVisibilityBasedProcessSuppressionEnabled())
+        m_preventProcessSuppression = nullptr;
+    else if (!m_preventProcessSuppression)
+        m_preventProcessSuppression = m_process->processSuppressionCounter();
+#endif
+}
+
 void WebPageProxy::layerHostingModeDidChange()
 {
     if (!isValid())
@@ -2489,7 +2494,7 @@ void WebPageProxy::preferencesDidChange()
         inspector()->enableRemoteInspection();
 #endif
 
-    m_process->pagePreferencesChanged(this);
+    updateProccessSuppressionState();
 
     m_pageClient.preferencesDidChange();
 
@@ -4558,6 +4563,9 @@ void WebPageProxy::resetStateAfterProcessExited()
 #if PLATFORM(IOS)
     m_activityToken = nullptr;
 #endif
+#if PLATFORM(COCOA)
+    m_preventProcessSuppression = nullptr;
+#endif
 
     m_isValid = false;
     m_isPageSuspended = false;
index bd398aa..766c2ec 100644 (file)
@@ -383,7 +383,6 @@ public:
     WebCore::IntSize viewSize() const;
     bool isViewVisible() const { return m_viewState & WebCore::ViewState::IsVisible; }
     bool isViewWindowActive() const;
-    bool isProcessSuppressible() const;
 
     void addMIMETypeWithCustomContentProvider(const String& mimeType);
 
@@ -959,6 +958,7 @@ private:
 
     void updateViewState(WebCore::ViewState::Flags flagsToUpdate = WebCore::ViewState::AllFlags);
     void updateActivityToken();
+    void updateProccessSuppressionState();
 
     enum class ResetStateReason {
         PageInvalidated,
@@ -1581,6 +1581,7 @@ private:
 #if PLATFORM(COCOA)
     HashMap<String, String> m_temporaryPDFFiles;
     std::unique_ptr<WebCore::RunLoopObserver> m_viewStateChangeDispatcher;
+    RefPtr<RefCounter::Count> m_preventProcessSuppression;
 #endif
         
     WebCore::ScrollPinningBehavior m_scrollPinningBehavior;
index 608e9bc..7cf7534 100644 (file)
@@ -94,6 +94,7 @@ WebProcessProxy::WebProcessProxy(WebContext& context)
     , m_mayHaveUniversalFileReadSandboxExtension(false)
     , m_customProtocolManagerProxy(this, context)
 #if PLATFORM(COCOA)
+    , m_pagesPreventingSuppressionCounter([this]() { updateProcessSuppressionState(); })
     , m_processSuppressionEnabled(false)
 #endif
     , m_numberOfTimesSuddenTerminationWasDisabled(0)
@@ -189,11 +190,6 @@ PassRefPtr<WebPageProxy> WebProcessProxy::createWebPage(PageClient& pageClient,
     RefPtr<WebPageProxy> webPage = WebPageProxy::create(pageClient, *this, pageID, configuration);
     m_pageMap.set(pageID, webPage.get());
     globalPageMap().set(pageID, webPage.get());
-#if PLATFORM(COCOA)
-    if (webPage->isProcessSuppressible())
-        m_processSuppressiblePages.add(pageID);
-    updateProcessSuppressionState();
-#endif
     return webPage.release();
 }
 
@@ -204,11 +200,6 @@ void WebProcessProxy::addExistingWebPage(WebPageProxy* webPage, uint64_t pageID)
 
     m_pageMap.set(pageID, webPage);
     globalPageMap().set(pageID, webPage);
-#if PLATFORM(COCOA)
-    if (webPage->isProcessSuppressible())
-        m_processSuppressiblePages.add(pageID);
-    updateProcessSuppressionState();
-#endif
 }
 
 void WebProcessProxy::removeWebPage(uint64_t pageID)
@@ -224,11 +215,6 @@ void WebProcessProxy::removeWebPage(uint64_t pageID)
     for (auto itemID : itemIDsToRemove)
         m_backForwardListItemMap.remove(itemID);
 
-#if PLATFORM(COCOA)
-    m_processSuppressiblePages.remove(pageID);
-    updateProcessSuppressionState();
-#endif
-
     // If this was the last WebPage open in that web process, and we have no other reason to keep it alive, let it go.
     // We only allow this when using a network process, as otherwise the WebProcess needs to preserve its session state.
     if (!m_context->usesNetworkProcess() || state() == State::Terminated || !canTerminateChildProcess())
@@ -683,32 +669,6 @@ void WebProcessProxy::didUpdateHistoryTitle(uint64_t pageID, const String& title
     m_context->historyClient().didUpdateHistoryTitle(m_context.ptr(), page, title, url, frame);
 }
 
-void WebProcessProxy::pageSuppressibilityChanged(WebKit::WebPageProxy *page)
-{
-#if PLATFORM(COCOA)
-    if (page->isProcessSuppressible())
-        m_processSuppressiblePages.add(page->pageID());
-    else
-        m_processSuppressiblePages.remove(page->pageID());
-    updateProcessSuppressionState();
-#else
-    UNUSED_PARAM(page);
-#endif
-}
-
-void WebProcessProxy::pagePreferencesChanged(WebKit::WebPageProxy *page)
-{
-#if PLATFORM(COCOA)
-    if (page->isProcessSuppressible())
-        m_processSuppressiblePages.add(page->pageID());
-    else
-        m_processSuppressiblePages.remove(page->pageID());
-    updateProcessSuppressionState();
-#else
-    UNUSED_PARAM(page);
-#endif
-}
-
 void WebProcessProxy::didSaveToPageCache()
 {
     m_context->processDidCachePage(this);
index ac1cfae..fbb170d 100644 (file)
@@ -43,6 +43,7 @@
 #include <wtf/HashMap.h>
 #include <wtf/PassRefPtr.h>
 #include <wtf/RefCounted.h>
+#include <wtf/RefCounter.h>
 
 #if PLATFORM(IOS)
 #include "ProcessThrottler.h"
@@ -118,8 +119,12 @@ public:
 
     DownloadProxy* createDownloadProxy(const WebCore::ResourceRequest&);
 
-    void pageSuppressibilityChanged(WebPageProxy*);
-    void pagePreferencesChanged(WebPageProxy*);
+#if PLATFORM(COCOA)
+    PassRefPtr<RefCounter::Count> processSuppressionCounter()
+    {
+        return m_pagesPreventingSuppressionCounter.count();
+    }
+#endif
 
     void didSaveToPageCache();
     void releasePageCache();
@@ -226,7 +231,7 @@ private:
     CustomProtocolManagerProxy m_customProtocolManagerProxy;
 
 #if PLATFORM(COCOA)
-    HashSet<uint64_t> m_processSuppressiblePages;
+    RefCounter m_pagesPreventingSuppressionCounter;
     bool m_processSuppressionEnabled;
 #endif
 
index 1968ec8..0eb89d5 100644 (file)
@@ -62,7 +62,7 @@ void WebProcessProxy::platformGetLaunchOptions(ProcessLauncher::LaunchOptions& l
 
 bool WebProcessProxy::allPagesAreProcessSuppressible() const
 {
-    return (m_processSuppressiblePages.size() == m_pageMap.size()) && !m_processSuppressiblePages.isEmpty();
+    return !m_pagesPreventingSuppressionCounter.value();
 }
 
 void WebProcessProxy::updateProcessSuppressionState()