Bug 1496257 - P1 socket operation should wait until socket process launched
authorJunior Hsu <juhsu@mozilla.com>
Wed, 10 Oct 2018 18:14:32 -0700
changeset 448025 2ada61a20690ee5b5afc81787c3dc7dd79a5996c
parent 448024 17204458665390f8649443e04bf0a07b78007125
child 448026 e97fe90fa93bb3df64d3a262c815971a20003d1a
push id20
push userjuhsu@mozilla.com
push dateThu, 13 Dec 2018 22:11:48 +0000
bugs1496257
milestone65.0a1
Bug 1496257 - P1 socket operation should wait until socket process launched Reviewers: mayhemer Tags: #secure-revision Bug #: 1496257 Differential Revision: https://phabricator.services.mozilla.com/D7668
netwerk/base/nsIOService.cpp
netwerk/ipc/SocketProcessHost.cpp
netwerk/ipc/SocketProcessHost.h
netwerk/protocol/http/nsHttpChannel.cpp
--- a/netwerk/base/nsIOService.cpp
+++ b/netwerk/base/nsIOService.cpp
@@ -1499,30 +1499,36 @@ nsIOService::Observe(nsISupports *subjec
             SetOffline(true);
         }
     } else if (!strcmp(topic, kProfileChangeNetRestoreTopic)) {
         if (mOfflineForProfileChange) {
             mOfflineForProfileChange = false;
             SetOffline(false);
         }
     } else if (!strcmp(topic, kProfileDoChange)) {
-        if (data && NS_LITERAL_STRING("startup").Equals(data)) {
+        if (!data) {
+            return NS_OK;
+        }
+        if (NS_LITERAL_STRING("startup").Equals(data)) {
             // Lazy initialization of network link service (see bug 620472)
             InitializeNetworkLinkService();
             // Set up the initilization flag regardless the actuall result.
             // If we fail here, we will fail always on.
             mNetworkLinkServiceInitialized = true;
 
             // And now reflect the preference setting
             PrefsChanged(MANAGE_OFFLINE_STATUS_PREF);
 
             // Bug 870460 - Read cookie database at an early-as-possible time
             // off main thread. Hence, we have more chance to finish db query
             // before something calls into the cookie service.
             nsCOMPtr<nsISupports> cookieServ = do_GetService(NS_COOKIESERVICE_CONTRACTID);
+        } else if (NS_LITERAL_STRING("xpcshell-do-get-profile").Equals(data)) {
+            // xpcshell doesn't read user profile.
+            LaunchSocketProcess();
         }
     } else if (!strcmp(topic, NS_XPCOM_SHUTDOWN_OBSERVER_ID)) {
         // Remember we passed XPCOM shutdown notification to prevent any
         // changes of the offline status from now. We must not allow going
         // online after this point.
         mShutdown = true;
 
         if (!mHttpHandlerAlreadyShutingDown && !mOfflineForProfileChange) {
--- a/netwerk/ipc/SocketProcessHost.cpp
+++ b/netwerk/ipc/SocketProcessHost.cpp
@@ -27,16 +27,17 @@ public:
   NS_DECL_THREADSAFE_ISUPPORTS
   explicit OfflineObserver(SocketProcessHost* aProcessHost)
     : mProcessHost(aProcessHost)
   {
     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()
   {
     mProcessHost = nullptr;
   }
 
@@ -51,28 +52,28 @@ private:
 
     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();
+      if (obs) {
+        obs->RemoveObserver(this, NS_IPC_IOSERVICE_SET_OFFLINE_TOPIC);
+        obs->RemoveObserver(this, NS_XPCOM_WILL_SHUTDOWN_OBSERVER_ID);
+      }
     }
 
     return NS_OK;
   }
-  virtual ~OfflineObserver()
-  {
-    nsCOMPtr<nsIObserverService> obs =
-      mozilla::services::GetObserverService();
-    if (obs) {
-      obs->RemoveObserver(this, NS_IPC_IOSERVICE_SET_OFFLINE_TOPIC);
-    }
-  }
+  virtual ~OfflineObserver() = default;
 
   SocketProcessHost* mProcessHost;
 };
 
 NS_IMPL_ISUPPORTS(OfflineObserver, nsIObserver)
 
 SocketProcessHost::SocketProcessHost(Listener* aListener)
  : GeckoChildProcessHost(GeckoProcessType_Socket),
@@ -90,16 +91,17 @@ 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;
 
   // TODO: reduce duplicate code about preference below. See bug 1484774.
   // Prefs information is passed via anonymous shared memory to avoid bloating
   // the command line.
   size_t prefMapSize;
   auto prefMapHandle = Preferences::EnsureSnapshot(&prefMapSize).ClonePlatformHandle();
@@ -155,25 +157,24 @@ SocketProcessHost::Launch()
   extraArgs.push_back(formatPtrArg(prefs.Length()).get());
 
   extraArgs.push_back("-prefMapSize");
   extraArgs.push_back(formatPtrArg(prefMapSize).get());
 
   nsAutoCString parentBuildID(mozilla::PlatformBuildID());
   extraArgs.push_back("-parentBuildID");
   extraArgs.push_back(parentBuildID.get());
+  mLaunchPhase = LaunchPhase::Waiting;
+  bool success = GeckoChildProcessHost::SyncLaunch(extraArgs);
+  InitAfterConnect(success);
 
-  mLaunchPhase = LaunchPhase::Waiting;
-  if (!GeckoChildProcessHost::LaunchAndWaitForProcessHandle(extraArgs)) {
-    mLaunchPhase = LaunchPhase::Complete;
-    return false;
-  }
+  return success;
+}
 
