From 36069ceee72786adb589307e5cc5b5a6c8346c88 Mon Sep 17 00:00:00 2001
From: "dbaron%dbaron.org"
 <dbaron%dbaron.org@18797224-902f-48f8-a5cc-f745e15eee43>
Date: Fri, 8 Feb 2008 19:53:59 +0000
Subject: [PATCH] Force reflow on default font change, since style data
 comparison is not sufficient for things not reflected in style data. 
 b=394057  r+sr=roc  a=blocking1.9+

git-svn-id: svn://10.0.0.236/trunk@245250 18797224-902f-48f8-a5cc-f745e15eee43
---
 mozilla/dom/src/base/nsHistory.cpp            |  2 +-
 mozilla/dom/src/base/nsLocation.cpp           |  2 +-
 mozilla/layout/base/nsCSSFrameConstructor.cpp | 30 +++---
 mozilla/layout/base/nsCSSFrameConstructor.h   |  2 +-
 mozilla/layout/base/nsFrameManager.cpp        | 11 ++-
 mozilla/layout/base/nsFrameManager.h          |  7 +-
 mozilla/layout/base/nsPresContext.cpp         | 69 +++++++------
 mozilla/layout/base/nsPresContext.h           |  6 +-
 mozilla/layout/base/tests/Makefile.in         |  1 +
 mozilla/layout/base/tests/test_bug394057.html | 98 +++++++++++++++++++
 mozilla/layout/style/nsStyleContext.cpp       |  6 +-
 11 files changed, 178 insertions(+), 56 deletions(-)
 create mode 100644 mozilla/layout/base/tests/test_bug394057.html

diff --git a/mozilla/dom/src/base/nsHistory.cpp b/mozilla/dom/src/base/nsHistory.cpp
index c39c480954b..f2b97d9beb6 100644
--- a/mozilla/dom/src/base/nsHistory.cpp
+++ b/mozilla/dom/src/base/nsHistory.cpp
@@ -295,7 +295,7 @@ nsHistory::Go()
       nsPresContext *pcx;
       if (doc && (shell = doc->GetPrimaryShell()) &&
           (pcx = shell->GetPresContext())) {
-        pcx->RebuildAllStyleData();
+        pcx->RebuildAllStyleData(NS_STYLE_HINT_REFLOW);
       }
 
       return NS_OK;
diff --git a/mozilla/dom/src/base/nsLocation.cpp b/mozilla/dom/src/base/nsLocation.cpp
index 8f9758f711b..a27174ec499 100644
--- a/mozilla/dom/src/base/nsLocation.cpp
+++ b/mozilla/dom/src/base/nsLocation.cpp
@@ -892,7 +892,7 @@ nsLocation::Reload()
     nsPresContext *pcx;
     if (doc && (shell = doc->GetPrimaryShell()) &&
         (pcx = shell->GetPresContext())) {
-      pcx->RebuildAllStyleData();
+      pcx->RebuildAllStyleData(NS_STYLE_HINT_REFLOW);
     }
 
     return NS_OK;
