Bug 1393098 part 0: Refactor logic (add helper bool & reduce GetParent calls), in nsTextFrame::CharacterDataChanged. r=jfkthame
authorDaniel Holbert <dholbert@cs.stanford.edu>
Thu, 31 Aug 2017 10:45:36 -0700
changeset 378063 007b471778c310660823bf7680e116a828bcb1e5
parent 378062 8ab1649365bb4e3cb484fb3bfc76364a1f800cd6
child 378064 9c13f2c5b9515e456a2a9abd1107affeb3709b93
push id32421
push userarchaeopteryx@coole-files.de
push dateFri, 01 Sep 2017 08:31:26 +0000
treeherdermozilla-central@583e73fb8e3c [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersjfkthame
bugs1393098
milestone57.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 1393098 part 0: Refactor logic (add helper bool & reduce GetParent calls), in nsTextFrame::CharacterDataChanged. r=jfkthame This patch doesn't affect behavior at all -- it just adjusts the logic slightly. Specifically, this patch: (a) Changes some code that currently tracks a frame, to now instead track that frame's parent, since we only ever call GetParent() on it anyway. (b) Drops a null-check that becomes unnecessary as a result of that change. (It was only there to protect us from calling GetParent() on a null pointer during the first loop iteration, and now that's not a risk since we're tracking the parent itself, and a null value will fail the equality comparison and do the right thing.) (c) Captures the "are ancestors already aware of a reflow request for my subtree" if-condition in a named boolean helper-variable. (d) Adds/improves documentation. MozReview-Commit-ID: 7dEflfiERYB
layout/generic/nsTextFrame.cpp
--- a/layout/generic/nsTextFrame.cpp
+++ b/layout/generic/nsTextFrame.cpp
@@ -4856,34 +4856,50 @@ nsTextFrame::CharacterDataChanged(Charac
   while (true) {
     next = textFrame->GetNextContinuation();
     if (!next || next->GetContentOffset() > int32_t(aInfo->mChangeStart))
       break;
     textFrame = next;
   }
 
   int32_t endOfChangedText = aInfo->mChangeStart + aInfo->mReplaceLength;
-  nsTextFrame* lastDirtiedFrame = nullptr;
+
+  // Parent of the last frame that we passed to FrameNeedsReflow.
+  // (For subsequent frames with this same parent, we can just set their
+  // dirty bit without bothering to call FrameNeedsReflow again.)
+  nsIFrame* lastDirtiedFrameParent = nullptr;
 
   nsIPresShell* shell = PresContext()->GetPresShell();
   do {
     // textFrame contained deleted text (or the insertion point,
     // if this was a pure insertion).
     textFrame->mState &= ~TEXT_WHITESPACE_FLAGS;
     textFrame->ClearTextRuns();
-    if (!lastDirtiedFrame ||
-        lastDirtiedFrame->GetParent() != textFrame->GetParent()) {
+
+    nsIFrame* parentOfTextFrame = textFrame->GetParent();
+    bool areAncestorsAwareOfReflowRequest = false;
+    if (lastDirtiedFrameParent == parentOfTextFrame) {
+      // An earlier iteration of this loop already called
+      // FrameNeedsReflow for a sibling of |textFrame|.
+      areAncestorsAwareOfReflowRequest = true;
+    } else {
+      lastDirtiedFrameParent = parentOfTextFrame;
+    }
+
+    if (!areAncestorsAwareOfReflowRequest) {
       // Ask the parent frame to reflow me.
       shell->FrameNeedsReflow(textFrame, nsIPresShell::eStyleChange,
                               NS_FRAME_IS_DIRTY);
-      lastDirtiedFrame = textFrame;
     } else {
-      // if the parent is a block, we're cheating here because we should
-      // be marking our line dirty, but we're not. nsTextFrame::SetLength
-      // will do that when it gets called during reflow.
+      // We already called FrameNeedsReflow on behalf of an earlier sibling,
+      // so we can just mark this frame as dirty and don't need to bother
+      // telling its ancestors.
+      // Note: if the parent is a block, we're cheating here because we should
+      // be marking our line dirty, but we're not. nsTextFrame::SetLength will
+      // do that when it gets called during reflow.
       textFrame->AddStateBits(NS_FRAME_IS_DIRTY);
     }
     textFrame->InvalidateFrame();
 
     // Below, frames that start after the deleted text will be adjusted so that
     // their offsets move with the trailing unchanged text. If this change
     // deletes more text than it inserts, those frame offsets will decrease.
     // We need to maintain the invariant that mContentOffset is non-decreasing