Bug 1550422 - P4. Sync preferences when they changed. r?mattwoodrow! draft
authorJean-Yves Avenard <jyavenard@mozilla.com>
Thu, 09 May 2019 14:50:31 +1000
changeset 2008581 faaa9a7490ceb2a6b5bcc517d4140a10b797ce35
parent 2008580 2b3724c42010f16f1735357e2e91683592b6627f
child 2008582 f47d32abe14b41358f8d7b7879f6e0955fef8c0a
push id363925
push userjyavenard@mozilla.com
push dateSat, 18 May 2019 07:53:18 +0000
treeherdertry@5082cd581229 [default view] [failures only]
reviewersmattwoodrow
bugs1550422
milestone68.0a1
Bug 1550422 - P4. Sync preferences when they changed. r?mattwoodrow! This will allow to remove gfxPrefs later. On Windows in particular, the need to decide gfxPrefs vs StaticPrefs for the WMF decoders has caused several bugs in the past. We will remove the confusion as a consequence. Differential Revision: https://phabricator.services.mozilla.com/D30589
dom/ipc/ContentParent.cpp
dom/ipc/ContentParent.h
gfx/ipc/GPUParent.cpp
gfx/ipc/GPUParent.h
gfx/ipc/GPUProcessManager.cpp
gfx/ipc/GPUProcessManager.h
gfx/ipc/PGPU.ipdl
--- a/dom/ipc/ContentParent.cpp
+++ b/dom/ipc/ContentParent.cpp
@@ -2952,40 +2952,18 @@ ContentParent::Observe(nsISupports* aSub
   }
 
   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.
-    static const BlacklistEntry sContentPrefBranchBlacklist[] = {
-        BLACKLIST_ENTRY(u"app.update.lastUpdateTime."),
-        BLACKLIST_ENTRY(u"datareporting.policy."),
-        BLACKLIST_ENTRY(u"browser.safebrowsing.provider."),
-        BLACKLIST_ENTRY(u"browser.shell."),
-        BLACKLIST_ENTRY(u"browser.slowStartup."),
-        BLACKLIST_ENTRY(u"extensions.getAddons.cache."),
-        BLACKLIST_ENTRY(u"media.gmp-manager."),
-        BLACKLIST_ENTRY(u"media.gmp-gmpopenh264."),
-        BLACKLIST_ENTRY(u"privacy.sanitize."),
-    };
-#undef BLACKLIST_ENTRY
-
-    for (const auto& entry : sContentPrefBranchBlacklist) {
-      if (NS_strncmp(entry.mPrefBranch, aData, entry.mLen) == 0) {
-        return NS_OK;
-      }
+    if (!ShouldSyncPreference(aData)) {
+      return NS_OK;
     }
 
     // We know prefs are ASCII here.
     NS_LossyConvertUTF16toASCII strData(aData);
 
     Pref pref(strData, /* isLocked */ false, Nothing(), Nothing());
     Preferences::GetPreference(&pref);
     if (IsAlive()) {
@@ -3123,16 +3101,47 @@ ContentParent::Observe(nsISupports* aSub
     }
   } else if (!strcmp(aTopic, NS_NETWORK_LINK_TYPE_TOPIC)) {
     UpdateNetworkLinkType();
   }
 
   return NS_OK;
 }
 
