Bug 1275434 - Refactor `PushNotifier` to clarify remoting logic. r=dragana
authorKit Cambridge <kcambridge@mozilla.com>
Thu, 19 May 2016 19:01:34 -0700
changeset 339034 23080ad39e3051ab76f181e7e3cb32875e87bd17
parent 339033 5885ad0c0a1d9fd25143692d3200a8ec2dcd80f0
child 339035 80fbbee5de1fb2616014704c01b0752c2bd2f963
push id6249
push userjlund@mozilla.com
push dateMon, 01 Aug 2016 13:59:36 +0000
treeherdermozilla-beta@bad9d4f5bf7e [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersdragana
bugs1275434, 1266433
milestone49.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1275434 - Refactor `PushNotifier` to clarify remoting logic. r=dragana Previously, all `PushNotifier` methods checked the process type, and either called `Notify*{Workers, Observers}` or sent an IPDL message. The message handlers then called back in to `PushNotifier` from the remote process. This was clearer when we only sent worker event notifications to the content process, and fired all observer notifications in the parent. It became more complicated once we started notifying observers for all subscriptions in both processes (bug 1266433). This makes it harder to see omissions; for example, "push-subscription-modified" isn't currently forwarded to the child, but "push-subscription-change" and "push-message" are. This patch moves the remoting code into `PushNotifier::Dispatch`, and adds a base `PushDispatcher` class. Each notification type subclasses this class and implements logic for sending messages and firing observers and worker events. It's more code, but a bit easier to see which methods are called where. MozReview-Commit-ID: 7Q0eD7qXOrW
dom/interfaces/push/nsIPushNotifier.idl
dom/ipc/ContentChild.cpp
dom/ipc/ContentChild.h
dom/ipc/ContentParent.cpp
dom/ipc/PContent.ipdl
dom/push/PushNotifier.cpp
dom/push/PushNotifier.h
--- a/dom/interfaces/push/nsIPushNotifier.idl
+++ b/dom/interfaces/push/nsIPushNotifier.idl
@@ -1,32 +1,27 @@
 /* -*- Mode: IDL; tab-width: 2; indent-tabs-mode: nil; c-basic-offset: 2 -*- */
 /* This Source Code Form is subject to the terms of the Mozilla Public
  * License, v. 2.0. If a copy of the MPL was not distributed with this
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
 #include "nsISupports.idl"
 
 %{C++
-#include "nsTArray.h"
-#include "mozilla/Maybe.h"
-
 #define PUSHNOTIFIER_CONTRACTID \
   "@mozilla.org/push/Notifier;1"
 
 // These constants are duplicated in `PushComponents.js`.
 #define OBSERVER_TOPIC_PUSH "push-message"
 #define OBSERVER_TOPIC_SUBSCRIPTION_CHANGE "push-subscription-change"
 #define OBSERVER_TOPIC_SUBSCRIPTION_MODIFIED "push-subscription-modified"
 %}
 
 interface nsIPrincipal;
 
-[ref] native MaybeData(const mozilla::Maybe<nsTArray<uint8_t>>);
-
 /**
  * Fires XPCOM observer notifications and service worker events for
  * messages sent to push subscriptions.
  */
 [scriptable, builtinclass, uuid(b00dfdeb-14e5-425b-adc7-b531442e3216)]
 interface nsIPushNotifier : nsISupports
 {
   /**
@@ -62,47 +57,16 @@ interface nsIPushNotifier : nsISupports
    * This is useful for Dev Tools and debugging add-ons that passively observe
    * when subscriptions are created or dropped. Other callers should listen for
    * `push-subscription-change` and resubscribe instead.
    */
   void notifySubscriptionModified(in ACString scope, in nsIPrincipal principal);
 
   void notifyError(in ACString scope, in nsIPrincipal principal,
                    in DOMString message, in uint32_t flags);
-
-  /**
-   * Internal methods used to fire service worker events and observer
-   * notifications. These are not exposed to JavaScript.
-   */
-
-  [noscript, nostdcall]
-  void notifyPushWorkers(in ACString scope,
-                         in nsIPrincipal principal,
-                         in DOMString messageId,
-                         in MaybeData data);
-
-  [noscript, nostdcall]
-  void notifyPushObservers(in ACString scope, in nsIPrincipal principal,
-                           in MaybeData data);
-
-  [noscript, nostdcall]
-  void notifySubscriptionChangeWorkers(in ACString scope,
-                                       in nsIPrincipal principal);
-
-  [noscript, nostdcall]
-  void notifySubscriptionChangeObservers(in ACString scope,
-                                         in nsIPrincipal principal);
-
-  [noscript, nostdcall]
-  void notifySubscriptionModifiedObservers(in ACString scope,
-                                           in nsIPrincipal principal);
-
-  [noscript, nostdcall, notxpcom]
-  void notifyErrorWorkers(in ACString scope, in DOMString message,
-                          in uint32_t flags);
 };
 
 /**
  * Provides methods for retrieving push message data in different formats.
  * This interface resembles the `PushMessageData` WebIDL interface.
  */
 [scriptable, builtinclass, uuid(dfc4f151-cead-40df-8eb7-7a7a67c54b16)]
 interface nsIPushData : nsISupports
--- a/dom/ipc/ContentChild.cpp
+++ b/dom/ipc/ContentChild.cpp
@@ -171,17 +171,17 @@
 #endif
 #include "NuwaChild.h"
 
 #ifdef MOZ_GAMEPAD
 #include "mozilla/dom/GamepadService.h"
 #endif
 
 #ifndef MOZ_SIMPLEPUSH
-#include "nsIPushNotifier.h"
+#include "mozilla/dom/PushNotifier.h"
 #endif
 
 #include "mozilla/dom/File.h"
 #include "mozilla/dom/cellbroadcast/CellBroadcastIPCService.h"
 #include "mozilla/dom/icc/IccChild.h"
 #include "mozilla/dom/mobileconnection/MobileConnectionChild.h"
 #include "mozilla/dom/mobilemessage/SmsChild.h"
 #include "mozilla/dom/devicestorage/DeviceStorageRequestChild.h"
@@ -3280,87 +3280,51 @@ ContentChild::RecvEndDragSession(const b
 }
 
 bool
 ContentChild::RecvPush(const nsCString& aScope,
                        const IPC::Principal& aPrincipal,
                        const nsString& aMessageId)
 {
 #ifndef MOZ_SIMPLEPUSH
-  nsCOMPtr<nsIPushNotifier> pushNotifier =
-      do_GetService("@mozilla.org/push/Notifier;1");
-  if (NS_WARN_IF(!pushNotifier)) {
-      return true;
-  }
-
-  nsresult rv = pushNotifier->NotifyPushObservers(aScope, aPrincipal,
-                                                  Nothing());
-  Unused << NS_WARN_IF(NS_FAILED(rv));
-
-  rv = pushNotifier->NotifyPushWorkers(aScope, aPrincipal,
-                                       aMessageId, Nothing());
-  Unused << NS_WARN_IF(NS_FAILED(rv));
+  PushMessageDispatcher dispatcher(aScope, aPrincipal, aMessageId, Nothing());
+  Unused << NS_WARN_IF(NS_FAILED(dispatcher.NotifyObserversAndWorkers()));
 #endif
   return true;
 }
 
 bool
 ContentChild::RecvPushWithData(const nsCString& aScope,
                                const IPC::Principal& aPrincipal,
                                const nsString& aMessageId,
                                InfallibleTArray<uint8_t>&& aData)
 {
 #ifndef MOZ_SIMPLEPUSH
-  nsCOMPtr<nsIPushNotifier> pushNotifier =
-      do_GetService("@mozilla.org/push/Notifier;1");
-  if (NS_WARN_IF(!pushNotifier)) {
-      return true;
-  }
-
-  nsresult rv = pushNotifier->NotifyPushObservers(aScope, aPrincipal,
-                                                  Some(aData));
-  Unused << NS_WARN_IF(NS_FAILED(rv));
-
-  rv = pushNotifier->NotifyPushWorkers(aScope, aPrincipal,
-                                       aMessageId, Some(aData));
-  Unused << NS_WARN_IF(NS_FAILED(rv));
+  PushMessageDispatcher dispatcher(aScope, aPrincipal, aMessageId, Some(aData));
+  Unused << NS_WARN_IF(NS_FAILED(dispatcher.NotifyObserversAndWorkers()));
 #endif
   return true;
 }
 
 bool
 ContentChild::RecvPushSubscriptionChange(const nsCString& aScope,
                                          const IPC::Principal& aPrincipal)
 {
 #ifndef MOZ_SIMPLEPUSH
-  nsCOMPtr<nsIPushNotifier> pushNotifier =
-      do_GetService("@mozilla.org/push/Notifier;1");
-  if (NS_WARN_IF(!pushNotifier)) {
-      return true;
-  }
-
-  nsresult rv = pushNotifier->NotifySubscriptionChangeObservers(aScope,
-                                                                aPrincipal);
-  Unused << NS_WARN_IF(NS_FAILED(rv));
-
-  rv = pushNotifier->NotifySubscriptionChangeWorkers(aScope, aPrincipal);
-  Unused << NS_WARN_IF(NS_FAILED(rv));
+  PushSubscriptionChangeDispatcher dispatcher(aScope, aPrincipal);
+  Unused << NS_WARN_IF(NS_FAILED(dispatcher.NotifyObserversAndWorkers()));
 #endif
   return true;
 }
 
 bool
-ContentChild::RecvPushError(const nsCString& aScope, const nsString& aMessage,
-                            const uint32_t& aFlags)
+ContentChild::RecvPushError(const nsCString& aScope, const IPC::Principal& aPrincipal,
+                            const nsString& aMessage, const uint32_t& aFlags)
 {
 #ifndef MOZ_SIMPLEPUSH
-  nsCOMPtr<nsIPushNotifier> pushNotifier =
-      do_GetService("@mozilla.org/push/Notifier;1");
-  if (NS_WARN_IF(!pushNotifier)) {
-      return true;
-  }
-  pushNotifier->NotifyErrorWorkers(aScope, aMessage, aFlags);
+  PushErrorDispatcher dispatcher(aScope, aPrincipal, aMessage, aFlags);
+  Unused << NS_WARN_IF(NS_FAILED(dispatcher.NotifyObserversAndWorkers()));
 #endif
   return true;
 }
 
 } // namespace dom
 } // namespace mozilla
