Rename and change parameter type of some GeometryMapper methods

Rename, simplify and change parameter type of some GeometryMapper methods

Rename
mapRectToDestinationSpace to sourceToDestinationRect
mapToVisualRectInDestinationSpace to sourceToDestinationVisualRect
localToVisualRectInAncestorSpace to localToAncestorVisualRect
so that all mapping methods are in the form
  (local|source|ancestor)To(local|source|ancestor)(Rect|VisualRect|Matrix|ClipRect)

Changed parameters to localToAncestorMatrix and getPrecomputedDataForAncestor()
from const PropertyTreeState& to const TransformPaintPropertyNode*
because some later changes will require the new form (
TransformationPaintPropertyNode without a state).

Simplified localToAncestorMatrix and localToAncestorClipRect because
we have early returns when the local node is the same as the ancestor
node and we no longer need to handle that condition in the loop.

CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2

Review-Url: https://codereview.chromium.org/2623823002
Cr-Commit-Position: refs/heads/master@{#443436}
Committed: https://chromium.googlesource.com/chromium/src/+/2bd97cb39b5e3f7371b77f7f6e9c48e699aa3cc5

[Xianzhu, 2017-01-10 03:13:21]

Description was changed from

==========
Rename and change parameter type of some GeometryMapper methods

Rename
mapRectToDestinationSpace to sourceToDestinationRect
mapToVisualRectInDestinationSpace to sourceToDestinationVisualRect
localToVisualRectInAncestorSpace to localToAncestorVisualRect
so that all mapping methods are in the form
 
(local|source|ancestor)To(local|source|ancestor)(Rect|VisualRect|Matrix|ClipRect)

Changed parameters to localToAncestorMatrix and getPrecomputedDataForAncestor()
from const PropertyTreeState& to const TransformPaintPropertyNode*
because some later changes will require the new form (
TransformationPaintPropertyNode without a state).
==========

to

==========
Rename and change parameter type of some GeometryMapper methods

Rename
mapRectToDestinationSpace to sourceToDestinationRect
mapToVisualRectInDestinationSpace to sourceToDestinationVisualRect
localToVisualRectInAncestorSpace to localToAncestorVisualRect
so that all mapping methods are in the form
 
(local|source|ancestor)To(local|source|ancestor)(Rect|VisualRect|Matrix|ClipRect)

Changed parameters to localToAncestorMatrix and getPrecomputedDataForAncestor()
from const PropertyTreeState& to const TransformPaintPropertyNode*
because some later changes will require the new form (
TransformationPaintPropertyNode without a state).
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2
==========

[Xianzhu, 2017-01-10 03:13:22]

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

[commit-bot: I haz the power, 2017-01-10 03:14:25]

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, 2017-01-10 05:15:58]

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

[commit-bot: I haz the power, 2017-01-10 05:15:59]

Dry run: Try jobs failed on following builders:
  linux_layout_tests_slimming_paint_v2 on master.tryserver.chromium.linux
(JOB_TIMED_OUT, no build URL)

[Xianzhu, 2017-01-10 17:11:58]

wangxianzhu@chromium.org changed reviewers:
+ chrishtr@chromium.org

[Xianzhu, 2017-01-10 17:11:58]

I'm not sure if the new names are good. Wdyt?

[Xianzhu, 2017-01-10 17:13:01]

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

[commit-bot: I haz the power, 2017-01-10 17:13:21]

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, 2017-01-10 19:08:34]

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

[commit-bot: I haz the power, 2017-01-10 19:08:35]

Dry run: This issue passed the CQ dry run.

[Xianzhu, 2017-01-10 19:22:16]

Description was changed from

==========
Rename and change parameter type of some GeometryMapper methods

Rename
mapRectToDestinationSpace to sourceToDestinationRect
mapToVisualRectInDestinationSpace to sourceToDestinationVisualRect
localToVisualRectInAncestorSpace to localToAncestorVisualRect
so that all mapping methods are in the form
 
(local|source|ancestor)To(local|source|ancestor)(Rect|VisualRect|Matrix|ClipRect)

