Make several functions in SVGRenderSupport templated for performance

Make several functions in SVGRenderSupport templated for performance

Make several functions in SVGRenderSupport templated for performance:
- layoutChildren()
- computeContainerBoundingBoxes()

This way, the implementation knows the exact render type and can use faster
firstChild() from RenderSVGRoot / RenderSVGContainer instead of the generic
RenderObject::slowFirstChild().

This CL also makes SVGRenderSupport a class instead of a namespace so that
we can make the private utility functions private (now that they need to
be defined in the header).

The impact on the release binary size is minimal: +560 bytes.

R=pdr

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

[pdr., 2014-05-21 20:38:19]

The change here is a good one but it comes at a pretty large cost in terms of
code complexity. I'm not sure this is worth it just to avoid a virtual call when
calling slowFirstChild.

I've cc'ed ojan and esprehn for a second opinion.

[Inactive, 2014-05-21 20:42:56]

On 2014/05/21 20:38:19, pdr wrote:
> The change here is a good one but it comes at a pretty large cost in terms of
> code complexity. I'm not sure this is worth it just to avoid a virtual call
when
> calling slowFirstChild.
> 
> I've cc'ed ojan and esprehn for a second opinion.

The patch looks big because I had to move the code to the header (due to
templates). However, the change is just making 2 functions templated so that the
argument is no longer a simple RenderObject* but a subclass. I personally don't
think this adds much code complexity but I can understand if you think it is not
worth it.

[pdr., 2014-05-21 21:50:10]

On 2014/05/21 20:42:56, Chris Dumez wrote:
> On 2014/05/21 20:38:19, pdr wrote:
> > The change here is a good one but it comes at a pretty large cost in terms
of
> > code complexity. I'm not sure this is worth it just to avoid a virtual call
> when
> > calling slowFirstChild.
> > 
> > I've cc'ed ojan and esprehn for a second opinion.
> 
> The patch looks big because I had to move the code to the header (due to
> templates). However, the change is just making 2 functions templated so that
the
> argument is no longer a simple RenderObject* but a subclass. I personally
don't
> think this adds much code complexity but I can understand if you think it is
not
> worth it.

I'm convinced this is worth it after all.

LGTM

[pdr., 2014-05-21 21:50:14]

The CQ bit was checked by pdr@chromium.org

[commit-bot: I haz the power, 2014-05-21 22:04:17]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/ch.dumez@samsung.com/291243007/1

[commit-bot: I haz the power, 2014-05-22 04:06:59]

FYI, CQ is re-trying this CL (attempt #1).
Please consider checking whether the failures are real,
and report flakes to chrome-troopers@google.com.
The failing builders are:
  win_gpu_triggered_tests on tryserver.chromium.gpu
(http://build.chromium.org/p/tryserver.chromium.gpu/builders/win_gpu_triggered...)

[commit-bot: I haz the power, 2014-05-22 04:09:08]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2014-05-22 04:09:09]

Try jobs failed on following builders:
  win_gpu_triggered_tests on tryserver.chromium.gpu
(http://build.chromium.org/p/tryserver.chromium.gpu/builders/win_gpu_triggered...)

[Inactive, 2014-05-22 12:20:41]

The CQ bit was checked by ch.dumez@samsung.com

[commit-bot: I haz the power, 2014-05-22 12:21:31]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/ch.dumez@samsung.com/291243007/1

[commit-bot: I haz the power, 2014-05-22 12:22:36]

Change committed as 174548