Don't adjust for scrolling of the ancestor, rather than un-adjusting for it later.

Don't adjust for scrolling of the ancestor, rather than un-adjusting for it later.

This simplifies the paint invalidation code and avoids code to depend on compositing state. It also brings non-LayoutView scrolling in line with LayoutView in terms
of whether it accounts for scroll, and makes clipping and scrolling apply together.

Note that there are seven use cases of mapToVisualRectInAncestorState:

1. Paint invalidation
2. IntersectionObserver
3. Layout*::absoluteClippedOverflowRect
4. WebPluginContainerImpl::computeClipRectsForPlugin
5. CompositedLayerMapping::recomputeInterestRect
6. LayoutObject::selectionRectInViewCoordinates
7. FrameView::windowClipRect

Use-case 1 doesn't want scrolling. Use case 2 does, so added code to handle that.
Use-cases 3-6 always pass a LayoutView as the ancestor parameter, which already
doesn't include scrolling.

Use-case 7 wants scrolling, so added code to do that.

Committed: https://crrev.com/28df6fb66bdd41a7d695141560933f701497fc2e
Cr-Commit-Position: refs/heads/master@{#410079}

[chrishtr, 2016-08-02 20:10:41]

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

[chrishtr, 2016-08-02 20:10:56]

Description was changed from

==========
none

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

to

==========
Don't adjust for scrolling of the ancestor, rather than un-adjusting for it
later.
==========

[commit-bot: I haz the power, 2016-08-02 20:11:12]

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-08-02 21:08:38]

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

[commit-bot: I haz the power, 2016-08-02 21:08:39]

Dry run: Try jobs failed on following builders:
  mac_chromium_rel_ng on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)

[chrishtr, 2016-08-02 23:43:34]

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

[commit-bot: I haz the power, 2016-08-02 23:44:22]

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-08-03 01:02:27]

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

[commit-bot: I haz the power, 2016-08-03 01:02:28]

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_...)

[chrishtr, 2016-08-03 04:54:45]

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

[commit-bot: I haz the power, 2016-08-03 04:55:04]

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-08-03 05:37:22]

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

[commit-bot: I haz the power, 2016-08-03 05:37:23]

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

[chrishtr, 2016-08-03 15:27:34]

Description was changed from

==========
Don't adjust for scrolling of the ancestor, rather than un-adjusting for it
later.
==========

to

==========
Don't adjust for scrolling of the ancestor, rather than un-adjusting for it
later.

This simplifies the paint invalidation code and avoids code to depend on
compositing state. It also brings non-LayoutView scrolling in line with
LayoutView in terms
of whether it accounts for scroll, and makes clipping and scrolling apply
together.

Note that there are four use cases of mapToVisualRectInAncestorState:

1. Paint invalidation
2. IntersectionObserver
3. Layout*::absoluteClippedOverflowRect
4. WebPluginContainerImpl::computeClipRectsForPlugin
5. CompositedLayerMapping::recomputeInterestRect
6. LayoutObject::selectionRectInViewCoordinates
7. FrameView::windowClipRect

Use-case 1 doesn't want scrolling. Use case 2 does, so added code to handle
that.
Use-cases 3-6 always pass a LayoutView as the ancestor parameter, which already
doesn't include scrolling.

Use-case 7 wants scrolling, so added code to do that.
==========

[chrishtr, 2016-08-03 16:17:44]

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

[chrishtr, 2016-08-03 16:17:48]

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

[commit-bot: I haz the power, 2016-08-03 16:18:15]

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

[Xianzhu, 2016-08-03 16:32:36]

Nit: s/four use cases/7 use cases/ in description.

What will use-case 3-6 behave for root-layer-scrolls?

How about a flag to control whether to apply clip and scroll?

[chrishtr, 2016-08-03 16:33:50]

Description was changed from

==========
Don't adjust for scrolling of the ancestor, rather than un-adjusting for it
later.

This simplifies the paint invalidation code and avoids code to depend on
compositing state. It also brings non-LayoutView scrolling in line with
LayoutView in terms
of whether it accounts for scroll, and makes clipping and scrolling apply
together.