Changed parameters to localToAncestorMatrix and getPrecomputedDataForAncestor()
from const PropertyTreeState& to const TransformPaintPropertyNode*
because some later changes will require the new form (
TransformationPaintPropertyNode without a state).
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2
==========

to

==========
Rename, simplify and change parameter type of some GeometryMapper methods

Rename
mapRectToDestinationSpace to sourceToDestinationRect
mapToVisualRectInDestinationSpace to sourceToDestinationVisualRect
localToVisualRectInAncestorSpace to localToAncestorVisualRect
so that all mapping methods are in the form
 
(local|source|ancestor)To(local|source|ancestor)(Rect|VisualRect|Matrix|ClipRect)

Changed parameters to localToAncestorMatrix and getPrecomputedDataForAncestor()
from const PropertyTreeState& to const TransformPaintPropertyNode*
because some later changes will require the new form (
TransformationPaintPropertyNode without a state).

Simplified localToAncestorMatrix and localToAncestorClipRect because
we have early returns when the local node is the same as the ancestor
node and we no longer need to handle that condition in the loop.

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

[Xianzhu, 2017-01-10 20:18:20]

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

[commit-bot: I haz the power, 2017-01-10 20:19:00]

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

[Xianzhu, 2017-01-10 20:31:37]

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

[commit-bot: I haz the power, 2017-01-10 20:32:31]

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

[Xianzhu, 2017-01-10 20:41:20]

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

[commit-bot: I haz the power, 2017-01-10 20:41:32]

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, 2017-01-10 22:05:09]

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

[commit-bot: I haz the power, 2017-01-10 22:05:10]

Dry run: Try jobs failed on following builders:
  android_n5x_swarming_rel on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/android_n5x_...)

[chrishtr, 2017-01-11 22:23:08]

https://codereview.chromium.org/2623823002/diff/100001/third_party/WebKit/Sou...
File third_party/WebKit/Source/platform/graphics/paint/GeometryMapper.h (right):

https://codereview.chromium.org/2623823002/diff/100001/third_party/WebKit/Sou...
third_party/WebKit/Source/platform/graphics/paint/GeometryMapper.h:73: const
TransformPaintPropertyNode* sourceTransformNode,
Curious: what is the new call site you're planning to add?

[Xianzhu, 2017-01-11 22:51:33]

https://codereview.chromium.org/2623823002/diff/100001/third_party/WebKit/Sou...
File third_party/WebKit/Source/platform/graphics/paint/GeometryMapper.h (right):

https://codereview.chromium.org/2623823002/diff/100001/third_party/WebKit/Sou...
third_party/WebKit/Source/platform/graphics/paint/GeometryMapper.h:73: const
TransformPaintPropertyNode* sourceTransformNode,
On 2017/01/11 22:23:08, chrishtr wrote:
> Curious: what is the new call site you're planning to add?

