Bug 1368589: handle the case where a RebuildAllStyleData event is posted and stylesheets are added at the same time. r=heycam
authorEmilio Cobos Álvarez <emilio@crisal.io>
Tue, 30 May 2017 02:46:33 +0200
changeset 409414 c6fb400017f150d652c0b2b01f8d619f58c82931
parent 409413 0c4c63d70d4290205431d3ac7f23ce7e12749b66
child 409415 6909c7487def14b559d00c5181c403e36d09ee62
push id7391
push usermtabara@mozilla.com
push dateMon, 12 Jun 2017 13:08:53 +0000
treeherdermozilla-beta@2191d7f87e2e [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersheycam
bugs1368589
milestone55.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 1368589: handle the case where a RebuildAllStyleData event is posted and stylesheets are added at the same time. r=heycam MozReview-Commit-ID: IWiTCCo55cg
image/test/reftest/jpeg/reftest.list
layout/style/ServoStyleSet.cpp
layout/style/ServoStyleSet.h
--- a/image/test/reftest/jpeg/reftest.list
+++ b/image/test/reftest/jpeg/reftest.list
@@ -46,9 +46,9 @@ random-if(Android) == jpg-srgb-icc.jpg j
 # \r\n
 # <contents of blue.jpg> (no newline)
 # --BOUNDARYOMG--\r\n
 # 
 # (The boundary is arbitrary, and just has to be defined as something that
 # won't be in the text of the contents themselves. --$(boundary)\r\n means
 # "Here is the beginning of a boundary," and --$(boundary)-- means "All done
 # sending you parts.")
-random-if(stylo||styloVsGecko) HTTP == webcam-simulacrum.mjpg blue.jpg # bug 1368589 for stylo
+HTTP == webcam-simulacrum.mjpg blue.jpg
--- a/layout/style/ServoStyleSet.cpp
+++ b/layout/style/ServoStyleSet.cpp
@@ -1088,25 +1088,24 @@ ServoStyleSet::EnsureUniqueInnerOnCSSShe
   return res;
 }
 
 void
 ServoStyleSet::RebuildData()
 {
   ClearNonInheritingStyleContexts();
   Servo_StyleSet_RebuildData(mRawSet.get());
-  mStylistState = StylistState::NotDirty;
 }
 
 void
 ServoStyleSet::ClearDataAndMarkDeviceDirty()
 {
   ClearNonInheritingStyleContexts();
   Servo_StyleSet_Clear(mRawSet.get());
-  mStylistState = StylistState::FullyDirty;
+  mStylistState |= StylistState::FullyDirty;
 }
 
 already_AddRefed<ServoComputedValues>
 ServoStyleSet::ResolveServoStyle(Element* aElement)
 {
   UpdateStylistIfNeeded();
   return Servo_ResolveStyle(aElement, mRawSet.get(),
                             mAllowResolveStaleStyles).Consume();
@@ -1198,19 +1197,38 @@ ServoStyleSet::ResolveForDeclarations(
                                                aParentOrNull,
                                                aDeclarations).Consume();
 }
 
 void
 ServoStyleSet::UpdateStylist()
 {
   MOZ_ASSERT(StylistNeedsUpdate());
-  if (mStylistState == StylistState::FullyDirty) {
+  if (mStylistState & StylistState::FullyDirty) {
     RebuildData();
+
+    if (mStylistState & StylistState::StyleSheetsDirty) {
+      // Normally, whoever was in charge of posting a RebuildAllStyleDataEvent,
+      // would also be in charge of posting a restyle/change hint according to
+      // it.
+      //
+      // However, other stylesheets may have been added to the document in the
+      // same period, so when both bits are set, we need to do a full subtree
+      // update, because we can no longer reason about the state of the style
+      // data.
+      //
+      // We could not clear the invalidations when rebuilding the data and
+      // process them here... But it's not clear if that complexity is worth
+      // to handle this edge case more efficiently.
+      if (Element* root = mPresContext->Document()->GetDocumentElement()) {
+        Servo_NoteExplicitHints(root, eRestyle_Subtree, nsChangeHint(0));
+      }
+    }
   } else {
+    MOZ_ASSERT(mStylistState & StylistState::StyleSheetsDirty);
     Element* root = mPresContext->Document()->GetDocumentElement();
     Servo_StyleSet_FlushStyleSheets(mRawSet.get(), root);
   }
   mStylistState = StylistState::NotDirty;
 }
 
 void
 ServoStyleSet::PrependSheetOfType(SheetType aType,
--- a/layout/style/ServoStyleSet.h
+++ b/layout/style/ServoStyleSet.h
@@ -42,16 +42,36 @@ class nsStyleContext;
 class nsPresContext;
 struct nsTimingFunction;
 struct RawServoRuleNode;
 struct TreeMatchContext;
 
 namespace mozilla {
 
 /**
+ * A few flags used to track which kind of stylist state we may need to
+ * update.
+ */
+enum class StylistState : uint8_t {
+  /** The stylist is not dirty, we should do nothing */
+  NotDirty = 0,
+
+  /** The style sheets have changed, so we need to update the style data. */
+  StyleSheetsDirty = 1 << 0,
+
+  /**
+   * All style data is dirty and both style sheet data and default computed
+   * values need to be recomputed.
+   */
+  FullyDirty = 1 << 1,
+};
+
+MOZ_MAKE_ENUM_CLASS_BITWISE_OPERATORS(StylistState)
+
+/**
  * The set of style sheets that apply to a document, backed by a Servo
  * Stylist.  A ServoStyleSet contains ServoStyleSheets.
  */
 class ServoStyleSet
 {
   friend class ServoRestyleManager;
   typedef ServoElementSnapshotTable SnapshotTable;
 
@@ -467,39 +487,21 @@ private:
    */
   void PreTraverse(dom::Element* aRoot = nullptr,
                    EffectCompositor::AnimationRestyleType =
                      EffectCompositor::AnimationRestyleType::Throttled);
   // Subset of the pre-traverse steps that involve syncing up data
   void PreTraverseSync();
 
   /**
-   * A tri-state used to track which kind of stylist state we may need to
-   * update.
-   */
-  enum class StylistState : uint8_t {
-    /** The stylist is not dirty, we should do nothing */
-    NotDirty,
-    /** The style sheets have changed, so we need to update the style data. */
-    StyleSheetsDirty,
-    /**
-     * All style data is dirty and both style sheet data and default computed
-     * values need to be recomputed.
-     */
-    FullyDirty,
-  };
-
-  /**
    * Note that the stylist needs a style flush due to style sheet changes.
    */
   void SetStylistStyleSheetsDirty()
   {
-    if (mStylistState == StylistState::NotDirty) {
-      mStylistState = StylistState::StyleSheetsDirty;
-    }
+    mStylistState |= StylistState::StyleSheetsDirty;
   }
 
   bool StylistNeedsUpdate() const
   {
     return mStylistState != StylistState::NotDirty;
   }
 
   /**