Reland "Let SVGForeignObject's local SVG coordinates mean what it should"

Reland "Let SVGForeignObject's local SVG coordinates mean what it should" (patchset #2 id:20001 of https://codereview.chromium.org/2767343003/ )

Fix wrong direction of mapping in LayoutSVGBlock::mapAncestorToLocal().

> Revert: "Let SVGForeignObject's local SVG coordinates mean what it should"
> 
> This patch caused a hit testing regression with foreign objects. While
> the fix is not hard, we are going to roll out as a precaution (and merge
> to stable).
> 
> This is a manual revert of https://crrev.com/f451d9b5de5a1b4691d60d8c74341ac8a466cf57
> (commit number #435337) due to revert conflicts.
> 
> BUG=704643
> 
> Review-Url: https://codereview.chromium.org/2767343003
> Cr-Commit-Position: refs/heads/master@{#459330}
> Committed: https://chromium.googlesource.com/chromium/src/+/0cfb512a4ae94e681bd10a15d93abd740f330e3e

> Original issue's description:
>> Let SVGForeignObject's local SVG coordinates mean what it should
>> 
>> We define "local SVG coordinates" as the space where SVG local transform
>> etc. apply. With this definition, SVGForeignObject's localSVGTransform
>> and localToSVGParentTransform should be the same which should both
>> transform from local SVG coordinates instead of local HTML coordinates
>> to parent SVG coordinates, and visualRectInLocalSVGCoordinates() should
>> return the visual rect in local SVG coordinates instead of local HTML
>> coordinates.
>> 
>> With the above coordinates fixed, LayoutSVGBlock becomes the same for
>> LayoutSVGText and LayoutSVGForeignObject regarding to SVG/HTML
>> coordinates: SVG coordinates = HTML coordinates + location() (though
>> location() is always zero for LayoutSVGText after
>> https://codereview.chromium.org/2531943002/). The HTML coordinate
>> mapping methods should also map between the HTML and SVG coordinates.
>> 
>> Review-Url: https://codereview.chromium.org/2536493002
>> Cr-Commit-Position: refs/heads/master@{#435337}

BUG=704643, 666416
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2

Review-Url: https://codereview.chromium.org/2778873002
Cr-Commit-Position: refs/heads/master@{#460209}
Committed: https://chromium.googlesource.com/chromium/src/+/09a6cd640fb5bf07aa06477362229c2e6969a718

[Xianzhu, 2017-03-27 20:27:02]

Created Revert of Revert: "Let SVGForeignObject's local SVG coordinates mean
what it should"

[Xianzhu, 2017-03-27 20:27:29]

Description was changed from

==========
Revert of Revert: "Let SVGForeignObject's local SVG coordinates mean what it
should" (patchset #2 id:20001 of https://codereview.chromium.org/2767343003/ )

Reason for revert:
Just to create the initial version of reland. 

Original issue's description:
> Revert: "Let SVGForeignObject's local SVG coordinates mean what it should"
> 
> This patch caused a hit testing regression with foreign objects. While
> the fix is not hard, we are going to roll out as a precaution (and merge
> to stable).
> 
> This is a manual revert of
https://crrev.com/f451d9b5de5a1b4691d60d8c74341ac8a466cf57
> (commit number #435337) due to revert conflicts.
> 
> BUG=704643
> 
> Review-Url: https://codereview.chromium.org/2767343003
> Cr-Commit-Position: refs/heads/master@{#459330}
> Committed:
https://chromium.googlesource.com/chromium/src/+/0cfb512a4ae94e681bd10a15d93a...

TBR=pdr@chromium.org
# Not skipping CQ checks because original CL landed more than 1 days ago.
BUG=704643
==========

to

==========
Reland "Let SVGForeignObject's local SVG coordinates mean what it should"
(patchset #2 id:20001 of https://codereview.chromium.org/2767343003/ )

Original issue's description:
> Revert: "Let SVGForeignObject's local SVG coordinates mean what it should"
> 
> This patch caused a hit testing regression with foreign objects. While
> the fix is not hard, we are going to roll out as a precaution (and merge
> to stable).
> 
> This is a manual revert of
https://crrev.com/f451d9b5de5a1b4691d60d8c74341ac8a466cf57
> (commit number #435337) due to revert conflicts.
> 
> BUG=704643
> 
> Review-Url: https://codereview.chromium.org/2767343003
> Cr-Commit-Position: refs/heads/master@{#459330}
> Committed:
https://chromium.googlesource.com/chromium/src/+/0cfb512a4ae94e681bd10a15d93a...

TBR=pdr@chromium.org
# Not skipping CQ checks because original CL landed more than 1 days ago.
BUG=704643
==========

[Xianzhu, 2017-03-28 17:27:08]

Description was changed from

==========
Reland "Let SVGForeignObject's local SVG coordinates mean what it should"
(patchset #2 id:20001 of https://codereview.chromium.org/2767343003/ )

Original issue's description:
> Revert: "Let SVGForeignObject's local SVG coordinates mean what it should"
> 
> This patch caused a hit testing regression with foreign objects. While
> the fix is not hard, we are going to roll out as a precaution (and merge
> to stable).
> 
> This is a manual revert of
https://crrev.com/f451d9b5de5a1b4691d60d8c74341ac8a466cf57
> (commit number #435337) due to revert conflicts.
> 
> BUG=704643
> 
> Review-Url: https://codereview.chromium.org/2767343003
> Cr-Commit-Position: refs/heads/master@{#459330}
> Committed:
https://chromium.googlesource.com/chromium/src/+/0cfb512a4ae94e681bd10a15d93a...

TBR=pdr@chromium.org
# Not skipping CQ checks because original CL landed more than 1 days ago.
BUG=704643
==========

to

==========
Reland "Let SVGForeignObject's local SVG coordinates mean what it should"
(patchset #2 id:20001 of https://codereview.chromium.org/2767343003/ )

Original issue's description:
> Revert: "Let SVGForeignObject's local SVG coordinates mean what it should"
> 
> This patch caused a hit testing regression with foreign objects. While
> the fix is not hard, we are going to roll out as a precaution (and merge
> to stable).
> 
> This is a manual revert of
https://crrev.com/f451d9b5de5a1b4691d60d8c74341ac8a466cf57
> (commit number #435337) due to revert conflicts.
> 
> BUG=704643
> 
> Review-Url: https://codereview.chromium.org/2767343003
> Cr-Commit-Position: refs/heads/master@{#459330}
> Committed:
https://chromium.googlesource.com/chromium/src/+/0cfb512a4ae94e681bd10a15d93a...

TBR=pdr@chromium.org
# Not skipping CQ checks because original CL landed more than 1 days ago.
BUG=704643
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2
==========

[Xianzhu, 2017-03-28 17:29:21]

Description was changed from

==========
Reland "Let SVGForeignObject's local SVG coordinates mean what it should"
(patchset #2 id:20001 of https://codereview.chromium.org/2767343003/ )

Original issue's description:
> Revert: "Let SVGForeignObject's local SVG coordinates mean what it should"
> 
> This patch caused a hit testing regression with foreign objects. While
> the fix is not hard, we are going to roll out as a precaution (and merge
> to stable).
> 
> This is a manual revert of
https://crrev.com/f451d9b5de5a1b4691d60d8c74341ac8a466cf57
> (commit number #435337) due to revert conflicts.
> 
> BUG=704643
> 
> Review-Url: https://codereview.chromium.org/2767343003
> Cr-Commit-Position: refs/heads/master@{#459330}
> Committed:
https://chromium.googlesource.com/chromium/src/+/0cfb512a4ae94e681bd10a15d93a...

TBR=pdr@chromium.org
# Not skipping CQ checks because original CL landed more than 1 days ago.
BUG=704643
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2
==========

to

==========
Reland "Let SVGForeignObject's local SVG coordinates mean what it should"
(patchset #2 id:20001 of https://codereview.chromium.org/2767343003/ )

Fix wrong direction of mapping in LayoutSVGBlock::mapAncestorToLocal().

> Revert: "Let SVGForeignObject's local SVG coordinates mean what it should"
> 
> This patch caused a hit testing regression with foreign objects. While
> the fix is not hard, we are going to roll out as a precaution (and merge
> to stable).
> 
> This is a manual revert of
https://crrev.com/f451d9b5de5a1b4691d60d8c74341ac8a466cf57
> (commit number #435337) due to revert conflicts.
> 
> BUG=704643
> 
> Review-Url: https://codereview.chromium.org/2767343003
> Cr-Commit-Position: refs/heads/master@{#459330}
> Committed:
https://chromium.googlesource.com/chromium/src/+/0cfb512a4ae94e681bd10a15d93a...

> Original issue's description:
>> Let SVGForeignObject's local SVG coordinates mean what it should
>> 
>> We define "local SVG coordinates" as the space where SVG local transform
>> etc. apply. With this definition, SVGForeignObject's localSVGTransform
>> and localToSVGParentTransform should be the same which should both
>> transform from local SVG coordinates instead of local HTML coordinates
>> to parent SVG coordinates, and visualRectInLocalSVGCoordinates() should
>> return the visual rect in local SVG coordinates instead of local HTML
>> coordinates.
>> 
>> With the above coordinates fixed, LayoutSVGBlock becomes the same for
>> LayoutSVGText and LayoutSVGForeignObject regarding to SVG/HTML
>> coordinates: SVG coordinates = HTML coordinates + location() (though
>> location() is always zero for LayoutSVGText after
>> https://codereview.chromium.org/2531943002/). The HTML coordinate
>> mapping methods should also map between the HTML and SVG coordinates.

BUG=704643,666416
==========

[Xianzhu, 2017-03-28 17:30:04]

Description was changed from

==========
Reland "Let SVGForeignObject's local SVG coordinates mean what it should"
(patchset #2 id:20001 of https://codereview.chromium.org/2767343003/ )

Fix wrong direction of mapping in LayoutSVGBlock::mapAncestorToLocal().

> Revert: "Let SVGForeignObject's local SVG coordinates mean what it should"
> 
> This patch caused a hit testing regression with foreign objects. While
> the fix is not hard, we are going to roll out as a precaution (and merge
> to stable).
> 
> This is a manual revert of
https://crrev.com/f451d9b5de5a1b4691d60d8c74341ac8a466cf57
> (commit number #435337) due to revert conflicts.
> 
> BUG=704643
> 
> Review-Url: https://codereview.chromium.org/2767343003
> Cr-Commit-Position: refs/heads/master@{#459330}
> Committed:
https://chromium.googlesource.com/chromium/src/+/0cfb512a4ae94e681bd10a15d93a...

> Original issue's description:
>> Let SVGForeignObject's local SVG coordinates mean what it should
>> 
>> We define "local SVG coordinates" as the space where SVG local transform
>> etc. apply. With this definition, SVGForeignObject's localSVGTransform
>> and localToSVGParentTransform should be the same which should both
>> transform from local SVG coordinates instead of local HTML coordinates
>> to parent SVG coordinates, and visualRectInLocalSVGCoordinates() should
>> return the visual rect in local SVG coordinates instead of local HTML
>> coordinates.
>> 
>> With the above coordinates fixed, LayoutSVGBlock becomes the same for
>> LayoutSVGText and LayoutSVGForeignObject regarding to SVG/HTML
>> coordinates: SVG coordinates = HTML coordinates + location() (though
>> location() is always zero for LayoutSVGText after
>> https://codereview.chromium.org/2531943002/). The HTML coordinate
>> mapping methods should also map between the HTML and SVG coordinates.

BUG=704643,666416
==========

to

==========
Reland "Let SVGForeignObject's local SVG coordinates mean what it should"
(patchset #2 id:20001 of https://codereview.chromium.org/2767343003/ )

Fix wrong direction of mapping in LayoutSVGBlock::mapAncestorToLocal().

> Revert: "Let SVGForeignObject's local SVG coordinates mean what it should"
> 
> This patch caused a hit testing regression with foreign objects. While
> the fix is not hard, we are going to roll out as a precaution (and merge
> to stable).
> 
> This is a manual revert of
https://crrev.com/f451d9b5de5a1b4691d60d8c74341ac8a466cf57
> (commit number #435337) due to revert conflicts.
> 
> BUG=704643
> 
> Review-Url: https://codereview.chromium.org/2767343003
> Cr-Commit-Position: refs/heads/master@{#459330}
> Committed:
https://chromium.googlesource.com/chromium/src/+/0cfb512a4ae94e681bd10a15d93a...

> Original issue's description:
>> Let SVGForeignObject's local SVG coordinates mean what it should
>> 
>> We define "local SVG coordinates" as the space where SVG local transform
>> etc. apply. With this definition, SVGForeignObject's localSVGTransform
>> and localToSVGParentTransform should be the same which should both
>> transform from local SVG coordinates instead of local HTML coordinates
>> to parent SVG coordinates, and visualRectInLocalSVGCoordinates() should
>> return the visual rect in local SVG coordinates instead of local HTML
>> coordinates.
>> 
>> With the above coordinates fixed, LayoutSVGBlock becomes the same for
>> LayoutSVGText and LayoutSVGForeignObject regarding to SVG/HTML
>> coordinates: SVG coordinates = HTML coordinates + location() (though
>> location() is always zero for LayoutSVGText after
>> https://codereview.chromium.org/2531943002/). The HTML coordinate
>> mapping methods should also map between the HTML and SVG coordinates.
>> 
>> Review-Url: https://codereview.chromium.org/2536493002
>> Cr-Commit-Position: refs/heads/master@{#435337}

BUG=704643,666416
==========

[Xianzhu, 2017-03-28 17:37:03]

Description was changed from

==========
Reland "Let SVGForeignObject's local SVG coordinates mean what it should"
(patchset #2 id:20001 of https://codereview.chromium.org/2767343003/ )

Fix wrong direction of mapping in LayoutSVGBlock::mapAncestorToLocal().

> Revert: "Let SVGForeignObject's local SVG coordinates mean what it should"
> 
> This patch caused a hit testing regression with foreign objects. While
> the fix is not hard, we are going to roll out as a precaution (and merge
> to stable).
> 
> This is a manual revert of
https://crrev.com/f451d9b5de5a1b4691d60d8c74341ac8a466cf57
> (commit number #435337) due to revert conflicts.
> 
> BUG=704643
> 
> Review-Url: https://codereview.chromium.org/2767343003
> Cr-Commit-Position: refs/heads/master@{#459330}
> Committed:
https://chromium.googlesource.com/chromium/src/+/0cfb512a4ae94e681bd10a15d93a...

> Original issue's description:
>> Let SVGForeignObject's local SVG coordinates mean what it should
>> 
>> We define "local SVG coordinates" as the space where SVG local transform
>> etc. apply. With this definition, SVGForeignObject's localSVGTransform
>> and localToSVGParentTransform should be the same which should both
>> transform from local SVG coordinates instead of local HTML coordinates
>> to parent SVG coordinates, and visualRectInLocalSVGCoordinates() should
>> return the visual rect in local SVG coordinates instead of local HTML
>> coordinates.
>> 
>> With the above coordinates fixed, LayoutSVGBlock becomes the same for
>> LayoutSVGText and LayoutSVGForeignObject regarding to SVG/HTML
>> coordinates: SVG coordinates = HTML coordinates + location() (though
>> location() is always zero for LayoutSVGText after
>> https://codereview.chromium.org/2531943002/). The HTML coordinate
>> mapping methods should also map between the HTML and SVG coordinates.
>> 
>> Review-Url: https://codereview.chromium.org/2536493002
>> Cr-Commit-Position: refs/heads/master@{#435337}

BUG=704643,666416
==========

to

==========
Reland "Let SVGForeignObject's local SVG coordinates mean what it should"
(patchset #2 id:20001 of https://codereview.chromium.org/2767343003/ )

Fix wrong direction of mapping in LayoutSVGBlock::mapAncestorToLocal().

> Revert: "Let SVGForeignObject's local SVG coordinates mean what it should"
> 
> This patch caused a hit testing regression with foreign objects. While
> the fix is not hard, we are going to roll out as a precaution (and merge
> to stable).
> 
> This is a manual revert of
https://crrev.com/f451d9b5de5a1b4691d60d8c74341ac8a466cf57
> (commit number #435337) due to revert conflicts.
> 
> BUG=704643
> 
> Review-Url: https://codereview.chromium.org/2767343003
> Cr-Commit-Position: refs/heads/master@{#459330}
> Committed:
https://chromium.googlesource.com/chromium/src/+/0cfb512a4ae94e681bd10a15d93a...

> Original issue's description:
>> Let SVGForeignObject's local SVG coordinates mean what it should
>> 
>> We define "local SVG coordinates" as the space where SVG local transform
>> etc. apply. With this definition, SVGForeignObject's localSVGTransform
>> and localToSVGParentTransform should be the same which should both
>> transform from local SVG coordinates instead of local HTML coordinates
>> to parent SVG coordinates, and visualRectInLocalSVGCoordinates() should
>> return the visual rect in local SVG coordinates instead of local HTML
>> coordinates.
>> 
>> With the above coordinates fixed, LayoutSVGBlock becomes the same for
>> LayoutSVGText and LayoutSVGForeignObject regarding to SVG/HTML
>> coordinates: SVG coordinates = HTML coordinates + location() (though
>> location() is always zero for LayoutSVGText after
>> https://codereview.chromium.org/2531943002/). The HTML coordinate
>> mapping methods should also map between the HTML and SVG coordinates.
>> 
>> Review-Url: https://codereview.chromium.org/2536493002
>> Cr-Commit-Position: refs/heads/master@{#435337}

BUG=704643,666416
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2
==========

[Xianzhu, 2017-03-28 17:40:23]

Patchset #2 (id:150001) has been deleted

[Xianzhu, 2017-03-28 17:40:29]

Patchset #2 (id:170001) has been deleted

[Xianzhu, 2017-03-28 17:41:13]

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

[commit-bot: I haz the power, 2017-03-28 17:41:47]

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

[Xianzhu, 2017-03-28 17:45:09]

https://codereview.chromium.org/2778873002/diff/190001/third_party/WebKit/Sou...
File third_party/WebKit/Source/core/layout/svg/LayoutSVGBlock.cpp (right):

https://codereview.chromium.org/2778873002/diff/190001/third_party/WebKit/Sou...
third_party/WebKit/Source/core/layout/svg/LayoutSVGBlock.cpp:117:
transformState.move(locationOffset());
This is the fix. In the original patch, this was "-locationOffset()". The
TransformState is already in UnapplyInverseTransformDirection mode for
mapAncestorToLocal(), so we should just apply transforms and offsets in the
normal direction.

[pdr., 2017-03-28 17:47:25]

LGTM!

[Xianzhu, 2017-03-28 19:11:49]

The CQ bit was unchecked by wangxianzhu@chromium.org

[Xianzhu, 2017-03-28 19:11:52]

The CQ bit was checked by wangxianzhu@chromium.org

[commit-bot: I haz the power, 2017-03-28 19:12:20]

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-03-28 20:38:16]

CQ is committing da patch.

Bot data: {"patchset_id": 190001, "attempt_start_ts": 1490728312417150,
"parent_rev": "6264dc5bf2672ef8629a00a030a3713adbee6b87", "commit_rev":
"09a6cd640fb5bf07aa06477362229c2e6969a718"}

[commit-bot: I haz the power, 2017-03-28 20:39:38]

Description was changed from

==========
Reland "Let SVGForeignObject's local SVG coordinates mean what it should"
(patchset #2 id:20001 of https://codereview.chromium.org/2767343003/ )

Fix wrong direction of mapping in LayoutSVGBlock::mapAncestorToLocal().

> Revert: "Let SVGForeignObject's local SVG coordinates mean what it should"
> 
> This patch caused a hit testing regression with foreign objects. While
> the fix is not hard, we are going to roll out as a precaution (and merge
> to stable).
> 
> This is a manual revert of
https://crrev.com/f451d9b5de5a1b4691d60d8c74341ac8a466cf57
> (commit number #435337) due to revert conflicts.
> 
> BUG=704643
> 
> Review-Url: https://codereview.chromium.org/2767343003
> Cr-Commit-Position: refs/heads/master@{#459330}
> Committed:
https://chromium.googlesource.com/chromium/src/+/0cfb512a4ae94e681bd10a15d93a...

> Original issue's description:
>> Let SVGForeignObject's local SVG coordinates mean what it should
>> 
>> We define "local SVG coordinates" as the space where SVG local transform
>> etc. apply. With this definition, SVGForeignObject's localSVGTransform
>> and localToSVGParentTransform should be the same which should both
>> transform from local SVG coordinates instead of local HTML coordinates
>> to parent SVG coordinates, and visualRectInLocalSVGCoordinates() should
>> return the visual rect in local SVG coordinates instead of local HTML
>> coordinates.
>> 
>> With the above coordinates fixed, LayoutSVGBlock becomes the same for
>> LayoutSVGText and LayoutSVGForeignObject regarding to SVG/HTML
>> coordinates: SVG coordinates = HTML coordinates + location() (though
>> location() is always zero for LayoutSVGText after
>> https://codereview.chromium.org/2531943002/). The HTML coordinate
>> mapping methods should also map between the HTML and SVG coordinates.
>> 
>> Review-Url: https://codereview.chromium.org/2536493002
>> Cr-Commit-Position: refs/heads/master@{#435337}

BUG=704643,666416
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2
==========

to

==========
Reland "Let SVGForeignObject's local SVG coordinates mean what it should"
(patchset #2 id:20001 of https://codereview.chromium.org/2767343003/ )

Fix wrong direction of mapping in LayoutSVGBlock::mapAncestorToLocal().

> Revert: "Let SVGForeignObject's local SVG coordinates mean what it should"
> 
> This patch caused a hit testing regression with foreign objects. While
> the fix is not hard, we are going to roll out as a precaution (and merge
> to stable).
> 
> This is a manual revert of
https://crrev.com/f451d9b5de5a1b4691d60d8c74341ac8a466cf57
> (commit number #435337) due to revert conflicts.
> 
> BUG=704643
> 
> Review-Url: https://codereview.chromium.org/2767343003
> Cr-Commit-Position: refs/heads/master@{#459330}
> Committed:
https://chromium.googlesource.com/chromium/src/+/0cfb512a4ae94e681bd10a15d93a...

> Original issue's description:
>> Let SVGForeignObject's local SVG coordinates mean what it should
>> 
>> We define "local SVG coordinates" as the space where SVG local transform
>> etc. apply. With this definition, SVGForeignObject's localSVGTransform
>> and localToSVGParentTransform should be the same which should both
>> transform from local SVG coordinates instead of local HTML coordinates
>> to parent SVG coordinates, and visualRectInLocalSVGCoordinates() should
>> return the visual rect in local SVG coordinates instead of local HTML
>> coordinates.
>> 
>> With the above coordinates fixed, LayoutSVGBlock becomes the same for
>> LayoutSVGText and LayoutSVGForeignObject regarding to SVG/HTML
>> coordinates: SVG coordinates = HTML coordinates + location() (though
>> location() is always zero for LayoutSVGText after
>> https://codereview.chromium.org/2531943002/). The HTML coordinate
>> mapping methods should also map between the HTML and SVG coordinates.
>> 
>> Review-Url: https://codereview.chromium.org/2536493002
>> Cr-Commit-Position: refs/heads/master@{#435337}

BUG=704643,666416
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2

Review-Url: https://codereview.chromium.org/2778873002
Cr-Commit-Position: refs/heads/master@{#460209}
Committed:
https://chromium.googlesource.com/chromium/src/+/09a6cd640fb5bf07aa0647736222...
==========

[commit-bot: I haz the power, 2017-03-28 20:39:39]

Committed patchset #2 (id:190001) as
https://chromium.googlesource.com/chromium/src/+/09a6cd640fb5bf07aa0647736222...