Let mapToVisualRectInAncestorSpace apply ancestor clip if not scrollsOverflow

Let mapToVisualRectInAncestorSpace apply ancestor clip if not scrollsOverflow

BUG=597902
Committed: https://crrev.com/ad7b21ace472d41b5b90ddab45f5e7bd44e737f2
Cr-Commit-Position: refs/heads/master@{#385919}

[Xianzhu, 2016-04-05 05:01:11]

Description was changed from

==========
Change how PaintInvalidationState slow path apply container clip
==========

to

==========
Change how PaintInvalidationState slow path apply container clip

BUG=597902
==========

[Xianzhu, 2016-04-06 23:12:45]

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

[chrishtr, 2016-04-06 23:18:19]

lgtm

Please update the CL title to specify that non-scrollsOverflow clips are
applied, but 
scrollsOverflow are not.

https://codereview.chromium.org/1856993004/diff/110001/third_party/WebKit/Sou...
File third_party/WebKit/Source/core/layout/PaintInvalidationState.cpp (right):

https://codereview.chromium.org/1856993004/diff/110001/third_party/WebKit/Sou...
third_party/WebKit/Source/core/layout/PaintInvalidationState.cpp:398: //
LayoutObject::mapToVisualRectInAncestorSpace() doesn't apply overflow clip of
ancestor.
Do any other call sites of mapToVisualRectInAncestorSpace need to apply this
clipping? If so please file a bug.

[Xianzhu, 2016-04-06 23:37:33]

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

[Xianzhu, 2016-04-06 23:37:33]

https://codereview.chromium.org/1856993004/diff/110001/third_party/WebKit/Sou...
File third_party/WebKit/Source/core/layout/PaintInvalidationState.cpp (right):

https://codereview.chromium.org/1856993004/diff/110001/third_party/WebKit/Sou...
third_party/WebKit/Source/core/layout/PaintInvalidationState.cpp:398: //
LayoutObject::mapToVisualRectInAncestorSpace() doesn't apply overflow clip of
ancestor.
On 2016/04/06 23:18:19, chrishtr - vacation wrote:
> Do any other call sites of mapToVisualRectInAncestorSpace need to apply this
> clipping? If so please file a bug.

Didn't think of this. Just looked at several callsites and I think we should
apply clipping there in the same way as paint invalidation rects. Will reopen
crbug.com/597902.

[Xianzhu, 2016-04-06 23:38:25]

Will use this CL to fix bug 597902. Will upload a new patch soon.

[Xianzhu, 2016-04-07 17:08:39]

Description was changed from

==========
Change how PaintInvalidationState slow path apply container clip

BUG=597902
==========

to

==========
Let mapToVisualRectInAncestorSpace apply ancestor clip if not scrollsOverflow

BUG=597902
==========

[Xianzhu, 2016-04-07 17:14:00]

pdr@ Ptal. The latest patch set contains more changes over the patch set that
chrishtr@ lgtmed.

The method used in the latest patch was proposed by chrishtr@ in
https://codereview.chromium.org/1826853007/#msg11

[pdr., 2016-04-07 18:23:45]

This patch looks sound to me. Just one request for cleanup.

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

https://codereview.chromium.org/1856993004/diff/190001/third_party/WebKit/Sou...
third_party/WebKit/Source/core/layout/LayoutBox.cpp:1992: if (container !=
ancestor || !containerBox->scrollsOverflow()) {
This block (check hasOverflowClip , mapScrollingContentsRectToBoxSpace, etc) is
repeated 3x verbatim. Can we refactor this logic into a single function,
possibly on LayoutBox?

[Xianzhu, 2016-04-07 19:04:51]

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

https://codereview.chromium.org/1856993004/diff/190001/third_party/WebKit/Sou...
third_party/WebKit/Source/core/layout/LayoutBox.cpp:1992: if (container !=
ancestor || !containerBox->scrollsOverflow()) {
On 2016/04/07 18:23:45, pdr wrote:
> This block (check hasOverflowClip , mapScrollingContentsRectToBoxSpace, etc)
is
> repeated 3x verbatim. Can we refactor this logic into a single function,
> possibly on LayoutBox?

Done.

[pdr., 2016-04-07 21:49:47]

LGTM

[Xianzhu, 2016-04-07 21:53:10]

The CQ bit was checked by wangxianzhu@chromium.org

[Xianzhu, 2016-04-07 21:53:10]

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

[commit-bot: I haz the power, 2016-04-07 21:53:52]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1856993004/210001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1856993004/210001

[commit-bot: I haz the power, 2016-04-07 23:25:36]

Description was changed from

==========
Let mapToVisualRectInAncestorSpace apply ancestor clip if not scrollsOverflow

BUG=597902
==========

to

==========
Let mapToVisualRectInAncestorSpace apply ancestor clip if not scrollsOverflow

BUG=597902
==========

[commit-bot: I haz the power, 2016-04-07 23:25:38]

Committed patchset #11 (id:210001)

[commit-bot: I haz the power, 2016-04-07 23:27:12]

Description was changed from

==========
Let mapToVisualRectInAncestorSpace apply ancestor clip if not scrollsOverflow

BUG=597902
==========

to

==========
Let mapToVisualRectInAncestorSpace apply ancestor clip if not scrollsOverflow

BUG=597902

Committed: https://crrev.com/ad7b21ace472d41b5b90ddab45f5e7bd44e737f2
Cr-Commit-Position: refs/heads/master@{#385919}
==========

[commit-bot: I haz the power, 2016-04-07 23:27:13]

Patchset 11 (id:??) landed as
https://crrev.com/ad7b21ace472d41b5b90ddab45f5e7bd44e737f2
Cr-Commit-Position: refs/heads/master@{#385919}