--- a/dom/ipc/ContentChild.h
+++ b/dom/ipc/ContentChild.h
@@ -534,18 +534,18 @@ public:
                    const nsString& aMessageId,
                    InfallibleTArray<uint8_t>&& aData) override;
 
   virtual bool
   RecvPushSubscriptionChange(const nsCString& aScope,
                              const IPC::Principal& aPrincipal) override;
 
   virtual bool
-  RecvPushError(const nsCString& aScope, const nsString& aMessage,
-                const uint32_t& aFlags) override;
+  RecvPushError(const nsCString& aScope, const IPC::Principal& aPrincipal,
+                const nsString& aMessage, const uint32_t& aFlags) override;
 
   // Get the directory for IndexedDB files. We query the parent for this and
   // cache the value
   nsString &GetIndexedDBPath();
 
   ContentParentId GetID() const { return mID; }
 
   bool IsForApp() const { return mIsForApp; }
--- a/dom/ipc/ContentParent.cpp
+++ b/dom/ipc/ContentParent.cpp
@@ -262,17 +262,17 @@ using namespace mozilla::system;
 #include "nsIProfileSaveEvent.h"
 #endif
 
 #ifdef MOZ_GAMEPAD
 #include "mozilla/dom/GamepadMonitoring.h"
 #endif
 
 #ifndef MOZ_SIMPLEPUSH
-#include "nsIPushNotifier.h"
+#include "mozilla/dom/PushNotifier.h"
 #endif
 
 #ifdef XP_WIN
 #include "mozilla/widget/AudioSession.h"
 #endif
 
 #include "VRManagerParent.h"            // for VRManagerParent
 