Note that there are four use cases of mapToVisualRectInAncestorState:

1. Paint invalidation
2. IntersectionObserver
3. Layout*::absoluteClippedOverflowRect
4. WebPluginContainerImpl::computeClipRectsForPlugin
5. CompositedLayerMapping::recomputeInterestRect
6. LayoutObject::selectionRectInViewCoordinates
7. FrameView::windowClipRect

Use-case 1 doesn't want scrolling. Use case 2 does, so added code to handle
that.
Use-cases 3-6 always pass a LayoutView as the ancestor parameter, which already
doesn't include scrolling.

Use-case 7 wants scrolling, so added code to do that.
==========

to

==========
Don't adjust for scrolling of the ancestor, rather than un-adjusting for it
later.

This simplifies the paint invalidation code and avoids code to depend on
compositing state. It also brings non-LayoutView scrolling in line with
LayoutView in terms
of whether it accounts for scroll, and makes clipping and scrolling apply
together.

Note that there are seven use cases of mapToVisualRectInAncestorState:

1. Paint invalidation
2. IntersectionObserver
3. Layout*::absoluteClippedOverflowRect
4. WebPluginContainerImpl::computeClipRectsForPlugin
5. CompositedLayerMapping::recomputeInterestRect
6. LayoutObject::selectionRectInViewCoordinates
7. FrameView::windowClipRect

Use-case 1 doesn't want scrolling. Use case 2 does, so added code to handle
that.
Use-cases 3-6 always pass a LayoutView as the ancestor parameter, which already
doesn't include scrolling.

Use-case 7 wants scrolling, so added code to do that.
==========

[chrishtr, 2016-08-03 16:38:34]

On 2016/08/03 at 16:32:36, wangxianzhu wrote:
> Nit: s/four use cases/7 use cases/ in description.

Fixed.

> 
> What will use-case 3-6 behave for root-layer-scrolls?

I think it should just work, because the ancestor element doesn't get its scroll
applied, even if it has overflow clip.

> How about a flag to control whether to apply clip and scroll?

I thought about that, but currently all use cases avoid clip of the ancestor,
and 5 of 7 avoid scroll. I felt it seemed cleaner
to not have a setting and not apply clip/scroll together for symmetry, and
instead adjust for scroll in the two use cases.

[chrishtr, 2016-08-03 16:39:20]

On 2016/08/03 at 16:38:34, chrishtr wrote:
> On 2016/08/03 at 16:32:36, wangxianzhu wrote:
> > Nit: s/four use cases/7 use cases/ in description.
> 
> Fixed.
> 
> > 
> > What will use-case 3-6 behave for root-layer-scrolls?
> 
> I think it should just work, because the ancestor element doesn't get its
scroll applied, even if it has overflow clip.

So I guess my point is that those use cases don't expect scrolling, which is
what mapToVisualRectInAncestorSpace provides under this CL.

> 
> > How about a flag to control whether to apply clip and scroll?
> 
> I thought about that, but currently all use cases avoid clip of the ancestor,
and 5 of 7 avoid scroll. I felt it seemed cleaner
> to not have a setting and not apply clip/scroll together for symmetry, and
instead adjust for scroll in the two use cases.

[Xianzhu, 2016-08-03 16:55:55]

https://codereview.chromium.org/2203123004/diff/60001/third_party/WebKit/Layo...
File
third_party/WebKit/LayoutTests/paint/invalidation/invalidate-after-composited-scroll.html
(right):

https://codereview.chromium.org/2203123004/diff/60001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/paint/invalidation/invalidate-after-composited-scroll.html:18:
}, 3000);
Is the timeout intended?

[chrishtr, 2016-08-03 17:00:36]

https://codereview.chromium.org/2203123004/diff/60001/third_party/WebKit/Layo...
File
third_party/WebKit/LayoutTests/paint/invalidation/invalidate-after-composited-scroll.html
(right):

