Bug 1424816: Remove the document state cache. r=smaug
authorEmilio Cobos Álvarez <emilio@crisal.io>
Mon, 11 Dec 2017 10:48:42 +0100
changeset 448128 5b18567e17374cee73f1a72ce4bbb513019c4f19
parent 448127 ae78c4fe253ab2618972271350f304947b4d4994
child 448129 e6d30f9822688aeeecda1504b8c591970ca1dab7
push id8527
push userCallek@gmail.com
push dateThu, 11 Jan 2018 21:05:50 +0000
treeherdermozilla-beta@95342d212a7a [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerssmaug
bugs1424816, 1422633
milestone59.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 1424816: Remove the document state cache. r=smaug See bug 1422633, there are assertions missing, and servo doesn't assert at all anymore. I don't think it's worth optimizing / lazily resolving it, each time the document state changes. We usually just restyle the world anyway (which requires recomputing it), and the changes that it's optimizing (nsWindow::SetActive() and XUL root element localedir attribute changes) aren't common enough to warrant the complexity I'd say. This doesn't handle invalidating the cache in the case the root element goes away, I haven't bothered because it was already broken, and GetRootElement() is already gone in RemoveSubtreeFromDocument. MozReview-Commit-ID: 9RuQhmmy7Kr
dom/base/nsDocument.cpp
dom/base/nsIDocument.h
dom/xul/XULDocument.cpp
layout/base/nsPresContext.cpp
layout/style/ServoStyleSet.cpp
layout/style/nsCSSPseudoClasses.cpp
--- a/dom/base/nsDocument.cpp
+++ b/dom/base/nsDocument.cpp
@@ -5051,16 +5051,34 @@ NotifyActivityChanged(nsISupports *aSupp
     olc->NotifyOwnerDocumentActivityChanged();
   }
   nsCOMPtr<nsIDocumentActivity> objectDocumentActivity(do_QueryInterface(aSupports));
   if (objectDocumentActivity) {
     objectDocumentActivity->NotifyOwnerDocumentActivityChanged();
   }
 }
 
