Cleanup SVGTextMetricsBuilder::measureTextLayoutObject

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

 /*
  * Copyright (C) Research In Motion Limited 2010-2012. All rights reserved.
  *
  * This library is free software; you can redistribute it and/or
  * modify it under the terms of the GNU Library General Public
  * License as published by the Free Software Foundation; either
  * version 2 of the License, or (at your option) any later version.
  *
  * This library is distributed in the hope that it will be useful,
  * but WITHOUT ANY WARRANTY; without even the implied warranty of
@@ skipping 10 matching lines @@
 
 #include "core/layout/api/LineLayoutSVGInlineText.h"
 #include "core/layout/svg/LayoutSVGInline.h"
 #include "core/layout/svg/LayoutSVGInlineText.h"
 #include "core/layout/svg/LayoutSVGText.h"
 #include "core/layout/svg/SVGTextMetrics.h"
 #include "platform/fonts/CharacterRange.h"
 #include "platform/text/BidiCharacterRun.h"
 #include "platform/text/BidiResolver.h"
 #include "platform/text/TextDirection.h"
-#include "platform/text/TextPath.h"
 #include "platform/text/TextRun.h"
 #include "platform/text/TextRunIterator.h"
 #include "wtf/Vector.h"
 
 namespace blink {
 
 class SVGTextMetricsCalculator {
 public:
     SVGTextMetricsCalculator(LayoutSVGInlineText*);
     ~SVGTextMetricsCalculator();
@@ skipping 124 matching lines @@
 }
 
 SVGTextMetrics SVGTextMetricsCalculator::currentCharacterMetrics()
 {
     unsigned currentSubrunPosition = updateSubrunRangesForCurrentPosition();
     unsigned length = currentCharacterStartsSurrogatePair() ? 2 : 1;
     float width = m_subrunRanges[currentSubrunPosition].width();
     return SVGTextMetrics(m_text, length, width);
 }
 
-struct MeasureTextData {
-    MeasureTextData(SVGCharacterDataMap* characterDataMap)
-        : allCharactersMap(characterDataMap)
-        , lastCharacterWasWhiteSpace(true)
-        , valueListPosition(0)
-    {
-    }
+struct TreeWalkTextState {
+    TreeWalkTextState()
+        : lastCharacterWasWhiteSpace(true)
+        , valueListPosition(0) { }
 
-    SVGCharacterDataMap* allCharactersMap;
     bool lastCharacterWasWhiteSpace;
     unsigned valueListPosition;
 };
 
-static void measureTextLayoutObject(LayoutSVGInlineText* text, MeasureTextData* data, bool processLayoutObject)
+// Struct for updating SVGTextLayoutAttributes. If an SVGCharacterDataMap is
+// available, the attribute's character data map will also be updated.

[fs, 2016/03/20 23:43:38]

Maybe mention the relation to valueListPosition too (since that part of
TreeWalkTextState is logically related to what's "going on" in this struct.)

[pdr., 2016/03/21 18:44:22]

Good idea, done.

+struct UpdateAttributes {
+    UpdateAttributes(SVGTextLayoutAttributes& textAttributes, const SVGCharacterDataMap* allCharacters)
+        : attributes(textAttributes)
+        , allCharactersMap(allCharacters) { }
+
+    void clearExistingAttributes()
+    {
+        if (allCharactersMap)
+            attributes.clear();
+        else
+            attributes.textMetricsValues().clear();
+    }
+
+    void addMetrics(SVGTextMetrics metrics)
+    {
+        attributes.textMetricsValues().append(metrics);
+    }
+
+    void updateCharacterDataMap(unsigned valueListPosition, unsigned currentTextPosition)
+    {
+        if (!allCharactersMap)
+            return;
+        const SVGCharacterDataMap::const_iterator it = allCharactersMap->find(valueListPosition);
+        if (it != allCharactersMap->end())
+            attributes.characterDataMap().set(currentTextPosition, it->value);
+    }
+
+    SVGTextLayoutAttributes& attributes;
+    const SVGCharacterDataMap* allCharactersMap;
+};
+
+static void walkInlineText(LayoutSVGInlineText* text, TreeWalkTextState& textState, UpdateAttributes* attributesToUpdate = nullptr)
 {
-    ASSERT(text);
-
-    SVGTextLayoutAttributes* attributes = text->layoutAttributes();
-    Vector<SVGTextMetrics>* textMetricsValues = &attributes->textMetricsValues();
-    if (processLayoutObject) {
-        if (data->allCharactersMap)
-            attributes->clear();
-        else
-            textMetricsValues->clear();
-    }
+    if (attributesToUpdate)
+        attributesToUpdate->clearExistingAttributes();
 
     if (!text->textLength())
         return;
 
     // TODO(pdr): This loop is too tightly coupled to SVGTextMetricsCalculator.
     // We should refactor SVGTextMetricsCalculator to be a simple bidi run
     // iterator and move all subrun logic to a single function.
     SVGTextMetricsCalculator calculator(text);
     bool preserveWhiteSpace = text->style()->whiteSpace() == PRE;
     unsigned surrogatePairCharacters = 0;
-    unsigned skippedCharacters = 0;
+    unsigned skippedWhitespace = 0;
     do {
-        SVGTextMetrics metrics = calculator.currentCharacterMetrics();
-        if (!metrics.length())
-            break;
-
-        bool characterIsWhiteSpace = calculator.currentCharacterIsWhiteSpace();
-        if (characterIsWhiteSpace && !preserveWhiteSpace && data->lastCharacterWasWhiteSpace) {
-            if (processLayoutObject)
-                textMetricsValues->append(SVGTextMetrics(SVGTextMetrics::SkippedSpaceMetrics));
-            if (data->allCharactersMap)
-                skippedCharacters += metrics.length();
+        bool currentCharacterIsWhiteSpace = calculator.currentCharacterIsWhiteSpace();
+        if (currentCharacterIsWhiteSpace && !preserveWhiteSpace && textState.lastCharacterWasWhiteSpace) {
+            if (attributesToUpdate)
+                attributesToUpdate->addMetrics(SVGTextMetrics(SVGTextMetrics::SkippedSpaceMetrics));
+            ASSERT(!calculator.currentCharacterStartsSurrogatePair());

[fs, 2016/03/20 23:43:38]

Maybe just assert calculator.currentCharacterMetrics().length() == 1 to cover
all bases?

[pdr., 2016/03/21 18:44:22]

Done

+            ASSERT(textState.lastCharacterWasWhiteSpace);

[fs, 2016/03/20 23:43:38]

This is part of the condition, so maybe a tad overzealous to assert.

[pdr., 2016/03/21 18:44:22]

Done

+            skippedWhitespace++;
             continue;
         }
 
-        if (processLayoutObject) {
-            if (data->allCharactersMap) {
-                const SVGCharacterDataMap::const_iterator it = data->allCharactersMap->find(data->valueListPosition + calculator.currentPosition() - skippedCharacters - surrogatePairCharacters + 1);
-                if (it != data->allCharactersMap->end())
-                    attributes->characterDataMap().set(calculator.currentPosition() + 1, it->value);
-            }
-            textMetricsValues->append(metrics);
+        if (attributesToUpdate) {
+            attributesToUpdate->updateCharacterDataMap(textState.valueListPosition - skippedWhitespace - surrogatePairCharacters + calculator.currentPosition() + 1, calculator.currentPosition() + 1);
+            attributesToUpdate->addMetrics(calculator.currentCharacterMetrics());
         }
 
-        if (data->allCharactersMap && calculator.currentCharacterStartsSurrogatePair())
+        if (calculator.currentCharacterStartsSurrogatePair())
             surrogatePairCharacters++;
-
-        data->lastCharacterWasWhiteSpace = characterIsWhiteSpace;
+        textState.lastCharacterWasWhiteSpace = currentCharacterIsWhiteSpace;
     } while (calculator.advancePosition());
 
-    if (!data->allCharactersMap)
-        return;
-
-    data->valueListPosition += calculator.currentPosition() - skippedCharacters;
+    textState.valueListPosition += calculator.currentPosition() - skippedWhitespace;
 }
 
-static void walkTree(LayoutSVGText* start, LayoutSVGInlineText* stopAtLeaf, MeasureTextData* data)
+static void walkTree(LayoutSVGText* start, LayoutSVGInlineText* stopAtText, SVGCharacterDataMap* allCharactersMap = nullptr)
 {
+    TreeWalkTextState textState;
     LayoutObject* child = start->firstChild();
     while (child) {
         if (child->isSVGInlineText()) {
             LayoutSVGInlineText* text = toLayoutSVGInlineText(child);
-            measureTextLayoutObject(text, data, !stopAtLeaf || stopAtLeaf == text);
-            if (stopAtLeaf && stopAtLeaf == text)
+            OwnPtr<UpdateAttributes> attributesToUpdate = nullptr;
+            if (!stopAtText || stopAtText == text)
+                attributesToUpdate = adoptPtr(new UpdateAttributes(*text->layoutAttributes(), allCharactersMap));
+            walkInlineText(text, textState, attributesToUpdate.get());
+            if (stopAtText == text)
                 return;
         } else if (child->isSVGInline()) {
             // Visit children of text content elements.
             if (LayoutObject* inlineChild = toLayoutSVGInline(child)->firstChild()) {
                 child = inlineChild;
                 continue;
             }
         }
         child = child->nextInPreOrderAfterChildren(start);
     }
 }
 
 void SVGTextMetricsBuilder::measureTextLayoutObject(LayoutSVGInlineText* text)
 {
     ASSERT(text);
-
-    LayoutSVGText* textRoot = LayoutSVGText::locateLayoutSVGTextAncestor(text);
-    if (!textRoot)
-        return;
-
-    MeasureTextData data(0);
-    walkTree(textRoot, text, &data);
+    if (LayoutSVGText* textRoot = LayoutSVGText::locateLayoutSVGTextAncestor(text))
+        walkTree(textRoot, text);
 }
 
-void SVGTextMetricsBuilder::buildMetricsAndLayoutAttributes(LayoutSVGText* textRoot, LayoutSVGInlineText* stopAtLeaf, SVGCharacterDataMap& allCharactersMap)
+void SVGTextMetricsBuilder::buildMetricsAndLayoutAttributes(LayoutSVGText* textRoot, LayoutSVGInlineText* stopAtText, SVGCharacterDataMap& allCharactersMap)
 {
     ASSERT(textRoot);
-    MeasureTextData data(&allCharactersMap);
-    walkTree(textRoot, stopAtLeaf, &data);
+    walkTree(textRoot, stopAtText, &allCharactersMap);
 }
 
 } // namespace blink