https://codereview.chromium.org/2203123004/diff/60001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/paint/invalidation/invalidate-after-composited-scroll.html:18:
}, 3000);
On 2016/08/03 at 16:55:55, Xianzhu wrote:
> Is the timeout intended?

Sorry that was sloppy, fixed.

[Xianzhu, 2016-08-03 17:03:51]

lgtm

https://codereview.chromium.org/2203123004/diff/80001/third_party/WebKit/Layo...
File
third_party/WebKit/LayoutTests/paint/invalidation/invalidate-after-composited-scroll.html
(right):

https://codereview.chromium.org/2203123004/diff/80001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/paint/invalidation/invalidate-after-composited-scroll.html:20:
</script>
Nit: Reset this file to origin/master.

[chrishtr, 2016-08-03 18:01:49]

https://codereview.chromium.org/2203123004/diff/80001/third_party/WebKit/Layo...
File
third_party/WebKit/LayoutTests/paint/invalidation/invalidate-after-composited-scroll.html
(right):

https://codereview.chromium.org/2203123004/diff/80001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/paint/invalidation/invalidate-after-composited-scroll.html:20:
</script>
On 2016/08/03 at 17:03:51, Xianzhu wrote:
> Nit: Reset this file to origin/master.

Done.

[chrishtr, 2016-08-03 18:01:54]

The CQ bit was checked by chrishtr@chromium.org

[chrishtr, 2016-08-03 18:01:55]

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/2203123004/#ps100001
(title: "none")

[commit-bot: I haz the power, 2016-08-03 18:03:22]

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-08-03 18:58:35]

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

[commit-bot: I haz the power, 2016-08-03 18:58:37]

Try jobs failed on following builders:
  linux_android_rel_ng on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/linux_androi...)

[chrishtr, 2016-08-03 23:32:05]

The CQ bit was checked by chrishtr@chromium.org

[chrishtr, 2016-08-03 23:32:06]

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/2203123004/#ps120001
(title: "none")

[commit-bot: I haz the power, 2016-08-03 23:32: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-08-04 01:01:10]

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

[commit-bot: I haz the power, 2016-08-04 01:01:11]

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_...)

[chrishtr, 2016-08-04 01:51:36]

The CQ bit was checked by chrishtr@chromium.org

[chrishtr, 2016-08-04 01:51:36]

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/2203123004/#ps140001
(title: "none")

[commit-bot: I haz the power, 2016-08-04 01:51:55]

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-08-04 02:46:54]

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

[commit-bot: I haz the power, 2016-08-04 02:46:56]

Try jobs failed on following builders:
  linux_android_rel_ng on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/linux_androi...)

[chrishtr, 2016-08-04 05:28:43]

The CQ bit was checked by chrishtr@chromium.org

[chrishtr, 2016-08-04 05:28:43]

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/2203123004/#ps160001
(title: "none")

[commit-bot: I haz the power, 2016-08-04 05:28:56]

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-08-04 06:26:17]

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

[commit-bot: I haz the power, 2016-08-04 06:26:18]

Try jobs failed on following builders:
  mac_chromium_rel_ng on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)

[chrishtr, 2016-08-04 16:27:51]

The CQ bit was checked by chrishtr@chromium.org

[chrishtr, 2016-08-04 16:27:51]

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/2203123004/#ps180001
(title: "none")

[commit-bot: I haz the power, 2016-08-04 16:28:15]

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

[chrishtr, 2016-08-04 16:28:52]

The CQ bit was unchecked by chrishtr@chromium.org

[chrishtr, 2016-08-04 16:30:15]

The CQ bit was checked by chrishtr@chromium.org

[chrishtr, 2016-08-04 16:30:16]

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/2203123004/#ps200001
(title: "none")

[commit-bot: I haz the power, 2016-08-04 16:31:14]

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-08-04 16:34:50]

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

[commit-bot: I haz the power, 2016-08-04 16:34:51]

Try jobs failed on following builders:
  ios-device on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-device/builds...)
  ios-simulator on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-simulator/bui...)
  mac_chromium_rel_ng on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)

