Invalidate text metrics when the <text> subtree is mutated

third_party/WebKit/Source/core/layout/svg/LayoutSVGText.cpp

 /*
  * Copyright (C) 2006 Apple Computer, Inc.
  * Copyright (C) 2006 Alexander Kellett <lypanov@kde.org>
  * Copyright (C) 2006 Oliver Hunt <ojh16@student.canterbury.ac.nz>
  * Copyright (C) 2007 Nikolas Zimmermann <zimmermann@kde.org>
  * Copyright (C) 2008 Rob Buis <buis@kde.org>
  * Copyright (C) 2009 Dirk Schulze <krit@webkit.org>
  * Copyright (C) Research In Motion Limited 2010-2012. All rights reserved.
  * Copyright (C) 2012 Google Inc.
  *
@@ skipping 61 matching lines @@
     , m_needsTransformUpdate(true)
     , m_needsTextMetricsUpdate(false)
 {
 }
 
 LayoutSVGText::~LayoutSVGText()
 {
     ASSERT(m_layoutAttributes.isEmpty());
 }
 
+void LayoutSVGText::willBeDestroyed()
+{
+    m_layoutAttributes.clear();
+    m_layoutAttributesBuilder.clearTextPositioningElements();
+
+    LayoutSVGBlock::willBeDestroyed();
+}
+
 bool LayoutSVGText::isChildAllowed(LayoutObject* child, const ComputedStyle&) const
 {
     return child->isSVGInline() || (child->isText() && SVGLayoutSupport::isLayoutableTextNode(child));
 }
 
 LayoutSVGText* LayoutSVGText::locateLayoutSVGTextAncestor(LayoutObject* start)
 {
     return const_cast<LayoutSVGText*>(findTextRoot(start));
 }
 
@@ skipping 23 matching lines @@
     if (beingDestroyed() || !everHadLayout()) {
         ASSERT(m_layoutAttributes.isEmpty());
         ASSERT(!m_layoutAttributesBuilder.numberOfTextPositioningElements());
         return;
     }
     if (documentBeingDestroyed())
         return;
 
     // The positioning elements cache depends on the size of each text layoutObject in the
     // subtree. If this changes, clear the cache. It will be rebuilt on the next layout.
     invalidatePositioningValues(LayoutInvalidationReason::ChildChanged);

[pdr., 2016/04/06 23:31:40]

setNeedsTextMetricsUpdate() requires that we mark the object for needing layout
too. This works because layout is invalidated in invalidatePositioningValues but
it'll be easy to forget that in the future. WDYT about folding the
setNeedsTextMetricsUpdate call into invalidatePositioningValues or having
setNeedsTextMetricsUpdate invalidate layout?

[fs, 2016/04/07 09:00:00]

I didn't want to fold this call into invalidatePositioningValues, because the
positioning values needing an update does not imply that the metrics value do
(but the reverse does hold I think...) I'm kind of hoping that eventually we
don't need *Added, *Removed et.c, but can just call (a tweaked version of)
invalidatePositioningValues directly instead.

-}
-
-void LayoutSVGText::willBeDestroyed()
-{
-    m_layoutAttributes.clear();
-    m_layoutAttributesBuilder.clearTextPositioningElements();
-
-    LayoutSVGBlock::willBeDestroyed();
+    setNeedsTextMetricsUpdate();

[pdr., 2016/04/06 23:31:40]

No tests seem fail without the change, but I don't understand why we don't need
this call in subtreeChildWillBeRemoved too.

[fs, 2016/04/07 09:00:00]

I think we do need it in subtreeChildWillBeRemoved too - and I added it
https://codereview.chromium.org/1861013003 (which is where I first noticed that
I had fixed this issue "by accident"...) I've yet to come up with a test to show
that though, but I'll look into that (I think it needs involve whitespace
collapsing, so before the metric/attribute resolution split it probably wouldn't
reproduce because both were sort of computed regardless.)

 }
 
 void LayoutSVGText::subtreeChildWillBeRemoved()
 {
     if (beingDestroyed() || !everHadLayout()) {
         ASSERT(m_layoutAttributes.isEmpty());
         ASSERT(!m_layoutAttributesBuilder.numberOfTextPositioningElements());
         return;
     }
 
     // The positioning elements cache depends on the size of each text layoutObject in the
     // subtree. If this changes, clear the cache. It will be rebuilt on the next layout.
     invalidatePositioningValues(LayoutInvalidationReason::ChildChanged);
 }
 
 void LayoutSVGText::subtreeTextDidChange()
 {
     ASSERT(!beingDestroyed());
     if (!everHadLayout()) {
         ASSERT(m_layoutAttributes.isEmpty());
         ASSERT(!m_layoutAttributesBuilder.numberOfTextPositioningElements());
         return;
     }
 
     // The positioning elements cache depends on the size of each text object in
     // the subtree. If this changes, clear the cache and mark it for rebuilding
     // in the next layout.
     invalidatePositioningValues(LayoutInvalidationReason::TextChanged);
+    setNeedsTextMetricsUpdate();
 }
 
 static inline void updateFontInAllDescendants(LayoutSVGText& textRoot, SVGTextLayoutAttributesBuilder* builder = nullptr)
 {
     for (LayoutObject* descendant = &textRoot; descendant; descendant = descendant->nextInPreOrder(&textRoot)) {
         if (!descendant->isSVGInlineText())
             continue;
         LayoutSVGInlineText* text = toLayoutSVGInlineText(descendant);
         text->updateScaledFont();
         if (builder)
@@ skipping 233 matching lines @@
         paintInvalidationState.pushDelayedPaintInvalidationTarget(*this);
 
     if (reason == PaintInvalidationSVGResourceChange)
         newPaintInvalidationState.setForceSubtreeInvalidationWithinContainer();
 
     newPaintInvalidationState.updateForChildren();
     invalidatePaintOfSubtreesIfNeeded(newPaintInvalidationState);
 }
 
 } // namespace blink