Bug 1637648 - P1: Make nsIOService the single point to forward observer notifications to socket process r=dragana
authorKershaw Chang <kershaw@mozilla.com>
Tue, 23 Jun 2020 11:11:01 +0000
changeset 536865 6fc6ec584f86d227413e2012f2cc574564f2e49d
parent 536864 38c2fa230269c6e1d0f6022cfb578b790cf2e6f5
child 536866 b284d915dc1c30907def49fc9d167299332702cf
push id37533
push userdluca@mozilla.com
push dateTue, 23 Jun 2020 21:38:40 +0000
treeherdermozilla-central@d48aa0f0aa0b [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersdragana
bugs1637648
milestone79.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 1637648 - P1: Make nsIOService the single point to forward observer notifications to socket process r=dragana Differential Revision: https://phabricator.services.mozilla.com/D77305
netwerk/base/nsIOService.cpp
netwerk/base/nsIOService.h
netwerk/ipc/PSocketProcess.ipdl
netwerk/ipc/SocketProcessChild.cpp
netwerk/ipc/SocketProcessChild.h
netwerk/ipc/SocketProcessHost.cpp
netwerk/ipc/SocketProcessHost.h
--- a/netwerk/base/nsIOService.cpp
+++ b/netwerk/base/nsIOService.cpp
@@ -265,44 +265,109 @@ nsresult nsIOService::Init() {
   for (int i = 0; gBadPortList[i]; i++)
     mRestrictedPortList.AppendElement(gBadPortList[i]);
 
   // Further modifications to the port list come from prefs
   Preferences::RegisterPrefixCallbacks(nsIOService::PrefsChanged,
                                        gCallbackPrefs, this);
   PrefsChanged();
 
+  mSocketProcessTopicBlackList.PutEntry(
+      NS_LITERAL_CSTRING(NS_XPCOM_WILL_SHUTDOWN_OBSERVER_ID));
+  mSocketProcessTopicBlackList.PutEntry(
+      NS_LITERAL_CSTRING(NS_XPCOM_SHUTDOWN_OBSERVER_ID));
+  mSocketProcessTopicBlackList.PutEntry(
+      NS_LITERAL_CSTRING("xpcom-shutdown-threads"));
+  mSocketProcessTopicBlackList.PutEntry(
+      NS_LITERAL_CSTRING("profile-do-change"));
+
   // Register for profile change notifications
-  nsCOMPtr<nsIObserverService> observerService = services::GetObserverService();
-  if (observerService) {
-    observerService->AddObserver(this, kProfileChangeNetTeardownTopic, true);
-    observerService->AddObserver(this, kProfileChangeNetRestoreTopic, true);
-    observerService->AddObserver(this, kProfileDoChange, true);
-    observerService->AddObserver(this, NS_XPCOM_SHUTDOWN_OBSERVER_ID, true);
-    observerService->AddObserver(this, NS_NETWORK_LINK_TOPIC, true);
-    observerService->AddObserver(this, NS_NETWORK_ID_CHANGED_TOPIC, true);
-    observerService->AddObserver(this, NS_WIDGET_WAKE_OBSERVER_TOPIC, true);
-  } else
-    NS_WARNING("failed to get observer service");
+  mObserverService = services::GetObserverService();
+  AddObserver(this, kProfileChangeNetTeardownTopic, true);
+  AddObserver(this, kProfileChangeNetRestoreTopic, true);
+  AddObserver(this, kProfileDoChange, true);
+  AddObserver(this, NS_XPCOM_SHUTDOWN_OBSERVER_ID, true);
+  AddObserver(this, NS_NETWORK_LINK_TOPIC, true);
+  AddObserver(this, NS_NETWORK_ID_CHANGED_TOPIC, true);
+  AddObserver(this, NS_WIDGET_WAKE_OBSERVER_TOPIC, true);
 
   if (IsSocketProcessChild()) {
     Preferences::RegisterCallbacks(nsIOService::OnTLSPrefChange,
                                    gCallbackSecurityPrefs, this);
   }
 
   gIOService = this;
 
   InitializeNetworkLinkService();
   InitializeProtocolProxyService();
 
   SetOffline(false);
 
   return NS_OK;
 }
 
+NS_IMETHODIMP
+nsIOService::AddObserver(nsIObserver* aObserver, const char* aTopic,
+                         bool aOwnsWeak) {
+  if (!mObserverService) {
+    return NS_ERROR_FAILURE;
+  }
+
+  // Register for the origional observer.
+  nsresult rv = mObserverService->AddObserver(aObserver, aTopic, aOwnsWeak);
+  if (NS_FAILED(rv)) {
+    return rv;
+  }
+
+  if (!XRE_IsParentProcess()) {
+    return NS_OK;
+  }
+
+  if (!UseSocketProcess()) {
+    return NS_OK;
+  }
+
+  nsAutoCString topic(aTopic);
+  if (mSocketProcessTopicBlackList.Contains(topic)) {
+    return NS_ERROR_FAILURE;
+  }
+
+  // Avoid registering  duplicate topics.
+  if (mObserverTopicForSocketProcess.Contains(topic)) {
+    return NS_ERROR_FAILURE;
+  }
+
+  mObserverTopicForSocketProcess.PutEntry(topic);
+
+  // This happens when AddObserver() is called by nsIOService::Init(). We don't
+  // want to add nsIOService again.
+  if (SameCOMIdentity(aObserver, static_cast<nsIObserver*>(this))) {
+    return NS_OK;
+  }
+
+  return mObserverService->AddObserver(this, aTopic, true);
+}
+
+NS_IMETHODIMP
+nsIOService::RemoveObserver(nsIObserver* aObserver, const char* aTopic) {
+  return NS_ERROR_NOT_IMPLEMENTED;
+}
+
+NS_IMETHODIMP
+nsIOService::EnumerateObservers(const char* aTopic,
+                                nsISimpleEnumerator** anEnumerator) {
+  return NS_ERROR_NOT_IMPLEMENTED;
+}
+
+NS_IMETHODIMP nsIOService::NotifyObservers(nsISupports* aSubject,
+                                           const char* aTopic,
+                                           const char16_t* aSomeData) {
+  return NS_ERROR_NOT_IMPLEMENTED;
+}
+
 nsIOService::~nsIOService() {
   if (gIOService) {
     MOZ_ASSERT(gIOService == this);
     gIOService = nullptr;
   }
 }
 
 // static
@@ -621,17 +686,18 @@ nsIOService::SocketProcessTelemetryPing(
   CallOrWaitForSocketProcess([]() {
     Unused << gIOService->mSocketProcess->GetActor()
                   ->SendSocketProcessTelemetryPing();
   });
   return NS_OK;
 }
 
 NS_IMPL_ISUPPORTS(nsIOService, nsIIOService, nsINetUtil, nsISpeculativeConnect,
-                  nsIObserver, nsIIOServiceInternal, nsISupportsWeakReference)
+                  nsIObserver, nsIIOServiceInternal, nsISupportsWeakReference,
+                  nsIObserverService)
 
 ////////////////////////////////////////////////////////////////////////////////
 
 nsresult nsIOService::RecheckCaptivePortal() {
   MOZ_ASSERT(NS_IsMainThread(), "Must be called on the main thread");
   if (!mCaptivePortalService) {
     return NS_OK;
   }
@@ -1190,16 +1256,19 @@ nsIOService::SetOffline(bool offline) {
   NS_ASSERTION(observerService, "The observer service should not be null");
 
   if (XRE_IsParentProcess()) {
     if (observerService) {
       (void)observerService->NotifyObservers(nullptr,
                                              NS_IPC_IOSERVICE_SET_OFFLINE_TOPIC,
                                              offline ? u"true" : u"false");
     }
+    if (SocketProcessReady()) {
+      Unused << mSocketProcess->GetActor()->SendSetOffline(offline);
+    }
   }
 
   nsIIOService* subject = static_cast<nsIIOService*>(this);
   while (mSetOfflineValue != mOffline) {
     offline = mSetOfflineValue;
 
     if (offline && !mOffline) {
       mOffline = true;  // indicate we're trying to shutdown
@@ -1579,16 +1648,27 @@ nsIOService::Observe(nsISupports* subjec
   } else if (!strcmp(topic, NS_WIDGET_WAKE_OBSERVER_TOPIC)) {
     // coming back alive from sleep
     // this indirection brought to you by:
     // https://bugzilla.mozilla.org/show_bug.cgi?id=1152048#c19
     nsCOMPtr<nsIRunnable> wakeupNotifier = new nsWakeupNotifier(this);
     NS_DispatchToMainThread(wakeupNotifier);
   }
 
+  if (UseSocketProcess() &&
+      mObserverTopicForSocketProcess.Contains(nsDependentCString(topic))) {
+    nsCString topicStr(topic);
+    nsString dataStr(data);
+    auto sendObserver = [topicStr, dataStr]() {
+      Unused << gIOService->mSocketProcess->GetActor()->SendNotifyObserver(
+          topicStr, dataStr);
+    };
+    CallOrWaitForSocketProcess(sendObserver);
+  }
+
   return NS_OK;
 }
 
 // nsINetUtil interface
 NS_IMETHODIMP
 nsIOService::ParseRequestContentType(const nsACString& aTypeHeader,
                                      nsACString& aCharset, bool* aHadCharset,
                                      nsACString& aContentType) {
--- a/netwerk/base/nsIOService.h
+++ b/netwerk/base/nsIOService.h
@@ -17,16 +17,17 @@
 #include "nsCategoryCache.h"
 #include "nsISpeculativeConnect.h"
 #include "nsDataHashtable.h"
 #include "mozilla/Atomics.h"
 #include "mozilla/Attributes.h"
 #include "mozilla/Mutex.h"
 #include "prtime.h"
 #include "nsICaptivePortalService.h"
+#include "nsIObserverService.h"
 
 #define NS_N(x) (sizeof(x) / sizeof(*x))
 
 // We don't want to expose this observer topic.
 // Intended internal use only for remoting offline/inline events.
 // See Bug 552829
 #define NS_IPC_IOSERVICE_SET_OFFLINE_TOPIC "ipc:network:set-offline"
 #define NS_IPC_IOSERVICE_SET_CONNECTIVITY_TOPIC "ipc:network:set-connectivity"
@@ -53,24 +54,26 @@ class nsAsyncRedirectVerifyHelper;
 class SocketProcessHost;
 class SocketProcessMemoryReporter;
 
 class nsIOService final : public nsIIOService,
                           public nsIObserver,
                           public nsINetUtil,
                           public nsISpeculativeConnect,
                           public nsSupportsWeakReference,
-                          public nsIIOServiceInternal {
+                          public nsIIOServiceInternal,
+                          public nsIObserverService {
  public:
   NS_DECL_THREADSAFE_ISUPPORTS
   NS_DECL_NSIIOSERVICE
   NS_DECL_NSIOBSERVER
   NS_DECL_NSINETUTIL
   NS_DECL_NSISPECULATIVECONNECT
   NS_DECL_NSIIOSERVICEINTERNAL
+  NS_DECL_NSIOBSERVERSERVICE
 
   // Gets the singleton instance of the IO Service, creating it as needed
   // Returns nullptr on out of memory or failure to initialize.
   static already_AddRefed<nsIOService> GetInstance();
 
   nsresult Init();
   nsresult NewURI(const char* aSpec, nsIURI* aBaseURI, nsIURI** result,
                   nsIProtocolHandler** hdlrResult);
@@ -241,16 +244,24 @@ class nsIOService final : public nsIIOSe
 
   SocketProcessHost* mSocketProcess;
 
   // Events should be executed after the socket process is launched. Will
   // dispatch these events while socket process fires OnProcessLaunchComplete.
   // Note: this array is accessed only on the main thread.
   nsTArray<std::function<void()>> mPendingEvents;
 
+  // The observer notifications need to be forwarded to socket process.
+  nsTHashtable<nsCStringHashKey> mObserverTopicForSocketProcess;
+  // Some noticications (e.g., NS_XPCOM_SHUTDOWN_OBSERVER_ID) are triggered in
+  // socket process, so we should not send the notifications again.
+  nsTHashtable<nsCStringHashKey> mSocketProcessTopicBlackList;
+
+  nsCOMPtr<nsIObserverService> mObserverService;
+
  public:
   // Used for all default buffer sizes that necko allocates.
   static uint32_t gDefaultSegmentSize;
   static uint32_t gDefaultSegmentCount;
 };
 
 /**
  * Reference to the IO service singleton. May be null.
--- a/netwerk/ipc/PSocketProcess.ipdl
+++ b/netwerk/ipc/PSocketProcess.ipdl
@@ -134,16 +134,17 @@ child:
                                    uint32_t maxBytesPerSecond);
   async PAltSvcTransaction(HttpConnectionInfoCloneArgs aConnInfo,
                            uint32_t aCaps);
   async ClearSessionCache();
   async PTRRService(bool aCaptiveIsPassed,
                     bool aParentalControlEnabled,
                     nsCString[] aDNSSuffixList);
   async PNativeDNSResolverOverride();
+  async NotifyObserver(nsCString aTopic, nsString aData);
 
 both:
   async PFileDescriptorSet(FileDescriptor fd);
   async PDNSRequest(nsCString hostName, nsCString trrServer, uint16_t type,
                     OriginAttributes originAttributes, uint32_t flags);
 };
 
 } // namespace net
--- a/netwerk/ipc/SocketProcessChild.cpp
+++ b/netwerk/ipc/SocketProcessChild.cpp
@@ -449,10 +449,19 @@ SocketProcessChild::AllocPNativeDNSResol
 }
 
 mozilla::ipc::IPCResult
 SocketProcessChild::RecvPNativeDNSResolverOverrideConstructor(
     PNativeDNSResolverOverrideChild* aActor) {
   return IPC_OK();
 }
 
+mozilla::ipc::IPCResult SocketProcessChild::RecvNotifyObserver(
+    const nsCString& aTopic, const nsString& aData) {
+  nsCOMPtr<nsIObserverService> obs = mozilla::services::GetObserverService();
+  if (obs) {
+    obs->NotifyObservers(nullptr, aTopic.get(), aData.get());
+  }
+  return IPC_OK();
+}
+
 }  // namespace net
 }  // namespace mozilla
--- a/netwerk/ipc/SocketProcessChild.h
+++ b/netwerk/ipc/SocketProcessChild.h
@@ -115,16 +115,19 @@ class SocketProcessChild final
       const bool& aParentalControlEnabled,
       nsTArray<nsCString>&& aDNSSuffixList) override;
 
   already_AddRefed<PNativeDNSResolverOverrideChild>
   AllocPNativeDNSResolverOverrideChild();
   mozilla::ipc::IPCResult RecvPNativeDNSResolverOverrideConstructor(
       PNativeDNSResolverOverrideChild* aActor) override;
 
+  mozilla::ipc::IPCResult RecvNotifyObserver(const nsCString& aTopic,
+                                             const nsString& aData);
+
  protected:
   friend class SocketProcessImpl;
   ~SocketProcessChild();
 
  private:
   // Mapping of content process id and the SocketProcessBridgeParent.
   // This table keeps SocketProcessBridgeParent alive in socket process.
   nsRefPtrHashtable<nsUint32HashKey, SocketProcessBridgeParent>
--- a/netwerk/ipc/SocketProcessHost.cpp
+++ b/netwerk/ipc/SocketProcessHost.cpp
@@ -26,75 +26,16 @@
 #  include "mozilla/Sandbox.h"
 #endif
 
 using namespace mozilla::ipc;
 
 namespace mozilla {
 namespace net {
 
-#define NS_IPC_IOSERVICE_SET_OFFLINE_TOPIC "ipc:network:set-offline"
-
-class OfflineObserver final : public nsIObserver {
- public:
-  NS_DECL_THREADSAFE_ISUPPORTS
-  explicit OfflineObserver(SocketProcessHost* aProcessHost)
-      : mProcessHost(aProcessHost) {
-    MOZ_ASSERT(NS_IsMainThread());
-
-    nsCOMPtr<nsIObserverService> obs = mozilla::services::GetObserverService();
-    if (obs) {
-      obs->AddObserver(this, NS_IPC_IOSERVICE_SET_OFFLINE_TOPIC, false);
-      obs->AddObserver(this, NS_XPCOM_WILL_SHUTDOWN_OBSERVER_ID, false);
-    }
-  }
-
-  void Destroy() {
-    MOZ_ASSERT(NS_IsMainThread());
-
-    nsCOMPtr<nsIObserverService> obs = mozilla::services::GetObserverService();
-    if (obs) {
-      obs->RemoveObserver(this, NS_IPC_IOSERVICE_SET_OFFLINE_TOPIC);
-    }
-    mProcessHost = nullptr;
-  }
-
- private:
-  // nsIObserver implementation.
-  NS_IMETHOD
-  Observe(nsISupports* aSubject, const char* aTopic,
-          const char16_t* aData) override {
-    if (!mProcessHost) {
-      return NS_OK;
-    }
-
-    if (!strcmp(aTopic, NS_IPC_IOSERVICE_SET_OFFLINE_TOPIC)) {
-      NS_ConvertUTF16toUTF8 dataStr(aData);
-      const char* offline = dataStr.get();
-      if (!mProcessHost->IsConnected() ||
-          mProcessHost->GetActor()->SendSetOffline(
-              !strcmp(offline, "true") ? true : false)) {
-        return NS_ERROR_NOT_AVAILABLE;
-      }
-    } else if (!strcmp(aTopic, NS_XPCOM_WILL_SHUTDOWN_OBSERVER_ID)) {
-      nsCOMPtr<nsIObserverService> obs =
-          mozilla::services::GetObserverService();
-      obs->RemoveObserver(this, NS_IPC_IOSERVICE_SET_OFFLINE_TOPIC);
-      obs->RemoveObserver(this, NS_XPCOM_WILL_SHUTDOWN_OBSERVER_ID);
-    }
-
-    return NS_OK;
-  }
-  virtual ~OfflineObserver() = default;
-
-  SocketProcessHost* mProcessHost;
-};
-
-NS_IMPL_ISUPPORTS(OfflineObserver, nsIObserver)
-
 #if defined(XP_MACOSX) && defined(MOZ_SANDBOX)
 bool SocketProcessHost::sLaunchWithMacSandbox = false;
 #endif
 
 SocketProcessHost::SocketProcessHost(Listener* aListener)
     : GeckoChildProcessHost(GeckoProcessType_Socket),
       mListener(aListener),
       mTaskFactory(this),
@@ -107,25 +48,17 @@ SocketProcessHost::SocketProcessHost(Lis
   if (!sLaunchWithMacSandbox) {
     sLaunchWithMacSandbox =
         (PR_GetEnv("MOZ_DISABLE_SOCKET_PROCESS_SANDBOX") == nullptr);
   }
   mDisableOSActivityMode = sLaunchWithMacSandbox;
 #endif
 }
 
-SocketProcessHost::~SocketProcessHost() {
-  MOZ_COUNT_DTOR(SocketProcessHost);
-  if (mOfflineObserver) {
-    RefPtr<OfflineObserver> observer = mOfflineObserver;
-    NS_DispatchToMainThread(
-        NS_NewRunnableFunction("SocketProcessHost::DestroyOfflineObserver",
-                               [observer]() { observer->Destroy(); }));
-  }
-}
+SocketProcessHost::~SocketProcessHost() { MOZ_COUNT_DTOR(SocketProcessHost); }
 
 bool SocketProcessHost::Launch() {
   MOZ_ASSERT(mLaunchPhase == LaunchPhase::Unlaunched);
   MOZ_ASSERT(!mSocketProcessParent);
   MOZ_ASSERT(NS_IsMainThread());
 
   std::vector<std::string> extraArgs;
 
@@ -234,34 +167,28 @@ void SocketProcessHost::InitAfterConnect
 #endif  // XP_LINUX && MOZ_SANDBOX
 
 #ifdef MOZ_GECKO_PROFILER
     Unused << GetActor()->SendInitProfiler(
         ProfilerParent::CreateForProcess(GetActor()->OtherPid()));
 #endif
 
     Unused << GetActor()->SendSetOffline(offline);
-
-    mOfflineObserver = new OfflineObserver(this);
   }
 
   if (mListener) {
     mListener->OnProcessLaunchComplete(this, aSucceeded);
   }
 }
 
 void SocketProcessHost::Shutdown() {
   MOZ_ASSERT(!mShutdownRequested);
   MOZ_ASSERT(NS_IsMainThread());
 
   mListener = nullptr;
-  if (mOfflineObserver) {
-    mOfflineObserver->Destroy();
-    mOfflineObserver = nullptr;
-  }
 
   if (mSocketProcessParent) {
     // OnChannelClosed uses this to check if the shutdown was expected or
     // unexpected.
     mShutdownRequested = true;
 
     // The channel might already be closed if we got here unexpectedly.
     if (!mChannelClosed) {
--- a/netwerk/ipc/SocketProcessHost.h
+++ b/netwerk/ipc/SocketProcessHost.h
@@ -14,17 +14,16 @@
 namespace mozilla {
 
 #if defined(XP_LINUX) && defined(MOZ_SANDBOX)
 class SandboxBroker;
 #endif
 
 namespace net {
 
-class OfflineObserver;
 class SocketProcessParent;
 
 // SocketProcessHost is the "parent process" container for a subprocess handle
 // and IPC connection. It owns the parent process IPDL actor, which in this
 // case, is a SocketProcessParent.
 // SocketProcessHost is allocated and managed by nsIOService in parent process.
 class SocketProcessHost final : public mozilla::ipc::GeckoChildProcessHost {
   friend class SocketProcessParent;
@@ -117,17 +116,16 @@ class SocketProcessHost final : public m
 
   UniquePtr<SocketProcessParent> mSocketProcessParent;
   // mShutdownRequested is set to true only when Shutdown() is called.
   // If mShutdownRequested is false and the IPC channel is closed,
   // OnProcessUnexpectedShutdown will be invoked.
   bool mShutdownRequested;
   bool mChannelClosed;
 
-  RefPtr<OfflineObserver> mOfflineObserver;
 #if defined(XP_LINUX) && defined(MOZ_SANDBOX)
   UniquePtr<SandboxBroker> mSandboxBroker;
 #endif
 };
 
 class SocketProcessMemoryReporter : public MemoryReportingProcess {
  public:
   NS_INLINE_DECL_THREADSAFE_REFCOUNTING(SocketProcessMemoryReporter, override)