Bug 1510934 - Ensure that pref changes during content process launch are delivered to the process. r=njn, a=RyanVM
authorJed Davis <jld@mozilla.com>
Fri, 14 Dec 2018 05:28:56 +0000
changeset 509071 097f70517f666382f7606678ab018570f99de907
parent 509070 ad653a697179ac2d603c30d3c4db9dea7e5e7929
child 509072 c94560ceb417fb956ef76b80cf5a993f0273d362
push id1905
push userffxbld-merge
push dateMon, 21 Jan 2019 12:33:13 +0000
treeherdermozilla-release@c2fca1944d8c [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersnjn, RyanVM
bugs1510934
milestone65.0
Bug 1510934 - Ensure that pref changes during content process launch are delivered to the process. r=njn, a=RyanVM Some prefs need to be available before IPC is started, so we serialize a snapshot when we start launching the process, and then stream further changes over IPC messages. However, async launch introduces a window between the snapshot and when the parent can start sending messages, during which other code can run on the main thread and change prefs. In order to not lose those updates, they're queued and sent when the launch is complete. Depends on D14089 Differential Revision: https://phabricator.services.mozilla.com/D14090
dom/ipc/ContentParent.cpp
dom/ipc/ContentParent.h
--- a/dom/ipc/ContentParent.cpp
+++ b/dom/ipc/ContentParent.cpp
@@ -1297,19 +1297,22 @@ void ContentParent::Init() {
   nsCOMPtr<nsIObserverService> obs = mozilla::services::GetObserverService();
   if (obs) {
     size_t length = ArrayLength(sObserverTopics);
     for (size_t i = 0; i < length; ++i) {
       obs->AddObserver(this, sObserverTopics[i], false);
     }
   }
 
-  // Register ContentParent as an observer for changes to any pref whose prefix
-  // matches the empty string, i.e. all of them.
-  Preferences::AddStrongObserver(this, "");
+  // Flush any pref updates that happened during launch and weren't
+  // included in the blobs set up in LaunchSubprocessInternal.
+  for (const Pref& pref : mQueuedPrefs) {
+    Unused << NS_WARN_IF(!SendPreferenceUpdate(pref));
+  }
+  mQueuedPrefs.Clear();
 
   if (obs) {
     nsAutoString cpId;
     cpId.AppendInt(static_cast<uint64_t>(this->ChildID()));
     obs->NotifyObservers(static_cast<nsIObserver*>(this), "ipc:content-created",
                          cpId.get());
   }
 
@@ -2128,16 +2131,22 @@ void ContentParent::LaunchSubprocessInte
     MarkAsDead();
     earlyReject();
     return;
   }
 
   // Copy the serialized prefs into the shared memory.
   memcpy(static_cast<char*>(shm.memory()), prefs.get(), prefs.Length());
 
+  // Register ContentParent as an observer for changes to any pref
+  // whose prefix matches the empty string, i.e. all of them.  The
+  // observation starts here in order to capture pref updates that
+  // happen during async launch.
+  Preferences::AddStrongObserver(this, "");
+
   // Formats a pointer or pointer-sized-integer as a string suitable for passing
   // in an arguments list.
   auto formatPtrArg = [](auto arg) {
     return nsPrintfCString("%zu", uintptr_t(arg));
   };
 
 #if defined(XP_WIN)
   // Record the handle as to-be-shared, and pass it via a command flag. This
@@ -3036,22 +3045,21 @@ ContentParent::Observe(nsISupports* aSub
 
     // Wait for shutdown to complete, so that we receive any shutdown
     // data (e.g. telemetry) from the child before we quit.
     // This loop terminate prematurely based on mForceKillTimer.
     SpinEventLoopUntil([&]() { return !mIPCOpen || mCalledKillHard; });
     NS_ASSERTION(!mSubprocess, "Close should have nulled mSubprocess");
   }
 
-  if (!IsAlive() || !mSubprocess) return NS_OK;
-
-  // listening for memory pressure event
-  if (!strcmp(aTopic, "memory-pressure")) {
-    Unused << SendFlushMemory(nsDependentString(aData));
-  } else if (!strcmp(aTopic, "nsPref:changed")) {
+  if (IsDead() || !mSubprocess) {
+    return NS_OK;
+  }
+
+  if (!strcmp(aTopic, "nsPref:changed")) {
     // A pref changed. If it's not on the blacklist, inform child processes.
 #define BLACKLIST_ENTRY(s) \
   { s, (sizeof(s) / sizeof(char16_t)) - 1 }
     struct BlacklistEntry {
       const char16_t* mPrefBranch;
       size_t mLen;
     };
     // These prefs are not useful in child processes.
@@ -3074,19 +3082,34 @@ ContentParent::Observe(nsISupports* aSub
       }
     }
 
     // We know prefs are ASCII here.
     NS_LossyConvertUTF16toASCII strData(aData);
 
     Pref pref(strData, /* isLocked */ false, null_t(), null_t());
     Preferences::GetPreference(&pref);
-    if (!SendPreferenceUpdate(pref)) {
-      return NS_ERROR_NOT_AVAILABLE;
+    if (IsAlive()) {
+      MOZ_ASSERT(mQueuedPrefs.IsEmpty());
+      if (!SendPreferenceUpdate(pref)) {
+        return NS_ERROR_NOT_AVAILABLE;
+      }
+    } else {
+      MOZ_ASSERT(IsLaunching());
+      mQueuedPrefs.AppendElement(pref);
     }
+  }
+
+  if (!IsAlive()) {
+    return NS_OK;
+  }
+
+  // listening for memory pressure event
+  if (!strcmp(aTopic, "memory-pressure")) {
+    Unused << SendFlushMemory(nsDependentString(aData));
   } else if (!strcmp(aTopic, NS_IPC_IOSERVICE_SET_OFFLINE_TOPIC)) {
     NS_ConvertUTF16toUTF8 dataStr(aData);
     const char* offline = dataStr.get();
     if (!SendSetOffline(!strcmp(offline, "true") ? true : false)) {
       return NS_ERROR_NOT_AVAILABLE;
     }
   } else if (!strcmp(aTopic, NS_IPC_IOSERVICE_SET_CONNECTIVITY_TOPIC)) {
     if (!SendSetConnectivity(NS_LITERAL_STRING("true").Equals(aData))) {
--- a/dom/ipc/ContentParent.h
+++ b/dom/ipc/ContentParent.h
@@ -359,16 +359,17 @@ class ContentParent final : public PCont
   bool RequestRunToCompletion();
 
   void UpdateCookieStatus(nsIChannel* aChannel);
 
   bool IsLaunching() const {
     return mLifecycleState == LifecycleState::LAUNCHING;
   }
   bool IsAlive() const override;
+  bool IsDead() const { return mLifecycleState == LifecycleState::DEAD; }
 
   virtual bool IsForBrowser() const override { return mIsForBrowser; }
   virtual bool IsForJSPlugin() const override {
     return mJSPluginID != nsFakePluginTag::NOT_JSPLUGIN;
   }
 
   GeckoChildProcessHost* Process() const { return mSubprocess; }
 
@@ -1317,16 +1318,21 @@ class ContentParent final : public PCont
   nsRefPtrHashtable<nsIDHashKey, GetFilesHelper> mGetFilesPendingRequests;
 
   nsTHashtable<nsCStringHashKey> mActivePermissionKeys;
 
   nsTArray<nsCString> mBlobURLs;
 
   UniquePtr<mozilla::ipc::CrashReporterHost> mCrashReporter;
 
+  // Collects any pref changes that occur during process launch (after
+  // the initial map is passed in command-line arguments) to be sent
+  // when the process can receive IPC messages.
+  nsTArray<Pref> mQueuedPrefs;
+
   static uint64_t sNextTabParentId;
   static nsDataHashtable<nsUint64HashKey, TabParent*> sNextTabParents;
 
 #if defined(XP_MACOSX) && defined(MOZ_CONTENT_SANDBOX)
   // When set to true, indicates that content processes should
   // initialize their sandbox during startup instead of waiting
   // for the SetProcessSandbox IPDL message.
   static bool sEarlySandboxInit;