Apply offset from the correct graphics layer; simplify code for link highlights.

Apply offset from the correct graphics layer; simplify code for link highlights.

The code simplification is to avoid deprecated Widget geometry methods,
and have less duplicated code. This leads to a MapCoordinatesFlag option to
absoluteQuads.

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

Committed: https://crrev.com/9b05ffde46a9ed06e7bb8366dc914f30d2ce1711
Cr-Commit-Position: refs/heads/master@{#440983}

[chrishtr, 2016-12-29 01:20:55]

Description was changed from

==========
none


none


none


Merge branch 'linkhighlight' into secondpatch


none

BUG=
==========

to

==========
none


none


none


Merge branch 'linkhighlight' into secondpatch


none

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

[chrishtr, 2016-12-29 01:21:48]

Description was changed from

==========
none


none


none


Merge branch 'linkhighlight' into secondpatch


none

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

to

==========
Apply offset from the correct graphics layer; compute quads in screen space.

Previously, 

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

[chrishtr, 2016-12-29 01:23:23]

Description was changed from

==========
Apply offset from the correct graphics layer; compute quads in screen space.

Previously, 

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

to

==========
Apply offset from the correct graphics layer; simplify code for link highlights.

The code simplification is to avoid deprecated Widget geometry methods,
and have less duplicated code. This leads to a MapCoordinatesFlag option to
absoluteQuads.

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

[chrishtr, 2016-12-29 01:23:38]

Description was changed from

==========
Apply offset from the correct graphics layer; simplify code for link highlights.

The code simplification is to avoid deprecated Widget geometry methods,
and have less duplicated code. This leads to a MapCoordinatesFlag option to
absoluteQuads.

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

to

==========
Apply offset from the correct graphics layer; simplify code for link highlights.

The code simplification is to avoid deprecated Widget geometry methods,
and have less duplicated code. This leads to a MapCoordinatesFlag option to
absoluteQuads.

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

[chrishtr, 2016-12-29 01:24:13]

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

[commit-bot: I haz the power, 2016-12-29 01:24:21]

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

[chrishtr, 2016-12-29 02:57:02]

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

[commit-bot: I haz the power, 2016-12-29 04:56:36]

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

[commit-bot: I haz the power, 2016-12-29 04:56:37]

Dry run: Try jobs failed on following builders:
  win_chromium_rel_ng on master.tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_rel_...)

[Xianzhu, 2016-12-29 05:48:11]

The CQ bit was checked by wangxianzhu@chromium.org

[Xianzhu, 2016-12-29 05:48:12]

lgtm

https://codereview.chromium.org/2604073002/diff/1/third_party/WebKit/Source/c...
File third_party/WebKit/Source/core/paint/PaintLayer.cpp (left):

https://codereview.chromium.org/2604073002/diff/1/third_party/WebKit/Source/c...
third_party/WebKit/Source/core/paint/PaintLayer.cpp:553:
->contentOffsetInCompositingLayer());
Was confused by this when working on VisualRectMappingTest for spinvalidation.
Glad to see this gone.

[commit-bot: I haz the power, 2016-12-29 05:48:24]

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

[Xianzhu, 2016-12-29 06:16:52]

The CQ bit was unchecked by wangxianzhu@chromium.org

[Xianzhu, 2016-12-29 06:33:00]

https://codereview.chromium.org/2604073002/diff/1/third_party/WebKit/Source/c...
File third_party/WebKit/Source/core/paint/PaintLayer.cpp (left):

https://codereview.chromium.org/2604073002/diff/1/third_party/WebKit/Source/c...
third_party/WebKit/Source/core/paint/PaintLayer.cpp:553:
->contentOffsetInCompositingLayer());
On 2016/12/29 05:48:12, Xianzhu wrote:
> Was confused by this when working on VisualRectMappingTest for spinvalidation.
> Glad to see this gone.

I just paused CQ because of a question:

Is there any caller depending on the behavior of the above removed lines?

- Many callers like the following seem fine:
  if (paintInvalidationContainer.layer()->groupedMapping())
    mapPointInPaintInvalidationContainerToBacking(...)

- LinkHighlightImpl should expect the behavior change;

- I'm not sure about LayoutObject::localToInvalidationBackingPoint(). One
callsite is FrameView::computeCompositedSelection(). Can you verify if the
behavior change for these callsites are OK?

Also we may need a followup to also remove the similar offset in
mapRectInPaintInvalidationContainerToBacking(), verify callsites, and (maybe
optionally) simplify callsites by removing the redundant groupdMapping() check.