diff --git a/mozilla/layout/base/nsCSSFrameConstructor.cpp b/mozilla/layout/base/nsCSSFrameConstructor.cpp
index 3b814039a6c..bde55ee5857 100644
--- a/mozilla/layout/base/nsCSSFrameConstructor.cpp
+++ b/mozilla/layout/base/nsCSSFrameConstructor.cpp
@@ -10047,22 +10047,18 @@ nsCSSFrameConstructor::RestyleElement(nsIContent     *aContent,
                                       nsIFrame       *aPrimaryFrame,
                                       nsChangeHint   aMinHint)
 {
+  NS_ASSERTION(aPrimaryFrame == mPresShell->GetPrimaryFrameFor(aContent),
+               "frame/content mismatch");
+  NS_ASSERTION(!aPrimaryFrame || aPrimaryFrame->GetContent() == aContent,
+               "frame/content mismatch");
+
   if (aMinHint & nsChangeHint_ReconstructFrame) {
     RecreateFramesForContent(aContent);
   } else if (aPrimaryFrame) {
     nsStyleChangeList changeList;
-    if (aMinHint) {
-      changeList.AppendChange(aPrimaryFrame, aContent, aMinHint);
-    }
-    nsChangeHint frameChange = mPresShell->FrameManager()->
+    mPresShell->FrameManager()->
       ComputeStyleChangeFor(aPrimaryFrame, &changeList, aMinHint);
-
-    if (frameChange & nsChangeHint_ReconstructFrame) {
-      RecreateFramesForContent(aContent);
-      changeList.Clear();
-    } else {
-      ProcessRestyledFrames(changeList);
-    }
+    ProcessRestyledFrames(changeList);
   } else {
     // no frames, reconstruct for content
     MaybeRecreateFramesForContent(aContent);
@@ -13182,8 +13178,12 @@ nsCSSFrameConstructor::ProcessOneRestyle(nsIContent* aContent,
 #define RESTYLE_ARRAY_STACKSIZE 128
 
 void
-nsCSSFrameConstructor::RebuildAllStyleData()
+nsCSSFrameConstructor::RebuildAllStyleData(nsChangeHint aExtraHint)
 {
+  NS_ASSERTION(!(aExtraHint & nsChangeHint_ReconstructFrame),
+               "Should not reconstruct the root of the frame tree.  "
+               "Use ReconstructDocElementHierarchy instead.");
+
   mRebuildAllStyleData = PR_FALSE;
 
   if (!mPresShell || !mPresShell->GetRootFrame())
@@ -13202,8 +13202,10 @@ nsCSSFrameConstructor::RebuildAllStyleData()
   // (but note that nsPresShell::SetPreferenceStyleRules currently depends
   // on us re-running rule matching here
   nsStyleChangeList changeList;
+  // XXX Does it matter that we're passing aExtraHint to the real root
+  // frame and not the root node's primary frame?
   mPresShell->FrameManager()->ComputeStyleChangeFor(mPresShell->GetRootFrame(),
-                                                    &changeList, nsChangeHint(0));
+                                                    &changeList, aExtraHint);
   // Process the required changes
   ProcessRestyledFrames(changeList);
   // Tell the style set it's safe to destroy the old rule tree.  We
@@ -13264,7 +13266,7 @@ nsCSSFrameConstructor::ProcessPendingRestyles()
   if (mRebuildAllStyleData) {
     // We probably wasted a lot of work up above, but this seems safest
     // and it should be rarely used.
-    RebuildAllStyleData();
+    RebuildAllStyleData(nsChangeHint(0));
   }
 }
 
diff --git a/mozilla/layout/base/nsCSSFrameConstructor.h b/mozilla/layout/base/nsCSSFrameConstructor.h
index 671537e1c16..0a025e2bf75 100644
--- a/mozilla/layout/base/nsCSSFrameConstructor.h
+++ b/mozilla/layout/base/nsCSSFrameConstructor.h
@@ -178,7 +178,7 @@ public:
   // itself.
   void ProcessPendingRestyles();
   
-  void RebuildAllStyleData();
+  void RebuildAllStyleData(nsChangeHint aExtraHint);
 
   void PostRestyleEvent(nsIContent* aContent, nsReStyleHint aRestyleHint,
                         nsChangeHint aMinChangeHint);
diff --git a/mozilla/layout/base/nsFrameManager.cpp b/mozilla/layout/base/nsFrameManager.cpp
index 131df83d6b0..42c7c58430f 100644
--- a/mozilla/layout/base/nsFrameManager.cpp
+++ b/mozilla/layout/base/nsFrameManager.cpp
@@ -1439,11 +1439,15 @@ nsFrameManager::ReResolveStyleContext(nsPresContext    *aPresContext,
   return aMinChange;
 }
 
-nsChangeHint
+void
 nsFrameManager::ComputeStyleChangeFor(nsIFrame          *aFrame, 
                                       nsStyleChangeList *aChangeList,
                                       nsChangeHint       aMinChange)
 {
+  if (aMinChange) {
+    aChangeList->AppendChange(aFrame, aFrame->GetContent(), aMinChange);
+  }
+
   nsChangeHint topLevelChange = aMinChange;
 
   nsIFrame* frame = aFrame;
@@ -1472,7 +1476,7 @@ nsFrameManager::ComputeStyleChangeFor(nsIFrame          *aFrame,
         // clobbered by the frame reconstruct anyway.
         NS_ASSERTION(!frame->GetPrevContinuation(),
                      "continuing frame had more severe impact than first-in-flow");
-        return topLevelChange;
+        return;
       }
 
       frame = frame->GetNextContinuation();
@@ -1481,14 +1485,13 @@ nsFrameManager::ComputeStyleChangeFor(nsIFrame          *aFrame,
     // Might we have special siblings?
     if (!(frame2->GetStateBits() & NS_FRAME_IS_SPECIAL)) {
       // nothing more to do here
-      return topLevelChange;
+      return;
     }
     
     frame2 = static_cast<nsIFrame*>
                         (propTable->GetProperty(frame2, nsGkAtoms::IBSplitSpecialSibling));
     frame = frame2;
   } while (frame2);
-  return topLevelChange;
 }
 
 
diff --git a/mozilla/layout/base/nsFrameManager.h b/mozilla/layout/base/nsFrameManager.h
index 31cd2cc2688..1fa82778b5e 100644
--- a/mozilla/layout/base/nsFrameManager.h
+++ b/mozilla/layout/base/nsFrameManager.h
@@ -181,10 +181,11 @@ public:
   NS_HIDDEN_(nsresult) ReParentStyleContext(nsIFrame* aFrame);
 
   /*
-   * Re-resolve the style contexts for a frame tree.  Returns the top-level
-   * change hint resulting from the style re-resolution.
+   * Re-resolve the style contexts for a frame tree, building
+   * aChangeList based on the resulting style changes, plus aMinChange
+   * applied to aFrame.
    */
-  NS_HIDDEN_(nsChangeHint)
+  NS_HIDDEN_(void)
     ComputeStyleChangeFor(nsIFrame* aFrame,
                           nsStyleChangeList* aChangeList,
                           nsChangeHint aMinChange);
diff --git a/mozilla/layout/base/nsPresContext.cpp b/mozilla/layout/base/nsPresContext.cpp
index 869d7278579..ba3b3079bdd 100644
--- a/mozilla/layout/base/nsPresContext.cpp
+++ b/mozilla/layout/base/nsPresContext.cpp
@@ -584,6 +584,10 @@ nsPresContext::GetUserPreferences()
     nsContentUtils::GetIntPref("browser.display.base_font_scaler",
                                mFontScaler);
 
+
+  mAutoQualityMinFontSizePixelsPref =
+    nsContentUtils::GetIntPref("browser.display.auto_quality_min_font_size");
+
   // * document colors
   GetDocumentColorPreferences();
 
@@ -661,6 +665,8 @@ nsPresContext::GetUserPreferences()
     mImageAnimationModePref = imgIContainer::kDontAnimMode;
   else if (animatePref.Equals("once"))
     mImageAnimationModePref = imgIContainer::kLoopOnceAnimMode;
+  else // dynamic change to invalid value should act like it does initially
+    mImageAnimationModePref = imgIContainer::kNormalAnimMode;
 
   PRUint32 bidiOptions = GetBidi();
 
@@ -701,12 +707,11 @@ nsPresContext::GetUserPreferences()
   SetBidi(bidiOptions, PR_FALSE);
 }
 
-static const char sMinFontSizePref[] = "browser.display.auto_quality_min_font_size";
-
 void
 nsPresContext::PreferenceChanged(const char* aPrefName)
 {
-  if (!nsCRT::strcmp(aPrefName, "layout.css.dpi")) {
+  nsDependentCString prefName(aPrefName);
+  if (prefName.EqualsLiteral("layout.css.dpi")) {
     PRInt32 oldAppUnitsPerDevPixel = AppUnitsPerDevPixel();
     if (mDeviceContext->CheckDPIChange() && mShell) {
       mDeviceContext->FlushFontCache();
@@ -723,14 +728,20 @@ nsPresContext::PreferenceChanged(const char* aPrefName)
       nscoord height = NSToCoordRound(oldHeightDevPixels*AppUnitsPerDevPixel());
       vm->SetWindowDimensions(width, height);
 
-      RebuildAllStyleData();
+      RebuildAllStyleData(NS_STYLE_HINT_REFLOW);
     }
     return;
   }
-  if (!nsCRT::strcmp(aPrefName, sMinFontSizePref)) {
-    mAutoQualityMinFontSizePixelsPref = nsContentUtils::GetIntPref(sMinFontSizePref);
-    RebuildAllStyleData();
-    return;
+  if (StringBeginsWith(prefName, NS_LITERAL_CSTRING("font."))) {
+    // Changes to font family preferences don't change anything in the
+    // computed style data, so the style system won't generate a reflow
+    // hint for us.  We need to do that manually.
+
+    // FIXME We could probably also handle changes to
+    // browser.display.auto_quality_min_font_size here, but that
+    // probably also requires clearing the text run cache, so don't
+    // bother (yet, anyway).
+    mPrefChangePendingNeedsReflow = PR_TRUE;
   }
   // we use a zero-delay timer to coalesce multiple pref updates
   if (!mPrefChangedTimer)
@@ -764,7 +775,14 @@ nsPresContext::UpdateAfterPreferencesChanged()
   }
 
   mDeviceContext->FlushFontCache();
-  RebuildAllStyleData();
+
+  nsChangeHint hint = nsChangeHint(0);
+
+  if (mPrefChangePendingNeedsReflow) {
+    NS_UpdateHint(hint, NS_STYLE_HINT_REFLOW);
+  }
+
+  RebuildAllStyleData(hint);
 }
 
 nsresult
@@ -829,9 +847,6 @@ nsPresContext::Init(nsIDeviceContext* aDeviceContext)
                                        nsPresContext::PrefChangedCallback,
                                        this);
 
-  // This is observed thanks to the browser.display. observer above.
-  mAutoQualityMinFontSizePixelsPref = nsContentUtils::GetIntPref(sMinFontSizePref);
-
   rv = mEventManager->Init();
   NS_ENSURE_SUCCESS(rv, rv);
 
@@ -950,7 +965,7 @@ nsPresContext::Observe(nsISupports* aSubject,
   if (!nsCRT::strcmp(aTopic, "charset")) {
     UpdateCharSet(NS_LossyConvertUTF16toASCII(aData));
     mDeviceContext->FlushFontCache();
-    RebuildAllStyleData();
+    RebuildAllStyleData(NS_STYLE_HINT_REFLOW);
     return NS_OK;
   }
 
@@ -1145,7 +1160,7 @@ nsPresContext::SetFullZoom(float aZoom)
   mFullZoom = aZoom;
   GetViewManager()->SetWindowDimensions(NSToCoordRound(oldWidthDevPixels * AppUnitsPerDevPixel()),
                                         NSToCoordRound(oldHeightDevPixels * AppUnitsPerDevPixel()));
-  RebuildAllStyleData();
+  RebuildAllStyleData(NS_STYLE_HINT_REFLOW);
 
   mSupressResizeReflow = PR_FALSE;
 
@@ -1286,7 +1301,7 @@ nsPresContext::SetBidi(PRUint32 aSource, PRBool aForceRestyle)
     }
   }
   if (aForceRestyle) {
-    RebuildAllStyleData();
+    RebuildAllStyleData(NS_STYLE_HINT_REFLOW);
   }
 }
 
