Bug 1337062: Transfer initial gfxVars with SendXPCOMProcessAttributes. Deal with potential (future) race condition. r?dvander draft
authorMilan Sreckovic <milan@mozilla.com>
Tue, 27 Jun 2017 17:04:17 -0400
changeset 600792 aa207e6f547b07ff2d52cae13294003b83002f42
parent 600694 8f80d594c08d5c7a112e5d4b9eb44ffca717eb7b
child 635100 84a5a0a84efb8e724966ae7fb045fa1cf02087b5
push id65881
push userbmo:milan@mozilla.com
push dateTue, 27 Jun 2017 21:06:14 +0000
reviewersdvander
bugs1337062
milestone56.0a1
Bug 1337062: Transfer initial gfxVars with SendXPCOMProcessAttributes. Deal with potential (future) race condition. r?dvander MozReview-Commit-ID: GYezEAg7ozQ
dom/ipc/ContentChild.cpp
dom/ipc/ContentParent.cpp
dom/ipc/PContent.ipdl
gfx/config/gfxVars.cpp
gfx/config/gfxVars.h
--- a/dom/ipc/ContentChild.cpp
+++ b/dom/ipc/ContentChild.cpp
@@ -530,16 +530,17 @@ NS_INTERFACE_MAP_END
 
 
 mozilla::ipc::IPCResult
 ContentChild::RecvSetXPCOMProcessAttributes(const XPCOMInitData& aXPCOMInit,
                                             const StructuredCloneData& aInitialData,
                                             nsTArray<LookAndFeelInt>&& aLookAndFeelIntCache)
 {
   mLookAndFeelCache = aLookAndFeelIntCache;
+  gfx::gfxVars::SetValuesForInitialize(aXPCOMInit.gfxNonDefaultVarUpdates());
   InitXPCOM(aXPCOMInit, aInitialData);
   InitGraphicsDeviceData(aXPCOMInit.contentDeviceData());
 
 #ifdef NS_PRINTING
   // Force the creation of the nsPrintingProxy so that it's IPC counterpart,
   // PrintingParent, is always available for printing initiated from the parent.
   // Create nsPrintingProxy instance later than the SystemGroup initialization.
   RefPtr<nsPrintingProxy> printingProxy = nsPrintingProxy::GetInstance();
@@ -995,18 +996,16 @@ ContentChild::AppendProcessId(nsACString
   }
   unsigned pid = getpid();
   aName.Append(nsPrintfCString("(pid %u)", pid));
 }
 
 void
 ContentChild::InitGraphicsDeviceData(const ContentDeviceData& aData)
 {
-  // Initialize the graphics platform. This may contact the parent process
-  // to read device preferences.
   gfxPlatform::InitChild(aData);
 }
 
 void
 ContentChild::InitXPCOM(const XPCOMInitData& aXPCOMInit,
                         const mozilla::dom::ipc::StructuredCloneData& aInitialData)
 {
   SET_PREF_PHASE(pref_initPhase::BEGIN_ALL_PREFS);
--- a/dom/ipc/ContentParent.cpp
+++ b/dom/ipc/ContentParent.cpp
@@ -2293,17 +2293,22 @@ ContentParent::InitInternal(ProcessPrior
   // send the file URL instead.
   StyleSheet* ucs = nsLayoutStylesheetCache::For(StyleBackendType::Gecko)->UserContentSheet();
   if (ucs) {
     SerializeURI(ucs->GetSheetURI(), xpcomInit.userContentSheetURL());
   } else {
     SerializeURI(nullptr, xpcomInit.userContentSheetURL());
   }
 
+  // 1. Build ContentDeviceData first, as it may affect some gfxVars.
   gfxPlatform::GetPlatform()->BuildContentDeviceData(&xpcomInit.contentDeviceData());
+  // 2. Gather non-default gfxVars.
+  xpcomInit.gfxNonDefaultVarUpdates() = gfxVars::FetchNonDefaultVars();
+  // 3. Start listening for gfxVars updates, to notify content process later on.
+  gfxVars::AddReceiver(this);
 
   nsCOMPtr<nsIGfxInfo> gfxInfo = services::GetGfxInfo();
   if (gfxInfo) {
     for (int32_t i = 1; i <= nsIGfxInfo::FEATURE_MAX_VALUE; ++i) {
       int32_t status = 0;
       nsAutoCString failureId;
       gfxInfo->GetFeatureStatus(i, failureId, &status);
       dom::GfxInfoFeatureStatus gfxFeatureStatus;
--- a/dom/ipc/PContent.ipdl
+++ b/dom/ipc/PContent.ipdl
@@ -263,16 +263,17 @@ struct XPCOMInitData
     bool haveBidiKeyboards;
     nsString[] dictionaries;
     ClipboardCapabilities clipboardCaps;
     DomainPolicyClone domainPolicy;
     /* used on MacOSX only */
     FontFamilyListEntry[] fontFamilies;
     OptionalURIParams userContentSheetURL;
     PrefSetting[] prefs;
+    GfxVarUpdate[] gfxNonDefaultVarUpdates;
     ContentDeviceData contentDeviceData;
     GfxInfoFeatureStatus[] gfxFeatureStatus;
     DataStorageEntry[] dataStorage;
     nsCString[] appLocales;
     nsCString[] requestedLocales;
 };
 
 /**
--- a/gfx/config/gfxVars.cpp
+++ b/gfx/config/gfxVars.cpp
@@ -9,57 +9,92 @@
 #include "mozilla/dom/ContentChild.h"
 
 namespace mozilla {
 namespace gfx {
 
 StaticAutoPtr<gfxVars> gfxVars::sInstance;
 StaticAutoPtr<nsTArray<gfxVars::VarBase*>> gfxVars::sVarList;
 
+StaticAutoPtr<nsTArray<GfxVarUpdate>> gGfxVarInitUpdates;
+
+void
+gfxVars::SetValuesForInitialize(const nsTArray<GfxVarUpdate>& aInitUpdates)
+{
+  // This should only be called once
+  MOZ_RELEASE_ASSERT(!gGfxVarInitUpdates);
+
+  // We expect aInitUpdates to be provided before any other gfxVars operation,
+  // and for sInstance to be null here, but handle the alternative.
+  if (sInstance) {
+    // Apply the updates, the object has been created already
+    for (const auto& varUpdate : aInitUpdates) {
+      ApplyUpdate(varUpdate);
+    }
+  } else {
+      // Save the values for Initialize call
+      gGfxVarInitUpdates = new nsTArray<GfxVarUpdate>(aInitUpdates);
+  }
+}
+
 void
 gfxVars::Initialize()
 {
   if (sInstance) {
+    MOZ_RELEASE_ASSERT(!gGfxVarInitUpdates, "Initial updates should not be present after any gfxVars operation");
     return;
   }
 
   // sVarList must be initialized first since it's used in the constructor for
   // sInstance.
   sVarList = new nsTArray<gfxVars::VarBase*>();
   sInstance = new gfxVars;
 
-  // Like Preferences, we want content to synchronously get initial data on
-  // init. Note the GPU process is not handled here - it cannot send sync
+  // Note the GPU process is not handled here - it cannot send sync
   // messages, so instead the initial data is pushed down.
   if (XRE_IsContentProcess()) {
-    InfallibleTArray<GfxVarUpdate> vars;
-    dom::ContentChild::GetSingleton()->SendGetGfxVars(&vars);
-    for (const auto& var : vars) {
-      ApplyUpdate(var);
+    MOZ_ASSERT(gGfxVarInitUpdates, "Initial updates should be provided in content process");
+    if (!gGfxVarInitUpdates) {
+      // No provided initial updates, sync-request them from parent.
+      InfallibleTArray<GfxVarUpdate> initUpdates;
+      dom::ContentChild::GetSingleton()->SendGetGfxVars(&initUpdates);
+      gGfxVarInitUpdates = new nsTArray<GfxVarUpdate>(Move(initUpdates));
     }
+    for (const auto& varUpdate : *gGfxVarInitUpdates) {
+      ApplyUpdate(varUpdate);
+    }
+    gGfxVarInitUpdates = nullptr;
   }
 }
 
 gfxVars::gfxVars()
 {
 }
 
 void
 gfxVars::Shutdown()
 {
   sInstance = nullptr;
   sVarList = nullptr;
+  gGfxVarInitUpdates = nullptr;
 }
 
 /* static */ void
 gfxVars::ApplyUpdate(const GfxVarUpdate& aUpdate)
 {
   // Only subprocesses receive updates and apply them locally.
   MOZ_ASSERT(!XRE_IsParentProcess());
-  sVarList->ElementAt(aUpdate.index())->SetValue(aUpdate.value());
+  MOZ_DIAGNOSTIC_ASSERT(sVarList || gGfxVarInitUpdates);
+  if (sVarList) {
+    sVarList->ElementAt(aUpdate.index())->SetValue(aUpdate.value());
+  } else if (gGfxVarInitUpdates) {
+    // Too early, we haven't been initialized, so just add to
+    // the array waiting for the initialization...
+    gGfxVarInitUpdates->AppendElement(aUpdate);
+  }
 }
 
 /* static */ void
 gfxVars::AddReceiver(gfxVarReceiver* aReceiver)
 {
   MOZ_ASSERT(NS_IsMainThread());
 
   // Don't double-add receivers, in case a broken content process sends two
--- a/gfx/config/gfxVars.h
+++ b/gfx/config/gfxVars.h
@@ -53,16 +53,20 @@ class gfxVarReceiver;
 //    void SetCxxName(const DataType& aValue);
 //
 // Note that the setter may only be called in the UI process; a gfxVar must be
 // a variable that is determined in the UI process and pushed to child
 // processes.
 class gfxVars final
 {
 public:
+  // These values will be used during the Initialize() call if set.  Any
+  // updates that come before initialization will get added to this array.
+  static void SetValuesForInitialize(const nsTArray<GfxVarUpdate>& aInitUpdates);
+
   static void Initialize();
   static void Shutdown();
 
   static void ApplyUpdate(const GfxVarUpdate& aUpdate);
   static void AddReceiver(gfxVarReceiver* aReceiver);
   static void RemoveReceiver(gfxVarReceiver* aReceiver);
 
   // Return a list of updates for all variables with non-default values.