Make LayoutSVGViewportContainer's viewport updates an internal detail

Make LayoutSVGViewportContainer's viewport updates an internal detail

determineIfLayoutSizeChanged was used by LayoutSVGContainer to let
a subclass, LayoutSVGViewportContainer, update its "layout size
changed" flag. This can be refactored by overriding layout (as
LayoutSVGRoot does) so other subclasses of LayoutSVGContainer are
not confused by this virtual.

With this refactoring it is possible to recalculate the viewport
only when needed (selfNeedsLayout), and move the viewport update
out of calculateLocalTransform.

No measurable perf difference, and no change in behavior.

CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2

Committed: https://crrev.com/81824be93544dd0936c78b5a24a30c07a818a8ea
Cr-Commit-Position: refs/heads/master@{#439437}

[pdr., 2016-12-17 04:52:36]

pdr@chromium.org changed reviewers:
+ fs@opera.com

[pdr., 2016-12-17 04:52:50]

The CQ bit was checked by pdr@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-12-17 04:53:05]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-12-17 06:12:16]

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

[commit-bot: I haz the power, 2016-12-17 06:12:17]

Dry run: Try jobs failed on following builders:
  linux_chromium_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[pdr., 2016-12-17 08:12:57]

pdr@chromium.org changed reviewers:
- fs@opera.com

[pdr., 2016-12-17 08:12:58]

On 2016/12/17 at 06:12:17, commit-bot wrote:
> Dry run: Try jobs failed on following builders:
>   linux_chromium_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

Oops, please hold off on this review.

[pdr., 2016-12-19 04:08:13]

Description was changed from

==========
Make LayoutSVGViewportContainer's viewport updates an internal detail

determineIfLayoutSizeChanged was used by LayoutSVGContainer to let
a subclass, LayoutSVGViewportContainer, update its "layout size
changed" flag. This can be refactored by overriding layout so other
subclasses of LayoutSVGContainer are not confused by this virtual.

With this refactoring it is possible to recalculate the viewport
only when needed (selfNeedsLayout), and move the viewport update
out of calculateLocalTransform.

No measurable perf difference, and no change in behavior.
==========

to

==========
Make LayoutSVGViewportContainer's viewport updates an internal detail

determineIfLayoutSizeChanged was used by LayoutSVGContainer to let
a subclass, LayoutSVGViewportContainer, update its "layout size
changed" flag. This can be refactored by overriding layout so other
subclasses of LayoutSVGContainer are not confused by this virtual.

With this refactoring it is possible to recalculate the viewport
only when needed (selfNeedsLayout), and move the viewport update
out of calculateLocalTransform.

No measurable perf difference, and no change in behavior.

CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2
==========

[pdr., 2016-12-19 05:35:00]

Description was changed from

==========
Make LayoutSVGViewportContainer's viewport updates an internal detail

determineIfLayoutSizeChanged was used by LayoutSVGContainer to let
a subclass, LayoutSVGViewportContainer, update its "layout size
changed" flag. This can be refactored by overriding layout so other
subclasses of LayoutSVGContainer are not confused by this virtual.

With this refactoring it is possible to recalculate the viewport
only when needed (selfNeedsLayout), and move the viewport update
out of calculateLocalTransform.

No measurable perf difference, and no change in behavior.

CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2
==========

to

==========
Make LayoutSVGViewportContainer's viewport updates an internal detail

determineIfLayoutSizeChanged was used by LayoutSVGContainer to let
a subclass, LayoutSVGViewportContainer, update its "layout size
changed" flag. This can be refactored by overriding layout (as
LayoutSVGRoot does) so other subclasses of LayoutSVGContainer are
not confused by this virtual.

With this refactoring it is possible to recalculate the viewport
only when needed (selfNeedsLayout), and move the viewport update
out of calculateLocalTransform.

No measurable perf difference, and no change in behavior.

CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2
==========

[pdr., 2016-12-19 05:35:00]

pdr@chromium.org changed reviewers:
+ fs@opera.com

[pdr., 2016-12-19 05:35:06]

The CQ bit was checked by pdr@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-12-19 05:35:20]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[pdr., 2016-12-19 06:44:30]

On 2016/12/17 at 08:12:58, pdr. wrote:
> On 2016/12/17 at 06:12:17, commit-bot wrote:
> > Dry run: Try jobs failed on following builders:
> >   linux_chromium_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
> 
> Oops, please hold off on this review.

This failure was due to m_isLayoutSizeChanged. This should be set whenever a
descendant's relative lengths could have changed, not just when the container's
layout size changes. For example, viewBox changes do not necessarily change an
object's layout size but they should set m_isLayoutSizeChanged because
descendants with relative lengths depend on it. We might want to rename
layoutSizeOfNearestViewportChanged to reflect this.

Ready for review.

