Improve a search logic to find a SVGFontFaceSrcElement

Source/core/svg/SVGFontFaceElement.cpp

 /*
  * Copyright (C) 2007 Eric Seidel <eric@webkit.org>
  * Copyright (C) 2007, 2008 Nikolas Zimmermann <zimmermann@kde.org>
  * Copyright (C) 2008 Apple Inc. 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.
  *
@@ skipping 255 matching lines @@
     return m_fontElement;
 }
 
 void SVGFontFaceElement::rebuildFontFace()
 {
     if (!inDocument()) {
         ASSERT(!m_fontElement);
         return;
     }
 
-    // we currently ignore all but the first src element, alternatively we could concat them
-    SVGFontFaceSrcElement* srcElement = 0;
-
-    for (Node* child = firstChild(); child && !srcElement; child = child->nextSibling()) {
-        if (child->hasTagName(font_face_srcTag))
-            srcElement = static_cast<SVGFontFaceSrcElement*>(child);
-    }
-
     bool describesParentFont = parentNode()->hasTagName(SVGNames::fontTag);
     RefPtr<CSSValueList> list;
 
     if (describesParentFont) {
         m_fontElement = toSVGFontElement(parentNode());
 
         list = CSSValueList::createCommaSeparated();
         list->append(CSSFontFaceSrcValue::createLocal(fontFamily()));
     } else {
         m_fontElement = 0;
+        // we currently ignore all but the first src element, alternatively we could concat them

[Stephen Chennney, 2014/01/10 12:22:34]

The comment was wrong before and it's still wrong now. You now ignore all but
the last.

[gyuyoung-inactive, 2014/01/10 15:46:23]

Fixed. Thanks.

[fs, 2014/01/10 17:43:00]

Friendly note: The comment was actually correct - maybe the iteration condition
was a tad too non-obvious: child && !srcElement
Meaning that the semantics just changed. No worries though since it's seems
quite unlikely that you'd ever encounter such a species - of the dying breed
that is SVG fonts - in the wild.

+        SVGFontFaceSrcElement* srcElement = 0;
+
+        for (Node* child = lastChild(); child && !srcElement; child = child->previousSibling()) {
+            if (child->hasTagName(font_face_srcTag)) {
+                srcElement = static_cast<SVGFontFaceSrcElement*>(child);
+                break;
+            }
+        }
+
         if (srcElement)
             list = srcElement->srcValue();

[Stephen Chennney, 2014/01/10 12:22:34]

This can be moved up inside the if. You don't need the srcElement variable at
all.

[gyuyoung-inactive, 2014/01/10 15:46:23]

Yes, right. Moved it into inside the if.

     }
 
     if (!list || !list->length())
         return;
 
     // Parse in-memory CSS rules
     m_fontFaceRule->mutableProperties()->addParsedProperty(CSSProperty(CSSPropertySrc, list));
 
     if (describesParentFont) {
         // Traverse parsed CSS values and associate CSSFontFaceSrcValue elements with ourselves.
@@ skipping 39 matching lines @@
 
 void SVGFontFaceElement::childrenChanged(bool changedByParser, Node* beforeChange, Node* afterChange, int childCountDelta)
 {
     SVGElement::childrenChanged(changedByParser, beforeChange, afterChange, childCountDelta);
     rebuildFontFace();
 }
 
 } // namespace WebCore
 
 #endif // ENABLE(SVG_FONTS)