+/* static */
+bool ContentParent::ShouldSyncPreference(const char16_t* aData) {
+#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.
+  static const BlacklistEntry sContentPrefBranchBlacklist[] = {
+      BLACKLIST_ENTRY(u"app.update.lastUpdateTime."),
+      BLACKLIST_ENTRY(u"datareporting.policy."),
+      BLACKLIST_ENTRY(u"browser.safebrowsing.provider."),
+      BLACKLIST_ENTRY(u"browser.shell."),
+      BLACKLIST_ENTRY(u"browser.slowStartup."),
+      BLACKLIST_ENTRY(u"browser.startup."),
+      BLACKLIST_ENTRY(u"extensions.getAddons.cache."),
+      BLACKLIST_ENTRY(u"media.gmp-manager."),
+      BLACKLIST_ENTRY(u"media.gmp-gmpopenh264."),
+      BLACKLIST_ENTRY(u"privacy.sanitize."),
+  };
+#undef BLACKLIST_ENTRY
+
+  for (const auto& entry : sContentPrefBranchBlacklist) {
+    if (NS_strncmp(entry.mPrefBranch, aData, entry.mLen) == 0) {
+      return false;
+    }
+  }
+  return true;
+}
+
 void ContentParent::UpdateNetworkLinkType() {
   nsresult rv;
   nsCOMPtr<nsINetworkLinkService> nls =
       do_GetService(NS_NETWORK_LINK_SERVICE_CONTRACTID, &rv);
   if (NS_FAILED(rv)) {
     return;
   }
 
--- a/dom/ipc/ContentParent.h
+++ b/dom/ipc/ContentParent.h
@@ -1212,16 +1212,18 @@ class ContentParent final : public PCont
     return mRecordReplayState != eNotRecordingOrReplaying;
   }
 
   void OnBrowsingContextGroupSubscribe(BrowsingContextGroup* aGroup);
   void OnBrowsingContextGroupUnsubscribe(BrowsingContextGroup* aGroup);
 
   void UpdateNetworkLinkType();
 
+  static bool ShouldSyncPreference(const char16_t* aData);
+
  private:
   // Released in ActorDestroy; deliberately not exposed to the CC.
   RefPtr<ContentParent> mSelfRef;
 
   // If you add strong pointers to cycle collected objects here, be sure to
   // release these objects in ShutDownProcess.  See the comment there for more
   // details.
 
--- a/gfx/ipc/GPUParent.cpp
+++ b/gfx/ipc/GPUParent.cpp
@@ -337,16 +337,21 @@ mozilla::ipc::IPCResult GPUParent::RecvU
   return IPC_OK();
 }
 
 mozilla::ipc::IPCResult GPUParent::RecvUpdateVar(const GfxVarUpdate& aUpdate) {
   gfxVars::ApplyUpdate(aUpdate);
   return IPC_OK();
 }
 
