Bug 1440537 - Don't propagate flushes across docgroup boundaries. r=bzbarsky
authorEmilio Cobos Álvarez <emilio@crisal.io>
Fri, 19 Apr 2019 19:04:35 +0200
changeset 530606 87f038262ac3d4431a569c1fad9bee5668a332de
parent 530605 f6766ba4ac77c6757c4e4db7598a2e685f23fcf6
child 530607 d7f798e98a7577980d1ab8985f1886cbd09fbcaa
push id11265
push userffxbld-merge
push dateMon, 13 May 2019 10:53:39 +0000
treeherdermozilla-beta@77e0fe8dbdd3 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersbzbarsky
bugs1440537
milestone68.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 1440537 - Don't propagate flushes across docgroup boundaries. r=bzbarsky We don't need to flush layout in the parent document if the parent and child documents can't observe each other. This will also match our behavior in a Fission world. I'm not attached to the name of the function, better ideas welcome. Differential Revision: https://phabricator.services.mozilla.com/D28217
dom/base/Document.cpp
dom/base/Document.h
dom/base/nsGlobalWindowOuter.cpp
layout/style/nsComputedDOMStyle.cpp
--- a/dom/base/Document.cpp
+++ b/dom/base/Document.cpp
@@ -3144,34 +3144,32 @@ void Document::SetPrincipals(nsIPrincipa
   // check. It's not unsafe to have a document which has a null principal in the
   // same docgroup as another document, so this should not be a problem.
   if (aNewPrincipal) {
     GetDocGroup();
   }
 #endif
 }
 
-mozilla::dom::DocGroup* Document::GetDocGroup() const {
 #ifdef DEBUG
+void Document::AssertDocGroupMatchesKey() const {
   // Sanity check that we have an up-to-date and accurate docgroup
   if (mDocGroup) {
     nsAutoCString docGroupKey;
 
     // GetKey() can fail, e.g. after the TLD service has shut down.
     nsresult rv = mozilla::dom::DocGroup::GetKey(NodePrincipal(), docGroupKey);
     if (NS_SUCCEEDED(rv)) {
       MOZ_ASSERT(mDocGroup->MatchesKey(docGroupKey));
     }
     // XXX: Check that the TabGroup is correct as well!
   }
+}
 #endif
 
-  return mDocGroup;
-}
-
 nsresult Document::Dispatch(TaskCategory aCategory,
                             already_AddRefed<nsIRunnable>&& aRunnable) {
   // Note that this method may be called off the main thread.
   if (mDocGroup) {
     return mDocGroup->Dispatch(aCategory, std::move(aRunnable));
   }
   return DispatcherTrait::Dispatch(aCategory, std::move(aRunnable));
 }
@@ -7329,17 +7327,18 @@ void Document::FlushPendingNotifications
   // container is reflowed if its size was changed.  But if it's not safe to
   // flush ourselves, then don't flush the parent, since that can cause things
   // like resizes of our frame's widget, which we can't handle while flushing
   // is unsafe.
   // Since media queries mean that a size change of our container can
   // affect style, we need to promote a style flush on ourself to a
   // layout flush on our parent, since we need our container to be the
   // correct size to determine the correct style.