-  return true;
-}
+#if 0
 
 void
 SocketProcessHost::OnChannelConnected(int32_t peer_pid)
 {
   MOZ_ASSERT(!NS_IsMainThread());
 
   GeckoChildProcessHost::OnChannelConnected(peer_pid);
 
@@ -217,16 +218,18 @@ SocketProcessHost::OnChannelConnectedTas
 void
 SocketProcessHost::OnChannelErrorTask()
 {
   if (mLaunchPhase == LaunchPhase::Waiting) {
     InitAfterConnect(false);
   }
 }
 
+#endif
+
 void
 SocketProcessHost::InitAfterConnect(bool aSucceeded)
 {
   MOZ_ASSERT(mLaunchPhase == LaunchPhase::Waiting);
   MOZ_ASSERT(!mSocketProcessParent);
 
   mLaunchPhase = LaunchPhase::Complete;
 
--- a/netwerk/ipc/SocketProcessHost.h
+++ b/netwerk/ipc/SocketProcessHost.h
@@ -35,17 +35,17 @@ public:
 
     // Called when the process of launching the process is complete.
     virtual void OnProcessLaunchComplete(SocketProcessHost* aHost,
                                          bool aSucceeded) = 0;
 
     // Called when the channel is closed but Shutdown() is not invoked.
     virtual void OnProcessUnexpectedShutdown(SocketProcessHost* aHost) = 0;
 
-protected:
+  protected:
     virtual ~Listener() = default;
   };
 
 public:
   explicit SocketProcessHost(Listener* listener);
   ~SocketProcessHost();
 
   // Launch the socket process asynchronously.
@@ -68,24 +68,29 @@ public:
   }
 
   bool IsConnected() const {
     MOZ_ASSERT(NS_IsMainThread());
 
     return !!mSocketProcessParent;
   }
 
+// We change to SyncLaunch as a workaround. Need these callbacks after back to
+// AsyncLaunch.
+#if 0
   // Called on the IO thread.
   void OnChannelConnected(int32_t peer_pid) override;
   void OnChannelError() override;
-
+#endif
 private:
+#if 0
   // Called on the main thread.
   void OnChannelConnectedTask();
   void OnChannelErrorTask();
+#endif
 
   // Called on the main thread after a connection has been established.
   void InitAfterConnect(bool aSucceeded);
 
   // Called on the main thread when the mSocketParent actor is shutting down.
   void OnChannelClosed();
 
   void DestroyProcess();
--- a/netwerk/protocol/http/nsHttpChannel.cpp
+++ b/netwerk/protocol/http/nsHttpChannel.cpp
@@ -1395,17 +1395,17 @@ nsHttpChannel::SetupTransaction()
 
     // create wrapper for this channel's notification callbacks
     nsCOMPtr<nsIInterfaceRequestor> callbacks;
     NS_NewNotificationCallbacksAggregation(mCallbacks, mLoadGroup,
                                            getter_AddRefs(callbacks));
 
     // create the transaction object
     if (UseSocketProcess()) {
-        if (!gIOService->IsSocketProcessConnected()) {
+        if (NS_WARN_IF(!gIOService->IsSocketProcessConnected())) {
            return NS_ERROR_NOT_AVAILABLE;
         }
 
         RefPtr<HttpTransactionParent> transParent = new HttpTransactionParent();
         mUsingSocketProcess = true;
         // TODO Update/comunicate to make the WP parser work again.
         LOG(("nsHttpChannel %p created HttpTransactionParent %p\n", this, mTransaction.get()));