@@ -1343,11 +1358,11 @@ nsPresContext::ThemeChangedInternal()
     sLookAndFeelChanged = PR_FALSE;
   }
 
-  // We have to clear style data because the assumption of style rule
-  // immutability has been violated since any style rule that uses
-  // system colors or fonts (and probably -moz-appearance as well) has
-  // changed.
-  RebuildAllStyleData();
+  // Changes in theme can change system colors (whose changes are
+  // properly reflected in computed style data), system fonts (whose
+  // changes are not), and -moz-appearance (whose changes likewise are
+  // not), so we need to reflow.
+  RebuildAllStyleData(NS_STYLE_HINT_REFLOW);
 }
 
 void
@@ -1379,23 +1394,19 @@ nsPresContext::SysColorChangedInternal()
   // they may be using system colors
   GetDocumentColorPreferences();
 
-  // We need to do a full reflow (and view update) here. Clearing the style
-  // data without reflowing/updating views will lead to incorrect change hints
-  // later, because when generating change hints, any style structs which have
-  // been cleared and not reread are assumed to not be used at all.
-  // XXXroc not sure what to make of the above comment, because we don't reflow
-  // synchronously here
-  RebuildAllStyleData();
+  // The system color values are computed to colors in the style data,
+  // so normal style data comparison is sufficient here.
+  RebuildAllStyleData(nsChangeHint(0));
 }
 
 void