[chrishtr, 2016-12-29 16:53:03]

The CQ bit was checked by chrishtr@chromium.org

[chrishtr, 2016-12-29 16:53:04]

https://codereview.chromium.org/2604073002/diff/1/third_party/WebKit/Source/c...
File third_party/WebKit/Source/core/paint/PaintLayer.cpp (left):

https://codereview.chromium.org/2604073002/diff/1/third_party/WebKit/Source/c...
third_party/WebKit/Source/core/paint/PaintLayer.cpp:553:
->contentOffsetInCompositingLayer());
On 2016/12/29 at 06:33:00, Xianzhu wrote:
> On 2016/12/29 05:48:12, Xianzhu wrote:
> > Was confused by this when working on VisualRectMappingTest for
spinvalidation.
> > Glad to see this gone.
> 
> I just paused CQ because of a question:
> 
> Is there any caller depending on the behavior of the above removed lines?
> 
> - Many callers like the following seem fine:
>   if (paintInvalidationContainer.layer()->groupedMapping())
>     mapPointInPaintInvalidationContainerToBacking(...)
> 
> - LinkHighlightImpl should expect the behavior change;
> 
> - I'm not sure about LayoutObject::localToInvalidationBackingPoint(). One
callsite is FrameView::computeCompositedSelection(). Can you verify if the
behavior change for these callsites are OK?

This one is not ok. Re-inserted this line. As you mention, only one call
site triggers this after my patch.

> 
> Also we may need a followup to also remove the similar offset in
mapRectInPaintInvalidationContainerToBacking(), verify callsites, and (maybe
optionally) simplify callsites by removing the redundant groupdMapping() check.

Will follow up with other patches along these lines.

[chrishtr, 2016-12-29 16:53:04]

The patchset sent to the CQ was uploaded after l-g-t-m from
wangxianzhu@chromium.org
Link to the patchset: https://codereview.chromium.org/2604073002/#ps20001
(title: "none")

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

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

[chrishtr, 2016-12-29 19:13:42]

The CQ bit was checked by chrishtr@chromium.org

[chrishtr, 2016-12-29 19:13:42]

The patchset sent to the CQ was uploaded after l-g-t-m from
wangxianzhu@chromium.org
Link to the patchset: https://codereview.chromium.org/2604073002/#ps40001
(title: "none")

[commit-bot: I haz the power, 2016-12-29 19:13:50]

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-29 20:43:14]

CQ is committing da patch.

Bot data: {"patchset_id": 40001, "attempt_start_ts": 1483038822249350,
"parent_rev": "ad8e5ccd565291dd00b03fde58b75c2e0da462e9", "commit_rev":
"b188c2ca293042a172955184a79041b10d2b2bd5"}

[commit-bot: I haz the power, 2016-12-29 20:43:40]

Description was changed from

==========
Apply offset from the correct graphics layer; simplify code for link highlights.

The code simplification is to avoid deprecated Widget geometry methods,
and have less duplicated code. This leads to a MapCoordinatesFlag option to
absoluteQuads.

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

to

==========
Apply offset from the correct graphics layer; simplify code for link highlights.

The code simplification is to avoid deprecated Widget geometry methods,
and have less duplicated code. This leads to a MapCoordinatesFlag option to
absoluteQuads.

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

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

[commit-bot: I haz the power, 2016-12-29 20:43:42]

Committed patchset #3 (id:40001)

[commit-bot: I haz the power, 2017-01-02 15:53:03]

Description was changed from

==========
Apply offset from the correct graphics layer; simplify code for link highlights.

The code simplification is to avoid deprecated Widget geometry methods,
and have less duplicated code. This leads to a MapCoordinatesFlag option to
absoluteQuads.

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

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

to

==========
Apply offset from the correct graphics layer; simplify code for link highlights.

The code simplification is to avoid deprecated Widget geometry methods,
and have less duplicated code. This leads to a MapCoordinatesFlag option to
absoluteQuads.

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

Committed: https://crrev.com/9b05ffde46a9ed06e7bb8366dc914f30d2ce1711
Cr-Commit-Position: refs/heads/master@{#440983}
==========

[commit-bot: I haz the power, 2017-01-02 15:53:04]

Patchset 3 (id:??) landed as
https://crrev.com/9b05ffde46a9ed06e7bb8366dc914f30d2ce1711
Cr-Commit-Position: refs/heads/master@{#440983}