Bug 1637648 - P2: Make TRRServiceParent use nsIOService to forward observer notifications r=dragana
authorKershaw Chang <kershaw@mozilla.com>
Tue, 23 Jun 2020 11:11:16 +0000
changeset 536866 b284d915dc1c30907def49fc9d167299332702cf
parent 536865 6fc6ec584f86d227413e2012f2cc574564f2e49d
child 536867 db984645da671b290773dc3736d48cc6b71b3ba1
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 - P2: Make TRRServiceParent use nsIOService to forward observer notifications r=dragana Differential Revision: https://phabricator.services.mozilla.com/D77306
netwerk/dns/PTRRService.ipdl
netwerk/dns/TRRService.cpp
netwerk/dns/TRRService.h
netwerk/dns/TRRServiceChild.cpp
netwerk/dns/TRRServiceParent.cpp
netwerk/dns/TRRServiceParent.h
--- a/netwerk/dns/PTRRService.ipdl
+++ b/netwerk/dns/PTRRService.ipdl
@@ -12,17 +12,16 @@ namespace mozilla {
 namespace net {
 
 async refcounted protocol PTRRService
 {
   manager PSocketProcess;
 
 child:
   async __delete__();
-  async NotifyObserver(nsCString aTopic, nsString aData);
   async UpdatePlatformDNSInformation(nsCString[] aSuffixList,
                                      bool aPlatformDisabledTRR);
   async InitTRRBLStorage(DataStorageEntry aEntry, FileDescriptor aWriteFd);
   async UpdateParentalControlEnabled(bool aEnabled);
   async ClearDNSCache(bool aTrrToo);
   async SetDetectedTrrURI(nsCString aURI);
 };
 
--- a/netwerk/dns/TRRService.cpp
+++ b/netwerk/dns/TRRService.cpp
@@ -61,19 +61,25 @@ TRRService::TRRService()
       mConfirmationState(CONFIRM_INIT),
       mRetryConfirmInterval(1000),
       mTRRFailures(0),
       mParentalControlEnabled(false) {
   MOZ_ASSERT(NS_IsMainThread(), "wrong thread");
 }
 
 // static
-void TRRService::AddObserver(nsIObserver* aObserver) {
-  nsCOMPtr<nsIObserverService> observerService =
-      mozilla::services::GetObserverService();
+void TRRService::AddObserver(nsIObserver* aObserver,
+                             nsIObserverService* aObserverService) {
+  nsCOMPtr<nsIObserverService> observerService;
+  if (aObserverService) {
+    observerService = aObserverService;
+  } else {
+    observerService = mozilla::services::GetObserverService();
+  }
+
   if (observerService) {
     observerService->AddObserver(aObserver, NS_CAPTIVE_PORTAL_CONNECTIVITY,
                                  true);
     observerService->AddObserver(aObserver, kOpenCaptivePortalLoginEvent, true);
     observerService->AddObserver(aObserver, kClearPrivateData, true);
     observerService->AddObserver(aObserver, kPurge, true);
     observerService->AddObserver(aObserver, NS_NETWORK_LINK_TOPIC, true);
     observerService->AddObserver(aObserver, NS_DNS_SUFFIX_LIST_UPDATED_TOPIC,
@@ -479,16 +485,43 @@ bool TRRService::IsOnTRRThread() {
   }
   if (!thread) {
     return false;
   }
 
   return thread->IsOnCurrentThread();
 }
 
+void TRRService::InitTRRBLStorage(DataStorage* aInitedStorage) {
+  if (mTRRBLStorage) {
+    return;
+  }
+
+  // We need a lock if we modify mTRRBLStorage variable because it is
+  // access off the main thread as well.
+  MutexAutoLock lock(mLock);
+  if (aInitedStorage) {
+    mTRRBLStorage = aInitedStorage;
+  } else {
+    mTRRBLStorage = DataStorage::Get(DataStorageClass::TRRBlacklist);
+    if (mTRRBLStorage) {
+      if (NS_FAILED(mTRRBLStorage->Init(nullptr))) {
+        mTRRBLStorage = nullptr;
+      }
+    }
+  }
+
+  if (mClearTRRBLStorage) {
+    if (mTRRBLStorage) {
+      mTRRBLStorage->Clear();
+    }
+    mClearTRRBLStorage = false;
+  }
+}
+
 NS_IMETHODIMP
 TRRService::Observe(nsISupports* aSubject, const char* aTopic,
                     const char16_t* aData) {
   MOZ_ASSERT(NS_IsMainThread(), "wrong thread");
   LOG(("TRR::Observe() topic=%s\n", aTopic));
   if (!strcmp(aTopic, NS_PREFBRANCH_PREFCHANGE_TOPIC_ID)) {
     ReadPrefs(NS_ConvertUTF16toUTF8(aData).get());
 
@@ -502,32 +535,20 @@ TRRService::Observe(nsISupports* aSubjec
 
   } else if (!strcmp(aTopic, kOpenCaptivePortalLoginEvent)) {
     // We are in a captive portal
     LOG(("TRRservice in captive portal\n"));
     mCaptiveIsPassed = false;
   } else if (!strcmp(aTopic, NS_CAPTIVE_PORTAL_CONNECTIVITY)) {
     nsAutoCString data = NS_ConvertUTF16toUTF8(aData);
     LOG(("TRRservice captive portal was %s\n", data.get()));
-    if (!mTRRBLStorage) {
-      // We need a lock if we modify mTRRBLStorage variable because it is
-      // access off the main thread as well.
-      MutexAutoLock lock(mLock);
-      mTRRBLStorage = DataStorage::Get(DataStorageClass::TRRBlacklist);
-      if (mTRRBLStorage) {
-        if (NS_FAILED(mTRRBLStorage->Init(nullptr))) {
-          mTRRBLStorage = nullptr;
-        }
-        if (mClearTRRBLStorage) {
-          if (mTRRBLStorage) {
-            mTRRBLStorage->Clear();
-          }
-          mClearTRRBLStorage = false;
-        }
-      }
+    // When TRRService is in socket process, InitTRRBLStorage() will be called
+    // by TRRServiceChild.
+    if (XRE_IsParentProcess()) {
+      InitTRRBLStorage(nullptr);
     }
 
     // We should avoid doing calling MaybeConfirm in response to a pref change
     // unless the service is in a TRR=enabled mode.
     if (mMode == MODE_TRRFIRST || mMode == MODE_TRRONLY) {
       if (!mCaptiveIsPassed) {
         if (mConfirmationState != CONFIRM_OK) {
           mConfirmationState = CONFIRM_TRYING;
--- a/netwerk/dns/TRRService.h
+++ b/netwerk/dns/TRRService.h
@@ -10,16 +10,17 @@
 #include "nsHostResolver.h"
 #include "nsIObserver.h"
 #include "nsWeakReference.h"
 #include "TRRServiceBase.h"
 
 class nsDNSService;
 class nsIPrefBranch;
 class nsINetworkLinkService;
+class nsIObserverService;
 
 namespace mozilla {
 namespace net {
 
 class TRRServiceChild;
 class TRRServiceParent;
 
 class TRRService : public TRRServiceBase,
@@ -77,17 +78,18 @@ class TRRService : public TRRServiceBase
   bool IsUsingAutoDetectedURL() { return mURISetByDetection; }
   static const nsCString& AutoDetectedKey();
 
  private:
   virtual ~TRRService();
 
   friend class TRRServiceChild;
   friend class TRRServiceParent;
-  static void AddObserver(nsIObserver* aObserver);
+  static void AddObserver(nsIObserver* aObserver,
+                          nsIObserverService* aObserverService = nullptr);
   static bool CheckCaptivePortalIsPassed();
   static bool GetParentalControlEnabledInternal();
   static bool CheckPlatformDNSStatus(nsINetworkLinkService* aLinkService);
 
   nsresult ReadPrefs(const char* name);
   void GetPrefBranch(nsIPrefBranch** result);
   void MaybeConfirm();
   void MaybeConfirm_locked();
@@ -104,16 +106,17 @@ class TRRService : public TRRServiceBase
   nsresult DispatchTRRRequestInternal(TRR* aTrrRequest, bool aWithLock);
   already_AddRefed<nsIThread> TRRThread_locked();
 
   // This method will process the URI and try to set mPrivateURI to that value.
   // Will return true if performed the change (if the value was different)
   // or false if mPrivateURI already had that value.
   bool MaybeSetPrivateURI(const nsACString& aURI) override;
   void ClearEntireCache();
+  void InitTRRBLStorage(DataStorage* aInitedStorage);
 
   bool mInitialized;
   Atomic<uint32_t, Relaxed> mTRRBlacklistExpireTime;
 
   Mutex mLock;
 
   nsCString mPrivateCred;  // main thread only
   nsCString mConfirmationNS;
--- a/netwerk/dns/TRRServiceChild.cpp
+++ b/netwerk/dns/TRRServiceChild.cpp
@@ -27,38 +27,31 @@ void TRRServiceChild::Init(const bool& a
   MOZ_ASSERT(sDNSService);
   MOZ_ASSERT(gTRRService);
 
   gTRRService->mCaptiveIsPassed = aCaptiveIsPassed;
   gTRRService->mParentalControlEnabled = aParentalControlEnabled;
   gTRRService->RebuildSuffixList(std::move(aDNSSuffixList));
 }
 
-mozilla::ipc::IPCResult TRRServiceChild::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();
-}
-
 mozilla::ipc::IPCResult TRRServiceChild::RecvUpdatePlatformDNSInformation(
     nsTArray<nsCString>&& aDNSSuffixList, const bool& aPlatformDisabledTRR) {
   gTRRService->RebuildSuffixList(std::move(aDNSSuffixList));
   gTRRService->mPlatformDisabledTRR = aPlatformDisabledTRR;
   return IPC_OK();
 }
 
 mozilla::ipc::IPCResult TRRServiceChild::RecvInitTRRBLStorage(
     const psm::DataStorageEntry& aEntry, const FileDescriptor& aWriteFd) {
   RefPtr<DataStorage> storage =
       DataStorage::Get(DataStorageClass::TRRBlacklist);
   if (storage) {
-    storage->Init(&aEntry.items(), aWriteFd);
+    if (NS_SUCCEEDED(storage->Init(&aEntry.items(), aWriteFd))) {
+      gTRRService->InitTRRBLStorage(storage);
+    }
   }
   return IPC_OK();
 }
 
 mozilla::ipc::IPCResult TRRServiceChild::RecvUpdateParentalControlEnabled(
     const bool& aEnabled) {
   gTRRService->mParentalControlEnabled = aEnabled;
   return IPC_OK();
--- a/netwerk/dns/TRRServiceParent.cpp
+++ b/netwerk/dns/TRRServiceParent.cpp
@@ -36,101 +36,73 @@ void TRRServiceParent::Init() {
     return;
   }
 
   SocketProcessParent* socketParent = SocketProcessParent::GetSingleton();
   if (!socketParent) {
     return;
   }
 
-  TRRService::AddObserver(this);
+  nsCOMPtr<nsIObserverService> obs =
+      static_cast<nsIObserverService*>(gIOService);
+  TRRService::AddObserver(this, obs);
+
   bool captiveIsPassed = TRRService::CheckCaptivePortalIsPassed();
   bool parentalControlEnabled = TRRService::GetParentalControlEnabledInternal();
 
   nsCOMPtr<nsINetworkLinkService> nls =
       do_GetService(NS_NETWORK_LINK_SERVICE_CONTRACTID);
   nsTArray<nsCString> suffixList;
   if (nls) {
     nls->GetDnsSuffixList(suffixList);
   }
 
   Preferences::RegisterPrefixCallbacks(TRRServiceParent::PrefsChanged,
                                        gTRRUriCallbackPrefs, this);
   prefsChanged(nullptr);
 
   Unused << socketParent->SendPTRRServiceConstructor(
       this, captiveIsPassed, parentalControlEnabled, suffixList);
+
+  RefPtr<DataStorage> storage =
+      DataStorage::Get(DataStorageClass::TRRBlacklist);
+  if (!storage) {
+    return;
+  }
+
+  if (NS_FAILED(storage->Init(nullptr))) {
+    return;
+  }
+
+  psm::DataStorageEntry entry;
+  storage->GetAll(&entry.items());
+
+  RefPtr<TRRServiceParent> self = this;
+  storage->AsyncTakeFileDesc([self, entry](ipc::FileDescriptor&& aWriteFd) {
+    Unused << self->SendInitTRRBLStorage(entry, aWriteFd);
+  });
 }
 
 NS_IMETHODIMP
 TRRServiceParent::Observe(nsISupports* aSubject, const char* aTopic,
                           const char16_t* aData) {
-  if (!strcmp(aTopic, kOpenCaptivePortalLoginEvent)) {
-    Unused << SendNotifyObserver(
-        nsDependentCString(aTopic),
-        aData ? nsDependentString(aData) : VoidString());
-  } else if (!strcmp(aTopic, NS_CAPTIVE_PORTAL_CONNECTIVITY)) {
-    if (!mTRRBLStorageInited) {
-      RefPtr<DataStorage> storage =
-          DataStorage::Get(DataStorageClass::TRRBlacklist);
-      if (storage) {
-        nsresult rv = storage->Init(nullptr);
-        if (NS_WARN_IF(NS_FAILED(rv))) {
-          Unused << SendNotifyObserver(
-              nsDependentCString(aTopic),
-              aData ? nsDependentString(aData) : VoidString());
-          return NS_OK;
-        }
-
-        psm::DataStorageEntry entry;
-        storage->GetAll(&entry.items());
-
-        RefPtr<TRRServiceParent> self = this;
-        nsCString topic(aTopic);
-        nsString data(aData);
-        rv = storage->AsyncTakeFileDesc(
-            [self, entry, topic, data](ipc::FileDescriptor&& aWriteFd) {
-              // We need to send this message before
-              // NS_CAPTIVE_PORTAL_CONNECTIVITY notification to make sure
-              // TRRBLStorage got inited properly.
-              Unused << self->SendInitTRRBLStorage(entry, aWriteFd);
-              Unused << self->SendNotifyObserver(topic, data);
-              self->mTRRBLStorageInited = true;
-            });
-
-        if (NS_SUCCEEDED(rv)) {
-          return NS_OK;
-        }
-      }
-    }
-
-    Unused << SendNotifyObserver(
-        nsDependentCString(aTopic),
-        aData ? nsDependentString(aData) : VoidString());
-  } else if (!strcmp(aTopic, kClearPrivateData) || !strcmp(aTopic, kPurge)) {
-    Unused << SendNotifyObserver(
-        nsDependentCString(aTopic),
-        aData ? nsDependentString(aData) : VoidString());
-  } else if (!strcmp(aTopic, NS_DNS_SUFFIX_LIST_UPDATED_TOPIC) ||
-             !strcmp(aTopic, NS_NETWORK_LINK_TOPIC)) {
+  if (!strcmp(aTopic, NS_DNS_SUFFIX_LIST_UPDATED_TOPIC) ||
+      !strcmp(aTopic, NS_NETWORK_LINK_TOPIC)) {
     nsCOMPtr<nsINetworkLinkService> link = do_QueryInterface(aSubject);
     // The network link service notification normally passes itself as the
     // subject, but some unit tests will sometimes pass a null subject.
     if (link) {
       nsTArray<nsCString> suffixList;
       link->GetDnsSuffixList(suffixList);
       bool platformDisabledTRR = TRRService::CheckPlatformDNSStatus(link);
       Unused << SendUpdatePlatformDNSInformation(suffixList,
                                                  platformDisabledTRR);
     }
 
     if (!strcmp(aTopic, NS_NETWORK_LINK_TOPIC) && mURISetByDetection) {
-      Unused << SendNotifyObserver(
-          nsDependentCString(aTopic),
-          aData ? nsDependentString(aData) : VoidString());
       CheckURIPrefs();
     }
   }
 
   return NS_OK;
 }
 
 bool TRRServiceParent::MaybeSetPrivateURI(const nsACString& aURI) {
--- a/netwerk/dns/TRRServiceParent.h
+++ b/netwerk/dns/TRRServiceParent.h
@@ -15,31 +15,29 @@
 namespace mozilla {
 namespace net {
 
 class TRRServiceParent : public TRRServiceBase,
                          public nsIObserver,
                          public nsSupportsWeakReference,
                          public PTRRServiceParent {
  public:
-  NS_DECL_ISUPPORTS
+  NS_DECL_THREADSAFE_ISUPPORTS
   NS_DECL_NSIOBSERVER
 
-  explicit TRRServiceParent() : mTRRBLStorageInited(false) {}
+  TRRServiceParent() = default;
   void Init();
   void UpdateParentalControlEnabled();
   static void PrefsChanged(const char* aName, void* aSelf);
   void SetDetectedTrrURI(const nsACString& aURI);
   bool MaybeSetPrivateURI(const nsACString& aURI) override;
   void GetTrrURI(nsACString& aURI);
 
  private:
   virtual ~TRRServiceParent() = default;
   virtual void ActorDestroy(ActorDestroyReason why) override;
   void prefsChanged(const char* aName);
-
-  bool mTRRBLStorageInited;
 };
 
 }  // namespace net
 }  // namespace mozilla
 
 #endif  // mozilla_net_TRRServiceParent_h