Bug 1342951, part 3 - Support recording of SVGTextFrame correspondence before reflow. r=heycam
authorJonathan Watt <jwatt@jwatt.org>
Fri, 01 Sep 2017 18:07:40 +0100
changeset 381843 302d9e49ac753edc83e1503c098cfe1007fdc8ce
parent 381842 a86e0e1c7c6318e9664af75c05bcb3ea79687cf4
child 381844 309140f65fc5820fa8ccab5f238d8d5c48283f82
push id32538
push userarchaeopteryx@coole-files.de
push dateWed, 20 Sep 2017 09:48:23 +0000
treeherdermozilla-central@a20de99fa3c1 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersheycam
bugs1342951
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 1342951, part 3 - Support recording of SVGTextFrame correspondence before reflow. r=heycam MozReview-Commit-ID: IG2etgpZCHN
layout/generic/nsBlockFrame.cpp
layout/generic/nsFrameStateBits.h
layout/svg/SVGTextFrame.cpp
layout/svg/SVGTextFrame.h
--- a/layout/generic/nsBlockFrame.cpp
+++ b/layout/generic/nsBlockFrame.cpp
@@ -5263,16 +5263,24 @@ nsBlockFrame::AppendFrames(ChildListID  
   nsFrame::ListTag(stdout, aFrameList);
   if (lastKid) {
     printf(" after ");
     nsFrame::ListTag(stdout, lastKid);
   }
   printf("\n");
 #endif
 
+  if (nsSVGUtils::IsInSVGTextSubtree(this)) {
+    MOZ_ASSERT(GetParent()->IsSVGTextFrame(),
+               "unexpected block frame in SVG text");
+    // Workaround for bug 1399425 in case this bit has been removed from the
+    // SVGTextFrame just before the parser adds more descendant nodes.
+    GetParent()->AddStateBits(NS_STATE_SVG_TEXT_CORRESPONDENCE_DIRTY);
+  }
+
   AddFrames(aFrameList, lastKid);
   if (aListID != kNoReflowPrincipalList) {
     PresContext()->PresShell()->
       FrameNeedsReflow(this, nsIPresShell::eTreeChange,
                        NS_FRAME_HAS_DIRTY_CHILDREN); // XXX sufficient?
   }
 }
 
--- a/layout/generic/nsFrameStateBits.h
+++ b/layout/generic/nsFrameStateBits.h
@@ -385,16 +385,20 @@ FRAME_STATE_BIT(SVG, 22, NS_STATE_SVG_PO
 // can be true even if a viewport size change will not affect mPositions,
 // determining a completley accurate value for
 // NS_STATE_SVG_POSITIONING_MAY_USE_PERCENTAGES would require extra work that is
 // probably not worth it.
 FRAME_STATE_BIT(SVG, 23, NS_STATE_SVG_POSITIONING_MAY_USE_PERCENTAGES)
 
 FRAME_STATE_BIT(SVG, 24, NS_STATE_SVG_TEXT_IN_REFLOW)
 
+// Set on SVGTextFrame frames when they need a
+// TextNodeCorrespondenceRecorder::RecordCorrespondence call
+// to update the cached nsTextNode indexes that they correspond to.
+FRAME_STATE_BIT(SVG, 25, NS_STATE_SVG_TEXT_CORRESPONDENCE_DIRTY)
 
 // == Frame state bits that apply to text frames ==============================
 
 FRAME_STATE_GROUP(Text, nsTextFrame)
 
 // -- Flags set during reflow -------------------------------------------------
 
 // nsTextFrame.cpp defines TEXT_REFLOW_FLAGS to be all of these bits.
--- a/layout/svg/SVGTextFrame.cpp
+++ b/layout/svg/SVGTextFrame.cpp
@@ -1387,18 +1387,23 @@ private:
    * The index into the current nsTextNode's character content.
    */
   uint32_t mNodeCharIndex;
 };
 
 /* static */ void
 TextNodeCorrespondenceRecorder::RecordCorrespondence(SVGTextFrame* aRoot)
 {
-  TextNodeCorrespondenceRecorder recorder(aRoot);
-  recorder.Record(aRoot);
+  if (aRoot->GetStateBits() & NS_STATE_SVG_TEXT_CORRESPONDENCE_DIRTY) {
+    // Resolve bidi so that continuation frames are created if necessary:
+    aRoot->MaybeResolveBidiForAnonymousBlockChild();
+    TextNodeCorrespondenceRecorder recorder(aRoot);
+    recorder.Record(aRoot);
+    aRoot->RemoveStateBits(NS_STATE_SVG_TEXT_CORRESPONDENCE_DIRTY);
+  }
 }
 
 void
 TextNodeCorrespondenceRecorder::Record(SVGTextFrame* aRoot)
 {
   if (!mNodeIterator.Current()) {
     // If there are no nsTextNodes then there is nothing to do.
     return;
@@ -1723,19 +1728,19 @@ private:
    * The iterator's current position relative to mSubtree.
    */
   SubtreePosition mSubtreePosition;
 };
 
 uint32_t
 TextFrameIterator::UndisplayedCharacters() const
 {
-  MOZ_ASSERT(!(mRootFrame->PrincipalChildList().FirstChild() &&
-               NS_SUBTREE_DIRTY(mRootFrame->PrincipalChildList().FirstChild())),
-             "should have already reflowed the anonymous block child");
+  MOZ_ASSERT(!(mRootFrame->GetStateBits() &
+               NS_STATE_SVG_TEXT_CORRESPONDENCE_DIRTY),
+             "Text correspondence must be up to date");
 
   if (!mCurrentFrame) {
     return mRootFrame->mTrailingUndisplayedCharacters;
   }
 
   nsTextFrame* frame = do_QueryFrame(mCurrentFrame);
   return GetUndisplayedCharactersBeforeFrame(frame);
 }
@@ -2426,17 +2431,17 @@ private:
   bool mPostReflow;
 };
 
 CharIterator::CharIterator(SVGTextFrame* aSVGTextFrame,
                            CharIterator::CharacterFilter aFilter,
                            nsIContent* aSubtree,
                            bool aPostReflow)
   : mFilter(aFilter),