+mozilla::ipc::IPCResult GPUParent::RecvPreferenceUpdate(const Pref& aPref) {
+  Preferences::SetPreference(aPref);
+  return IPC_OK();
+}
+
 static void CopyFeatureChange(Feature aFeature, Maybe<FeatureFailure>* aOut) {
   FeatureState& feature = gfxConfig::GetFeature(aFeature);
   if (feature.DisabledByDefault() || feature.IsEnabled()) {
     // No change:
     //   - Disabled-by-default means the parent process told us not to use this
     //   feature.
     //   - Enabled means we were told to use this feature, and we didn't
     //   discover anything
--- a/gfx/ipc/GPUParent.h
+++ b/gfx/ipc/GPUParent.h
@@ -52,16 +52,17 @@ class GPUParent final : public PGPUParen
   mozilla::ipc::IPCResult RecvInitVR(Endpoint<PVRGPUChild>&& aVRGPUChild);
   mozilla::ipc::IPCResult RecvInitUiCompositorController(
       const LayersId& aRootLayerTreeId,
       Endpoint<PUiCompositorControllerParent>&& aEndpoint);
   mozilla::ipc::IPCResult RecvInitProfiler(
       Endpoint<PProfilerChild>&& aEndpoint);
   mozilla::ipc::IPCResult RecvUpdatePref(const GfxPrefSetting& pref);
   mozilla::ipc::IPCResult RecvUpdateVar(const GfxVarUpdate& pref);
+  mozilla::ipc::IPCResult RecvPreferenceUpdate(const Pref& pref);
   mozilla::ipc::IPCResult RecvNewContentCompositorManager(
       Endpoint<PCompositorManagerParent>&& aEndpoint);
   mozilla::ipc::IPCResult RecvNewContentImageBridge(
       Endpoint<PImageBridgeParent>&& aEndpoint);
   mozilla::ipc::IPCResult RecvNewContentVRManager(
       Endpoint<PVRManagerParent>&& aEndpoint);
   mozilla::ipc::IPCResult RecvNewContentVideoDecoderManager(
       Endpoint<PVideoDecoderManagerParent>&& aEndpoint);
--- a/gfx/ipc/GPUProcessManager.cpp
+++ b/gfx/ipc/GPUProcessManager.cpp
@@ -5,16 +5,17 @@
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
 #include "GPUProcessManager.h"
 
 #include "gfxPrefs.h"
 #include "GPUProcessHost.h"
 #include "GPUProcessListener.h"
 #include "mozilla/MemoryReportingProcess.h"
+#include "mozilla/Preferences.h"
 #include "mozilla/Sprintf.h"
 #include "mozilla/StaticPtr.h"
 #include "mozilla/StaticPrefs.h"
 #include "mozilla/VideoDecoderManagerChild.h"
 #include "mozilla/VideoDecoderManagerParent.h"
 #include "mozilla/dom/ContentParent.h"
 #include "mozilla/gfx/gfxVars.h"
 #include "mozilla/layers/APZCTreeManagerChild.h"
@@ -79,16 +80,17 @@ GPUProcessManager::GPUProcessManager()
       mProcess(nullptr),
       mProcessToken(0),
       mGPUChild(nullptr) {
   MOZ_COUNT_CTOR(GPUProcessManager);
 
   mIdNamespace = AllocateNamespace();
   mObserver = new Observer(this);
   nsContentUtils::RegisterShutdownObserver(mObserver);
+  Preferences::AddStrongObserver(mObserver, "");
 
   mDeviceResetLastTime = TimeStamp::Now();
 
   LayerTreeOwnerTracker::Initialize();
 }
 
 GPUProcessManager::~GPUProcessManager() {
   MOZ_COUNT_DTOR(GPUProcessManager);
@@ -107,29 +109,51 @@ NS_IMPL_ISUPPORTS(GPUProcessManager::Obs
 GPUProcessManager::Observer::Observer(GPUProcessManager* aManager)
     : mManager(aManager) {}
 
 NS_IMETHODIMP
 GPUProcessManager::Observer::Observe(nsISupports* aSubject, const char* aTopic,
                                      const char16_t* aData) {
   if (!strcmp(aTopic, NS_XPCOM_SHUTDOWN_OBSERVER_ID)) {
     mManager->OnXPCOMShutdown();
+  } else if (!strcmp(aTopic, "nsPref:changed")) {
+    mManager->OnPreferenceChange(aData);
   }
   return NS_OK;
 }
 
 void GPUProcessManager::OnXPCOMShutdown() {
   if (mObserver) {
     nsContentUtils::UnregisterShutdownObserver(mObserver);
+    Preferences::RemoveObserver(mObserver, "");
     mObserver = nullptr;
   }
 
   CleanShutdown();
 }
 
+void GPUProcessManager::OnPreferenceChange(const char16_t* aData) {
+  // A pref changed. If it's not on the blacklist, inform child processes.
+  if (!dom::ContentParent::ShouldSyncPreference(aData)) {
+    return;
+  }
+
+  // We know prefs are ASCII here.
+  NS_LossyConvertUTF16toASCII strData(aData);
+
+  mozilla::dom::Pref pref(strData, /* isLocked */ false, Nothing(), Nothing());
+  Preferences::GetPreference(&pref);
+  if (!!mGPUChild) {
+    MOZ_ASSERT(mQueuedPrefs.IsEmpty());
+    mGPUChild->SendPreferenceUpdate(pref);
+  } else {
+    mQueuedPrefs.AppendElement(pref);
+  }
+}
+
 void GPUProcessManager::LaunchGPUProcess() {
   if (mProcess) {
     return;
   }
 
   // Start the Vsync I/O thread so can use it as soon as the process launches.
   EnsureVsyncIOThread();
 
@@ -337,16 +361,23 @@ void GPUProcessManager::OnProcessLaunchC
     DisableGPUProcess("Failed to create PVsyncBridge endpoints");
     return;
   }
 
   mVsyncBridge = VsyncBridgeChild::Create(mVsyncIOThread, mProcessToken,
                                           std::move(vsyncChild));
   mGPUChild->SendInitVsyncBridge(std::move(vsyncParent));
 
+  // Flush any pref updates that happened during launch and weren't
+  // included in the blobs set up in LaunchGPUProcess.
+  for (const mozilla::dom::Pref& pref : mQueuedPrefs) {
+    Unused << NS_WARN_IF(!mGPUChild->SendPreferenceUpdate(pref));
+  }
+  mQueuedPrefs.Clear();
+
   CrashReporter::AnnotateCrashReport(
       CrashReporter::Annotation::GPUProcessStatus,
       NS_LITERAL_CSTRING("Running"));
 
   CrashReporter::AnnotateCrashReport(
       CrashReporter::Annotation::GPUProcessLaunchCount,
       static_cast<int>(mNumProcessAttempts));
 }
--- a/gfx/ipc/GPUProcessManager.h
+++ b/gfx/ipc/GPUProcessManager.h
@@ -174,16 +174,17 @@ class GPUProcessManager final : public G
   GPUChild* GetGPUChild() { return mGPUChild; }
 
   // Returns whether or not a GPU process was ever launched.
   bool AttemptedGPUProcess() const { return mNumProcessAttempts > 0; }
 
  private:
   // Called from our xpcom-shutdown observer.
   void OnXPCOMShutdown();
+  void OnPreferenceChange(const char16_t* aData);
 
   bool CreateContentCompositorManager(
       base::ProcessId aOtherProcess,
       mozilla::ipc::Endpoint<PCompositorManagerChild>* aOutEndpoint);
   bool CreateContentImageBridge(
       base::ProcessId aOtherProcess,
       mozilla::ipc::Endpoint<PImageBridgeChild>* aOutEndpoint);
   bool CreateContentVRManager(
@@ -272,14 +273,18 @@ class GPUProcessManager final : public G
   uint32_t mDeviceResetCount;
   TimeStamp mDeviceResetLastTime;
 
   // Fields that are associated with the current GPU process.
   GPUProcessHost* mProcess;
   uint64_t mProcessToken;
   GPUChild* mGPUChild;
   RefPtr<VsyncBridgeChild> mVsyncBridge;
+  // 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<mozilla::dom::Pref> mQueuedPrefs;
 };
 
 }  // namespace gfx
 }  // namespace mozilla
 
 #endif  // _include_mozilla_gfx_ipc_GPUProcessManager_h_
--- a/gfx/ipc/PGPU.ipdl
+++ b/gfx/ipc/PGPU.ipdl
@@ -1,16 +1,17 @@
 /* -*- Mode: C++; tab-width: 8; 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 GraphicsMessages;
 include MemoryReportTypes;
 include HangTypes;
+include PrefsTypes;
 include protocol PAPZInputBridge;
 include protocol PCompositorManager;
 include protocol PImageBridge;
 include protocol PProfiler;
 include protocol PVRGPU;
 include protocol PVRManager;
 include protocol PVsyncBridge;
 include protocol PUiCompositorController;
@@ -61,16 +62,18 @@ parent:
   async InitUiCompositorController(LayersId rootLayerTreeId, Endpoint<PUiCompositorControllerParent> endpoint);
   async InitProfiler(Endpoint<PProfilerChild> endpoint);
   // Forward GPU process its endpoints to the VR process.
   async InitVR(Endpoint<PVRGPUChild> endpoint);
   // Called to update a gfx preference or variable.
   async UpdatePref(GfxPrefSetting pref);
   async UpdateVar(GfxVarUpdate var);
 
+  async PreferenceUpdate(Pref pref);
+
   // Create a new content-process compositor bridge.
   async NewContentCompositorManager(Endpoint<PCompositorManagerParent> endpoint);
   async NewContentImageBridge(Endpoint<PImageBridgeParent> endpoint);
   async NewContentVRManager(Endpoint<PVRManagerParent> endpoint);
   async NewContentVideoDecoderManager(Endpoint<PVideoDecoderManagerParent> endpoint);
 
   // Called to notify the GPU process of who owns a layersId.
   sync AddLayerTreeIdMapping(LayerTreeIdMapping mapping);