Bug 983465. Disable collapsing-text frame construction optimization on a per-document basis. r=bz
authorRobert O'Callahan <robert@ocallahan.org>
Thu, 03 Apr 2014 03:48:51 -0400
changeset 178454 e0af8be2215b53c54adc6591d35a8c97d7b0effc
parent 178453 0a228725aa5c633676d73bde754061f240234d81
child 178455 a53ff44f6546098033fb8963304afc3b5a9903af
push id42287
push userrocallahan@mozilla.com
push dateTue, 15 Apr 2014 07:43:49 +0000
treeherdermozilla-inbound@c6becd0b9a5b [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersbz
bugs983465
milestone31.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 983465. Disable collapsing-text frame construction optimization on a per-document basis. r=bz
content/base/src/nsGenericDOMDataNode.h
content/base/src/nsRange.cpp
layout/base/nsCSSFrameConstructor.cpp
layout/base/nsCSSFrameConstructor.h
--- a/content/base/src/nsGenericDOMDataNode.h
+++ b/content/base/src/nsGenericDOMDataNode.h
@@ -46,20 +46,16 @@ enum {
   // This bit is set to indicate that we have a cached
   // TextIsOnlyWhitespace value
   NS_CACHED_TEXT_IS_ONLY_WHITESPACE =     DATA_NODE_FLAG_BIT(2),
 
   // This bit is only meaningful if the NS_CACHED_TEXT_IS_ONLY_WHITESPACE
   // bit is set, and if so it indicates whether we're only whitespace or
   // not.
   NS_TEXT_IS_ONLY_WHITESPACE =            DATA_NODE_FLAG_BIT(3),
-
-  // This bit is set to indicate that we must create frames for this text node
-  // even if it's only whitespace next to a block boundary.
-  NS_CREATE_FRAME_FOR_IGNORABLE_WHITESPACE = DATA_NODE_FLAG_BIT(4)
 };
 
 // Make sure we have enough space for those bits
 ASSERT_NODE_FLAGS_SPACE(NODE_TYPE_SPECIFIC_BITS_OFFSET + 4);
 
 #undef DATA_NODE_FLAG_BIT
 
 class nsGenericDOMDataNode : public nsIContent
--- a/content/base/src/nsRange.cpp
+++ b/content/base/src/nsRange.cpp
@@ -2746,18 +2746,20 @@ static void ExtractRectFromOffset(nsIFra
   }
 }
 
 static nsTextFrame*
 GetTextFrameForContent(nsIContent* aContent)
 {
   nsIPresShell* presShell = aContent->OwnerDoc()->GetShell();
   if (presShell) {
-    nsIFrame* frame = presShell->FrameConstructor()->EnsureFrameForTextNode(
+    presShell->FrameConstructor()->EnsureFrameForTextNode(
         static_cast<nsGenericDOMDataNode*>(aContent));
+    aContent->OwnerDoc()->FlushPendingNotifications(Flush_Layout);
+    nsIFrame* frame = aContent->GetPrimaryFrame();
     if (frame && frame->GetType() == nsGkAtoms::textFrame) {
       return static_cast<nsTextFrame*>(frame);
     }
   }
   return nullptr;
 }
 
 static nsresult GetPartialTextRect(nsLayoutUtils::RectCallback* aCallback,
@@ -2799,17 +2801,17 @@ static void CollectClientRects(nsLayoutU
   nsCOMPtr<nsINode> startContainer = aStartParent;
   nsCOMPtr<nsINode> endContainer = aEndParent;
 
   // Flush out layout so our frames are up to date.
   if (!aStartParent->IsInDoc()) {
     return;
   }
 
-  aStartParent->GetCurrentDoc()->FlushPendingNotifications(Flush_Layout);
+  aStartParent->OwnerDoc()->FlushPendingNotifications(Flush_Layout);
 
   // Recheck whether we're still in the document
   if (!aStartParent->IsInDoc()) {
     return;
   }
 
   RangeSubtreeIterator iter;
 
--- a/layout/base/nsCSSFrameConstructor.cpp
+++ b/layout/base/nsCSSFrameConstructor.cpp
@@ -1420,16 +1420,17 @@ nsCSSFrameConstructor::nsCSSFrameConstru
   , mGfxScrollFrame(nullptr)
   , mPageSequenceFrame(nullptr)
   , mCurrentDepth(0)
   , mUpdateCount(0)
   , mQuotesDirty(false)
   , mCountersDirty(false)
   , mIsDestroyingFrameTree(false)
   , mHasRootAbsPosContainingBlock(false)
+  , mAlwaysCreateFramesForIgnorableWhitespace(false)
 {
 #ifdef DEBUG
   static bool gFirstTime = true;
   if (gFirstTime) {
     gFirstTime = false;
     char* flags = PR_GetEnv("GECKO_FRAMECTOR_DEBUG_FLAGS");
     if (flags) {
       bool error = false;
@@ -5547,26 +5548,23 @@ nsCSSFrameConstructor::ConstructFramesFr
     // (We check mAdditionalStateBits because only the generated content
     // container's frame construction item is marked with
     // mIsGeneratedContent, and we might not have an aParentFrame.)
     // We don't do it for content that may have XBL anonymous siblings,
     // because they make it difficult to correctly create the frame
     // due to dynamic changes.
     // We don't do it for SVG text, since we might need to position and
     // measure the white space glyphs due to x/y/dx/dy attributes.
-    // We check that NS_CREATE_FRAME_FOR_IGNORABLE_WHITESPACE is not set on
-    // the node. This lets us disable this optimization for specific nodes
-    // (e.g. nodes whose geometry is being queried via DOM APIs).
     if (AtLineBoundary(aIter) &&
         !styleContext->StyleText()->WhiteSpaceOrNewlineIsSignificant() &&
         aIter.List()->ParentHasNoXBLChildren() &&
         !(aState.mAdditionalStateBits & NS_FRAME_GENERATED_CONTENT) &&
         (item.mFCData->mBits & FCDATA_IS_LINE_PARTICIPANT) &&
         !(item.mFCData->mBits & FCDATA_IS_SVG_TEXT) &&
-        !item.mContent->HasFlag(NS_CREATE_FRAME_FOR_IGNORABLE_WHITESPACE) &&
+        !mAlwaysCreateFramesForIgnorableWhitespace &&
         item.IsWhitespace(aState))
       return;
 
     ConstructTextFrame(item.mFCData, aState, item.mContent,
                        adjParentFrame, styleContext,
                        aFrameItems);
     return;
   }
@@ -7779,23 +7777,25 @@ InvalidateCanvasIfNeeded(nsIPresShell* p
   nsIFrame* rootFrame = presShell->GetRootFrame();
   rootFrame->InvalidateFrameSubtree();
 }
 
 nsIFrame*
 nsCSSFrameConstructor::EnsureFrameForTextNode(nsGenericDOMDataNode* aContent)
 {
   if (aContent->HasFlag(NS_CREATE_FRAME_IF_NON_WHITESPACE) &&
-      !aContent->HasFlag(NS_CREATE_FRAME_FOR_IGNORABLE_WHITESPACE)) {
+      !mAlwaysCreateFramesForIgnorableWhitespace) {
     // Text frame may have been suppressed. Disable suppression and signal
-    // that a flush should be performed.
-    aContent->SetFlags(NS_CREATE_FRAME_FOR_IGNORABLE_WHITESPACE);
+    // that a flush should be performed. We do this on a document-wide
+    // basis so that pages that repeatedly query metrics for
+    // collapsed-whitespace text nodes don't trigger pathological behavior.
+    mAlwaysCreateFramesForIgnorableWhitespace = true;
     nsAutoScriptBlocker blocker;
     BeginUpdate();
-    RecreateFramesForContent(aContent, false);
+    ReconstructDocElementHierarchy();
     EndUpdate();
   }
   return aContent->GetPrimaryFrame();
 }
 
 nsresult
 nsCSSFrameConstructor::CharacterDataChanged(nsIContent* aContent,
                                             CharacterDataChangeInfo* aInfo)
--- a/layout/base/nsCSSFrameConstructor.h
+++ b/layout/base/nsCSSFrameConstructor.h
@@ -1796,13 +1796,14 @@ private:
   // Current ProcessChildren depth.
   uint16_t            mCurrentDepth;
   uint16_t            mUpdateCount;
   bool                mQuotesDirty : 1;
   bool                mCountersDirty : 1;
   bool                mIsDestroyingFrameTree : 1;
   // This is true if mDocElementContainingBlock supports absolute positioning
   bool                mHasRootAbsPosContainingBlock : 1;
+  bool                mAlwaysCreateFramesForIgnorableWhitespace : 1;
 
   nsCOMPtr<nsILayoutHistoryState> mTempFrameTreeState;
 };
 
 #endif /* nsCSSFrameConstructor_h___ */