Improve a search logic to find a SVGFontFaceSrcElement

Improve a search logic to find a SVGFontFaceSrcElement

In SVGFontFaceElement::rebuildFontFace(), it tries to find a SVGFontFaceSrcElement. However, current search logic looks inefficient. The search logic starts to find SVGFontFaceSrcElement from a first child node, then it replaces a discovered element whenever finding next SVGFontFaceSrcElement. Thus, it would be good if we find SVGFontFaceSrcElement from a last node. Then, we go out of the searching loop as soon as we find a first SVGFontFaceSrcElement. It will help to reduce iteration number in the search loop.

Additionally the search loop is moved to the place which uses a SVGFontFaceSrcElement actually.

BUG=N/A

Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=164881

[gyuyoung-inactive, 2014-01-10 10:16:47]

When I run whole layout test with this patch, there wasn't any regression.

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

Reasonable change, but some room for improvement.

https://codereview.chromium.org/133493002/diff/1/Source/core/svg/SVGFontFaceE...
File Source/core/svg/SVGFontFaceElement.cpp (right):

https://codereview.chromium.org/133493002/diff/1/Source/core/svg/SVGFontFaceE...
Source/core/svg/SVGFontFaceElement.cpp:286: // we currently ignore all but the
first src element, alternatively we could concat them
The comment was wrong before and it's still wrong now. You now ignore all but
the last.

https://codereview.chromium.org/133493002/diff/1/Source/core/svg/SVGFontFaceE...
Source/core/svg/SVGFontFaceElement.cpp:297: list = srcElement->srcValue();
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]

https://codereview.chromium.org/133493002/diff/1/Source/core/svg/SVGFontFaceE...
File Source/core/svg/SVGFontFaceElement.cpp (right):

https://codereview.chromium.org/133493002/diff/1/Source/core/svg/SVGFontFaceE...
Source/core/svg/SVGFontFaceElement.cpp:286: // we currently ignore all but the
first src element, alternatively we could concat them
On 2014/01/10 12:22:34, Stephen Chenney wrote:
> The comment was wrong before and it's still wrong now. You now ignore all but
> the last.

Fixed. Thanks.

https://codereview.chromium.org/133493002/diff/1/Source/core/svg/SVGFontFaceE...
Source/core/svg/SVGFontFaceElement.cpp:297: list = srcElement->srcValue();
On 2014/01/10 12:22:34, Stephen Chenney wrote:
> This can be moved up inside the if. You don't need the srcElement variable at
> all.

Yes, right. Moved it into inside the if.

[Stephen Chennney, 2014-01-10 15:57:50]

lgtm

[commit-bot: I haz the power, 2014-01-10 15:58:05]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/gyuyoung.kim@samsung.com/133493002/100001

[commit-bot: I haz the power, 2014-01-10 17:00:38]

Change committed as 164881

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

https://codereview.chromium.org/133493002/diff/1/Source/core/svg/SVGFontFaceE...
File Source/core/svg/SVGFontFaceElement.cpp (right):

https://codereview.chromium.org/133493002/diff/1/Source/core/svg/SVGFontFaceE...
Source/core/svg/SVGFontFaceElement.cpp:286: // we currently ignore all but the
first src element, alternatively we could concat them
On 2014/01/10 12:22:34, Stephen Chenney wrote:
> The comment was wrong before and it's still wrong now. You now ignore all but
> the last.

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.