+bool
+nsIDocument::IsTopLevelWindowInactive() const
+{
+  nsCOMPtr<nsIDocShellTreeItem> treeItem = GetDocShell();
+  if (!treeItem) {
+    return false;
+  }
+
+  nsCOMPtr<nsIDocShellTreeItem> rootItem;
+  treeItem->GetRootTreeItem(getter_AddRefs(rootItem));
+  if (!rootItem) {
+    return false;
+  }
+
+  nsCOMPtr<nsPIDOMWindowOuter> domWindow = rootItem->GetWindow();
+  return domWindow && !domWindow->IsActive();
+}
+
 void
 nsIDocument::SetContainer(nsDocShell* aContainer)
 {
   if (aContainer) {
     mDocumentContainer = aContainer;
   } else {
     mDocumentContainer = WeakPtr<nsDocShell>();
   }
@@ -5746,20 +5764,17 @@ nsDocument::ContentStateChanged(nsIConte
                   "Someone forgot a scriptblocker");
   NS_DOCUMENT_NOTIFY_OBSERVERS(ContentStateChanged,
                                (this, aContent, aStateMask));
 }
 
 void
 nsDocument::DocumentStatesChanged(EventStates aStateMask)
 {
-  // Invalidate our cached state.
-  mGotDocumentState &= ~aStateMask;
-  mDocumentState &= ~aStateMask;
-
+  UpdateDocumentStates(aStateMask);
   NS_DOCUMENT_NOTIFY_OBSERVERS(DocumentStatesChanged, (this, aStateMask));
 }
 
 void
 nsDocument::StyleRuleChanged(StyleSheet* aSheet, css::Rule* aStyleRule)
 {
   if (!StyleSheetChangeEventsEnabled()) {
     return;
@@ -9870,31 +9885,32 @@ nsDocument::ForgetImagePreload(nsIURI* a
     if (req) {
       // Make sure to cancel the request so imagelib knows it's gone.
       req->CancelAndForgetObserver(NS_BINDING_ABORTED);
     }
   }
 }
 
 void
-nsIDocument::UpdatePossiblyStaleDocumentState()
-{
-  if (!mGotDocumentState.HasState(NS_DOCUMENT_STATE_RTL_LOCALE)) {
+nsIDocument::UpdateDocumentStates(EventStates aChangedStates)
+{
+  if (aChangedStates.HasState(NS_DOCUMENT_STATE_RTL_LOCALE)) {
     if (IsDocumentRightToLeft()) {
       mDocumentState |= NS_DOCUMENT_STATE_RTL_LOCALE;
-    }
-    mGotDocumentState |= NS_DOCUMENT_STATE_RTL_LOCALE;
-  }
-  if (!mGotDocumentState.HasState(NS_DOCUMENT_STATE_WINDOW_INACTIVE)) {
-    nsIPresShell* shell = GetShell();
-    if (shell && shell->GetPresContext() &&
-        shell->GetPresContext()->IsTopLevelWindowInactive()) {
+    } else {
+      mDocumentState &= ~NS_DOCUMENT_STATE_RTL_LOCALE;
+    }
+  }
+
+  if (aChangedStates.HasState(NS_DOCUMENT_STATE_WINDOW_INACTIVE)) {
+    if (IsTopLevelWindowInactive()) {
       mDocumentState |= NS_DOCUMENT_STATE_WINDOW_INACTIVE;
-    }
-    mGotDocumentState |= NS_DOCUMENT_STATE_WINDOW_INACTIVE;
+    } else {
+      mDocumentState &= ~NS_DOCUMENT_STATE_WINDOW_INACTIVE;
+    }
   }
 }
 
 namespace {
 
 /**
  * Stub for LoadSheet(), since all we want is to get the sheet into
  * the CSSLoader's style cache
--- a/dom/base/nsIDocument.h
+++ b/dom/base/nsIDocument.h
@@ -1549,16 +1549,18 @@ public:
    * Return the inner window ID.
    */
   uint64_t InnerWindowID() const
   {
     nsPIDOMWindowInner* window = GetInnerWindow();
     return window ? window->WindowID() : 0;
   }
 
+  bool IsTopLevelWindowInactive() const;
+
   /**
    * Get the script loader for this document
    */
   virtual mozilla::dom::ScriptLoader* ScriptLoader() = 0;
 
   /**
    * Add/Remove an element to the document's id and name hashes
    */
@@ -2632,27 +2634,17 @@ public:
   virtual DocumentTheme GetDocumentLWTheme() { return Doc_Theme_None; }
   virtual DocumentTheme ThreadSafeGetDocumentLWTheme() const { return Doc_Theme_None; }
 
   /**
    * Returns the document state.
    * Document state bits have the form NS_DOCUMENT_STATE_* and are declared in
    * nsIDocument.h.
    */
-  mozilla::EventStates GetDocumentState()
-  {
-    UpdatePossiblyStaleDocumentState();
-    return ThreadSafeGetDocumentState();
-  }
-
-  // GetDocumentState() mutates the state due to lazy resolution;
-  // and can't be used during parallel traversal. Use this instead,
-  // and ensure GetDocumentState() has been called first.
-  // This will assert if the state is stale.
-  mozilla::EventStates ThreadSafeGetDocumentState() const
+  mozilla::EventStates GetDocumentState() const
   {
     return mDocumentState;
   }
 
   virtual nsISupports* GetCurrentContentSink() = 0;
 
   virtual void SetScrollToRef(nsIURI *aDocumentURI) = 0;
   virtual void ScrollToRef() = 0;
@@ -3270,19 +3262,19 @@ protected:
     }
   }
 
   bool GetChildDocumentUseCounter(mozilla::UseCounter aUseCounter)
   {
     return mChildDocumentUseCounters[aUseCounter];
   }
 
+  void UpdateDocumentStates(mozilla::EventStates);
+
 private:
-  void UpdatePossiblyStaleDocumentState();
-
   mutable std::bitset<eDeprecatedOperationCount> mDeprecationWarnedAbout;
   mutable std::bitset<eDocumentWarningCount> mDocWarningWarnedAbout;
 
   // Lazy-initialization to have mDocGroup initialized in prior to the
   // SelectorCaches.
   // FIXME(emilio): We can use a single cache when all CSSOM methods are
   // implemented for the Servo backend.
   mozilla::UniquePtr<SelectorCache> mServoSelectorCache;
@@ -3428,17 +3420,16 @@ protected:
   // container for per-context fonts (downloadable, SVG, etc.)
   RefPtr<mozilla::dom::FontFaceSet> mFontFaceSet;
 
   // Last time this document or a one of its sub-documents was focused.  If
   // focus has never occurred then mLastFocusTime.IsNull() will be true.
   mozilla::TimeStamp mLastFocusTime;
 
   mozilla::EventStates mDocumentState;
-  mozilla::EventStates mGotDocumentState;
 
   // True if BIDI is enabled.
   bool mBidiEnabled : 1;
   // True if a MathML element has ever been owned by this document.
   bool mMathMLEnabled : 1;
 
   // True if this document is the initial document for a window.  This should
   // basically be true only for documents that exist in newly-opened windows or
--- a/dom/xul/XULDocument.cpp
+++ b/dom/xul/XULDocument.cpp
@@ -1663,16 +1663,20 @@ XULDocument::AddElementToDocumentPre(Ele
     }
 
     return NS_OK;
 }
 
 nsresult
 XULDocument::AddElementToDocumentPost(Element* aElement)
 {
+    if (aElement == GetRootElement()) {
+        ResetDocumentDirection();
+    }
+
     // We need to pay special attention to the keyset tag to set up a listener
     if (aElement->NodeInfo()->Equals(nsGkAtoms::keyset, kNameSpaceID_XUL)) {
         // Create our XUL key listener and hook it up.
         nsXBLService::AttachGlobalKeyHandler(aElement);
     }
 
     // See if we need to attach a XUL template to this node
     bool needsHookup;
--- a/layout/base/nsPresContext.cpp
+++ b/layout/base/nsPresContext.cpp
@@ -1684,29 +1684,17 @@ uint32_t
 nsPresContext::GetBidi() const
 {
   return Document()->GetBidiOptions();
 }
 
 bool
 nsPresContext::IsTopLevelWindowInactive()
 {
-  nsCOMPtr<nsIDocShellTreeItem> treeItem(mContainer);
-  if (!treeItem)
-    return false;
-
-  nsCOMPtr<nsIDocShellTreeItem> rootItem;
-  treeItem->GetRootTreeItem(getter_AddRefs(rootItem));
-  if (!rootItem) {
-    return false;
-  }
-
-  nsCOMPtr<nsPIDOMWindowOuter> domWindow = rootItem->GetWindow();
-
-  return domWindow && !domWindow->IsActive();
+  return Document()->IsTopLevelWindowInactive();
 }
 
 void
 nsPresContext::RecordInteractionTime(InteractionType aType,
                                      const TimeStamp& aTimeStamp)
 {
   if (!mInteractionTimeEnabled || aTimeStamp.IsNull()) {
     return;
--- a/layout/style/ServoStyleSet.cpp
+++ b/layout/style/ServoStyleSet.cpp
@@ -368,20 +368,16 @@ ServoStyleSet::PreTraverseSync()
   mozilla::Unused << mPresContext->Document()->GetRootElement();
 
   ResolveMappedAttrDeclarationBlocks();
 
   nsMediaFeatures::InitSystemMetrics();
 
   LookAndFeel::NativeInit();
 
-  // This is lazily computed and pseudo matching needs to access
-  // it so force computation early.
-  mPresContext->Document()->GetDocumentState();
-
   if (gfxUserFontSet* userFontSet = mPresContext->Document()->GetUserFontSet()) {
     // Ensure that the @font-face data is not stale
     uint64_t generation = userFontSet->GetGeneration();
     if (generation != mUserFontSetUpdateGeneration) {
       mPresContext->DeviceContext()->UpdateFontCacheUserFonts(userFontSet);
       mUserFontSetUpdateGeneration = generation;
     }
 
--- a/layout/style/nsCSSPseudoClasses.cpp
+++ b/layout/style/nsCSSPseudoClasses.cpp
@@ -191,26 +191,18 @@ nsCSSPseudoClasses::StringPseudoMatches(
                                         const nsIDocument* aDocument,
                                         EventStates aStateMask,
                                         bool* const aDependence)
 {
 
   switch (aPseudo) {
     case CSSPseudoClassType::mozLocaleDir:
       {
-        bool docIsRTL;
-        if (ServoStyleSet::IsInServoTraversal()) {
-          docIsRTL = aDocument->ThreadSafeGetDocumentState()
-                              .HasState(NS_DOCUMENT_STATE_RTL_LOCALE);
-        } else {
-          auto doc = const_cast<nsIDocument*>(aDocument);
-          docIsRTL = doc->GetDocumentState()
-                        .HasState(NS_DOCUMENT_STATE_RTL_LOCALE);
-        }
-
+        const bool docIsRTL =
+          aDocument->GetDocumentState().HasState(NS_DOCUMENT_STATE_RTL_LOCALE);
         nsDependentString dirString(aString);
 
         if (dirString.EqualsLiteral("rtl")) {
           if (!docIsRTL) {
             return false;
           }
         } else if (dirString.EqualsLiteral("ltr")) {
           if (docIsRTL) {