Bug 1864522 - Remove unused functionality from ModifyWakeLock. r=gsvelto

- WakeLock is never registered as an observer, so remove its nsIObserver
  interface to eliminate a use of mContentParentID.

- mContentParentID is always CONTENT_PROCESS_ID_UNKNOWN, so propagate that
  through to hal::ModifyWakeLock().

- Compute the child ID on the receiver side of the ModifyWakeLock message,
  instead of passing it in the message. We always use the child ID of the
  ContentChild.

- Rename the old ModifyWakeLock into ModifyWakeLockWithChildID. Rename
  aProcessID to aChildID, because it is not a PID.

- Add a new hal::ModifyWakeLock for the parent process that uses
  CONTENT_PROCESS_ID_MAIN as the child id. This method is not actually called
  right now.

Differential Revision: https://phabricator.services.mozilla.com/D193577

dom/power/WakeLock.cpp

@@ -21,7 +21,6 @@ namespace mozilla::dom {
 NS_INTERFACE_MAP_BEGIN(WakeLock)
   NS_INTERFACE_MAP_ENTRY_AMBIGUOUS(nsISupports, nsIDOMEventListener)
   NS_INTERFACE_MAP_ENTRY(nsIDOMEventListener)
-  NS_INTERFACE_MAP_ENTRY(nsIObserver)
   NS_INTERFACE_MAP_ENTRY(nsISupportsWeakReference)
   NS_INTERFACE_MAP_ENTRY(nsIWakeLock)
 NS_INTERFACE_MAP_END
@@ -29,8 +28,6 @@ NS_INTERFACE_MAP_END
 NS_IMPL_ADDREF(WakeLock)
 NS_IMPL_RELEASE(WakeLock)
 
-WakeLock::WakeLock() : mContentParentID(CONTENT_PROCESS_ID_UNKNOWN) {}
-
 WakeLock::~WakeLock() {
   DoUnlock();
   DetachEventListener();
@@ -64,37 +61,6 @@ nsresult WakeLock::Init(const nsAString& aTopic, nsPIDOMWindowInner* aWindow) {
   return NS_OK;
 }
 
-NS_IMETHODIMP
-WakeLock::Observe(nsISupports* aSubject, const char* aTopic,
-                  const char16_t* data) {
-  // If this wake lock was acquired on behalf of another process, unlock it
-  // when that process dies.
-  //
-  // Note that we do /not/ call DoUnlock() here!  The wake lock back-end is
-  // already listening for ipc:content-shutdown messages and will clear out its
-  // tally for the process when it dies.  All we need to do here is ensure that
-  // unlock() becomes a nop.
-
-  MOZ_ASSERT(!strcmp(aTopic, "ipc:content-shutdown"));
-
-  nsCOMPtr<nsIPropertyBag2> props = do_QueryInterface(aSubject);
-  if (!props) {
-    NS_WARNING("ipc:content-shutdown message without property bag as subject");
-    return NS_OK;
-  }
-
-  uint64_t childID = 0;
-  nsresult rv = props->GetPropertyAsUint64(u"childID"_ns, &childID);
-  if (NS_SUCCEEDED(rv)) {
-    if (childID == mContentParentID) {
-      mLocked = false;
-    }
-  } else {
-    NS_WARNING("ipc:content-shutdown message without childID property");
-  }
-  return NS_OK;
-}
-
 void WakeLock::DoLock() {
   if (!mLocked) {
     // Change the flag immediately to prevent recursive reentering
@@ -102,8 +68,7 @@ void WakeLock::DoLock() {
 
     hal::ModifyWakeLock(
         mTopic, hal::WAKE_LOCK_ADD_ONE,
-        mHidden ? hal::WAKE_LOCK_ADD_ONE : hal::WAKE_LOCK_NO_CHANGE,
+        mHidden ? hal::WAKE_LOCK_ADD_ONE : hal::WAKE_LOCK_NO_CHANGE);
-        mContentParentID);
   }
 }
 
@@ -114,8 +79,7 @@ void WakeLock::DoUnlock() {
 
     hal::ModifyWakeLock(
         mTopic, hal::WAKE_LOCK_REMOVE_ONE,
-        mHidden ? hal::WAKE_LOCK_REMOVE_ONE : hal::WAKE_LOCK_NO_CHANGE,
+        mHidden ? hal::WAKE_LOCK_REMOVE_ONE : hal::WAKE_LOCK_NO_CHANGE);
-        mContentParentID);
   }
 }
 
@@ -183,8 +147,7 @@ WakeLock::HandleEvent(Event* aEvent) {
     if (mLocked && oldHidden != mHidden) {
       hal::ModifyWakeLock(
           mTopic, hal::WAKE_LOCK_NO_CHANGE,
-          mHidden ? hal::WAKE_LOCK_ADD_ONE : hal::WAKE_LOCK_REMOVE_ONE,
+          mHidden ? hal::WAKE_LOCK_ADD_ONE : hal::WAKE_LOCK_REMOVE_ONE);
-          mContentParentID);
     }
 
     return NS_OK;

dom/power/WakeLock.h