-  if (mParentDocument && IsSafeToFlush()) {
+  if (StyleOrLayoutObservablyDependsOnParentDocumentLayout() &&
+      IsSafeToFlush()) {
     mozilla::ChangesToFlush parentFlush = aFlush;
     if (flushType >= FlushType::Style) {
       parentFlush.mFlushType = std::max(FlushType::Layout, flushType);
     }
     mParentDocument->FlushPendingNotifications(parentFlush);
   }
 
   if (RefPtr<PresShell> presShell = GetPresShell()) {
--- a/dom/base/Document.h
+++ b/dom/base/Document.h
@@ -3491,17 +3491,40 @@ class Document : public nsINode,
 
   bool HasScriptsBlockedBySandbox();
 
   bool InlineScriptAllowedByCSP();
 
   void ReportHasScrollLinkedEffect();
   bool HasScrollLinkedEffect() const { return mHasScrollLinkedEffect; }
 
-  DocGroup* GetDocGroup() const;
+#ifdef DEBUG
+  void AssertDocGroupMatchesKey() const;
+#endif
+
+  DocGroup* GetDocGroup() const {
+#ifdef DEBUG
+    AssertDocGroupMatchesKey();
+#endif
+    return mDocGroup;
+  }
+
+  /**
+   * If we're a sub-document, the parent document's layout can affect our style
+   * and layout (due to the viewport size, viewport units, media queries...).
+   *
+   * This function returns true if our parent document and our child document
+   * can observe each other. If they cannot, then we don't need to synchronously
+   * update the parent document layout every time the child document may need
+   * up-to-date layout information.
+   */
+  bool StyleOrLayoutObservablyDependsOnParentDocumentLayout() const {
+    return GetParentDocument() &&
+           GetDocGroup() == GetParentDocument()->GetDocGroup();
+  }
 
   void AddIntersectionObserver(DOMIntersectionObserver* aObserver) {
     MOZ_ASSERT(!mIntersectionObservers.Contains(aObserver),
                "Intersection observer already in the list");
     mIntersectionObservers.PutEntry(aObserver);
   }
 
   void RemoveIntersectionObserver(DOMIntersectionObserver* aObserver) {
--- a/dom/base/nsGlobalWindowOuter.cpp
+++ b/dom/base/nsGlobalWindowOuter.cpp
@@ -7361,26 +7361,22 @@ nsresult nsGlobalWindowOuter::SecurityCh
 
 void nsGlobalWindowOuter::FlushPendingNotifications(FlushType aType) {
   if (mDoc) {
     mDoc->FlushPendingNotifications(aType);
   }
 }
 
 void nsGlobalWindowOuter::EnsureSizeAndPositionUpToDate() {
-  // If we're a subframe, make sure our size is up to date.  It's OK that this
-  // crosses the content/chrome boundary, since chrome can have pending reflows
-  // too.
-  //
-  // Make sure to go through the document chain rather than the window chain to
-  // not flush on detached iframes, see bug 1545516.
-  if (mDoc) {
-    if (RefPtr<Document> parent = mDoc->GetParentDocument()) {
-      parent->FlushPendingNotifications(FlushType::Layout);
-    }
+  // If we're a subframe, make sure our size is up to date.  Make sure to go
+  // through the document chain rather than the window chain to not flush on
+  // detached iframes, see bug 1545516.
+  if (mDoc && mDoc->StyleOrLayoutObservablyDependsOnParentDocumentLayout()) {
+    RefPtr<Document> parent = mDoc->GetParentDocument();
+    parent->FlushPendingNotifications(FlushType::Layout);
   }
 }
 
 already_AddRefed<nsISupports> nsGlobalWindowOuter::SaveWindowState() {
   if (!mContext || !GetWrapperPreserveColor()) {
     // The window may be getting torn down; don't bother saving state.
     return nullptr;
   }
--- a/layout/style/nsComputedDOMStyle.cpp
+++ b/layout/style/nsComputedDOMStyle.cpp
@@ -780,17 +780,18 @@ bool nsComputedDOMStyle::NeedsToFlush() 
   // We always compute styles from the element's owner document.
   if (ElementNeedsRestyle(mElement, mPseudo)) {
     return true;
   }
 
   Document* doc = mElement->OwnerDoc();
   // If parent document is there, also needs to check if there is some change
   // that needs to flush this document (e.g. size change for iframe).
-  while (Document* parentDocument = doc->GetParentDocument()) {
+  while (doc->StyleOrLayoutObservablyDependsOnParentDocumentLayout()) {
+    Document* parentDocument = doc->GetParentDocument();
     Element* element = parentDocument->FindContentForSubDocument(doc);
     if (ElementNeedsRestyle(element, nullptr)) {
       return true;
     }
     doc = parentDocument;
   }
 
   return false;