-    mFrameIterator(FrameIfAnonymousChildReflowed(aSVGTextFrame), aSubtree),
+    mFrameIterator(aSVGTextFrame, aSubtree),
     mFrameForTrimCheck(nullptr),
     mTrimmedOffset(0),
     mTrimmedLength(0),
     mTextElementCharIndex(0),
     mGlyphStartTextElementCharIndex(0),
     mLengthAdjustScaleFactor(aSVGTextFrame->mLengthAdjustScaleFactor)
   , mPostReflow(aPostReflow)
 {
@@ -3774,17 +3779,19 @@ SVGTextFrame::ReflowSVG()
 {
   NS_ASSERTION(nsSVGUtils::OuterSVGIsCallingReflowSVG(this),
                "This call is probaby a wasteful mistake");
 
   MOZ_ASSERT(!(GetStateBits() & NS_FRAME_IS_NONDISPLAY),
              "ReflowSVG mechanism not designed for this");
 
   if (!nsSVGUtils::NeedsReflowSVG(this)) {
-    NS_ASSERTION(!(mState & NS_STATE_SVG_POSITIONING_DIRTY), "How did this happen?");
+    MOZ_ASSERT(!HasAnyStateBits(NS_STATE_SVG_TEXT_CORRESPONDENCE_DIRTY |
+                                NS_STATE_SVG_POSITIONING_DIRTY),
+               "How did this happen?");
     return;
   }
 
   MaybeReflowAnonymousBlockChild();
   UpdateGlyphPositioning();
 
   nsPresContext* presContext = PresContext();
 
@@ -5031,16 +5038,20 @@ SVGTextFrame::DoGlyphPositioning()
   RemoveStateBits(NS_STATE_SVG_POSITIONING_DIRTY);
 
   nsIFrame* kid = PrincipalChildList().FirstChild();
   if (kid && NS_SUBTREE_DIRTY(kid)) {
     MOZ_ASSERT(false, "should have already reflowed the kid");
     return;
   }
 
+  // Since we can be called directly via GetBBoxContribution, our correspondence
+  // may not be up to date.
+  TextNodeCorrespondenceRecorder::RecordCorrespondence(this);
+
   // Determine the positions of each character in app units.
   nsTArray<nsPoint> charPositions;
   DetermineCharPositions(charPositions);
 
   if (charPositions.IsEmpty()) {
     // No characters, so nothing to do.
     return;
   }
@@ -5217,17 +5228,21 @@ SVGTextFrame::ScheduleReflowSVG()
   } else {
     nsSVGUtils::ScheduleReflowSVG(this);
   }
 }
 
 void
 SVGTextFrame::NotifyGlyphMetricsChange()
 {
-  AddStateBits(NS_STATE_SVG_POSITIONING_DIRTY);
+  // TODO: perf - adding NS_STATE_SVG_TEXT_CORRESPONDENCE_DIRTY is overly
+  // aggressive here.  Ideally we would only set that bit when our descendant
+  // frame tree changes (i.e. after frame construction).
+  AddStateBits(NS_STATE_SVG_TEXT_CORRESPONDENCE_DIRTY |
+               NS_STATE_SVG_POSITIONING_DIRTY);
   nsLayoutUtils::PostRestyleEvent(
     mContent->AsElement(), nsRestyleHint(0),
     nsChangeHint_InvalidateRenderingObservers);
   ScheduleReflowSVG();
 }
 
 void
 SVGTextFrame::UpdateGlyphPositioning()