@@ -5823,81 +5823,53 @@ ContentParent::StartProfiler(nsIProfiler
 }
 
 bool
 ContentParent::RecvNotifyPushObservers(const nsCString& aScope,
                                        const IPC::Principal& aPrincipal,
                                        const nsString& aMessageId)
 {
 #ifndef MOZ_SIMPLEPUSH
-  nsCOMPtr<nsIPushNotifier> pushNotifier =
-      do_GetService("@mozilla.org/push/Notifier;1");
-  if (NS_WARN_IF(!pushNotifier)) {
-      return true;
-  }
-
-  nsresult rv = pushNotifier->NotifyPushObservers(aScope, aPrincipal,
-                                                  Nothing());
-  Unused << NS_WARN_IF(NS_FAILED(rv));
+  PushMessageDispatcher dispatcher(aScope, aPrincipal, aMessageId, Nothing());
+  Unused << NS_WARN_IF(NS_FAILED(dispatcher.NotifyObservers()));
 #endif
   return true;
 }
 
 bool
 ContentParent::RecvNotifyPushObserversWithData(const nsCString& aScope,
                                                const IPC::Principal& aPrincipal,
                                                const nsString& aMessageId,
                                                InfallibleTArray<uint8_t>&& aData)
 {
 #ifndef MOZ_SIMPLEPUSH
-  nsCOMPtr<nsIPushNotifier> pushNotifier =
-      do_GetService("@mozilla.org/push/Notifier;1");
-  if (NS_WARN_IF(!pushNotifier)) {
-      return true;
-  }
-
-  nsresult rv = pushNotifier->NotifyPushObservers(aScope, aPrincipal,
-                                                  Some(aData));
-  Unused << NS_WARN_IF(NS_FAILED(rv));
+  PushMessageDispatcher dispatcher(aScope, aPrincipal, aMessageId, Some(aData));
+  Unused << NS_WARN_IF(NS_FAILED(dispatcher.NotifyObservers()));
 #endif
   return true;
 }
 
 bool
 ContentParent::RecvNotifyPushSubscriptionChangeObservers(const nsCString& aScope,
                                                          const IPC::Principal& aPrincipal)
 {
 #ifndef MOZ_SIMPLEPUSH
-  nsCOMPtr<nsIPushNotifier> pushNotifier =
-      do_GetService("@mozilla.org/push/Notifier;1");
-  if (NS_WARN_IF(!pushNotifier)) {
-      return true;
-  }
-
-  nsresult rv = pushNotifier->NotifySubscriptionChangeObservers(aScope,
-                                                                aPrincipal);
-  Unused << NS_WARN_IF(NS_FAILED(rv));
+  PushSubscriptionChangeDispatcher dispatcher(aScope, aPrincipal);
+  Unused << NS_WARN_IF(NS_FAILED(dispatcher.NotifyObservers()));
 #endif
   return true;
 }
 
 bool
 ContentParent::RecvNotifyPushSubscriptionModifiedObservers(const nsCString& aScope,
                                                            const IPC::Principal& aPrincipal)
 {
 #ifndef MOZ_SIMPLEPUSH
-  nsCOMPtr<nsIPushNotifier> pushNotifier =
-      do_GetService("@mozilla.org/push/Notifier;1");
-  if (NS_WARN_IF(!pushNotifier)) {
-      return true;
-  }
-
-  nsresult rv = pushNotifier->NotifySubscriptionModifiedObservers(aScope,
-                                                                  aPrincipal);
-  Unused << NS_WARN_IF(NS_FAILED(rv));
+  PushSubscriptionModifiedDispatcher dispatcher(aScope, aPrincipal);
+  Unused << NS_WARN_IF(NS_FAILED(dispatcher.NotifyObservers()));
 #endif
   return true;
 }
 
 } // namespace dom
 } // namespace mozilla
 
 NS_IMPL_ISUPPORTS(ParentIdleListener, nsIObserver)
--- a/dom/ipc/PContent.ipdl
+++ b/dom/ipc/PContent.ipdl
@@ -667,17 +667,18 @@ child:
     /**
      * Send a `pushsubscriptionchange` event to a service worker in the child.
      */
     async PushSubscriptionChange(nsCString scope, Principal principal);
 
     /**
      * Send a Push error message to all service worker clients in the child.
      */
-    async PushError(nsCString scope, nsString message, uint32_t flags);
+    async PushError(nsCString scope, Principal principal, nsString message,
+                    uint32_t flags);
 
     /**
      * Windows specific: associate this content process with the browsers
      * audio session.
      */
     async SetAudioSessionData(nsID aID,
                               nsString aDisplayName,
                               nsString aIconPath);