-nsPresContext::RebuildAllStyleData()
+nsPresContext::RebuildAllStyleData(nsChangeHint aExtraHint)
 {
   if (!mShell) {
     // We must have been torn down. Nothing to do here.
     return;
   }
-  mShell->FrameConstructor()->RebuildAllStyleData();
+  mShell->FrameConstructor()->RebuildAllStyleData(aExtraHint);
 }
 
 void
diff --git a/mozilla/layout/base/nsPresContext.h b/mozilla/layout/base/nsPresContext.h
index 3279802ee9f..6beaa616e7c 100644
--- a/mozilla/layout/base/nsPresContext.h
+++ b/mozilla/layout/base/nsPresContext.h
@@ -61,6 +61,7 @@
 #include "nsIDocument.h"
 #include "nsInterfaceHashtable.h"
 #include "nsCycleCollectionParticipant.h"
+#include "nsChangeHint.h"
 // XXX we need only gfxTypes.h, but we cannot include it directly.
 #include "gfxPoint.h"
 class nsImageLoader;
@@ -200,7 +201,7 @@ public:
     { return GetPresShell()->FrameManager(); } 
 #endif
 
-  void RebuildAllStyleData();
+  void RebuildAllStyleData(nsChangeHint aExtraHint);
   void PostRebuildAllStyleDataEvent();
 
   /**
@@ -465,7 +466,7 @@ public:
   float TextZoom() { return mTextZoom; }
   void SetTextZoom(float aZoom) {
     mTextZoom = aZoom;
-    RebuildAllStyleData();
+    RebuildAllStyleData(NS_STYLE_HINT_REFLOW);
   }
 
   float GetFullZoom() { return mFullZoom; }
@@ -849,6 +850,7 @@ protected:
   unsigned              mPrefScrollbarSide : 2;
   unsigned              mPendingSysColorChanged : 1;
   unsigned              mPendingThemeChanged : 1;
+  unsigned              mPrefChangePendingNeedsReflow : 1;
   unsigned              mRenderedPositionVaryingContent : 1;
 
   // resize reflow is supressed when the only change has been to zoom
diff --git a/mozilla/layout/base/tests/Makefile.in b/mozilla/layout/base/tests/Makefile.in
index c69dca7a566..eee5d0dacb4 100644
--- a/mozilla/layout/base/tests/Makefile.in
+++ b/mozilla/layout/base/tests/Makefile.in
@@ -84,6 +84,7 @@ _TEST_FILES =	\
 		test_bug66619.html \
 		test_bug386575.xhtml \
 		test_bug388019.html \
+		test_bug394057.html \
 		test_bug396024.html \
 		test_bug399284.html \
 		test_bug399951.html \
diff --git a/mozilla/layout/base/tests/test_bug394057.html b/mozilla/layout/base/tests/test_bug394057.html
new file mode 100644
index 00000000000..5c0fce0cdbe
--- /dev/null
+++ b/mozilla/layout/base/tests/test_bug394057.html
@@ -0,0 +1,98 @@
+<!DOCTYPE HTML>
+<html lang="en">
+<!--
+https://bugzilla.mozilla.org/show_bug.cgi?id=394057
+-->
+<head>
+  <title>Test for Bug 394057</title>
+  <script type="text/javascript" src="/MochiKit/MochiKit.js"></script>
+  <script type="text/javascript" src="/tests/SimpleTest/SimpleTest.js"></script>
+  <link rel="stylesheet" type="text/css" href="/tests/SimpleTest/test.css" />
+  <style type="text/css">
+
+  #display { background: yellow; color: black; font-family: serif; }
+
+  </style>
+</head>
+<body>
+<a target="_blank" href="https://bugzilla.mozilla.org/show_bug.cgi?id=394057">Mozilla Bug 394057</a>
+<table id="display"><tr><td>MmMmMm...iiiIIIlll---</td></tr></table>
+<div id="content" style="display: none">
+  
+</div>
+<pre id="test">
+<script class="testbody" type="text/javascript">
+
+/** Test for Bug 394057 **/
+
+SimpleTest.waitForExplicitFinish();
+netscape.security.PrivilegeManager.enablePrivilege('UniversalXPConnect');
+
+var tableElement = document.getElementById("display");
+
+var CC = Components.classes;
+var CI = Components.interfaces;
+
+var fe =
+  CC["@mozilla.org/gfx/fontenumerator;1"].createInstance(CI.nsIFontEnumerator);
+var serifFonts = fe.EnumerateFonts("x-western", "serif", {});
+var monospaceFonts = fe.EnumerateFonts("x-western", "monospace", {});
+
+function table_width_for_font(font) {
+  tableElement.style.fontFamily = '"' + font + '"';
+  var result = tableElement.offsetWidth;
+  tableElement.style.fontFamily = "";
+  return result;
+}
+
+var serifIdx = 0;
+var monospaceIdx = 0;
+var monospaceWidth, serifWidth;
+monospaceWidth = table_width_for_font(monospaceFonts[monospaceIdx]);
+for (serifIdx in serifFonts) {
+  serifWidth = table_width_for_font(serifFonts[serifIdx]);
+  if (serifWidth != monospaceWidth)
+    break;
+}
+if (serifWidth == monospaceWidth) {
+  for (monospaceIdx in monospaceFonts) {
+    monospaceWidth = table_width_for_font(monospaceFonts[monospaceIdx]);
+    if (serifWidth != monospaceWidth)
+      break;
+  }
+}
+
+isnot(serifWidth, monospaceWidth,
+      "can't find serif and monospace fonts of different width");
+
+var prefs =
+  CC["@mozilla.org/preferences-service;1"].getService(CI.nsIPrefBranch);
+prefs.setCharPref('font.name.serif.x-western', serifFonts[serifIdx]);
+
+var serifWidthFromPref;
+setTimeout(setTimeout, 0, step2, 0);
+function step2() {
+    netscape.security.PrivilegeManager.enablePrivilege('UniversalXPConnect');
+    serifWidthFromPref = tableElement.offsetWidth;
+    prefs.setCharPref('font.name.serif.x-western',
+                        monospaceFonts[monospaceIdx]);
+    setTimeout(setTimeout, 0, step3, 0);
+}
+var monospaceWidthFromPref;
+function step3() {
+    netscape.security.PrivilegeManager.enablePrivilege('UniversalXPConnect');
+    monospaceWidthFromPref = tableElement.offsetWidth;
+    prefs.clearUserPref('font.name.serif.x-western');
+
+    is(serifWidthFromPref, serifWidth,
+       "changing font pref should change width of table (serif)");
+    is(monospaceWidthFromPref, monospaceWidth,
+       "changing font pref should change width of table (monospace)");
+    SimpleTest.finish();
+}
+
+</script>
+</pre>
+</body>
+</html>
+
diff --git a/mozilla/layout/style/nsStyleContext.cpp b/mozilla/layout/style/nsStyleContext.cpp
index 6d22cc49f49..f06095fd1fb 100644
--- a/mozilla/layout/style/nsStyleContext.cpp
+++ b/mozilla/layout/style/nsStyleContext.cpp
@@ -391,7 +391,11 @@ nsStyleContext::CalcStyleDifference(nsStyleContext* aOther)
   // We must always ensure that we populate the structs on the new style
   // context that are filled in on the old context, so that if we get
   // two style changes in succession, the second of which causes a real
-  // style change, the PeekStyleData doesn't fail.
+  // style change, the PeekStyleData doesn't return null (implying that
+  // nobody ever looked at that struct's data).  In other words, we
+  // can't skip later structs if we get a big change up front, because
+  // we could later get a small change in one of those structs that we
+  // don't want to miss.
 
   // If our rule nodes are the same, then we are looking at the same
   // style data.  We know this because CalcStyleDifference is always