@@ -5269,29 +5284,37 @@ SVGTextFrame::MaybeReflowAnonymousBlockC
   if (NS_SUBTREE_DIRTY(this)) {
     if (mState & NS_FRAME_IS_DIRTY) {
       // If we require a full reflow, ensure our kid is marked fully dirty.
       // (Note that our anonymous nsBlockFrame is not an nsSVGDisplayableFrame, so
       // even when we are called via our ReflowSVG this will not be done for us
       // by nsSVGDisplayContainerFrame::ReflowSVG.)
       kid->AddStateBits(NS_FRAME_IS_DIRTY);
     }
+
+    TextNodeCorrespondenceRecorder::RecordCorrespondence(this);
+
     MOZ_ASSERT(nsSVGUtils::AnyOuterSVGIsCallingReflowSVG(this),
                "should be under ReflowSVG");
     nsPresContext::InterruptPreventer noInterrupts(PresContext());
     DoReflow();
   }
 }
 
 void
 SVGTextFrame::DoReflow()
 {
   // Since we are going to reflow the anonymous block frame, we will
   // need to update mPositions.
-  AddStateBits(NS_STATE_SVG_POSITIONING_DIRTY);
+  // We also mark our text correspondence as dirty since we can end up needing
+  // reflow in ways that do not set NS_STATE_SVG_TEXT_CORRESPONDENCE_DIRTY.
+  // (We'd then fail the "expected a TextNodeCorrespondenceProperty" assertion
+  // when UpdateGlyphPositioning() is called after we return.)
+  AddStateBits(NS_STATE_SVG_TEXT_CORRESPONDENCE_DIRTY |
+               NS_STATE_SVG_POSITIONING_DIRTY);
 
   if (mState & NS_FRAME_IS_NONDISPLAY) {
     // Normally, these dirty flags would be cleared in ReflowSVG(), but that
     // doesn't get called for non-display frames. We don't want to reflow our
     // descendants every time SVGTextFrame::PaintSVG makes sure that we have
     // valid positions by calling UpdateGlyphPositioning(), so we need to clear
     // these dirty bits. Note that this also breaks an invalidation loop where
     // our descendants invalidate as they reflow, which invalidates rendering
@@ -5331,18 +5354,16 @@ SVGTextFrame::DoReflow()
                "style system should ensure that :-moz-svg-text "
                "does not get styled");
 
   kid->Reflow(presContext, desiredSize, reflowInput, status);
   kid->DidReflow(presContext, &reflowInput, nsDidReflowStatus::FINISHED);
   kid->SetSize(wm, desiredSize.Size(wm));
 
   RemoveStateBits(NS_STATE_SVG_TEXT_IN_REFLOW);
-
-  TextNodeCorrespondenceRecorder::RecordCorrespondence(this);
 }
 
 // Usable font size range in devpixels / user-units
 #define CLAMP_MIN_SIZE 8.0
 #define CLAMP_MAX_SIZE 200.0
 #define PRECISE_SIZE   200.0
 
 bool
--- a/layout/svg/SVGTextFrame.h
+++ b/layout/svg/SVGTextFrame.h
@@ -198,17 +198,18 @@ class SVGTextFrame final : public nsSVGD
 protected:
   explicit SVGTextFrame(nsStyleContext* aContext)
     : nsSVGDisplayContainerFrame(aContext, kClassID)
     , mTrailingUndisplayedCharacters(0)
     , mFontSizeScaleFactor(1.0f)
     , mLastContextScale(1.0f)
     , mLengthAdjustScaleFactor(1.0f)
   {
-    AddStateBits(NS_STATE_SVG_POSITIONING_DIRTY);
+    AddStateBits(NS_STATE_SVG_TEXT_CORRESPONDENCE_DIRTY |
+                 NS_STATE_SVG_POSITIONING_DIRTY);
   }
 
   ~SVGTextFrame() {}
 
 public:
   NS_DECL_QUERYFRAME
   NS_DECL_FRAMEARENA_HELPERS(SVGTextFrame)