[commit-bot: I haz the power, 2016-12-19 06:55:22]

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

[commit-bot: I haz the power, 2016-12-19 06:55:23]

Dry run: This issue passed the CQ dry run.

[fs, 2016-12-19 08:43:21]

lgtm

[fs, 2016-12-19 08:47:46]

On 2016/12/19 at 06:44:30, pdr wrote:
> On 2016/12/17 at 08:12:58, pdr. wrote:
> > On 2016/12/17 at 06:12:17, commit-bot wrote:
> > > Dry run: Try jobs failed on following builders:
> > >   linux_chromium_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
> > 
> > Oops, please hold off on this review.
> 
> This failure was due to m_isLayoutSizeChanged. This should be set whenever a
descendant's relative lengths could have changed, not just when the container's
layout size changes. For example, viewBox changes do not necessarily change an
object's layout size but they should set m_isLayoutSizeChanged because
descendants with relative lengths depend on it. We might want to rename
layoutSizeOfNearestViewportChanged to reflect this.

Yes, we lack this fidelity wrt 'viewBox' (it's marked as a transform change and
a layout change, but we can't fully infer that we have a potential viewport
change on the other side.)

[pdr., 2016-12-19 08:53:31]

The CQ bit was checked by pdr@chromium.org

[commit-bot: I haz the power, 2016-12-19 08:53:41]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-12-19 08:57:09]

CQ is committing da patch.

Bot data: {"patchset_id": 20001, "attempt_start_ts": 1482137611971260,
"parent_rev": "7e5614acdc7f34823af4256b855437439aae27ea", "commit_rev":
"e374d8f2318a9e0e6f199bdfd60c0c0fb29622bd"}

[commit-bot: I haz the power, 2016-12-19 08:57:37]

Description was changed from

==========
Make LayoutSVGViewportContainer's viewport updates an internal detail

determineIfLayoutSizeChanged was used by LayoutSVGContainer to let
a subclass, LayoutSVGViewportContainer, update its "layout size
changed" flag. This can be refactored by overriding layout (as
LayoutSVGRoot does) so other subclasses of LayoutSVGContainer are
not confused by this virtual.

With this refactoring it is possible to recalculate the viewport
only when needed (selfNeedsLayout), and move the viewport update
out of calculateLocalTransform.

No measurable perf difference, and no change in behavior.

CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2
==========

to

==========
Make LayoutSVGViewportContainer's viewport updates an internal detail

determineIfLayoutSizeChanged was used by LayoutSVGContainer to let
a subclass, LayoutSVGViewportContainer, update its "layout size
changed" flag. This can be refactored by overriding layout (as
LayoutSVGRoot does) so other subclasses of LayoutSVGContainer are
not confused by this virtual.

With this refactoring it is possible to recalculate the viewport
only when needed (selfNeedsLayout), and move the viewport update
out of calculateLocalTransform.

No measurable perf difference, and no change in behavior.

CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2

Review-Url: https://codereview.chromium.org/2587623002
==========

[commit-bot: I haz the power, 2016-12-19 08:57:38]

Committed patchset #2 (id:20001)

[commit-bot: I haz the power, 2016-12-19 08:59:22]

Description was changed from

==========
Make LayoutSVGViewportContainer's viewport updates an internal detail

determineIfLayoutSizeChanged was used by LayoutSVGContainer to let
a subclass, LayoutSVGViewportContainer, update its "layout size
changed" flag. This can be refactored by overriding layout (as
LayoutSVGRoot does) so other subclasses of LayoutSVGContainer are
not confused by this virtual.

With this refactoring it is possible to recalculate the viewport
only when needed (selfNeedsLayout), and move the viewport update
out of calculateLocalTransform.

No measurable perf difference, and no change in behavior.

CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2

Review-Url: https://codereview.chromium.org/2587623002
==========

to

==========
Make LayoutSVGViewportContainer's viewport updates an internal detail

determineIfLayoutSizeChanged was used by LayoutSVGContainer to let
a subclass, LayoutSVGViewportContainer, update its "layout size
changed" flag. This can be refactored by overriding layout (as
LayoutSVGRoot does) so other subclasses of LayoutSVGContainer are
not confused by this virtual.

With this refactoring it is possible to recalculate the viewport
only when needed (selfNeedsLayout), and move the viewport update
out of calculateLocalTransform.

No measurable perf difference, and no change in behavior.

CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2

Committed: https://crrev.com/81824be93544dd0936c78b5a24a30c07a818a8ea
Cr-Commit-Position: refs/heads/master@{#439437}
==========

[commit-bot: I haz the power, 2016-12-19 08:59:23]

Patchset 2 (id:??) landed as
https://crrev.com/81824be93544dd0936c78b5a24a30c07a818a8ea
Cr-Commit-Position: refs/heads/master@{#439437}