--- a/dom/push/PushNotifier.cpp
+++ b/dom/push/PushNotifier.cpp
@@ -49,322 +49,92 @@ PushNotifier::NotifyPushWithData(const n
 {
   nsTArray<uint8_t> data;
   if (!data.SetCapacity(aDataLen, fallible)) {
     return NS_ERROR_OUT_OF_MEMORY;
   }
   if (!data.InsertElementsAt(0, aData, aDataLen, fallible)) {
     return NS_ERROR_OUT_OF_MEMORY;
   }
-  return NotifyPush(aScope, aPrincipal, aMessageId, Some(data));
+  PushMessageDispatcher dispatcher(aScope, aPrincipal, aMessageId, Some(data));
+  return Dispatch(dispatcher);
 }
 
 NS_IMETHODIMP
 PushNotifier::NotifyPush(const nsACString& aScope, nsIPrincipal* aPrincipal,
                          const nsAString& aMessageId)
 {
-  return NotifyPush(aScope, aPrincipal, aMessageId, Nothing());
+  PushMessageDispatcher dispatcher(aScope, aPrincipal, aMessageId, Nothing());
+  return Dispatch(dispatcher);
 }
 
 NS_IMETHODIMP
 PushNotifier::NotifySubscriptionChange(const nsACString& aScope,
                                        nsIPrincipal* aPrincipal)
 {
-  nsresult rv = NotifySubscriptionChangeObservers(aScope, aPrincipal);
-  Unused << NS_WARN_IF(NS_FAILED(rv));
-
-  if (XRE_IsContentProcess()) {
-    // Forward XPCOM observer notifications to the parent.
-    ContentChild* parentActor = ContentChild::GetSingleton();
-    if (!NS_WARN_IF(!parentActor)) {
-      Unused << NS_WARN_IF(
-        !parentActor->SendNotifyPushSubscriptionChangeObservers(
-          PromiseFlatCString(aScope), IPC::Principal(aPrincipal)));
-    }
-  }
-
-  rv = NotifySubscriptionChangeWorkers(aScope, aPrincipal);
-  if (NS_WARN_IF(NS_FAILED(rv))) {
-    return rv;
-  }
-  return NS_OK;
+  PushSubscriptionChangeDispatcher dispatcher(aScope, aPrincipal);
+  return Dispatch(dispatcher);
 }
 
 NS_IMETHODIMP
 PushNotifier::NotifySubscriptionModified(const nsACString& aScope,
                                          nsIPrincipal* aPrincipal)
 {
-  nsresult rv = NotifySubscriptionModifiedObservers(aScope, aPrincipal);
-  Unused << NS_WARN_IF(NS_FAILED(rv));
-
-  if (XRE_IsContentProcess()) {
-    ContentChild* parentActor = ContentChild::GetSingleton();
-    if (!NS_WARN_IF(!parentActor)) {
-      Unused << NS_WARN_IF(
-        !parentActor->SendNotifyPushSubscriptionModifiedObservers(
-          PromiseFlatCString(aScope), IPC::Principal(aPrincipal)));
-    }
-  }
-
-  return NS_OK;
+  PushSubscriptionModifiedDispatcher dispatcher(aScope, aPrincipal);
+  return Dispatch(dispatcher);
 }
 
 NS_IMETHODIMP
 PushNotifier::NotifyError(const nsACString& aScope, nsIPrincipal* aPrincipal,
                           const nsAString& aMessage, uint32_t aFlags)
 {
-  if (ShouldNotifyWorkers(aPrincipal)) {
-    // For service worker subscriptions, report the error to all clients.
-    NotifyErrorWorkers(aScope, aMessage, aFlags);
-    return NS_OK;
-  }
-  // For system subscriptions, log the error directly to the browser console.
-  return nsContentUtils::ReportToConsoleNonLocalized(aMessage,
-                                                     aFlags,
-                                                     NS_LITERAL_CSTRING("Push"),
-                                                     nullptr, /* aDocument */
-                                                     nullptr, /* aURI */
-                                                     EmptyString(), /* aLine */
-                                                     0, /* aLineNumber */
-                                                     0, /* aColumnNumber */
-                                                     nsContentUtils::eOMIT_LOCATION);
-}
-
-nsresult
-PushNotifier::NotifyPush(const nsACString& aScope, nsIPrincipal* aPrincipal,
-                         const nsAString& aMessageId,
-                         const Maybe<nsTArray<uint8_t>>& aData)
-{
-  // Notify XPCOM observers in the current process.
-  nsresult rv = NotifyPushObservers(aScope, aPrincipal, aData);
-  Unused << NS_WARN_IF(NS_FAILED(rv));
-
-  if (XRE_IsContentProcess()) {
-    // If we're in the content process, forward the notification to the parent.
-    // We don't need to do anything if we're already in the parent;
-    // `ContentChild::RecvPush` will notify content process observers.
-    ContentChild* parentActor = ContentChild::GetSingleton();
-    if (!NS_WARN_IF(!parentActor)) {
-      if (aData) {
-        Unused << NS_WARN_IF(
-          !parentActor->SendNotifyPushObserversWithData(
-            PromiseFlatCString(aScope), IPC::Principal(aPrincipal),
-            PromiseFlatString(aMessageId), aData.ref()));
-      } else {
-        Unused << NS_WARN_IF(
-          !parentActor->SendNotifyPushObservers(
-            PromiseFlatCString(aScope), IPC::Principal(aPrincipal),
-            PromiseFlatString(aMessageId)));
-      }
-    }
-  }
-
-  rv = NotifyPushWorkers(aScope, aPrincipal, aMessageId, aData);
-  if (NS_WARN_IF(NS_FAILED(rv))) {
-    return rv;
-  }
-  return NS_OK;
-}
-
-nsresult
-PushNotifier::NotifyPushWorkers(const nsACString& aScope,
-                                nsIPrincipal* aPrincipal,
-                                const nsAString& aMessageId,
-                                const Maybe<nsTArray<uint8_t>>& aData)
-{
-  AssertIsOnMainThread();
-  NS_ENSURE_ARG(aPrincipal);
-
-  if (XRE_IsContentProcess() || !BrowserTabsRemoteAutostart()) {
-    // Notify the worker from the current process. Either we're running in
-    // the content process and received a message from the parent, or e10s
-    // is disabled.
-    if (!ShouldNotifyWorkers(aPrincipal)) {
-      return NS_OK;
-    }
-    RefPtr<ServiceWorkerManager> swm = ServiceWorkerManager::GetInstance();
-    if (!swm) {
-      return NS_ERROR_FAILURE;
-    }
-    nsAutoCString originSuffix;
-    nsresult rv = aPrincipal->GetOriginSuffix(originSuffix);
-    if (NS_WARN_IF(NS_FAILED(rv))) {
-      return rv;
-    }
-    return swm->SendPushEvent(originSuffix, aScope, aMessageId, aData);
-  }
-
-  // Otherwise, we're in the parent and e10s is enabled. Broadcast the event
-  // to all content processes.
-  bool ok = true;
-  nsTArray<ContentParent*> contentActors;
-  ContentParent::GetAll(contentActors);
-  for (uint32_t i = 0; i < contentActors.Length(); ++i) {
-    if (aData) {
-      ok &= contentActors[i]->SendPushWithData(PromiseFlatCString(aScope),
-        IPC::Principal(aPrincipal), PromiseFlatString(aMessageId), aData.ref());
-    } else {
-      ok &= contentActors[i]->SendPush(PromiseFlatCString(aScope),
-        IPC::Principal(aPrincipal), PromiseFlatString(aMessageId));
-    }
-  }
-  return ok ? NS_OK : NS_ERROR_FAILURE;
+  PushErrorDispatcher dispatcher(aScope, aPrincipal, aMessage, aFlags);
+  return Dispatch(dispatcher);
 }
 
 nsresult
-PushNotifier::NotifySubscriptionChangeWorkers(const nsACString& aScope,
-                                              nsIPrincipal* aPrincipal)
+PushNotifier::Dispatch(PushDispatcher& aDispatcher)
 {
-  AssertIsOnMainThread();
-  NS_ENSURE_ARG(aPrincipal);
+  if (XRE_IsParentProcess()) {
+    // Always notify XPCOM observers in the parent process.
+    Unused << NS_WARN_IF(NS_FAILED(aDispatcher.NotifyObservers()));
 
-  if (XRE_IsContentProcess() || !BrowserTabsRemoteAutostart()) {
-    // Content process or e10s disabled.
-    if (!ShouldNotifyWorkers(aPrincipal)) {
+    nsTArray<ContentParent*> contentActors;
+    ContentParent::GetAll(contentActors);
+    if (!contentActors.IsEmpty()) {
+      // At least one content process is active, so e10s must be enabled.
+      // Broadcast a message to notify observers and service workers.
+      for (uint32_t i = 0; i < contentActors.Length(); ++i) {
+        Unused << NS_WARN_IF(!aDispatcher.SendToChild(contentActors[i]));
+      }
       return NS_OK;
     }
-    RefPtr<ServiceWorkerManager> swm = ServiceWorkerManager::GetInstance();
-    if (!swm) {
-      return NS_ERROR_FAILURE;
-    }
-    nsAutoCString originSuffix;
-    nsresult rv = aPrincipal->GetOriginSuffix(originSuffix);
-    if (NS_WARN_IF(NS_FAILED(rv))) {
-      return rv;
-    }
-    return swm->SendPushSubscriptionChangeEvent(originSuffix, aScope);
-  }
 
-  // Parent process, e10s enabled.
-  bool ok = true;
-  nsTArray<ContentParent*> contentActors;
-  ContentParent::GetAll(contentActors);
-  for (uint32_t i = 0; i < contentActors.Length(); ++i) {
-    ok &= contentActors[i]->SendPushSubscriptionChange(
-      PromiseFlatCString(aScope), IPC::Principal(aPrincipal));
-  }
-  return ok ? NS_OK : NS_ERROR_FAILURE;
-}
+    if (BrowserTabsRemoteAutostart()) {
+      // e10s is enabled, but no content processes are active.
+      return aDispatcher.HandleNoChildProcesses();
+    }
 
-void
-PushNotifier::NotifyErrorWorkers(const nsACString& aScope,
-                                 const nsAString& aMessage,
-                                 uint32_t aFlags)
-{
-  AssertIsOnMainThread();
-
-  if (XRE_IsContentProcess() || !BrowserTabsRemoteAutostart()) {
-    // Content process or e10s disabled.
-    RefPtr<ServiceWorkerManager> swm = ServiceWorkerManager::GetInstance();
-    if (swm) {
-      swm->ReportToAllClients(PromiseFlatCString(aScope),
-                              PromiseFlatString(aMessage),
-                              NS_ConvertUTF8toUTF16(aScope), /* aFilename */
-                              EmptyString(), /* aLine */
-                              0, /* aLineNumber */
-                              0, /* aColumnNumber */
-                              aFlags);
-    }
-    return;
+    // e10s is disabled; notify workers in the parent.
+    return aDispatcher.NotifyWorkers();
   }
 
-  // Parent process, e10s enabled.
-  nsTArray<ContentParent*> contentActors;
-  ContentParent::GetAll(contentActors);
-  if (!contentActors.IsEmpty()) {
-    // At least one content process active.
-    for (uint32_t i = 0; i < contentActors.Length(); ++i) {
-      Unused << NS_WARN_IF(
-        !contentActors[i]->SendPushError(PromiseFlatCString(aScope),
-          PromiseFlatString(aMessage), aFlags));
-    }
-    return;
-  }
-  // Report to the console if no content processes are active.
-  nsCOMPtr<nsIURI> scopeURI;
-  nsresult rv = NS_NewURI(getter_AddRefs(scopeURI), aScope);
-  if (NS_WARN_IF(NS_FAILED(rv))) {
-    return;
-  }
-  Unused << NS_WARN_IF(NS_FAILED(
-    nsContentUtils::ReportToConsoleNonLocalized(aMessage,
-                                                aFlags,
-                                                NS_LITERAL_CSTRING("Push"),
-                                                nullptr, /* aDocument */
-                                                scopeURI, /* aURI */
-                                                EmptyString(), /* aLine */
-                                                0, /* aLineNumber */
-                                                0, /* aColumnNumber */
-                                                nsContentUtils::eOMIT_LOCATION)));
-}
+  // Otherwise, we're in the content process, so e10s must be enabled. Notify
+  // observers and workers, then send a message to notify observers in the
+  // parent.
+  MOZ_ASSERT(XRE_IsContentProcess());
 
-nsresult
-PushNotifier::NotifyPushObservers(const nsACString& aScope,
-                                  nsIPrincipal* aPrincipal,
-                                  const Maybe<nsTArray<uint8_t>>& aData)
-{
-  nsCOMPtr<nsIPushData> data;
-  if (aData) {
-    data = new PushData(aData.ref());
-  }
-  nsCOMPtr<nsIPushMessage> message = new PushMessage(aPrincipal, data);
-  return DoNotifyObservers(message, OBSERVER_TOPIC_PUSH, aScope);
-}
+  nsresult rv = aDispatcher.NotifyObserversAndWorkers();
 
-nsresult
-PushNotifier::NotifySubscriptionChangeObservers(const nsACString& aScope,
-                                                nsIPrincipal* aPrincipal)
-{
-  return DoNotifyObservers(aPrincipal, OBSERVER_TOPIC_SUBSCRIPTION_CHANGE,
-                           aScope);
-}
-
-nsresult
-PushNotifier::NotifySubscriptionModifiedObservers(const nsACString& aScope,
-                                                  nsIPrincipal* aPrincipal)
-{
-  return DoNotifyObservers(aPrincipal, OBSERVER_TOPIC_SUBSCRIPTION_MODIFIED,
-                           aScope);
-}
-
-nsresult
-PushNotifier::DoNotifyObservers(nsISupports *aSubject, const char *aTopic,
-                                const nsACString& aScope)
-{
-  nsCOMPtr<nsIObserverService> obsService =
-    mozilla::services::GetObserverService();
-  if (!obsService) {
-    return NS_ERROR_FAILURE;
+  ContentChild* parentActor = ContentChild::GetSingleton();
+  if (!NS_WARN_IF(!parentActor)) {
+    Unused << NS_WARN_IF(!aDispatcher.SendToParent(parentActor));
   }
-  // If there's a service for this push category, make sure it is alive.
-  nsCOMPtr<nsICategoryManager> catMan =
-    do_GetService(NS_CATEGORYMANAGER_CONTRACTID);
-  if (catMan) {
-    nsXPIDLCString contractId;
-    nsresult rv = catMan->GetCategoryEntry("push",
-                                           PromiseFlatCString(aScope).get(),
-                                           getter_Copies(contractId));
-    if (NS_SUCCEEDED(rv)) {
-      // Ensure the service is created - we don't need to do anything with
-      // it though - we assume the service constructor attaches a listener.
-      nsCOMPtr<nsISupports> service = do_GetService(contractId);
-    }
-  }
-  return obsService->NotifyObservers(aSubject, aTopic,
-                                     NS_ConvertUTF8toUTF16(aScope).get());
-}
 
-bool
-PushNotifier::ShouldNotifyWorkers(nsIPrincipal* aPrincipal)
-{
-  // System subscriptions use observer notifications instead of service worker
-  // events. The `testing.notifyWorkers` pref disables worker events for
-  // non-system subscriptions.
-  return !nsContentUtils::IsSystemPrincipal(aPrincipal) &&
-         Preferences::GetBool("dom.push.testing.notifyWorkers", true);
+  return rv;
 }
 
 PushData::PushData(const nsTArray<uint8_t>& aData)
   : mData(aData)
 {}
 
 PushData::~PushData()
 {}
@@ -476,10 +246,296 @@ PushMessage::GetData(nsIPushData** aData
 {
   NS_ENSURE_ARG_POINTER(aData);
 
   nsCOMPtr<nsIPushData> data = mData;
   data.forget(aData);
   return NS_OK;
 }
 
+PushDispatcher::PushDispatcher(const nsACString& aScope,
+                               nsIPrincipal* aPrincipal)
+  : mScope(aScope)
+  , mPrincipal(aPrincipal)
+{}
+
+PushDispatcher::~PushDispatcher()
+{}
+
+nsresult
+PushDispatcher::HandleNoChildProcesses()
+{
+  return NS_OK;
+}
+
+nsresult
+PushDispatcher::NotifyObserversAndWorkers()
+{
+  Unused << NS_WARN_IF(NS_FAILED(NotifyObservers()));
+  return NotifyWorkers();
+}
+
+bool
+PushDispatcher::ShouldNotifyWorkers()
+{
+  // System subscriptions use observer notifications instead of service worker
+  // events. The `testing.notifyWorkers` pref disables worker events for
+  // non-system subscriptions.
+  return !nsContentUtils::IsSystemPrincipal(mPrincipal) &&
+         Preferences::GetBool("dom.push.testing.notifyWorkers", true);
+}
+
+nsresult
+PushDispatcher::DoNotifyObservers(nsISupports *aSubject, const char *aTopic,
+                                  const nsACString& aScope)
+{
+  nsCOMPtr<nsIObserverService> obsService =
+    mozilla::services::GetObserverService();
+  if (!obsService) {
+    return NS_ERROR_FAILURE;
+  }
+  // If there's a service for this push category, make sure it is alive.
+  nsCOMPtr<nsICategoryManager> catMan =
+    do_GetService(NS_CATEGORYMANAGER_CONTRACTID);
+  if (catMan) {
+    nsXPIDLCString contractId;
+    nsresult rv = catMan->GetCategoryEntry("push",
+                                           mScope.BeginReading(),
+                                           getter_Copies(contractId));
+    if (NS_SUCCEEDED(rv)) {
+      // Ensure the service is created - we don't need to do anything with
+      // it though - we assume the service constructor attaches a listener.
+      nsCOMPtr<nsISupports> service = do_GetService(contractId);
+    }
+  }
+  return obsService->NotifyObservers(aSubject, aTopic,
+                                     NS_ConvertUTF8toUTF16(mScope).get());
+}
+
+PushMessageDispatcher::PushMessageDispatcher(const nsACString& aScope,
+                                             nsIPrincipal* aPrincipal,
+                                             const nsAString& aMessageId,
+                                             const Maybe<nsTArray<uint8_t>>& aData)
+  : PushDispatcher(aScope, aPrincipal)
+  , mMessageId(aMessageId)
+  , mData(aData)
+{}
+
+PushMessageDispatcher::~PushMessageDispatcher()
+{}
+
+nsresult
+PushMessageDispatcher::NotifyObservers()
+{
+  nsCOMPtr<nsIPushData> data;
+  if (mData) {
+    data = new PushData(mData.ref());
+  }
+  nsCOMPtr<nsIPushMessage> message = new PushMessage(mPrincipal, data);
+  return DoNotifyObservers(message, OBSERVER_TOPIC_PUSH, mScope);
+}
+
+nsresult
+PushMessageDispatcher::NotifyWorkers()
+{
+  if (!ShouldNotifyWorkers()) {
+    return NS_OK;
+  }
+  RefPtr<ServiceWorkerManager> swm = ServiceWorkerManager::GetInstance();
+  if (!swm) {
+    return NS_ERROR_FAILURE;
+  }
+  nsAutoCString originSuffix;
+  nsresult rv = mPrincipal->GetOriginSuffix(originSuffix);
+  if (NS_WARN_IF(NS_FAILED(rv))) {
+    return rv;
+  }
+  return swm->SendPushEvent(originSuffix, mScope, mMessageId, mData);
+}
+
+bool
+PushMessageDispatcher::SendToParent(ContentChild* aParentActor)
+{
+  if (mData) {
+    return aParentActor->SendNotifyPushObserversWithData(mScope,
+                                                         IPC::Principal(mPrincipal),
+                                                         mMessageId,
+                                                         mData.ref());
+  }
+  return aParentActor->SendNotifyPushObservers(mScope,
+                                               IPC::Principal(mPrincipal),
+                                               mMessageId);
+}
+
+bool
+PushMessageDispatcher::SendToChild(ContentParent* aContentActor)
+{
+  if (mData) {
+    return aContentActor->SendPushWithData(mScope, IPC::Principal(mPrincipal),
+                                           mMessageId, mData.ref());
+  }
+  return aContentActor->SendPush(mScope, IPC::Principal(mPrincipal),
+                                 mMessageId);
+}
+
+PushSubscriptionChangeDispatcher::PushSubscriptionChangeDispatcher(const nsACString& aScope,
+                                                                   nsIPrincipal* aPrincipal)
+  : PushDispatcher(aScope, aPrincipal)
+{}
+
+PushSubscriptionChangeDispatcher::~PushSubscriptionChangeDispatcher()
+{}
+
+nsresult
+PushSubscriptionChangeDispatcher::NotifyObservers()
+{
+  return DoNotifyObservers(mPrincipal, OBSERVER_TOPIC_SUBSCRIPTION_CHANGE,
+                           mScope);
+}
+
+nsresult
+PushSubscriptionChangeDispatcher::NotifyWorkers()
+{
+  if (!ShouldNotifyWorkers()) {
+    return NS_OK;
+  }
+  RefPtr<ServiceWorkerManager> swm = ServiceWorkerManager::GetInstance();
+  if (!swm) {
+    return NS_ERROR_FAILURE;
+  }
+  nsAutoCString originSuffix;
+  nsresult rv = mPrincipal->GetOriginSuffix(originSuffix);
+  if (NS_WARN_IF(NS_FAILED(rv))) {
+    return rv;
+  }
+  return swm->SendPushSubscriptionChangeEvent(originSuffix, mScope);
+}
+
+bool
+PushSubscriptionChangeDispatcher::SendToParent(ContentChild* aParentActor)
+{
+  return aParentActor->SendNotifyPushSubscriptionChangeObservers(mScope,
+                                                                 IPC::Principal(mPrincipal));
+}
+
+bool
+PushSubscriptionChangeDispatcher::SendToChild(ContentParent* aContentActor)
+{
+  return aContentActor->SendPushSubscriptionChange(mScope,
+                                                   IPC::Principal(mPrincipal));
+}
+
+PushSubscriptionModifiedDispatcher::PushSubscriptionModifiedDispatcher(const nsACString& aScope,
+                                                                       nsIPrincipal* aPrincipal)
+  : PushDispatcher(aScope, aPrincipal)
+{}
+
+PushSubscriptionModifiedDispatcher::~PushSubscriptionModifiedDispatcher()
+{}
+
+nsresult
+PushSubscriptionModifiedDispatcher::NotifyObservers()
+{
+  return DoNotifyObservers(mPrincipal, OBSERVER_TOPIC_SUBSCRIPTION_MODIFIED,
+                           mScope);
+}
+
+nsresult
+PushSubscriptionModifiedDispatcher::NotifyWorkers()
+{
+  return NS_OK;
+}
+
+bool
+PushSubscriptionModifiedDispatcher::SendToParent(ContentChild* aParentActor)
+{
+  return aParentActor->SendNotifyPushSubscriptionModifiedObservers(mScope,
+                                                                   IPC::Principal(mPrincipal));
+}
+
+bool
+PushSubscriptionModifiedDispatcher::SendToChild(ContentParent*)
+{
+  return true;
+}
+
+PushErrorDispatcher::PushErrorDispatcher(const nsACString& aScope,
+                                         nsIPrincipal* aPrincipal,
+                                         const nsAString& aMessage,
+                                         uint32_t aFlags)
+  : PushDispatcher(aScope, aPrincipal)
+  , mMessage(aMessage)
+  , mFlags(aFlags)
+{}
+
+PushErrorDispatcher::~PushErrorDispatcher()
+{}
+
+nsresult
+PushErrorDispatcher::NotifyObservers()
+{
+  return NS_OK;
+}
+
+nsresult
+PushErrorDispatcher::NotifyWorkers()
+{
+  if (!ShouldNotifyWorkers()) {
+    // For system subscriptions, log the error directly to the browser console.
+    return nsContentUtils::ReportToConsoleNonLocalized(mMessage,
+                                                       mFlags,
+                                                       NS_LITERAL_CSTRING("Push"),
+                                                       nullptr, /* aDocument */
+                                                       nullptr, /* aURI */
+                                                       EmptyString(), /* aLine */
+                                                       0, /* aLineNumber */
+                                                       0, /* aColumnNumber */
+                                                       nsContentUtils::eOMIT_LOCATION);
+  }
+  // For service worker subscriptions, report the error to all clients.
+  RefPtr<ServiceWorkerManager> swm = ServiceWorkerManager::GetInstance();
+  if (swm) {
+    swm->ReportToAllClients(mScope,
+                            mMessage,
+                            NS_ConvertUTF8toUTF16(mScope), /* aFilename */
+                            EmptyString(), /* aLine */
+                            0, /* aLineNumber */
+                            0, /* aColumnNumber */
+                            mFlags);
+  }
+  return NS_OK;
+}
+
+bool
+PushErrorDispatcher::SendToParent(ContentChild*)
+{
+  return true;
+}
+
+bool
+PushErrorDispatcher::SendToChild(ContentParent* aContentActor)
+{
+  return aContentActor->SendPushError(mScope, IPC::Principal(mPrincipal),
+                                      mMessage, mFlags);
+}
+
+nsresult
+PushErrorDispatcher::HandleNoChildProcesses()
+{
+  // Report to the console if no content processes are active.
+  nsCOMPtr<nsIURI> scopeURI;
+  nsresult rv = NS_NewURI(getter_AddRefs(scopeURI), mScope);
+  if (NS_WARN_IF(NS_FAILED(rv))) {
+    return rv;
+  }
+  return nsContentUtils::ReportToConsoleNonLocalized(mMessage,
+                                                     mFlags,
+                                                     NS_LITERAL_CSTRING("Push"),
+                                                     nullptr, /* aDocument */
+                                                     scopeURI, /* aURI */
+                                                     EmptyString(), /* aLine */
+                                                     0, /* aLineNumber */
+                                                     0, /* aColumnNumber */
+                                                     nsContentUtils::eOMIT_LOCATION);
+}
+
 } // namespace dom
 } // namespace mozilla
--- a/dom/push/PushNotifier.h
+++ b/dom/push/PushNotifier.h
@@ -6,19 +6,70 @@
 #define mozilla_dom_PushNotifier_h
 
 #include "nsIPushNotifier.h"
 
 #include "nsCycleCollectionParticipant.h"
 #include "nsIPrincipal.h"
 #include "nsString.h"
 
+#include "mozilla/Maybe.h"
+
 namespace mozilla {
 namespace dom {
 
+class ContentChild;
+class ContentParent;
+
+/**
+ * `PushDispatcher` is a base class used to forward observer notifications and
+ * service worker events to the correct process.
+ */
+class MOZ_STACK_CLASS PushDispatcher
+{
+public:
+  // Fires an XPCOM observer notification. This method may be called from both
+  // processes.
+  virtual nsresult NotifyObservers() = 0;
+
+  // Fires a service worker event. This method is called from the content
+  // process if e10s is enabled, or the parent otherwise.
+  virtual nsresult NotifyWorkers() = 0;
+
+  // A convenience method that calls `NotifyObservers` and `NotifyWorkers`.
+  nsresult NotifyObserversAndWorkers();
+
+  // Sends an IPDL message to fire an observer notification in the parent
+  // process. This method is only called from the content process, and only
+  // if e10s is enabled.
+  virtual bool SendToParent(ContentChild* aParentActor) = 0;
+
+  // Sends an IPDL message to fire an observer notification and a service worker
+  // event in the content process. This method is only called from the parent,
+  // and only if e10s is enabled.
+  virtual bool SendToChild(ContentParent* aContentActor) = 0;
+
+  // An optional method, called from the parent if e10s is enabled and there
+  // are no active content processes. The default behavior is a no-op.
+  virtual nsresult HandleNoChildProcesses();
+
+protected:
+  PushDispatcher(const nsACString& aScope,
+                 nsIPrincipal* aPrincipal);
+
+  virtual ~PushDispatcher();
+
+  bool ShouldNotifyWorkers();
+  nsresult DoNotifyObservers(nsISupports *aSubject, const char *aTopic,
+                             const nsACString& aScope);
+
+  const nsCString mScope;
+  nsCOMPtr<nsIPrincipal> mPrincipal;
+};
+
 /**
  * `PushNotifier` implements the `nsIPushNotifier` interface. This service
  * broadcasts XPCOM observer notifications for incoming push messages, then
  * forwards incoming push messages to service workers.
  *
  * All scriptable methods on this interface may be called from the parent or
  * content process. Observer notifications are broadcasted to both processes.
  */
@@ -27,41 +78,36 @@ class PushNotifier final : public nsIPus
 public:
   PushNotifier();
 
   NS_DECL_CYCLE_COLLECTING_ISUPPORTS
   NS_DECL_CYCLE_COLLECTION_CLASS_AMBIGUOUS(PushNotifier, nsIPushNotifier)
   NS_DECL_NSIPUSHNOTIFIER
 
 private:
-  virtual ~PushNotifier();
+  ~PushNotifier();
 
-  nsresult NotifyPush(const nsACString& aScope, nsIPrincipal* aPrincipal,
-                      const nsAString& aMessageId,
-                      const Maybe<nsTArray<uint8_t>>& aData);
-  nsresult DoNotifyObservers(nsISupports *aSubject, const char *aTopic,
-                             const nsACString& aScope);
-  bool ShouldNotifyWorkers(nsIPrincipal* aPrincipal);
+  nsresult Dispatch(PushDispatcher& aDispatcher);
 };
 
 /**
  * `PushData` provides methods for retrieving push message data in different
  * formats. This class is similar to the `PushMessageData` WebIDL interface.
  */
 class PushData final : public nsIPushData
 {
 public:
   explicit PushData(const nsTArray<uint8_t>& aData);
 
   NS_DECL_CYCLE_COLLECTING_ISUPPORTS
   NS_DECL_CYCLE_COLLECTION_CLASS_AMBIGUOUS(PushData, nsIPushData)
   NS_DECL_NSIPUSHDATA
 
 private:
-  virtual ~PushData();
+  ~PushData();
 
   nsresult EnsureDecodedText();
 
   nsTArray<uint8_t> mData;
   nsString mDecodedText;
 };
 
 /**
@@ -74,18 +120,84 @@ class PushMessage final : public nsIPush
 public:
   PushMessage(nsIPrincipal* aPrincipal, nsIPushData* aData);
 
   NS_DECL_CYCLE_COLLECTING_ISUPPORTS
   NS_DECL_CYCLE_COLLECTION_CLASS_AMBIGUOUS(PushMessage, nsIPushMessage)
   NS_DECL_NSIPUSHMESSAGE
 
 private:
-  virtual ~PushMessage();
+  ~PushMessage();
 
   nsCOMPtr<nsIPrincipal> mPrincipal;
   nsCOMPtr<nsIPushData> mData;
 };
 
+class PushMessageDispatcher final : public PushDispatcher
+{
+public:
+  PushMessageDispatcher(const nsACString& aScope,
+               nsIPrincipal* aPrincipal,
+               const nsAString& aMessageId,
+               const Maybe<nsTArray<uint8_t>>& aData);
+  ~PushMessageDispatcher();
+
+  nsresult NotifyObservers() override;
+  nsresult NotifyWorkers() override;
+  bool SendToParent(ContentChild* aParentActor) override;
+  bool SendToChild(ContentParent* aContentActor) override;
+
+private:
+  const nsString mMessageId;
+  const Maybe<nsTArray<uint8_t>> mData;
+};
+
+class PushSubscriptionChangeDispatcher final : public PushDispatcher
+{
+public:
+  PushSubscriptionChangeDispatcher(const nsACString& aScope,
+                                 nsIPrincipal* aPrincipal);
+  ~PushSubscriptionChangeDispatcher();
+
+  nsresult NotifyObservers() override;
+  nsresult NotifyWorkers() override;
+  bool SendToParent(ContentChild* aParentActor) override;
+  bool SendToChild(ContentParent* aContentActor) override;
+};
+
+class PushSubscriptionModifiedDispatcher : public PushDispatcher
+{
+public:
+  PushSubscriptionModifiedDispatcher(const nsACString& aScope,
+                                     nsIPrincipal* aPrincipal);
+  ~PushSubscriptionModifiedDispatcher();
+
+  nsresult NotifyObservers() override;
+  nsresult NotifyWorkers() override;
+  bool SendToParent(ContentChild* aParentActor) override;
+  bool SendToChild(ContentParent* aContentActor) override;
+};
+
+class PushErrorDispatcher final : public PushDispatcher
+{
+public:
+  PushErrorDispatcher(const nsACString& aScope,
+                      nsIPrincipal* aPrincipal,
+                      const nsAString& aMessage,
+                      uint32_t aFlags);
+  ~PushErrorDispatcher();
+
+  nsresult NotifyObservers() override;
+  nsresult NotifyWorkers() override;
+  bool SendToParent(ContentChild* aParentActor) override;
+  bool SendToChild(ContentParent* aContentActor) override;
+
+private:
+  nsresult HandleNoChildProcesses() override;
+
+  const nsString mMessage;
+  uint32_t mFlags;
+};
+
 } // namespace dom
 } // namespace mozilla
 
 #endif // mozilla_dom_PushNotifier_h