Bug 874353 - Remove CPU wake lock control from ContentParent

dom/browser-element/mochitest/priority/test_ExpectingSystemMessage.html

@@ -43,8 +43,8 @@ function runTest() {
 }
 
 addEventListener('testready', function() {
-  // Cause the CPU wake lock taken on behalf of this new process to time out
+  // Cause the grace period of priority privilege for this new process to time
-  // after 1s.
+  // out after 1s.
   SpecialPowers.pushPrefEnv(
     {set: [["dom.ipc.systemMessageCPULockTimeoutSec", 1]]},
     runTest);

dom/browser-element/mochitest/priority/test_ExpectingSystemMessage2.html

@@ -57,9 +57,9 @@ function runTest() {
 }
 
 addEventListener('testready', function() {
-  // We don't want this wake lock to time out during the test; if it did, then
+  // We don't want the grace period of priority privilege to time out during the
-  // we might see BACKGROUND priority instead of BACKGROUND_PERCEIVABLE.  So
+  // test; should it really happen, we would see BACKGROUND priority instead of
-  // set the timeout to a large value.
+  // BACKGROUND_PERCEIVABLE.  So set the timeout to a large value.
   SpecialPowers.pushPrefEnv(
     {set: [["dom.ipc.systemMessageCPULockTimeoutSec", 99999]]},
     runTest);

dom/browser-element/mochitest/priority/test_HighPriorityDowngrade.html

@@ -69,8 +69,8 @@ function runTest() {
 addEventListener('testready', function() {
   SpecialPowers.pushPrefEnv(
     {set: [
-      /* Cause the CPU wake lock taken on behalf of the high-priority process
+      /* Cause the grace period of priority privilege for the high-priority
-       * to time out after 1s. */
+       * process to time out after 1s. */
        ["dom.ipc.systemMessageCPULockTimeoutSec", 1],
        ["dom.wakelock.enabled", true]
     ]},

dom/browser-element/mochitest/priority/test_HighPriorityDowngrade2.html

@@ -65,8 +65,8 @@ function runTest() {
 }
 
 addEventListener('testready', function() {
-  // Cause the CPU wake lock taken on behalf of the high-priority process never
+  // Cause the grace period of priority privilege for the high-priority process
-  // to time out during this test.
+  // never to time out during this test.
   SpecialPowers.pushPrefEnv(
     {set: [["dom.ipc.systemMessageCPULockTimeoutSec", 1000]]},
     runTest);

dom/ipc/ContentParent.cpp

@@ -41,7 +41,6 @@
 #include "mozilla/dom/PFileDescriptorSetParent.h"
 #include "mozilla/dom/PCycleCollectWithLogsParent.h"
 #include "mozilla/dom/PMemoryReportRequestParent.h"
-#include "mozilla/dom/power/PowerManagerService.h"
 #include "mozilla/dom/DOMStorageIPC.h"
 #include "mozilla/dom/bluetooth/PBluetoothParent.h"
 #include "mozilla/dom/PFMRadioParent.h"
@@ -84,7 +83,6 @@
 #include "nsIClipboard.h"
 #include "nsICycleCollectorListener.h"
 #include "nsIDOMGeoGeolocation.h"
-#include "mozilla/dom/WakeLock.h"
 #include "nsIDOMWindow.h"
 #include "nsIExternalProtocolService.h"
 #include "nsIGfxInfo.h"
@@ -184,7 +182,6 @@ using base::KillProcess;
 using namespace mozilla::dom::bluetooth;
 using namespace mozilla::dom::devicestorage;
 using namespace mozilla::dom::indexedDB;
-using namespace mozilla::dom::power;
 using namespace mozilla::dom::mobilemessage;
 using namespace mozilla::dom::telephony;
 using namespace mozilla::hal;
@@ -838,6 +835,95 @@ ContentParent::AnswerBridgeToChildProcess(const uint64_t& id)
     }
 }
 
+namespace {
+
+class SystemMessageHandledListener MOZ_FINAL
+    : public nsITimerCallback
+    , public LinkedListElement<SystemMessageHandledListener>
+{
+public:
+    NS_DECL_ISUPPORTS
+
+    SystemMessageHandledListener() {}
+
+    static void OnSystemMessageHandled()
+    {
+        if (!sListeners) {
+            return;
+        }
+
+        SystemMessageHandledListener* listener = sListeners->popFirst();
+        if (!listener) {
+            return;
+        }
+
+        // Careful: ShutDown() may delete |this|.
+        listener->ShutDown();
+    }
+
+    void Init(ContentParent* aContentParent)
+    {
+        MOZ_ASSERT(!mContentParent);
+        MOZ_ASSERT(!mTimer);
+
+        // mTimer keeps a strong reference to |this|.  When this object's
+        // destructor runs, it will remove itself from the LinkedList.
+
+        if (!sListeners) {
+            sListeners = new LinkedList<SystemMessageHandledListener>();
+            ClearOnShutdown(&sListeners);
+        }
+        sListeners->insertBack(this);
+
+        mContentParent = aContentParent;
+
+        mTimer = do_CreateInstance("@mozilla.org/timer;1");
+
+        uint32_t timeoutSec =
+            Preferences::GetInt("dom.ipc.systemMessageCPULockTimeoutSec", 30);
+        mTimer->InitWithCallback(this, timeoutSec * 1000,
+                                 nsITimer::TYPE_ONE_SHOT);
+    }
+
+    NS_IMETHOD Notify(nsITimer* aTimer)
+    {
+        // Careful: ShutDown() may delete |this|.
+        ShutDown();
+        return NS_OK;
+    }
+
+private:
+    ~SystemMessageHandledListener() {}
+
+    static StaticAutoPtr<LinkedList<SystemMessageHandledListener> > sListeners;
+
+    void ShutDown()
+    {
+        ProcessPriorityManager::ResetProcessPriority(mContentParent, false);
+
+        nsRefPtr<SystemMessageHandledListener> kungFuDeathGrip = this;
+
+        if (mContentParent) {
+            mContentParent = nullptr;
+        }
+        if (mTimer) {
+            mTimer->Cancel();
+            mTimer = nullptr;
+        }
+    }
+
+    nsRefPtr<ContentParent> mContentParent;
+    nsCOMPtr<nsITimer> mTimer;
+};
+
+StaticAutoPtr<LinkedList<SystemMessageHandledListener> >
+    SystemMessageHandledListener::sListeners;
+
+NS_IMPL_ISUPPORTS(SystemMessageHandledListener,
+                  nsITimerCallback)
+
+} // anonymous namespace
+
 /*static*/ TabParent*
 ContentParent::CreateBrowserOrApp(const TabContext& aContext,
                                   Element* aFrameElement,
@@ -1042,7 +1128,21 @@ ContentParent::CreateBrowserOrApp(const TabContext& aContext,
         return nullptr;
     }
 
-    p->MaybeTakeCPUWakeLock(aFrameElement);
+    // Request a higher priority above BACKGROUND if the child process is
+    // "critical" and probably has system messages coming soon. (A CPU wake lock
+    // may already be controlled by the B2G process in SystemMessageInternal.js
+    // for message handling.) This privilege is revoked once the message is
+    // delivered, or the grace period is up, whichever comes first.
+    nsCOMPtr<nsIMozBrowserFrame> browserFrame = do_QueryInterface(aFrameElement);
+    if (browserFrame && browserFrame->GetIsExpectingSystemMessage()) {
+      ProcessPriorityManager::ResetProcessPriority(p, true);
+
+      // This object's Init() function keeps it alive.
+      nsRefPtr<SystemMessageHandledListener> listener =
+          new SystemMessageHandledListener();
+      listener->Init(p);
+    }
+
 
     return static_cast<TabParent*>(browser);
 }
@@ -1106,117 +1206,6 @@ ContentParent::Init()
     NS_ASSERTION(observer, "FileUpdateDispatcher is null");
 }
 
-namespace {
-
-class SystemMessageHandledListener MOZ_FINAL
-    : public nsITimerCallback
-    , public LinkedListElement<SystemMessageHandledListener>
-{
-public:
-    NS_DECL_ISUPPORTS
-
-    SystemMessageHandledListener() {}
-
-    static void OnSystemMessageHandled()
-    {
-        if (!sListeners) {
-            return;
-        }
-
-        SystemMessageHandledListener* listener = sListeners->popFirst();
-        if (!listener) {
-            return;
-        }
-
-        // Careful: ShutDown() may delete |this|.
-        listener->ShutDown();
-    }
-
-    void Init(WakeLock* aWakeLock)
-    {
-        MOZ_ASSERT(!mWakeLock);
-        MOZ_ASSERT(!mTimer);
-
-        // mTimer keeps a strong reference to |this|.  When this object's
-        // destructor runs, it will remove itself from the LinkedList.
-
-        if (!sListeners) {
-            sListeners = new LinkedList<SystemMessageHandledListener>();
-            ClearOnShutdown(&sListeners);
-        }
-        sListeners->insertBack(this);
-
-        mWakeLock = aWakeLock;
-
-        mTimer = do_CreateInstance("@mozilla.org/timer;1");
-
-        uint32_t timeoutSec =
-            Preferences::GetInt("dom.ipc.systemMessageCPULockTimeoutSec", 30);
-        mTimer->InitWithCallback(this, timeoutSec * 1000,
-                                 nsITimer::TYPE_ONE_SHOT);
-    }
-
-    NS_IMETHOD Notify(nsITimer* aTimer)
-    {
-        // Careful: ShutDown() may delete |this|.
-        ShutDown();
-        return NS_OK;
-    }
-
-private:
-    ~SystemMessageHandledListener() {}
-
-    static StaticAutoPtr<LinkedList<SystemMessageHandledListener> > sListeners;
-
-    void ShutDown()
-    {
-        nsRefPtr<SystemMessageHandledListener> kungFuDeathGrip = this;
-
-        ErrorResult rv;
-        mWakeLock->Unlock(rv);
-
-        if (mTimer) {
-            mTimer->Cancel();
-            mTimer = nullptr;
-        }
-    }
-
-    nsRefPtr<WakeLock> mWakeLock;
-    nsCOMPtr<nsITimer> mTimer;
-};
-
-StaticAutoPtr<LinkedList<SystemMessageHandledListener> >
-    SystemMessageHandledListener::sListeners;
-
-NS_IMPL_ISUPPORTS(SystemMessageHandledListener,
-                  nsITimerCallback)
-
-} // anonymous namespace
-
-void
-ContentParent::MaybeTakeCPUWakeLock(Element* aFrameElement)
-{
-    // Take the CPU wake lock on behalf of this processs if it's expecting a
-    // system message.  We'll release the CPU lock once the message is
-    // delivered, or after some period of time, which ever comes first.
-
-    nsCOMPtr<nsIMozBrowserFrame> browserFrame =
-        do_QueryInterface(aFrameElement);
-    if (!browserFrame ||
-        !browserFrame->GetIsExpectingSystemMessage()) {
-        return;
-    }
-
-    nsRefPtr<PowerManagerService> pms = PowerManagerService::GetInstance();
-    nsRefPtr<WakeLock> lock =
-        pms->NewWakeLockOnBehalfOfProcess(NS_LITERAL_STRING("cpu"), this);
-
-    // This object's Init() function keeps it alive.
-    nsRefPtr<SystemMessageHandledListener> listener =
-        new SystemMessageHandledListener();
-    listener->Init(lock);
-}
-
 bool
 ContentParent::SetPriorityAndCheckIsAlive(ProcessPriority aPriority)
 {

dom/ipc/ContentParent.h

@@ -333,12 +333,6 @@ private:
 
     void Init();
 
-    // If the frame element indicates that the child process is "critical" and
-    // has a pending system message, this function acquires the CPU wake lock on
-    // behalf of the child.  We'll release the lock when the system message is
-    // handled or after a timeout, whichever comes first.
-    void MaybeTakeCPUWakeLock(Element* aFrameElement);
-
     // Set the child process's priority and then check whether the child is
     // still alive.  Returns true if the process is still alive, and false
     // otherwise.  If you pass a FOREGROUND* priority here, it's (hopefully)

dom/ipc/ProcessPriorityManager.cpp

@@ -131,6 +131,12 @@ public:
                           ProcessPriority aPriority,
                           uint32_t aBackgroundLRU = 0);
 
+  /**
+   * This function implements ProcessPriorityManager::ResetProcessPriority.
+   */
+  void ResetProcessPriority(ContentParent* aContentParent,
+                            bool aHandleSystemMessage);
+
   /**
    * If a magic testing-only pref is set, notify the observer service on the
    * given topic with the given data.  This is used for testing
@@ -287,6 +293,8 @@ public:
 
   void ShutDown();
 
+  void SetHandlesSystemMessage(bool aHandlesSystemMessage);
+
 private:
   void FireTestOnlyObserverNotification(
     const char* aTopic,
@@ -302,6 +310,7 @@ private:
   ProcessCPUPriority mCPUPriority;
   bool mHoldsCPUWakeLock;
   bool mHoldsHighPriorityWakeLock;
+  bool mHandlesSystemMessage;
 
   /**
    * Used to implement NameWithComma().
@@ -496,6 +505,17 @@ ProcessPriorityManagerImpl::SetProcessPriority(ContentParent* aContentParent,
   pppm->SetPriorityNow(aPriority, aBackgroundLRU);
 }
 
+void
+ProcessPriorityManagerImpl::ResetProcessPriority(ContentParent* aContentParent,
+                                                 bool aHandleSystemMessage)
+{
+  MOZ_ASSERT(aContentParent);
+  nsRefPtr<ParticularProcessPriorityManager> pppm =
+    GetParticularProcessPriorityManager(aContentParent);
+  pppm->SetHandlesSystemMessage(aHandleSystemMessage);
+  pppm->ResetPriority();
+}
+
 void
 ProcessPriorityManagerImpl::ObserveContentParentCreated(
   nsISupports* aContentParent)
@@ -647,6 +667,7 @@ ParticularProcessPriorityManager::ParticularProcessPriorityManager(
   , mCPUPriority(PROCESS_CPU_PRIORITY_NORMAL)
   , mHoldsCPUWakeLock(false)
   , mHoldsHighPriorityWakeLock(false)
+  , mHandlesSystemMessage(false)
 {
   MOZ_ASSERT(XRE_GetProcessType() == GeckoProcessType_Default);
   LOGP("Creating ParticularProcessPriorityManager.");
@@ -969,7 +990,7 @@ ParticularProcessPriorityManager::CurrentPriority()
 ProcessPriority
 ParticularProcessPriorityManager::ComputePriority()
 {
-  if ((mHoldsCPUWakeLock || mHoldsHighPriorityWakeLock) &&
+  if ((mHandlesSystemMessage || mHoldsCPUWakeLock || mHoldsHighPriorityWakeLock) &&
       HasAppType("critical")) {
     return PROCESS_PRIORITY_FOREGROUND_HIGH;
   }
@@ -990,7 +1011,7 @@ ParticularProcessPriorityManager::ComputePriority()
       PROCESS_PRIORITY_FOREGROUND;
   }
 
-  if ((mHoldsCPUWakeLock || mHoldsHighPriorityWakeLock) &&
+  if ((mHandlesSystemMessage || mHoldsCPUWakeLock || mHoldsHighPriorityWakeLock) &&
       IsExpectingSystemMessage()) {
     return PROCESS_PRIORITY_BACKGROUND_PERCEIVABLE;
   }
@@ -1137,6 +1158,12 @@ ParticularProcessPriorityManager::ShutDown()
   mContentParent = nullptr;
 }
 
+void
+ParticularProcessPriorityManager::SetHandlesSystemMessage(bool aHandlesSystemMessage)
+{
+  mHandlesSystemMessage = aHandlesSystemMessage;
+}
+
 void
 ProcessPriorityManagerImpl::FireTestOnlyObserverNotification(
   const char* aTopic,
@@ -1473,6 +1500,19 @@ ProcessPriorityManager::SetProcessPriority(ContentParent* aContentParent,
   }
 }
 
+/* static */ void
+ProcessPriorityManager::ResetProcessPriority(ContentParent* aContentParent,
+                                             bool aHandleSystemMessage)
+{
+  MOZ_ASSERT(aContentParent);
+
+  ProcessPriorityManagerImpl* singleton =
+    ProcessPriorityManagerImpl::GetSingleton();
+  if (singleton) {
+    singleton->ResetProcessPriority(aContentParent, aHandleSystemMessage);
+  }
+}
+
 /* static */ void
 ProcessPriorityManager::RemoveFromBackgroundLRUPool(
     ContentParent* aContentParent)

dom/ipc/ProcessPriorityManager.h

@@ -59,6 +59,20 @@ public:
   static void SetProcessPriority(dom::ContentParent* aContentParent,
                                  hal::ProcessPriority aPriority);
 
+  /**
+   * Reset the process priority of a given ContentParent's process in
+   * consideration of system message handling.
+   *
+   * Note that because this method takes a ContentParent*, you can only set the
+   * priority of your subprocesses.  In fact, because we don't support nested
+   * content processes (bug 761935), you can only call this method from the
+   * main process.
+   *
+   * The process priority manager will determine a new appropriate priority.
+   */
+  static void ResetProcessPriority(dom::ContentParent* aContentParent,
+                                   bool aHandleSystemMessage);
+
   /**
    * Returns true iff this process's priority is FOREGROUND*.
    *

dom/messages/SystemMessageInternal.js

@@ -39,6 +39,16 @@ try {
   kMaxPendingMessages = 5;
 }
 
+//Limit the duration to hold the CPU wake lock.
+let kCpuWakeLockTimeoutSec;
+try {
+  kCpuWakeLockTimeoutSec =
+    Services.prefs.getIntPref("dom.ipc.systemMessageCPULockTimeoutSec");
+} catch (e) {
+  // getIntPref throws when the pref is not set.
+  kCpuWakeLockTimeoutSec = 30;
+}
+
 const kMessages =["SystemMessageManager:GetPendingMessages",
                   "SystemMessageManager:HasPendingMessages",
                   "SystemMessageManager:Register",
@@ -149,7 +159,7 @@ SystemMessageInternal.prototype = {
       debug("Releasing the CPU wake lock because the system messages " +
             "were not handled by its registered page before time out.");
       this._cancelCpuWakeLock(aPageKey);
-    }.bind(this), 30000, Ci.nsITimer.TYPE_ONE_SHOT);
+    }.bind(this), kCpuWakeLockTimeoutSec * 1000, Ci.nsITimer.TYPE_ONE_SHOT);
   },
 
   _releaseCpuWakeLock: function _releaseCpuWakeLock(aPageKey, aHandledCount) {

dom/messages/SystemMessageManager.js

@@ -108,10 +108,8 @@ SystemMessageManager.prototype = {
       this._dispatchMessage(aType, aDispatcher, aDispatcher.messages.shift());
     } else {
       // No more messages that need to be handled, we can notify the
-      // ContentChild to release the CPU wake lock grabbed by the ContentParent
+      // ContentChild to propogate the event, so that the ContentParent can
-      // (i.e. NewWakeLockOnBehalfOfProcess()) and reset the process's priority.
+      // reset the process's priority.
-      //
-      // TODO: Bug 874353 - Remove SystemMessageHandledListener in ContentParent
       Services.obs.notifyObservers(/* aSubject */ null,
                                    "handle-system-messages-done",
                                    /* aData */ null);
@@ -249,11 +247,8 @@ SystemMessageManager.prototype = {
                               pageURL: this._pageURL,
                               handledCount: messages.length });
 
-      // We also need to notify the ContentChild to release the CPU wake lock
+      // We also need to notify the ContentChild to propogate the event, so that
-      // grabbed by the ContentParent (i.e. NewWakeLockOnBehalfOfProcess()) and
+      // the ContentParent can reset the process's priority.
-      // reset the process's priority.
-      //
-      // TODO: Bug 874353 - Remove SystemMessageHandledListener in ContentParent
       Services.obs.notifyObservers(/* aSubject */ null,
                                    "handle-system-messages-done",
                                    /* aData */ null);