@@ -9,7 +9,6 @@
 
 #include "nsCOMPtr.h"
 #include "nsIDOMEventListener.h"
-#include "nsIObserver.h"
 #include "nsIWakeLock.h"
 #include "nsString.h"
 #include "nsWeakReference.h"
@@ -24,12 +23,10 @@ namespace dom {
 class Document;
 
 class WakeLock final : public nsIDOMEventListener,
-                       public nsIObserver,
                        public nsSupportsWeakReference,
                        public nsIWakeLock {
  public:
   NS_DECL_NSIDOMEVENTLISTENER
-  NS_DECL_NSIOBSERVER
   NS_DECL_NSIWAKELOCK
 
   NS_DECL_ISUPPORTS
@@ -39,7 +36,7 @@ class WakeLock final : public nsIDOMEventListener,
   // |var foo = navigator.requestWakeLock('cpu'); foo = null;|
   // doesn't unlock the 'cpu' resource.
 
-  WakeLock();
+  WakeLock() = default;
 
   // Initialize this wake lock on behalf of the given window.  Null windows are
   // allowed; a lock without an associated window is always considered
@@ -65,10 +62,6 @@ class WakeLock final : public nsIDOMEventListener,
   bool mLocked = false;
   bool mHidden = true;
 
-  // The ID of the ContentParent on behalf of whom we acquired this lock, or
-  // CONTENT_PROCESS_UNKNOWN_ID if this lock was acquired on behalf of the
-  // current process.
-  uint64_t mContentParentID;
   nsString mTopic;
 
   // window that this was created for.  Weak reference.

hal/Hal.cpp

@@ -351,17 +351,10 @@ void NotifyNetworkChange(const NetworkInformation& aInfo) {
 MOZ_IMPL_HAL_OBSERVER(WakeLock)
 
 void ModifyWakeLock(const nsAString& aTopic, WakeLockControl aLockAdjust,
-                    WakeLockControl aHiddenAdjust,
+                    WakeLockControl aHiddenAdjust) {
-                    uint64_t aProcessID /* = CONTENT_PROCESS_ID_UNKNOWN */) {
   AssertMainThread();
 
-  if (aProcessID == CONTENT_PROCESS_ID_UNKNOWN) {
+  PROXY_IF_SANDBOXED(ModifyWakeLock(aTopic, aLockAdjust, aHiddenAdjust));
-    aProcessID = InSandbox() ? ContentChild::GetSingleton()->GetID()
-                             : CONTENT_PROCESS_ID_MAIN;
-  }
-
-  PROXY_IF_SANDBOXED(
-      ModifyWakeLock(aTopic, aLockAdjust, aHiddenAdjust, aProcessID));
 }
 
 void GetWakeLockInfo(const nsAString& aTopic,

hal/Hal.h

@@ -183,24 +183,14 @@ void DisableWakeLockNotifications();
 MOZ_DEFINE_HAL_OBSERVER(WakeLock);
 
 /**
- * Adjust a wake lock's counts on behalf of a given process.
+ * Adjust a wake lock's counts for the current process.
- *
- * In most cases, you shouldn't need to pass the aProcessID argument; the
- * default of CONTENT_PROCESS_ID_UNKNOWN is probably what you want.
  *
  * @param aTopic        lock topic
  * @param aLockAdjust   to increase or decrease active locks
  * @param aHiddenAdjust to increase or decrease hidden locks
- * @param aProcessID    indicates which process we're modifying the wake lock
- *                      on behalf of.  It is interpreted as
- *
- *                      CONTENT_PROCESS_ID_UNKNOWN: The current process
- *                      CONTENT_PROCESS_ID_MAIN: The root process
- *                      X: The process with ContentChild::GetID() == X
  */
 void ModifyWakeLock(const nsAString& aTopic, hal::WakeLockControl aLockAdjust,
-                    hal::WakeLockControl aHiddenAdjust,
+                    hal::WakeLockControl aHiddenAdjust);
-                    uint64_t aProcessID = hal::CONTENT_PROCESS_ID_UNKNOWN);
 
 /**
  * Query the wake lock numbers of aTopic.

hal/HalWakeLock.cpp

@@ -169,10 +169,12 @@ void EnableWakeLockNotifications() { sActiveListeners++; }
 
 void DisableWakeLockNotifications() { sActiveListeners--; }
 
-void ModifyWakeLock(const nsAString& aTopic, hal::WakeLockControl aLockAdjust,
+void ModifyWakeLockWithChildID(const nsAString& aTopic,
-                    hal::WakeLockControl aHiddenAdjust, uint64_t aProcessID) {
+                               hal::WakeLockControl aLockAdjust,
+                               hal::WakeLockControl aHiddenAdjust,
+                               uint64_t aChildID) {
   MOZ_ASSERT(NS_IsMainThread());
-  MOZ_ASSERT(aProcessID != CONTENT_PROCESS_ID_UNKNOWN);
+  MOZ_ASSERT(aChildID != CONTENT_PROCESS_ID_UNKNOWN);
 
   if (sIsShuttingDown) {
     return;
@@ -185,7 +187,7 @@ void ModifyWakeLock(const nsAString& aTopic, hal::WakeLockControl aLockAdjust,
         if (!entry) {
           entry.Insert(MakeUnique<ProcessLockTable>());
         } else {
-          Unused << entry.Data()->Get(aProcessID, &processCount);
+          Unused << entry.Data()->Get(aChildID, &processCount);
           CountWakeLocks(entry->get(), &totalCount);
         }
         return entry->get();
@@ -217,9 +219,9 @@ void ModifyWakeLock(const nsAString& aTopic, hal::WakeLockControl aLockAdjust,
   totalCount.numHidden += aHiddenAdjust;
 
   if (processCount.numLocks) {
-    table->InsertOrUpdate(aProcessID, processCount);
+    table->InsertOrUpdate(aChildID, processCount);
   } else {
-    table->Remove(aProcessID);
+    table->Remove(aChildID);
   }
   if (!totalCount.numLocks) {
     sLockTable->Remove(aTopic);
@@ -235,6 +237,12 @@ void ModifyWakeLock(const nsAString& aTopic, hal::WakeLockControl aLockAdjust,
   }
 }
 
+void ModifyWakeLock(const nsAString& aTopic, hal::WakeLockControl aLockAdjust,
+                    hal::WakeLockControl aHiddenAdjust) {
+  ModifyWakeLockWithChildID(aTopic, aLockAdjust, aHiddenAdjust,
+                            CONTENT_PROCESS_ID_MAIN);
+}
+
 void GetWakeLockInfo(const nsAString& aTopic,
                      WakeLockInformation* aWakeLockInfo) {
   if (sIsShuttingDown) {

hal/HalWakeLock.h

@@ -21,6 +21,14 @@ enum WakeLockState {
 WakeLockState ComputeWakeLockState(int aNumLocks, int aNumHidden);
 
 }  // namespace hal
+
+namespace hal_impl {
+void ModifyWakeLockWithChildID(const nsAString& aTopic,
+                               hal::WakeLockControl aLockAdjust,
+                               hal::WakeLockControl aHiddenAdjust,
+                               uint64_t aChildID);
+}  // namespace hal_impl
+
 }  // namespace mozilla
 
 #endif /* __HAL_WAKELOCK_H_ */

hal/sandbox/PHal.ipdl

@@ -74,8 +74,7 @@ parent:
 
     async ModifyWakeLock(nsString aTopic,
                          WakeLockControl aLockAdjust,
-                         WakeLockControl aHiddenAdjust,
+                         WakeLockControl aHiddenAdjust);
-                         uint64_t aProcessID);
     async EnableWakeLockNotifications();
     async DisableWakeLockNotifications();
     sync GetWakeLockInfo(nsString aTopic)

hal/sandbox/SandboxHal.cpp

@@ -7,11 +7,13 @@
 #include "Hal.h"
 #include "HalLog.h"
 #include "mozilla/dom/ContentChild.h"
+#include "mozilla/dom/ContentParent.h"
 #include "mozilla/hal_sandbox/PHalChild.h"
 #include "mozilla/hal_sandbox/PHalParent.h"
 #include "mozilla/dom/BrowserParent.h"
 #include "mozilla/dom/BrowserChild.h"
 #include "mozilla/EnumeratedRange.h"
+#include "mozilla/HalWakeLock.h"
 #include "mozilla/Observer.h"
 #include "mozilla/Unused.h"
 #include "WindowIdentifier.h"
@@ -108,9 +110,8 @@ void DisableWakeLockNotifications() {
 }
 
 void ModifyWakeLock(const nsAString& aTopic, WakeLockControl aLockAdjust,
-                    WakeLockControl aHiddenAdjust, uint64_t aProcessID) {
+                    WakeLockControl aHiddenAdjust) {
-  MOZ_ASSERT(aProcessID != CONTENT_PROCESS_ID_UNKNOWN);
+  Hal()->SendModifyWakeLock(aTopic, aLockAdjust, aHiddenAdjust);
-  Hal()->SendModifyWakeLock(aTopic, aLockAdjust, aHiddenAdjust, aProcessID);
 }
 
 void GetWakeLockInfo(const nsAString& aTopic,
@@ -267,12 +268,10 @@ class HalParent : public PHalParent,
 
   virtual mozilla::ipc::IPCResult RecvModifyWakeLock(
       const nsAString& aTopic, const WakeLockControl& aLockAdjust,
-      const WakeLockControl& aHiddenAdjust,
+      const WakeLockControl& aHiddenAdjust) override {
-      const uint64_t& aProcessID) override {
-    MOZ_ASSERT(aProcessID != CONTENT_PROCESS_ID_UNKNOWN);
-
     // We allow arbitrary content to use wake locks.
-    hal::ModifyWakeLock(aTopic, aLockAdjust, aHiddenAdjust, aProcessID);
+    uint64_t id = static_cast<ContentParent*>(Manager())->ChildID();
+    hal_impl::ModifyWakeLockWithChildID(aTopic, aLockAdjust, aHiddenAdjust, id);
     return IPC_OK();
   }