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 178540 e0af8be2215b53c54adc6591d35a8c97d7b0effc
parent 178539 0a228725aa5c633676d73bde754061f240234d81
child 178541 a53ff44f6546098033fb8963304afc3b5a9903af
push id6323
push userryanvm@gmail.com
push dateTue, 15 Apr 2014 16:27:50 +0000
treeherderfx-team@9a6be8f108fd [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersbz
bugs983465
milestone31.0a1
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___ */