Bug 1299348 - Remove StylingStarted(). r=emilio
authorBobby Holley <bobbyholley@gmail.com>
Tue, 30 Aug 2016 18:06:47 -0700
changeset 312053 3f6b84d701ae0df834dfa6efbd56a9d915026998
parent 312052 db6ce29aa0bbebc1de94bc1041c151c7e851bd43
child 312054 bc4801b96009f6dfd51deb4ef87347ccd813776c
push id81271
push userbholley@mozilla.com
push dateWed, 31 Aug 2016 17:33:35 +0000
treeherdermozilla-inbound@3f6b84d701ae [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersemilio
bugs1299348
milestone51.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 1299348 - Remove StylingStarted(). r=emilio StylingStarted is a kind of nebulous and not-very-useful concept. The concept that _is_ useful is whether the presshell has been initialized or not, but the root element may not exist at that point. So we need to make sure we that we can trigger the initial document style in both presshell initialized _and_ ContentInserted, which has the nice effect of handling root element reinsertions. We also take the opportunity to make StyleDocument assert the existence of a root element, and align the responsibility for clearing the dirty descendant bits between document and non-document nodes.
layout/base/ServoRestyleManager.cpp
layout/base/nsPresShell.cpp
layout/style/ServoStyleSet.cpp
layout/style/ServoStyleSet.h
--- a/layout/base/ServoRestyleManager.cpp
+++ b/layout/base/ServoRestyleManager.cpp
@@ -275,27 +275,31 @@ ServoRestyleManager::NoteRestyleHint(Ele
   }
 }
 
 void
 ServoRestyleManager::ProcessPendingRestyles()
 {
   MOZ_ASSERT(PresContext()->Document(), "No document?  Pshaw!");
   MOZ_ASSERT(!nsContentUtils::IsSafeToRunScript(), "Missing a script blocker!");
+
+  if (MOZ_UNLIKELY(!PresContext()->PresShell()->DidInitialize())) {
+    // PresShell::FlushPendingNotifications doesn't early-return in the case
+    // where the PreShell hasn't yet been initialized (and therefore we haven't
+    // yet done the initial style traversal of the DOM tree). We should arguably
+    // fix up the callers and assert against this case, but we just detect and
+    // handle it for now.
+    return;
+  }
+
   if (!HasPendingRestyles()) {
     return;
   }
 
   ServoStyleSet* styleSet = StyleSet();
-  if (!styleSet->StylingStarted()) {
-    // If something caused us to restyle, and we haven't started styling yet,
-    // do nothing. Everything is dirty, and we'll style it all later.
-    return;
-  }
-
   nsIDocument* doc = PresContext()->Document();
   Element* root = doc->GetRootElement();
   if (root) {
     for (auto iter = mModifiedElements.Iter(); !iter.Done(); iter.Next()) {
       ServoElementSnapshot* snapshot = iter.UserData();
       Element* element = iter.Key();
 
       // TODO: avoid the ComputeRestyleHint call if we already have the highest
@@ -326,17 +330,17 @@ ServoRestyleManager::ProcessPendingResty
       RecreateStyleContexts(root, nullptr, styleSet, changeList);
       ProcessRestyledFrames(changeList);
 
       mInStyleRefresh = false;
     }
   }
 
   MOZ_ASSERT(!doc->IsDirtyForServo());
-  MOZ_ASSERT(!doc->HasDirtyDescendantsForServo());
+  doc->UnsetHasDirtyDescendantsForServo();
 
   mModifiedElements.Clear();
 
   IncrementRestyleGeneration();
 }
 
 void
 ServoRestyleManager::RestyleForInsertOrChange(nsINode* aContainer,
@@ -349,16 +353,30 @@ ServoRestyleManager::RestyleForInsertOrC
   //
   // Bug 1297899 tracks this work.
   //
 }
 
 void
 ServoRestyleManager::ContentInserted(nsINode* aContainer, nsIContent* aChild)
 {
+  if (aContainer == aContainer->OwnerDoc()) {
+    // If we're getting this notification for the insertion of a root element,
+    // that means either:
+    //   (a) We initialized the PresShell before the root element existed, or
+    //   (b) The root element was removed and it or another root is being
+    //       inserted.
+    //
+    // Either way the whole tree is dirty, so we should style the document.
+    MOZ_ASSERT(aChild == aChild->OwnerDoc()->GetRootElement());
+    MOZ_ASSERT(aChild->IsDirtyForServo());
+    StyleSet()->StyleDocument(/* aLeaveDirtyBits = */ false);
+    return;
+  }
+
   if (!aContainer->ServoData().get()) {
     // This can happen with display:none. Bug 1297249 tracks more investigation
     // and assertions here.
     return;
   }
 
   // Style the new subtree because we will most likely need it during subsequent
   // frame construction. Bug 1298281 tracks deferring this work in the lazy
--- a/layout/base/nsPresShell.cpp
+++ b/layout/base/nsPresShell.cpp
@@ -1677,18 +1677,25 @@ PresShell::Initialize(nscoord aWidth, ns
   }
 #endif
 
   // XXX Do a full invalidate at the beginning so that invalidates along
   // the way don't have region accumulation issues?
 
   mPresContext->SetVisibleArea(nsRect(0, 0, aWidth, aHeight));
 
-  if (mStyleSet->IsServo()) {
-    mStyleSet->AsServo()->StartStyling(GetPresContext());
+  if (mStyleSet->IsServo() && mDocument->GetRootElement()) {
+    // If we have the root element already, go ahead style it along with any
+    // descendants.
+    //
+    // Some things, like nsDocumentViewer::GetPageMode, recreate the PresShell
+    // while keeping the content tree alive (see bug 1292280) - so we
+    // unconditionally mark the root as dirty.
+    mDocument->GetRootElement()->SetIsDirtyForServo();
+    mStyleSet->AsServo()->StyleDocument(/* aLeaveDirtyBits = */ false);
   }
 
   // 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?");
--- a/layout/style/ServoStyleSet.cpp
+++ b/layout/style/ServoStyleSet.cpp
@@ -17,17 +17,16 @@
 
 using namespace mozilla;
 using namespace mozilla::dom;
 
 ServoStyleSet::ServoStyleSet()
   : mPresContext(nullptr)
   , mRawSet(Servo_StyleSet_Init())
   , mBatching(0)
-  , mStylingStarted(false)
 {
 }
 
 void
 ServoStyleSet::Init(nsPresContext* aPresContext)
 {
   mPresContext = aPresContext;
 }
@@ -68,34 +67,16 @@ ServoStyleSet::EndUpdate()
   if (--mBatching > 0) {
     return NS_OK;
   }
 
   // ... do something ...
   return NS_OK;
 }
 
-void
-ServoStyleSet::StartStyling(nsPresContext* aPresContext)
-{
-  MOZ_ASSERT(!mStylingStarted);
-
-  // Some things, like nsDocumentViewer::GetPageMode, recreate the presShell,
-  // while keeping the content tree alive. See bug 1292280.
-  //
-  // That's why we need to force a restyle.
-  nsIContent* root = mPresContext->Document()->GetRootElement();
-  if (root) {
-    root->SetIsDirtyForServo();
-  }
-
-  StyleDocument(/* aLeaveDirtyBits = */ false);
-  mStylingStarted = true;
-}
-
 already_AddRefed<nsStyleContext>
 ServoStyleSet::ResolveStyleFor(Element* aElement,
                                nsStyleContext* aParentContext)
 {
   return GetContext(aElement, aParentContext, nullptr,
                     CSSPseudoElementType::NotPseudo);
 }
 
@@ -479,31 +460,26 @@ ClearDirtyBits(nsIContent* aContent)
   for (nsIContent* n = it.GetNextChild(); n; n = it.GetNextChild()) {
     ClearDirtyBits(n);
   }
 }
 
 void
 ServoStyleSet::StyleDocument(bool aLeaveDirtyBits)
 {
-  // Unconditionally clear the flag on the document so that HasPendingRestyles
-  // returns false.
+  // Grab the root.
   nsIDocument* doc = mPresContext->Document();
-  doc->UnsetHasDirtyDescendantsForServo();
-
-  // Grab the root.
-  nsIContent* root = mPresContext->Document()->GetRootElement();
-  if (!root) {
-    return;
-  }
+  nsIContent* root = doc->GetRootElement();
+  MOZ_ASSERT(root);
 
   // Restyle the document, clearing the dirty bits if requested.
   Servo_RestyleSubtree(root, mRawSet.get());
   if (!aLeaveDirtyBits) {
     ClearDirtyBits(root);
+    doc->UnsetHasDirtyDescendantsForServo();
   }
 }
 
 void
 ServoStyleSet::StyleNewSubtree(nsIContent* aContent)
 {
   MOZ_ASSERT(aContent->IsDirtyForServo());
   Servo_RestyleSubtree(aContent, mRawSet.get());
--- a/layout/style/ServoStyleSet.h
+++ b/layout/style/ServoStyleSet.h
@@ -50,18 +50,16 @@ public:
   void Shutdown();
 
   bool GetAuthorStyleDisabled() const;
   nsresult SetAuthorStyleDisabled(bool aStyleDisabled);
 
   void BeginUpdate();
   nsresult EndUpdate();
 
-  void StartStyling(nsPresContext* aPresContext);
-
   already_AddRefed<nsStyleContext>
   ResolveStyleFor(dom::Element* aElement,
                   nsStyleContext* aParentContext);
 
   already_AddRefed<nsStyleContext>
   ResolveStyleFor(dom::Element* aElement,
                   nsStyleContext* aParentContext,
                   TreeMatchContext& aTreeMatchContext);
@@ -123,17 +121,17 @@ public:
   /**
    * Computes a restyle hint given a element and a previous element snapshot.
    */
   nsRestyleHint ComputeRestyleHint(dom::Element* aElement,
                                    ServoElementSnapshot* aSnapshot);
 
   /**
    * Performs a Servo traversal to compute style for all dirty nodes in the
-   * document.
+   * document. The root element must be non-null.
    *
    * If aLeaveDirtyBits is true, the dirty/dirty-descendant bits are not
    * cleared.
    */
   void StyleDocument(bool aLeaveDirtyBits);
 
   /**
    * Eagerly styles a subtree of dirty nodes that were just appended to the
@@ -150,32 +148,29 @@ public:
    * Like the above, but does not assume that the root node is dirty. When
    * appending multiple children to a potentially-non-dirty node, it's
    * preferable to call StyleNewChildren on the node rather than making multiple
    * calls to StyleNewSubtree on each child, since it allows for more
    * parallelism.
    */
   void StyleNewChildren(nsIContent* aParent);
 
-  bool StylingStarted() const { return mStylingStarted; }
-
 private:
   already_AddRefed<nsStyleContext> GetContext(already_AddRefed<ServoComputedValues>,
                                               nsStyleContext* aParentContext,
                                               nsIAtom* aPseudoTag,
                                               CSSPseudoElementType aPseudoType);
 
   already_AddRefed<nsStyleContext> GetContext(nsIContent* aContent,
                                               nsStyleContext* aParentContext,
                                               nsIAtom* aPseudoTag,
                                               CSSPseudoElementType aPseudoType);
 
   nsPresContext* mPresContext;
   UniquePtr<RawServoStyleSet> mRawSet;
   EnumeratedArray<SheetType, SheetType::Count,
                   nsTArray<RefPtr<ServoStyleSheet>>> mSheets;
   int32_t mBatching;
-  bool mStylingStarted;
 };
 
 } // namespace mozilla
 
 #endif // mozilla_ServoStyleSet_h