From 4938f0925aa1cb5513b3a7a0794ec57a23a57722 Mon Sep 17 00:00:00 2001
From: "L. David Baron" <dbaron@dbaron.org>
Date: Fri, 5 Dec 2014 11:37:39 -0800
Subject: [PATCH] Bug 1089417 patch 8 - Only drop MediumFeaturesChanged on the
 floor if we've never computed style before, rather than never computed style
 using this rule processor.  r=heycam

This depends on patches 4 and 7.
---
 layout/style/nsCSSRuleProcessor.cpp    | 34 +++++++++++++++++++++-----
 layout/style/test/test_bug1089417.html |  2 +-
 2 files changed, 29 insertions(+), 7 deletions(-)

diff --git a/layout/style/nsCSSRuleProcessor.cpp b/layout/style/nsCSSRuleProcessor.cpp
index 364cd32ef968..621cc9de4788 100644
--- a/layout/style/nsCSSRuleProcessor.cpp
+++ b/layout/style/nsCSSRuleProcessor.cpp
@@ -2917,17 +2917,39 @@ nsCSSRuleProcessor::HasAttributeDependentStyle(AttributeRuleProcessorData* aData
 /* virtual */ bool
 nsCSSRuleProcessor::MediumFeaturesChanged(nsPresContext* aPresContext)
 {
-  RuleCascadeData *old = mRuleCascades;
   // We don't want to do anything if there aren't any sets of rules
-  // cached yet (or somebody cleared them and is thus responsible for
-  // rebuilding things), since we should not build the rule cascade too
-  // early (e.g., before we know whether the quirk style sheet should be
+  // cached yet, since we should not build the rule cascade too early
+  // (e.g., before we know whether the quirk style sheet should be
   // enabled).  And if there's nothing cached, it doesn't matter if
-  // anything changed.  See bug 448281.
+  // anything changed.  But in the cases where it does matter, we've
+  // cached a previous cache key to test against, instead of our current
+  // rule cascades.  See bug 448281 and bug 1089417.
+  MOZ_ASSERT(!(mRuleCascades && mPreviousCacheKey));
+  RuleCascadeData *old = mRuleCascades;
   if (old) {
     RefreshRuleCascade(aPresContext);
+    return (old != mRuleCascades);
   }
-  return (old != mRuleCascades);
+
+  if (mPreviousCacheKey) {
+    // RefreshRuleCascade will get rid of mPreviousCacheKey anyway to
+    // maintain the invariant that we can't have both an mRuleCascades
+    // and an mPreviousCacheKey.  But we need to hold it a little
+    // longer.
+    UniquePtr<nsMediaQueryResultCacheKey> previousCacheKey(
+      Move(mPreviousCacheKey));
+    RefreshRuleCascade(aPresContext);
+
+    // This test is a bit pessimistic since the cache key's operator==
+    // just does list comparison rather than set comparison, but it
+    // should catch all the cases we care about (i.e., where the cascade
+    // order hasn't changed).  Other cases will do a restyle anyway, so
+    // we shouldn't need to worry about posting a second.
+    return !mRuleCascades || // all sheets gone, but we had sheets before
+           mRuleCascades->mCacheKey != *previousCacheKey;
+  }
+
+  return false;
 }
 
 UniquePtr<nsMediaQueryResultCacheKey>
diff --git a/layout/style/test/test_bug1089417.html b/layout/style/test/test_bug1089417.html
index 20ead99b1937..ee4ac7b55cc8 100644
--- a/layout/style/test/test_bug1089417.html
+++ b/layout/style/test/test_bug1089417.html
@@ -21,7 +21,7 @@ https://bugzilla.mozilla.org/show_bug.cgi?id=1089417
 
     f.height = "400";
     fdoc.getElementById("s").disabled = false;
-    todo_is(fwin.getComputedStyle(fdoc.documentElement).backgroundColor,
+    is(fwin.getComputedStyle(fdoc.documentElement).backgroundColor,
        "rgb(0, 128, 0)",
        "media query change should have restyled");
     SimpleTest.finish();