I will use this for handle geometry effects of filters for crbug.com/637313 (WIP
CL https://codereview.chromium.org/2625133003). This is used to map a rect from
localTransformSpace of one effect node to that of another.

Actually we already have similar usage when handling clipping. Previously we
created a temporary PropertyTreeState to meet the requirement of this method.

I think this change also makes it clear that only transform nodes are used in
this method.

[chrishtr, 2017-01-11 23:23:10]

https://codereview.chromium.org/2623823002/diff/100001/third_party/WebKit/Sou...
File third_party/WebKit/Source/platform/graphics/paint/GeometryMapper.cpp
(left):

https://codereview.chromium.org/2623823002/diff/100001/third_party/WebKit/Sou...
third_party/WebKit/Source/platform/graphics/paint/GeometryMapper.cpp:57:
FloatRect result = localToAncestorRect(rect, sourceState, lcaState, success);
Why remove this?

https://codereview.chromium.org/2623823002/diff/100001/third_party/WebKit/Sou...
third_party/WebKit/Source/platform/graphics/paint/GeometryMapper.cpp:111: } else
{
Why remove this?

https://codereview.chromium.org/2623823002/diff/100001/third_party/WebKit/Sou...
File third_party/WebKit/Source/platform/graphics/paint/GeometryMapper.cpp
(right):

https://codereview.chromium.org/2623823002/diff/100001/third_party/WebKit/Sou...
third_party/WebKit/Source/platform/graphics/paint/GeometryMapper.cpp:190: if
(!clipNode) {
What about if clipNode == ancestorState.clip()?

[Xianzhu, 2017-01-11 23:54:43]

https://codereview.chromium.org/2623823002/diff/100001/third_party/WebKit/Sou...
File third_party/WebKit/Source/platform/graphics/paint/GeometryMapper.cpp
(left):

https://codereview.chromium.org/2623823002/diff/100001/third_party/WebKit/Sou...
third_party/WebKit/Source/platform/graphics/paint/GeometryMapper.cpp:57:
FloatRect result = localToAncestorRect(rect, sourceState, lcaState, success);
On 2017/01/11 23:23:09, chrishtr wrote:
> Why remove this?

Line 53-59 now becomes a call to localToAncestorVisualRect() which is almost
equivalent to the original code. One difference is about the special handling of
'success' in localToAncestorVisualRect() which is actually also needed here for
spv1.

https://codereview.chromium.org/2623823002/diff/100001/third_party/WebKit/Sou...
third_party/WebKit/Source/platform/graphics/paint/GeometryMapper.cpp:111: } else
{
On 2017/01/11 23:23:09, chrishtr wrote:
> Why remove this?

Now slowSourceToDestinationVisualRect (previously
slowMapToVisualRectInDestinationSpace) calls this function. Some unit tests
(e.g. GeometryMapperTest, SiblingTransformsWithClip) expect success==false
instead of DCHECK failure here.

https://codereview.chromium.org/2623823002/diff/100001/third_party/WebKit/Sou...
File third_party/WebKit/Source/platform/graphics/paint/GeometryMapper.cpp
(right):

https://codereview.chromium.org/2623823002/diff/100001/third_party/WebKit/Sou...
third_party/WebKit/Source/platform/graphics/paint/GeometryMapper.cpp:190: if
(!clipNode) {
On 2017/01/11 23:23:09, chrishtr wrote:
> What about if clipNode == ancestorState.clip()?

In the case we'll execute the loop below, the same as before. (!clipNode) is
equivalent to the original (clipNode != ancestorState.clip() && !found).

These changes are possible because now we early return if localState.clip() ==
ancestorState.clip(). Previously the two loops needed to handle the case by
putting infinite clip rect into the map and returning the entry from the map.

[chrishtr, 2017-01-12 18:07:12]

https://codereview.chromium.org/2623823002/diff/100001/third_party/WebKit/Sou...
File third_party/WebKit/Source/platform/graphics/paint/GeometryMapper.cpp
(left):

https://codereview.chromium.org/2623823002/diff/100001/third_party/WebKit/Sou...
third_party/WebKit/Source/platform/graphics/paint/GeometryMapper.cpp:111: } else
{
On 2017/01/11 at 23:54:43, Xianzhu wrote:
> On 2017/01/11 23:23:09, chrishtr wrote:
> > Why remove this?
> 
> Now slowSourceToDestinationVisualRect (previously
slowMapToVisualRectInDestinationSpace) calls this function. Some unit tests
(e.g. GeometryMapperTest, SiblingTransformsWithClip) expect success==false
instead of DCHECK failure here.

The DCHECK is to ensure that it's actually an ancestor. Are the unittests at
fault here? Is the assumption that there is no use case
for failure in SPv2 incorrect?

[Xianzhu, 2017-01-12 18:39:17]

https://codereview.chromium.org/2623823002/diff/100001/third_party/WebKit/Sou...
File third_party/WebKit/Source/platform/graphics/paint/GeometryMapper.cpp
(left):

https://codereview.chromium.org/2623823002/diff/100001/third_party/WebKit/Sou...
third_party/WebKit/Source/platform/graphics/paint/GeometryMapper.cpp:111: } else
{
On 2017/01/12 18:07:12, chrishtr wrote:
> On 2017/01/11 at 23:54:43, Xianzhu wrote:
> > On 2017/01/11 23:23:09, chrishtr wrote:
> > > Why remove this?
> > 
> > Now slowSourceToDestinationVisualRect (previously
> slowMapToVisualRectInDestinationSpace) calls this function. Some unit tests
> (e.g. GeometryMapperTest, SiblingTransformsWithClip) expect success==false
> instead of DCHECK failure here.
> 
> The DCHECK is to ensure that it's actually an ancestor. Are the unittests at
> fault here? Is the assumption that there is no use case
> for failure in SPv2 incorrect?

No DCHECK(success) here is mainly a requirement of
slowSourceToDestinationVisualRect() because ancestorState here contains
lcaTransform and destinationState.clip(), and destinationState.clip() may not be
an ancestor of localState.clip().

Is the |success| parameter just for testing and nested calls from other
GeometryMapper methods? If yes, can we add several internal methods having the
parameter, and remove the parameter from public methods which will
DCHECK(success) in them?

[chrishtr, 2017-01-12 18:46:36]

On 2017/01/12 at 18:39:17, wangxianzhu wrote:
>
https://codereview.chromium.org/2623823002/diff/100001/third_party/WebKit/Sou...
> File third_party/WebKit/Source/platform/graphics/paint/GeometryMapper.cpp
(left):
> 
>
https://codereview.chromium.org/2623823002/diff/100001/third_party/WebKit/Sou...
> third_party/WebKit/Source/platform/graphics/paint/GeometryMapper.cpp:111: }
else {
> On 2017/01/12 18:07:12, chrishtr wrote:
> > On 2017/01/11 at 23:54:43, Xianzhu wrote:
> > > On 2017/01/11 23:23:09, chrishtr wrote:
> > > > Why remove this?
> > > 
> > > Now slowSourceToDestinationVisualRect (previously
> > slowMapToVisualRectInDestinationSpace) calls this function. Some unit tests
> > (e.g. GeometryMapperTest, SiblingTransformsWithClip) expect success==false
> > instead of DCHECK failure here.
> > 
> > The DCHECK is to ensure that it's actually an ancestor. Are the unittests at
> > fault here? Is the assumption that there is no use case
> > for failure in SPv2 incorrect?
> 
> No DCHECK(success) here is mainly a requirement of
slowSourceToDestinationVisualRect() because ancestorState here contains
lcaTransform and destinationState.clip(), and destinationState.clip() may not be
an ancestor of localState.clip().
> 
> Is the |success| parameter just for testing and nested calls from other
GeometryMapper methods? If yes, can we add several internal methods having the
parameter, and remove the parameter from public methods which will
DCHECK(success) in them?

Yes it is. I think we always DCHECK(success) from external code. I like your
suggestion.

[Xianzhu, 2017-01-12 19:13:17]

On 2017/01/12 18:46:36, chrishtr wrote:
> On 2017/01/12 at 18:39:17, wangxianzhu wrote:
> >
>
https://codereview.chromium.org/2623823002/diff/100001/third_party/WebKit/Sou...
> > File third_party/WebKit/Source/platform/graphics/paint/GeometryMapper.cpp
> (left):
> > 
> >
>
https://codereview.chromium.org/2623823002/diff/100001/third_party/WebKit/Sou...
> > third_party/WebKit/Source/platform/graphics/paint/GeometryMapper.cpp:111: }
> else {
> > On 2017/01/12 18:07:12, chrishtr wrote:
> > > On 2017/01/11 at 23:54:43, Xianzhu wrote:
> > > > On 2017/01/11 23:23:09, chrishtr wrote:
> > > > > Why remove this?
> > > > 
> > > > Now slowSourceToDestinationVisualRect (previously
> > > slowMapToVisualRectInDestinationSpace) calls this function. Some unit
tests
> > > (e.g. GeometryMapperTest, SiblingTransformsWithClip) expect success==false
> > > instead of DCHECK failure here.
> > > 
> > > The DCHECK is to ensure that it's actually an ancestor. Are the unittests
at
> > > fault here? Is the assumption that there is no use case
> > > for failure in SPv2 incorrect?
> > 
> > No DCHECK(success) here is mainly a requirement of
> slowSourceToDestinationVisualRect() because ancestorState here contains
> lcaTransform and destinationState.clip(), and destinationState.clip() may not
be
> an ancestor of localState.clip().
> > 
> > Is the |success| parameter just for testing and nested calls from other
> GeometryMapper methods? If yes, can we add several internal methods having the
> parameter, and remove the parameter from public methods which will
> DCHECK(success) in them?
> 
> Yes it is. I think we always DCHECK(success) from external code. I like your
> suggestion.

Great. Would you mind if I address this in a follow-up?

[chrishtr, 2017-01-12 19:14:48]

On 2017/01/12 at 19:13:17, wangxianzhu wrote:
> On 2017/01/12 18:46:36, chrishtr wrote:
> > On 2017/01/12 at 18:39:17, wangxianzhu wrote:
> > >
> >
https://codereview.chromium.org/2623823002/diff/100001/third_party/WebKit/Sou...
> > > File third_party/WebKit/Source/platform/graphics/paint/GeometryMapper.cpp
> > (left):
> > > 
> > >
> >
https://codereview.chromium.org/2623823002/diff/100001/third_party/WebKit/Sou...
> > > third_party/WebKit/Source/platform/graphics/paint/GeometryMapper.cpp:111:
}
> > else {
> > > On 2017/01/12 18:07:12, chrishtr wrote:
> > > > On 2017/01/11 at 23:54:43, Xianzhu wrote:
> > > > > On 2017/01/11 23:23:09, chrishtr wrote:
> > > > > > Why remove this?
> > > > > 
> > > > > Now slowSourceToDestinationVisualRect (previously
> > > > slowMapToVisualRectInDestinationSpace) calls this function. Some unit
tests
> > > > (e.g. GeometryMapperTest, SiblingTransformsWithClip) expect
success==false
> > > > instead of DCHECK failure here.
> > > > 
> > > > The DCHECK is to ensure that it's actually an ancestor. Are the
unittests at
> > > > fault here? Is the assumption that there is no use case
> > > > for failure in SPv2 incorrect?
> > > 
> > > No DCHECK(success) here is mainly a requirement of
> > slowSourceToDestinationVisualRect() because ancestorState here contains
> > lcaTransform and destinationState.clip(), and destinationState.clip() may
not be
> > an ancestor of localState.clip().
> > > 
> > > Is the |success| parameter just for testing and nested calls from other
> > GeometryMapper methods? If yes, can we add several internal methods having
the
> > parameter, and remove the parameter from public methods which will
> > DCHECK(success) in them?
> > 
> > Yes it is. I think we always DCHECK(success) from external code. I like your
> > suggestion.
> 
> Great. Would you mind if I address this in a follow-up?

Sure. But my earlier comment/question about the unittests still stands. Is there
something
broken about them or a lack of our understanding of use cases?

[Xianzhu, 2017-01-12 19:28:56]

On 2017/01/12 19:14:48, chrishtr wrote:
> On 2017/01/12 at 19:13:17, wangxianzhu wrote:
> > On 2017/01/12 18:46:36, chrishtr wrote:
> > > On 2017/01/12 at 18:39:17, wangxianzhu wrote:
> > > >
> > >
>
https://codereview.chromium.org/2623823002/diff/100001/third_party/WebKit/Sou...
> > > > File
third_party/WebKit/Source/platform/graphics/paint/GeometryMapper.cpp
> > > (left):
> > > > 
> > > >
> > >
>
https://codereview.chromium.org/2623823002/diff/100001/third_party/WebKit/Sou...
> > > >
third_party/WebKit/Source/platform/graphics/paint/GeometryMapper.cpp:111:
> }
> > > else {
> > > > On 2017/01/12 18:07:12, chrishtr wrote:
> > > > > On 2017/01/11 at 23:54:43, Xianzhu wrote:
> > > > > > On 2017/01/11 23:23:09, chrishtr wrote:
> > > > > > > Why remove this?
> > > > > > 
> > > > > > Now slowSourceToDestinationVisualRect (previously
> > > > > slowMapToVisualRectInDestinationSpace) calls this function. Some unit
> tests
> > > > > (e.g. GeometryMapperTest, SiblingTransformsWithClip) expect
> success==false
> > > > > instead of DCHECK failure here.
> > > > > 
> > > > > The DCHECK is to ensure that it's actually an ancestor. Are the
> unittests at
> > > > > fault here? Is the assumption that there is no use case
> > > > > for failure in SPv2 incorrect?
> > > > 
> > > > No DCHECK(success) here is mainly a requirement of
> > > slowSourceToDestinationVisualRect() because ancestorState here contains
> > > lcaTransform and destinationState.clip(), and destinationState.clip() may
> not be
> > > an ancestor of localState.clip().
> > > > 
> > > > Is the |success| parameter just for testing and nested calls from other
> > > GeometryMapper methods? If yes, can we add several internal methods having
> the
> > > parameter, and remove the parameter from public methods which will
> > > DCHECK(success) in them?
> > > 
> > > Yes it is. I think we always DCHECK(success) from external code. I like
your
> > > suggestion.
> > 
> > Great. Would you mind if I address this in a follow-up?
> 
> Sure. But my earlier comment/question about the unittests still stands. Is
there
> something
> broken about them or a lack of our understanding of use cases?

The unit tests call GeometryMapper with abnormal parameters and expect failures.
I'm not sure if the cases in the unit tests are realistic for real world of
spv2, but I think they are useful because they cover the code paths in
GeometryMapper checking for failures. The follow-up CL will let the unit tests
call internal methods with the success parameter.

[chrishtr, 2017-01-12 20:42:00]

The CQ bit was checked by chrishtr@chromium.org

[chrishtr, 2017-01-12 20:42:00]

lgtm

Sounds good, thanks.

[commit-bot: I haz the power, 2017-01-12 20:42:58]

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, 2017-01-13 01:29:53]

CQ is committing da patch.

Bot data: {"patchset_id": 100001, "attempt_start_ts": 1484253720075090,
"parent_rev": "e7225c311f5be2450e84dc6f8e0548a699787bb3", "commit_rev":
"2bd97cb39b5e3f7371b77f7f6e9c48e699aa3cc5"}

[commit-bot: I haz the power, 2017-01-13 01:30:48]

Description was changed from

==========
Rename, simplify and change parameter type of some GeometryMapper methods

Rename
mapRectToDestinationSpace to sourceToDestinationRect
mapToVisualRectInDestinationSpace to sourceToDestinationVisualRect
localToVisualRectInAncestorSpace to localToAncestorVisualRect
so that all mapping methods are in the form
 
(local|source|ancestor)To(local|source|ancestor)(Rect|VisualRect|Matrix|ClipRect)

Changed parameters to localToAncestorMatrix and getPrecomputedDataForAncestor()
from const PropertyTreeState& to const TransformPaintPropertyNode*
because some later changes will require the new form (
TransformationPaintPropertyNode without a state).

Simplified localToAncestorMatrix and localToAncestorClipRect because
we have early returns when the local node is the same as the ancestor
node and we no longer need to handle that condition in the loop.

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

to

==========
Rename, simplify and change parameter type of some GeometryMapper methods

Rename
mapRectToDestinationSpace to sourceToDestinationRect
mapToVisualRectInDestinationSpace to sourceToDestinationVisualRect
localToVisualRectInAncestorSpace to localToAncestorVisualRect
so that all mapping methods are in the form
 
(local|source|ancestor)To(local|source|ancestor)(Rect|VisualRect|Matrix|ClipRect)

Changed parameters to localToAncestorMatrix and getPrecomputedDataForAncestor()
from const PropertyTreeState& to const TransformPaintPropertyNode*
because some later changes will require the new form (
TransformationPaintPropertyNode without a state).

Simplified localToAncestorMatrix and localToAncestorClipRect because
we have early returns when the local node is the same as the ancestor
node and we no longer need to handle that condition in the loop.

CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2

Review-Url: https://codereview.chromium.org/2623823002
Cr-Commit-Position: refs/heads/master@{#443436}
Committed:
https://chromium.googlesource.com/chromium/src/+/2bd97cb39b5e3f7371b77f7f6e9c...
==========

[commit-bot: I haz the power, 2017-01-13 01:30:49]

Committed patchset #6 (id:100001) as
https://chromium.googlesource.com/chromium/src/+/2bd97cb39b5e3f7371b77f7f6e9c...