Bug 1413143: Make font inflation computation less lazy. r=bz,JanH
authorEmilio Cobos Álvarez <emilio@crisal.io>
Wed, 01 Nov 2017 11:24:17 +0100
changeset 443460 1ce74c61c5c3355689b65ca7453020e90897ee00
parent 443459 39d71149faea86d74a770711992b745892e49801
child 443461 bfd94ce6cddd5977e8151437dd0ac755fd5d7775
push id1618
push userCallek@gmail.com
push dateThu, 11 Jan 2018 17:45:48 +0000
treeherdermozilla-release@882ca853e05a [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersbz, JanH
bugs1413143, 1404545
milestone58.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1413143: Make font inflation computation less lazy. r=bz,JanH This makes it a bit more straight-forward to change the system font scale, preserving the sync MediaFeatureChanged event. This also avoids notifying media queries when the shell is not initialized. In particular, the patch in bug 1404545 allows calling MediaFeatureValuesChanged on a still-initializing pres-shell. This is nasty, and all this initialization order is kind of a mess, but I'm not reworking it for now... Also, this drops the invalidation of font-inflation when a doctype is added to the document. GetViewportInfo() already relies on the doctype not changing, as noted in a comment. MozReview-Commit-ID: Knw7dM1B04Y
dom/base/nsDocument.cpp
layout/base/PresShell.cpp
layout/base/nsIPresShell.h
layout/base/nsLayoutUtils.cpp
layout/base/nsPresContext.cpp
layout/reftests/bugs/reftest.list
--- a/dom/base/nsDocument.cpp
+++ b/dom/base/nsDocument.cpp
@@ -8167,16 +8167,18 @@ nsIDocument::AdoptNode(nsINode& aAdopted
                "Should still be in the document we just got adopted into");
 
   return adoptedNode;
 }
 
 nsViewportInfo
 nsDocument::GetViewportInfo(const ScreenIntSize& aDisplaySize)
 {
+  MOZ_ASSERT(mPresShell);
+
   // Compute the CSS-to-LayoutDevice pixel scale as the product of the
   // widget scale and the full zoom.
   nsPresContext* context = mPresShell->GetPresContext();
   // When querying the full zoom, get it from the device context rather than
   // directly from the pres context, because the device context's value can
   // include an adjustment necessay to keep the number of app units per device
   // pixel an integer, and we want the adjusted value.
   float fullZoom = context ? context->DeviceContext()->GetFullZoom() : 1.0;
--- a/layout/base/PresShell.cpp
+++ b/layout/base/PresShell.cpp
@@ -760,17 +760,16 @@ nsIPresShell::nsIPresShell()
     , mNeedThrottledAnimationFlush(true)
     , mPresShellId(0)
     , mFontSizeInflationEmPerLine(0)
     , mFontSizeInflationMinTwips(0)
     , mFontSizeInflationLineThreshold(0)
     , mFontSizeInflationForceEnabled(false)
     , mFontSizeInflationDisabledInMasterProcess(false)
     , mFontSizeInflationEnabled(false)
-    , mFontSizeInflationEnabledIsDirty(false)
     , mPaintingIsFrozen(false)
     , mIsNeverPainting(false)
     , mInFlush(false)
   {}
 
 PresShell::PresShell()
   : mCaretEnabled(false)
 #ifdef DEBUG
@@ -1005,17 +1004,23 @@ PresShell::Init(nsIDocument* aDocument,
   for (DocumentTimeline* timeline : mDocument->Timelines()) {
     timeline->NotifyRefreshDriverCreated(GetPresContext()->RefreshDriver());
   }
 
   // Get our activeness from the docShell.
   QueryIsActive();
 
   // Setup our font inflation preferences.
-  SetupFontInflation();
+  mFontSizeInflationEmPerLine = nsLayoutUtils::FontSizeInflationEmPerLine();
+  mFontSizeInflationMinTwips = nsLayoutUtils::FontSizeInflationMinTwips();
+  mFontSizeInflationLineThreshold = nsLayoutUtils::FontSizeInflationLineThreshold();
+  mFontSizeInflationForceEnabled = nsLayoutUtils::FontSizeInflationForceEnabled();
+  mFontSizeInflationDisabledInMasterProcess = nsLayoutUtils::FontSizeInflationDisabledInMasterProcess();
+  // We'll compute the font size inflation state in Initialize(), when we know
+  // the document type.
 
   mTouchManager.Init(this, mDocument);
 
   if (mPresContext->IsRootContentDocument()) {
     mZoomConstraintsClient = new ZoomConstraintsClient();
     mZoomConstraintsClient->Init(this, mDocument);
     if (gfxPrefs::MetaViewportEnabled() || gfxPrefs::APZAllowZooming()) {
       mMobileViewportManager = new MobileViewportManager(this, mDocument);
@@ -1695,16 +1700,20 @@ PresShell::Initialize(nscoord aWidth, ns
     return NS_OK;
   }
 
   MOZ_LOG(gLog, LogLevel::Debug, ("PresShell::Initialize this=%p", this));
 
   NS_ASSERTION(!mDidInitialize, "Why are we being called?");
 
   nsCOMPtr<nsIPresShell> kungFuDeathGrip(this);
+
+  RecomputeFontSizeInflationEnabled();
+  MOZ_DIAGNOSTIC_ASSERT(!mIsDestroying);
+
   mDidInitialize = true;
 
 #ifdef DEBUG
   if (VERIFY_REFLOW_NOISY_RC & gVerifyReflowFlags) {
     if (mDocument) {
       nsIURI *uri = mDocument->GetDocumentURI();
       if (uri) {
         printf("*** PresShell::Initialize (this=%p, url='%s')\n",
@@ -1720,16 +1729,17 @@ PresShell::Initialize(nscoord aWidth, ns
   mPresContext->SetVisibleArea(nsRect(0, 0, aWidth, aHeight));
 
   // Get the root frame from the frame manager
   // XXXbz it would be nice to move this somewhere else... like frame manager
   // Init(), say.  But we need to make sure our views are all set up by the
   // time we do this!
   nsIFrame* rootFrame = mFrameConstructor->GetRootFrame();
   NS_ASSERTION(!rootFrame, "How did that happen, exactly?");
+
   if (!rootFrame) {
     nsAutoScriptBlocker scriptBlocker;
     mFrameConstructor->BeginUpdate();
     rootFrame = mFrameConstructor->ConstructRootFrame();
     mFrameConstructor->SetRootFrame(rootFrame);
     mFrameConstructor->EndUpdate();
   }
 
@@ -4451,21 +4461,16 @@ PresShell::ContentInserted(nsIDocument* 
   mPresContext->RestyleManager()->ContentInserted(container, aChild);
 
   mFrameConstructor->ContentInserted(
       aMaybeContainer,
       aChild,
       nullptr,
       nsCSSFrameConstructor::InsertionKind::Async);
 
-  if (aChild->NodeType() == nsIDOMNode::DOCUMENT_TYPE_NODE) {
-    MOZ_ASSERT(container == aDocument);
-    NotifyFontSizeInflationEnabledIsDirty();
-  }
-
   VERIFY_STYLE_TREE;
 }
 
 void
 PresShell::ContentRemoved(nsIDocument *aDocument,
                           nsIContent* aMaybeContainer,
                           nsIContent* aChild,
                           nsIContent* aPreviousSibling)
@@ -4500,21 +4505,16 @@ PresShell::ContentRemoved(nsIDocument *a
   if (mPointerEventTarget &&
       nsContentUtils::ContentIsDescendantOf(mPointerEventTarget, aChild)) {
     mPointerEventTarget = aMaybeContainer;
   }
 
   mFrameConstructor->ContentRemoved(aMaybeContainer, aChild, oldNextSibling,
                                     nsCSSFrameConstructor::REMOVE_CONTENT);
 
-  if (aChild->NodeType() == nsIDOMNode::DOCUMENT_TYPE_NODE) {
-    MOZ_ASSERT(container == aDocument);
-    NotifyFontSizeInflationEnabledIsDirty();
-  }
-
   VERIFY_STYLE_TREE;
 }
 
 void
 PresShell::NotifyCounterStylesAreDirty()
 {
   nsAutoCauseReflowNotifier reflowNotifier(this);
   mFrameConstructor->BeginUpdate();
@@ -10697,42 +10697,43 @@ nsIPresShell::SetScrollPositionClampingS
     if (nsIScrollableFrame* rootScrollFrame = GetRootScrollFrameAsScrollable()) {
       rootScrollFrame->MarkScrollbarsDirtyForReflow();
     }
     MarkFixedFramesForReflow(nsIPresShell::eResize);
   }
 }
 
 void
-PresShell::SetupFontInflation()
-{
-  mFontSizeInflationEmPerLine = nsLayoutUtils::FontSizeInflationEmPerLine();
-  mFontSizeInflationMinTwips = nsLayoutUtils::FontSizeInflationMinTwips();
-  mFontSizeInflationLineThreshold = nsLayoutUtils::FontSizeInflationLineThreshold();
-  mFontSizeInflationForceEnabled = nsLayoutUtils::FontSizeInflationForceEnabled();
-  mFontSizeInflationDisabledInMasterProcess = nsLayoutUtils::FontSizeInflationDisabledInMasterProcess();
-
-  NotifyFontSizeInflationEnabledIsDirty();
-}
-
-void
 nsIPresShell::RecomputeFontSizeInflationEnabled()
 {
-  mFontSizeInflationEnabledIsDirty = false;
   mFontSizeInflationEnabled = DetermineFontSizeInflationState();
 
-  HandleSystemFontScale();
+  float fontScale = nsLayoutUtils::SystemFontScale();
+  if (fontScale == 0.0f) {
+    return;
+  }
+
+  MOZ_ASSERT(mDocument);
+  MOZ_ASSERT(mPresContext);
+  if (mFontSizeInflationEnabled || mDocument->IsSyntheticDocument()) {
+    mPresContext->SetSystemFontScale(1.0f);
+  } else {
+    mPresContext->SetSystemFontScale(fontScale);
+  }
 }
 
 bool
 nsIPresShell::DetermineFontSizeInflationState()
 {
   MOZ_ASSERT(mPresContext, "our pres context should not be null");
-  if ((FontSizeInflationEmPerLine() == 0 &&
-      FontSizeInflationMinTwips() == 0) || mPresContext->IsChrome()) {
+  if (mPresContext->IsChrome()) {
+    return false;
+  }
+
+  if (FontSizeInflationEmPerLine() == 0 && FontSizeInflationMinTwips() == 0) {
     return false;
   }
 
   // Force-enabling font inflation always trumps the heuristics here.
   if (!FontSizeInflationForceEnabled()) {
     if (TabChild* tab = TabChild::GetFrom(this)) {
       // We're in a child process.  Cancel inflation if we're not
       // async-pan zoomed.
@@ -10783,43 +10784,16 @@ nsIPresShell::DetermineFontSizeInflation
     if (vInf.GetDefaultZoom() >= CSSToScreenScale(1.0f) || vInf.IsAutoSizeEnabled()) {
       return false;
     }
   }
 
   return true;
 }
 
-bool
-nsIPresShell::FontSizeInflationEnabled()
-{
-  if (mFontSizeInflationEnabledIsDirty) {
-    RecomputeFontSizeInflationEnabled();
-  }
-
-  return mFontSizeInflationEnabled;
-}
-
-void
-nsIPresShell::HandleSystemFontScale()
-{
-  float fontScale = nsLayoutUtils::SystemFontScale();
-  if (fontScale == 0.0f) {
-    return;
-  }
-
-  MOZ_ASSERT(mDocument && mPresContext, "our document and pres context should not be null");
-
-  if (!mFontSizeInflationEnabled && !mDocument->IsSyntheticDocument()) {
-    mPresContext->SetSystemFontScale(fontScale);
-  } else {
-    mPresContext->SetSystemFontScale(1.0f);
-  }
-}
-
 void
 PresShell::PausePainting()
 {
   if (GetPresContext()->RefreshDriver()->GetPresContext() != GetPresContext())
     return;
 
   mPaintingIsFrozen = true;
   GetPresContext()->RefreshDriver()->Freeze();
--- a/layout/base/nsIPresShell.h
+++ b/layout/base/nsIPresShell.h
@@ -1502,34 +1502,24 @@ public:
   bool FontSizeInflationForceEnabled() const {
     return mFontSizeInflationForceEnabled;
   }
 
   bool FontSizeInflationDisabledInMasterProcess() const {
     return mFontSizeInflationDisabledInMasterProcess;
   }
 
-  /**
-   * Determine if font size inflation is enabled. This value is cached until
-   * it becomes dirty.
-   *
-   * @returns true, if font size inflation is enabled; false otherwise.
-   */
-  bool FontSizeInflationEnabled();
+  bool FontSizeInflationEnabled() const {
+    return mFontSizeInflationEnabled;
+  }
 
   /**
-   * Notify the pres shell that an event occurred making the current value of
-   * mFontSizeInflationEnabled invalid. This will schedule a recomputation of
-   * whether font size inflation is enabled on the next call to
-   * FontSizeInflationEnabled().
+   * Recomputes whether font-size inflation is enabled.
    */
-  void NotifyFontSizeInflationEnabledIsDirty()
-  {
-    mFontSizeInflationEnabledIsDirty = true;
-  }
+  void RecomputeFontSizeInflationEnabled();
 
   /**
    * Return true if the most recent interruptible reflow was interrupted.
    */
   bool IsReflowInterrupted() const {
     return mWasLastReflowInterrupted;
   }
 
@@ -1585,33 +1575,21 @@ public:
   /**
    * Refresh observer management.
    */
 protected:
   void DoObserveStyleFlushes();
   void DoObserveLayoutFlushes();
 
   /**
-   * Do computations necessary to determine if font size inflation is enabled.
-   * This value is cached after computation, as the computation is somewhat
-   * expensive.
-   */
-  void RecomputeFontSizeInflationEnabled();
-
-  /**
-   * Does the actual work of figuring out the current state of font size inflation.
+   * Does the actual work of figuring out the current state of font size
+   * inflation.
    */
   bool DetermineFontSizeInflationState();
 
-  /**
-   * Apply the system font scale from the corresponding pref to the PresContext,
-   * taking into account the current state of font size inflation.
-   */
-  void HandleSystemFontScale();
-
   void RecordAlloc(void* aPtr) {
 #ifdef DEBUG
     MOZ_ASSERT(!mAllocatedPointers.Contains(aPtr));
     mAllocatedPointers.PutEntry(aPtr);
 #endif
   }
 
   void RecordFree(void* aPtr) {
--- a/layout/base/nsLayoutUtils.cpp
+++ b/layout/base/nsLayoutUtils.cpp
@@ -9125,26 +9125,26 @@ nsLayoutUtils::HasDocumentLevelListeners
   return false;
 }
 
 static void
 MaybeReflowForInflationScreenSizeChange(nsPresContext *aPresContext)
 {
   if (aPresContext) {
     nsIPresShell* presShell = aPresContext->GetPresShell();
-    bool fontInflationWasEnabled = presShell->FontSizeInflationEnabled();
-    presShell->NotifyFontSizeInflationEnabledIsDirty();
+    const bool fontInflationWasEnabled = presShell->FontSizeInflationEnabled();
+    presShell->RecomputeFontSizeInflationEnabled();
     bool changed = false;
-    if (presShell && presShell->FontSizeInflationEnabled() &&
+    if (presShell->FontSizeInflationEnabled() &&
         presShell->FontSizeInflationMinTwips() != 0) {
       aPresContext->ScreenSizeInchesForFontInflation(&changed);
     }
 
     changed = changed ||
-      (fontInflationWasEnabled != presShell->FontSizeInflationEnabled());
+      fontInflationWasEnabled != presShell->FontSizeInflationEnabled();
     if (changed) {
       nsCOMPtr<nsIDocShell> docShell = aPresContext->GetDocShell();
       if (docShell) {
         nsCOMPtr<nsIContentViewer> cv;
         docShell->GetContentViewer(getter_AddRefs(cv));
         if (cv) {
           nsTArray<nsCOMPtr<nsIContentViewer> > array;
           cv->AppendSubtree(array);
--- a/layout/base/nsPresContext.cpp
+++ b/layout/base/nsPresContext.cpp
@@ -2118,24 +2118,28 @@ nsPresContext::MediaFeatureValuesChanged
   }
 
   if (aRestyleHint || aChangeHint) {
     RebuildAllStyleData(aChangeHint, aRestyleHint);
   }
 
   mPendingViewportChange = false;
 
+  if (!mShell || !mShell->DidInitialize()) {
+    return;
+  }
+
   if (mDocument->IsBeingUsedAsImage()) {
     MOZ_ASSERT(mDocument->MediaQueryLists().isEmpty());
     return;
   }
 
   mDocument->NotifyMediaFeatureValuesChanged();
 
-  MOZ_ASSERT(nsContentUtils::IsSafeToRunScript());
+  MOZ_DIAGNOSTIC_ASSERT(nsContentUtils::IsSafeToRunScript());
 
   // Media query list listeners should be notified from a queued task
   // (in HTML5 terms), although we also want to notify them on certain
   // flushes.  (We're already running off an event.)
   //
   // Note that we do this after the new style from media queries in
   // style sheets has been computed.
 
--- a/layout/reftests/bugs/reftest.list
+++ b/layout/reftests/bugs/reftest.list
@@ -2042,9 +2042,9 @@ needs-focus != 1377447-1.html 1377447-2.
 == 1398500-1.html 1398500-1-ref.html
 == 1401317.html 1401317-ref.html
 == 1401992.html 1401992-ref.html
 == 1405878-1.xml 1405878-1-ref.xml
 == 1404057.html 1404057-ref.html
 != 1404057.html 1404057-noref.html
 == 1406183-1.html 1406183-1-ref.html
 == 1410028.html 1410028-ref.html
-test-pref(font.size.systemFontScale,200) skip-if(isDebugBuild) == 1412743.html 1412743-ref.html # bug 1413143
+test-pref(font.size.systemFontScale,200) == 1412743.html 1412743-ref.html