Bug 1369411: Use a runnable instead of a timer for pref changes in nsPresContext. r=bholley a=jcristau
authorEmilio Cobos Álvarez <emilio@crisal.io>
Mon, 05 Mar 2018 10:57:12 +0100
changeset 463385 d7611ae5168b937b7304348d511ae0e4f691e46e
parent 463384 d84374efea519773d438a536be511c23c4a5d755
child 463386 99b358bc5ef4a64dfb2c13984261b2fd3af77f68
push id1683
push usersfraser@mozilla.com
push dateThu, 26 Apr 2018 16:43:40 +0000
treeherdermozilla-release@5af6cb21869d [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersbholley, jcristau
bugs1369411
milestone60.0
Bug 1369411: Use a runnable instead of a timer for pref changes in nsPresContext. r=bholley a=jcristau This should hopefully make pushPrefEnv able to deal with it. I optimistically enabled Android too, pending a CI run. MozReview-Commit-ID: 47C4q0lzIek
layout/base/nsPresContext.cpp
layout/base/nsPresContext.h
layout/base/tests/mochitest.ini
--- a/layout/base/nsPresContext.cpp
+++ b/layout/base/nsPresContext.cpp
@@ -214,32 +214,23 @@ nsPresContext::IsDOMPaintEventPending()
 }
 
 void
 nsPresContext::PrefChangedCallback(const char* aPrefName, void* instance_data)
 {
   RefPtr<nsPresContext>  presContext =
     static_cast<nsPresContext*>(instance_data);
 
-  NS_ASSERTION(nullptr != presContext, "bad instance data");
-  if (nullptr != presContext) {
+  NS_ASSERTION(presContext, "bad instance data");
+  if (presContext) {
     presContext->PreferenceChanged(aPrefName);
   }
 }
 
 void
-nsPresContext::PrefChangedUpdateTimerCallback(nsITimer *aTimer, void *aClosure)
-{
-  nsPresContext*  presContext = (nsPresContext*)aClosure;
-  NS_ASSERTION(presContext != nullptr, "bad instance data");
-  if (presContext)
-    presContext->UpdateAfterPreferencesChanged();
-}
-
-void
 nsPresContext::ForceReflowForFontInfoUpdate()
 {
   // We can trigger reflow by pretending a font.* preference has changed;
   // this is the same mechanism as gfxPlatform::ForceGlobalReflow() uses
   // if new fonts are installed during the session, for example.
   PreferenceChanged("font.internaluseonly.changed");
 }
 
@@ -373,22 +364,16 @@ nsPresContext::Destroy()
 {
   if (mEventManager) {
     // unclear if these are needed, but can't hurt
     mEventManager->NotifyDestroyPresContext(this);
     mEventManager->SetPresContext(nullptr);
     mEventManager = nullptr;
   }
 
-  if (mPrefChangedTimer)
-  {
-    mPrefChangedTimer->Cancel();
-    mPrefChangedTimer = nullptr;
-  }
-
   // Unregister preference callbacks
   Preferences::UnregisterPrefixCallback(nsPresContext::PrefChangedCallback,
                                         "font.",
                                         this);
   Preferences::UnregisterPrefixCallback(nsPresContext::PrefChangedCallback,
                                         "browser.display.",
                                         this);
   Preferences::UnregisterCallback(nsPresContext::PrefChangedCallback,
@@ -471,17 +456,16 @@ NS_IMPL_CYCLE_COLLECTION_TRAVERSE_BEGIN(
   // NS_IMPL_CYCLE_COLLECTION_TRAVERSE_RAWPTR(mDeviceContext); // not xpcom
   NS_IMPL_CYCLE_COLLECTION_TRAVERSE(mEffectCompositor);
   NS_IMPL_CYCLE_COLLECTION_TRAVERSE(mEventManager);
   // NS_IMPL_CYCLE_COLLECTION_TRAVERSE_RAWPTR(mLanguage); // an atom
 
   // NS_IMPL_CYCLE_COLLECTION_TRAVERSE(mTheme); // a service
   // NS_IMPL_CYCLE_COLLECTION_TRAVERSE(mLangService); // a service
   NS_IMPL_CYCLE_COLLECTION_TRAVERSE(mPrintSettings);
-  NS_IMPL_CYCLE_COLLECTION_TRAVERSE(mPrefChangedTimer);
 NS_IMPL_CYCLE_COLLECTION_TRAVERSE_END
 
 NS_IMPL_CYCLE_COLLECTION_UNLINK_BEGIN(nsPresContext)
   NS_IMPL_CYCLE_COLLECTION_UNLINK(mAnimationEventDispatcher);
   NS_IMPL_CYCLE_COLLECTION_UNLINK(mDocument);
   NS_IMPL_CYCLE_COLLECTION_UNLINK(mDeviceContext); // worth bothering?
   NS_IMPL_CYCLE_COLLECTION_UNLINK(mEffectCompositor);
   // NS_RELEASE(tmp->mLanguage); // an atom
@@ -818,61 +802,74 @@ nsPresContext::PreferenceChanged(const c
 
     // Changes to bidi.numeral also needs to empty the text run cache.
     // This is handled in gfxTextRunWordCache.cpp.
   }
   if (StringBeginsWith(prefName, NS_LITERAL_CSTRING("gfx.font_rendering."))) {
     // Changes to font_rendering prefs need to trigger a reflow
     mPrefChangePendingNeedsReflow = true;
   }
-  // we use a zero-delay timer to coalesce multiple pref updates
-  if (!mPrefChangedTimer)
-  {
-    // We will end up calling InvalidatePreferenceSheets one from each pres
-    // context, but all it's doing is clearing its cached sheet pointers,
-    // so it won't be wastefully recreating the sheet multiple times.
-    // The first pres context that has its mPrefChangedTimer called will
-    // be the one to cause the reconstruction of the pref style sheet.
-    nsLayoutStylesheetCache::InvalidatePreferenceSheets();
-    mPrefChangedTimer = CreateTimer(PrefChangedUpdateTimerCallback,
-                                    "PrefChangedUpdateTimerCallback", 0);
-    if (!mPrefChangedTimer) {
-      return;
-    }
-  }
+
+  // We will end up calling InvalidatePreferenceSheets one from each pres
+  // context, but all it's doing is clearing its cached sheet pointers, so it
+  // won't be wastefully recreating the sheet multiple times.
+  //
+  // The first pres context that has its pref changed runnable called will
+  // be the one to cause the reconstruction of the pref style sheet.
+  nsLayoutStylesheetCache::InvalidatePreferenceSheets();
+  DispatchPrefChangedRunnableIfNeeded();
+
   if (prefName.EqualsLiteral("nglayout.debug.paint_flashing") ||
       prefName.EqualsLiteral("nglayout.debug.paint_flashing_chrome")) {
     mPaintFlashingInitialized = false;
     return;
   }
 }
 
 void
+nsPresContext::DispatchPrefChangedRunnableIfNeeded()
+{
+  if (mPostedPrefChangedRunnable) {
+    return;
+  }
+
+  nsCOMPtr<nsIRunnable> runnable =
+    NewRunnableMethod("nsPresContext::UpdateAfterPreferencesChanged",
+                      this,
+                      &nsPresContext::UpdateAfterPreferencesChanged);
+  nsresult rv = Document()->Dispatch(TaskCategory::Other, runnable.forget());
+  if (NS_SUCCEEDED(rv)) {
+    mPostedPrefChangedRunnable = true;
+  }
+}
+
+void
 nsPresContext::UpdateAfterPreferencesChanged()
 {
-  mPrefChangedTimer = nullptr;
+  mPostedPrefChangedRunnable = false;
+  if (!mShell) {
+    return;
+  }
 
   if (!mContainer) {
     // Delay updating until there is a container
     mNeedsPrefUpdate = true;
     return;
   }
 
   nsCOMPtr<nsIDocShellTreeItem> docShell(mContainer);
   if (docShell && nsIDocShellTreeItem::typeChrome == docShell->ItemType()) {
     return;
   }
 
   // Initialize our state from the user preferences
   GetUserPreferences();
 
   // update the presShell: tell it to set the preference style rules up
-  if (mShell) {
-    mShell->UpdatePreferenceStyles();
-  }
+  mShell->UpdatePreferenceStyles();
 
   InvalidatePaintedLayers();
   mDeviceContext->FlushFontCache();
 
   nsChangeHint hint = nsChangeHint(0);
 
   if (mPrefChangePendingNeedsReflow) {
     hint |= NS_STYLE_HINT_REFLOW;
@@ -1632,20 +1629,17 @@ nsPresContext::ElementWouldPropagateScro
 void
 nsPresContext::SetContainer(nsIDocShell* aDocShell)
 {
   if (aDocShell) {
     NS_ASSERTION(!(mContainer && mNeedsPrefUpdate),
                  "Should only need pref update if mContainer is null.");
     mContainer = static_cast<nsDocShell*>(aDocShell);
     if (mNeedsPrefUpdate) {
-      if (!mPrefChangedTimer) {
-        mPrefChangedTimer = CreateTimer(PrefChangedUpdateTimerCallback,
-                                        "PrefChangedUpdateTimerCallback", 0);
-      }
+      DispatchPrefChangedRunnableIfNeeded();
       mNeedsPrefUpdate = false;
     }
   } else {
     mContainer = WeakPtr<nsDocShell>();
   }
   UpdateIsChrome();
   if (mContainer) {
     GetDocumentColorPreferences();
@@ -1828,18 +1822,17 @@ nsPresContext::ThemeChanged()
   if (!mPendingThemeChanged) {
     sLookAndFeelChanged = true;
     sThemeChanged = true;
 
     nsCOMPtr<nsIRunnable> ev =
       NewRunnableMethod("nsPresContext::ThemeChangedInternal",
                         this,
                         &nsPresContext::ThemeChangedInternal);
-    nsresult rv = Document()->Dispatch(TaskCategory::Other,
-                                       ev.forget());
+    nsresult rv = Document()->Dispatch(TaskCategory::Other, ev.forget());
     if (NS_SUCCEEDED(rv)) {
       mPendingThemeChanged = true;
     }
   }
 }
 
 static bool
 NotifyThemeChanged(TabParent* aTabParent, void* aArg)
@@ -1883,18 +1876,17 @@ void
 nsPresContext::SysColorChanged()
 {
   if (!mPendingSysColorChanged) {
     sLookAndFeelChanged = true;
     nsCOMPtr<nsIRunnable> ev =
       NewRunnableMethod("nsPresContext::SysColorChangedInternal",
                         this,
                         &nsPresContext::SysColorChangedInternal);
-    nsresult rv = Document()->Dispatch(TaskCategory::Other,
-                                       ev.forget());
+    nsresult rv = Document()->Dispatch(TaskCategory::Other, ev.forget());
     if (NS_SUCCEEDED(rv)) {
       mPendingSysColorChanged = true;
     }
   }
 }
 
 void
 nsPresContext::SysColorChangedInternal()
--- a/layout/base/nsPresContext.h
+++ b/layout/base/nsPresContext.h
@@ -1223,17 +1223,17 @@ protected:
   void SetSMILAnimations(nsIDocument *aDoc, uint16_t aNewMode,
                                      uint16_t aOldMode);
   void GetDocumentColorPreferences();
 
   void PreferenceChanged(const char* aPrefName);
   static void PrefChangedCallback(const char*, void*);
 
   void UpdateAfterPreferencesChanged();
-  static void PrefChangedUpdateTimerCallback(nsITimer *aTimer, void *aClosure);
+  void DispatchPrefChangedRunnableIfNeeded();
 
   void GetUserPreferences();
 
   /**
    * Fetch the user's font preferences for the given aLanguage's
    * langugage group.
    */
   const LangGroupFontPrefs* GetFontPrefsForLang(nsAtom *aLanguage, bool* aNeedsToCache = nullptr) const
@@ -1354,17 +1354,16 @@ protected:
   gfxSize               mLastFontInflationScreenSize;
 
   int32_t               mCurAppUnitsPerDevPixel;
   int32_t               mAutoQualityMinFontSizePixelsPref;
 
   nsCOMPtr<nsITheme> mTheme;
   nsLanguageAtomService* mLangService;
   nsCOMPtr<nsIPrintSettings> mPrintSettings;
-  nsCOMPtr<nsITimer>    mPrefChangedTimer;
 
   mozilla::UniquePtr<nsBidi> mBidiEngine;
 
   AutoTArray<TransactionInvalidations, 4> mTransactions;
 
   // text performance metrics
   nsAutoPtr<gfxTextPerfMetrics>   mTextPerf;
 
@@ -1462,16 +1461,17 @@ protected:
   unsigned              mDoScaledTwips : 1;
   unsigned              mIsRootPaginatedDocument : 1;
   unsigned              mPrefBidiDirection : 1;
   unsigned              mPrefScrollbarSide : 2;
   unsigned              mPendingSysColorChanged : 1;
   unsigned              mPendingThemeChanged : 1;
   unsigned              mPendingUIResolutionChanged : 1;
   unsigned              mPrefChangePendingNeedsReflow : 1;
+  unsigned              mPostedPrefChangedRunnable : 1;
   unsigned              mIsEmulatingMedia : 1;
 
   // Are we currently drawing an SVG glyph?
   unsigned              mIsGlyph : 1;
 
   // Does the associated document use root-em (rem) units?
   unsigned              mUsesRootEMUnits : 1;
   // Does the associated document use ex or ch units?
--- a/layout/base/tests/mochitest.ini
+++ b/layout/base/tests/mochitest.ini
@@ -23,17 +23,16 @@ support-files = border_radius_hit_testin
 [test_bug369950.html]
 skip-if = true # Bug 492575
 support-files = bug369950-subframe.xml
 [test_bug370436.html]
 skip-if = toolkit == 'android' # Bug 1355815
 [test_bug386575.xhtml]
 [test_bug388019.html]
 [test_bug394057.html]
-skip-if = toolkit == 'android' || os == "linux" # Bug 1355817, linux Bug 1369411
 [test_bug399284.html]
 [test_bug399951.html]
 [test_bug404209.xhtml]
 [test_bug416896.html]
 [test_bug423523.html]
 [test_bug435293-interaction.html]
 [test_bug435293-scale.html]
 [test_bug435293-skew.html]