[chrishtr, 2016-08-04 20:47:28]

The CQ bit was checked by chrishtr@chromium.org

[chrishtr, 2016-08-04 20:47:29]

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/2203123004/#ps220001
(title: "none")

[commit-bot: I haz the power, 2016-08-04 20:48:09]

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-08-04 22:15:04]

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

[commit-bot: I haz the power, 2016-08-04 22:15:06]

Try jobs failed on following builders:
  linux_android_rel_ng on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/linux_androi...)

[chrishtr, 2016-08-04 23:59:52]

The CQ bit was checked by chrishtr@chromium.org

[chrishtr, 2016-08-04 23:59:53]

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/2203123004/#ps240001
(title: "none")

[commit-bot: I haz the power, 2016-08-05 00:00:13]

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-08-05 00:55:49]

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

[commit-bot: I haz the power, 2016-08-05 00:55:50]

Try jobs failed on following builders:
  mac_chromium_rel_ng on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)

[chrishtr, 2016-08-05 03:47:54]

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

[commit-bot: I haz the power, 2016-08-05 03:48:10]

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-08-05 06:09:54]

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

[commit-bot: I haz the power, 2016-08-05 06:09:55]

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

[chrishtr, 2016-08-05 15:57:35]

The CQ bit was checked by chrishtr@chromium.org

[chrishtr, 2016-08-05 15:57:35]

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/2203123004/#ps260001
(title: "none")

[commit-bot: I haz the power, 2016-08-05 15:57:46]

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-08-05 16:35:29]

Committed patchset #14 (id:260001)

[commit-bot: I haz the power, 2016-08-05 16:37:49]

Description was changed from

==========
Don't adjust for scrolling of the ancestor, rather than un-adjusting for it
later.

This simplifies the paint invalidation code and avoids code to depend on
compositing state. It also brings non-LayoutView scrolling in line with
LayoutView in terms
of whether it accounts for scroll, and makes clipping and scrolling apply
together.

Note that there are seven use cases of mapToVisualRectInAncestorState:

1. Paint invalidation
2. IntersectionObserver
3. Layout*::absoluteClippedOverflowRect
4. WebPluginContainerImpl::computeClipRectsForPlugin
5. CompositedLayerMapping::recomputeInterestRect
6. LayoutObject::selectionRectInViewCoordinates
7. FrameView::windowClipRect

Use-case 1 doesn't want scrolling. Use case 2 does, so added code to handle
that.
Use-cases 3-6 always pass a LayoutView as the ancestor parameter, which already
doesn't include scrolling.

Use-case 7 wants scrolling, so added code to do that.
==========

to

==========
Don't adjust for scrolling of the ancestor, rather than un-adjusting for it
later.

This simplifies the paint invalidation code and avoids code to depend on
compositing state. It also brings non-LayoutView scrolling in line with
LayoutView in terms
of whether it accounts for scroll, and makes clipping and scrolling apply
together.

Note that there are seven use cases of mapToVisualRectInAncestorState:

1. Paint invalidation
2. IntersectionObserver
3. Layout*::absoluteClippedOverflowRect
4. WebPluginContainerImpl::computeClipRectsForPlugin
5. CompositedLayerMapping::recomputeInterestRect
6. LayoutObject::selectionRectInViewCoordinates
7. FrameView::windowClipRect

Use-case 1 doesn't want scrolling. Use case 2 does, so added code to handle
that.
Use-cases 3-6 always pass a LayoutView as the ancestor parameter, which already
doesn't include scrolling.

Use-case 7 wants scrolling, so added code to do that.

Committed: https://crrev.com/28df6fb66bdd41a7d695141560933f701497fc2e
Cr-Commit-Position: refs/heads/master@{#410079}
==========

[commit-bot: I haz the power, 2016-08-05 16:37:51]

Patchset 14 (id:??) landed as
https://crrev.com/28df6fb66bdd41a7d695141560933f701497fc2e
Cr-Commit-Position: refs/heads/master@{#410079}