Invalidate text metrics when the <text> subtree is mutated

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

Index: third_party/WebKit/Source/core/layout/svg/LayoutSVGText.cpp
diff --git a/third_party/WebKit/Source/core/layout/svg/LayoutSVGText.cpp b/third_party/WebKit/Source/core/layout/svg/LayoutSVGText.cpp
index 899e1708cfc4e5208cb27eb0e8e16bea1f71afc0..9d0d385cb0e269e935a80a827aca59d1012112f9 100644
--- a/third_party/WebKit/Source/core/layout/svg/LayoutSVGText.cpp
+++ b/third_party/WebKit/Source/core/layout/svg/LayoutSVGText.cpp
@@ -79,6 +79,14 @@ 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));
@@ -123,14 +131,7 @@ void LayoutSVGText::subtreeChildWasAdded()
     // 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()
@@ -159,6 +160,7 @@ void LayoutSVGText::subtreeTextDidChange()
     // 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)