Fix paint invalidation of paintInvalidationContainer itself

Fix paint invalidation of paintInvalidationContainer itself

LayoutBlock::invalidatePaintOfSubtreesIfNeeded() has a condition.
paintInvalidationContainerForChild != childPaintInvalidationState.paintInvalidationContainer()
to use the ForceHorriblySlowRectMapping path in case that the
PaintInvalidationState fails to get the correct paintInvalidationContainer.

However, the childPaintInvalidationState actually contains the parent
paintInvalidationContainer if the current object itself is a
paintInvalidationContainer so the above condition is false for
out-of-flow-positioned composited objects, and will use the
ForceHorriblySlowRectMapping path for paint invalidation of the subtree.

Also LayoutBox::mapToVisibleRectInAncestorSpace() depends on the fact
that paintInvalidationState contains parent paintInvalidationContainer
if the current object itself is a paintInvalidationContainer. To avoid
the dependency we should check 'ancestor == this' before checking
PaintInvalidationState.

BUG=371485, 591199
Committed: https://crrev.com/df97b1c6526b25450d767b2098c8ee1c3d124b95
Cr-Commit-Position: refs/heads/master@{#380865}

[Xianzhu, 2016-03-11 01:47:10]

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

[Xianzhu, 2016-03-11 01:49:14]

Description was changed from

==========
Fix paint invalidation of paintInvalidationContainer itself

LayoutBlock::invalidatePaintOfSubtreesIfNeeded() has a condition.
paintInvalidationContainerForChild !=
childPaintInvalidationState.paintInvalidationContainer().
The childPaintInvalidationState actually contains the parent
paintInvalidationContainer if the current object itself is a
paintInvalidationContainer so the above condition is false for
out-of-flow-positioned composited objects, and will use the
ForceHorriblySlowRectMapping path.

Also LayoutBox::mapToVisibleRectInAncestorSpace() depends on the fact
that paintInvalidationState contains parent paintInvalidationContainer
if the current object itself is a paintInvalidationContainer. To avoid
the dependency we should check 'ancestor == this' before checking
PaintInvalidationState.

BUG=371485,591199
==========

to

==========
Fix paint invalidation of paintInvalidationContainer itself

LayoutBlock::invalidatePaintOfSubtreesIfNeeded() has a condition.
paintInvalidationContainerForChild !=
childPaintInvalidationState.paintInvalidationContainer()
to use the ForceHorriblySlowRectMapping path in case that the
PaintInvalidationState fails to get the correct paintInvalidationContainer.

However, the childPaintInvalidationState actually contains the parent
paintInvalidationContainer if the current object itself is a
paintInvalidationContainer so the above condition is false for
out-of-flow-positioned composited objects, and will use the
ForceHorriblySlowRectMapping path for paint invalidation of the subtree.

Also LayoutBox::mapToVisibleRectInAncestorSpace() depends on the fact
that paintInvalidationState contains parent paintInvalidationContainer
if the current object itself is a paintInvalidationContainer. To avoid
the dependency we should check 'ancestor == this' before checking
PaintInvalidationState.

BUG=371485,591199
==========

[chrishtr, 2016-03-11 18:19:59]

https://codereview.chromium.org/1786513002/diff/1/third_party/WebKit/Source/c...
File third_party/WebKit/Source/core/layout/LayoutBlock.cpp (right):

https://codereview.chromium.org/1786513002/diff/1/third_party/WebKit/Source/c...
third_party/WebKit/Source/core/layout/LayoutBlock.cpp:359: if
(&paintInvalidationContainerForChild != box &&
paintInvalidationContainerForChild !=
childPaintInvalidationState.paintInvalidationContainer()) {
If box establishes a paint invalidation container, why is it that the paint
offsets are still correct? In other words, consider a box that leads to this
code path and
is not composited. In that case, we have to force the slow mapping because its
paint invalidation container is not correctly tracked. Adding compositing means
we
are sure the paint invalidation container is tracked, but why is the offset
correct?

The comment in line 356 seems to indicate otherwise.

I also don't remember the cases when this code path would be hit. Can you remind
me of one?

[Xianzhu, 2016-03-11 18:30:33]

https://codereview.chromium.org/1786513002/diff/1/third_party/WebKit/Source/c...
File third_party/WebKit/Source/core/layout/LayoutBlock.cpp (right):

https://codereview.chromium.org/1786513002/diff/1/third_party/WebKit/Source/c...
third_party/WebKit/Source/core/layout/LayoutBlock.cpp:359: if
(&paintInvalidationContainerForChild != box &&
paintInvalidationContainerForChild !=
childPaintInvalidationState.paintInvalidationContainer()) {
On 2016/03/11 18:19:59, chrishtr wrote:
> If box establishes a paint invalidation container, why is it that the paint
> offsets are still correct? In other words, consider a box that leads to this
> code path and
> is not composited. In that case, we have to force the slow mapping because its
> paint invalidation container is not correctly tracked. Adding compositing
means
> we
> are sure the paint invalidation container is tracked, but why is the offset
> correct?
>

In the case the paint offset won't be used, for either old code or with this
patch:
- The old code used ForceHorriblySlowRectMapping;
- The new code will early return in mapToVisibleRectInAncestorSpace() without
using PaintInvalidationState.
 
> The comment in line 356 seems to indicate otherwise.
> 
> I also don't remember the cases when this code path would be hit. Can you
remind
> me of one?

This code was intended for the case that the actual paint invalidation container
is not in the containing block chain. For example:
<div style="composited-stacking-context">
  <div style="fixed-not-composited"></div>
</div>
At here, childPaintInvalidationState.paintInvalidationContainer() is the
LayoutView, but the actual paintInvalidationContainer of the fixed-position div
is the composited-stacking-context.

However, this path was also used when the box itself establishes a paint
invalidation container, forcing slow rect mapping for the whole subtree.

The above issues will be fixed in new paint invalidation for spv1. For the
self-paint-invalidation-container case, LayoutObject::invalidatePaintIfNeeded()
will get a PaintInvalidationState with correct paintOffset (zero) and
paintInvalidationContainer (itself).

[chrishtr, 2016-03-11 21:17:16]

https://codereview.chromium.org/1786513002/diff/1/third_party/WebKit/Source/c...
File third_party/WebKit/Source/core/layout/LayoutBlock.cpp (right):

https://codereview.chromium.org/1786513002/diff/1/third_party/WebKit/Source/c...
third_party/WebKit/Source/core/layout/LayoutBlock.cpp:359: if
(&paintInvalidationContainerForChild != box &&
paintInvalidationContainerForChild !=
childPaintInvalidationState.paintInvalidationContainer()) {
On 2016/03/11 at 18:30:32, Xianzhu wrote:
> On 2016/03/11 18:19:59, chrishtr wrote:
> > If box establishes a paint invalidation container, why is it that the paint
> > offsets are still correct? In other words, consider a box that leads to this
> > code path and
> > is not composited. In that case, we have to force the slow mapping because
its
> > paint invalidation container is not correctly tracked. Adding compositing
means
> > we
> > are sure the paint invalidation container is tracked, but why is the offset
> > correct?
> >
> 
> In the case the paint offset won't be used, for either old code or with this
patch:
> - The old code used ForceHorriblySlowRectMapping;
> - The new code will early return in mapToVisibleRectInAncestorSpace() without
using PaintInvalidationState.
>  
> > The comment in line 356 seems to indicate otherwise.
> > 
> > I also don't remember the cases when this code path would be hit. Can you
remind
> > me of one?
> 
> This code was intended for the case that the actual paint invalidation
container is not in the containing block chain. For example:
> <div style="composited-stacking-context">
>   <div style="fixed-not-composited"></div>
> </div>
> At here, childPaintInvalidationState.paintInvalidationContainer() is the
LayoutView, but the actual paintInvalidationContainer of the fixed-position div
is the composited-stacking-context.
> 
> However, this path was also used when the box itself establishes a paint
invalidation container, forcing slow rect mapping for the whole subtree.

Is there a test for this case?

> 
> The above issues will be fixed in new paint invalidation for spv1. For the
self-paint-invalidation-container case, LayoutObject::invalidatePaintIfNeeded()
will get a PaintInvalidationState with correct paintOffset (zero) and
paintInvalidationContainer (itself).

https://codereview.chromium.org/1786513002/diff/1/third_party/WebKit/Source/c...
File third_party/WebKit/Source/core/layout/LayoutBox.cpp (right):

https://codereview.chromium.org/1786513002/diff/1/third_party/WebKit/Source/c...
third_party/WebKit/Source/core/layout/LayoutBox.cpp:1904: if (ancestor == this)
{
This seems fragile. Can it just be moved down into
PaintInvalidationState::canMapToContainer?

[Xianzhu, 2016-03-11 21:39:56]

https://codereview.chromium.org/1786513002/diff/1/third_party/WebKit/Source/c...
File third_party/WebKit/Source/core/layout/LayoutBlock.cpp (right):

https://codereview.chromium.org/1786513002/diff/1/third_party/WebKit/Source/c...
third_party/WebKit/Source/core/layout/LayoutBlock.cpp:359: if
(&paintInvalidationContainerForChild != box &&
paintInvalidationContainerForChild !=
childPaintInvalidationState.paintInvalidationContainer()) {
On 2016/03/11 21:17:16, chrishtr wrote:
> On 2016/03/11 at 18:30:32, Xianzhu wrote:
> > On 2016/03/11 18:19:59, chrishtr wrote:
> > > If box establishes a paint invalidation container, why is it that the
paint
> > > offsets are still correct? In other words, consider a box that leads to
this
> > > code path and
> > > is not composited. In that case, we have to force the slow mapping because
> its
> > > paint invalidation container is not correctly tracked. Adding compositing
> means
> > > we
> > > are sure the paint invalidation container is tracked, but why is the
offset
> > > correct?
> > >
> > 
> > In the case the paint offset won't be used, for either old code or with this
> patch:
> > - The old code used ForceHorriblySlowRectMapping;
> > - The new code will early return in mapToVisibleRectInAncestorSpace()
without
> using PaintInvalidationState.
> >  
> > > The comment in line 356 seems to indicate otherwise.
> > > 
> > > I also don't remember the cases when this code path would be hit. Can you
> remind
> > > me of one?
> > 
> > This code was intended for the case that the actual paint invalidation
> container is not in the containing block chain. For example:
> > <div style="composited-stacking-context">
> >   <div style="fixed-not-composited"></div>
> > </div>
> > At here, childPaintInvalidationState.paintInvalidationContainer() is the
> LayoutView, but the actual paintInvalidationContainer of the fixed-position
div
> is the composited-stacking-context.
> > 
> > However, this path was also used when the box itself establishes a paint
> invalidation container, forcing slow rect mapping for the whole subtree.
> 
> Is there a test for this case?
> 
> > 
> > The above issues will be fixed in new paint invalidation for spv1. For the
> self-paint-invalidation-container case,
LayoutObject::invalidatePaintIfNeeded()
> will get a PaintInvalidationState with correct paintOffset (zero) and
> paintInvalidationContainer (itself).
> 

Many existing layout tests cover the code here and the cases for correctness of
paint invalidation, but not for whether ForceHorriblySlowRectMapping is used. It
seems not worth testing the latter as the code here is about to be obsoleted by
slimmingPaintInvalidation.

https://codereview.chromium.org/1786513002/diff/1/third_party/WebKit/Source/c...
File third_party/WebKit/Source/core/layout/LayoutBox.cpp (right):

https://codereview.chromium.org/1786513002/diff/1/third_party/WebKit/Source/c...
third_party/WebKit/Source/core/layout/LayoutBox.cpp:1904: if (ancestor == this)
{
On 2016/03/11 21:17:16, chrishtr wrote:
> This seems fragile. Can it just be moved down into
> PaintInvalidationState::canMapToContainer?

This code is also needed when there is no paintInvalidationState, so we can't
move it into PaintInvalidationState::canMapToContainer() which is only called
when there is paintInvalidationContainer.

In next steps for new paint invalidation, I plan to add
PaintInvalidationState::mapObjectRectToAncestor() to include the code below from
1910 to 1922, and remove PaintInvalidationState parameter from this function,
and let PaintInvalidationState::mapObjectRectToAncestor() fallback to this
function if fast mapping is not possible.

[chrishtr, 2016-03-11 21:43:06]

https://codereview.chromium.org/1786513002/diff/1/third_party/WebKit/Source/c...
File third_party/WebKit/Source/core/layout/LayoutBox.cpp (right):

https://codereview.chromium.org/1786513002/diff/1/third_party/WebKit/Source/c...
third_party/WebKit/Source/core/layout/LayoutBox.cpp:1904: if (ancestor == this)
{
On 2016/03/11 at 21:39:56, Xianzhu wrote:
> On 2016/03/11 21:17:16, chrishtr wrote:
> > This seems fragile. Can it just be moved down into
> > PaintInvalidationState::canMapToContainer?
> 
> This code is also needed when there is no paintInvalidationState, so we can't
move it into PaintInvalidationState::canMapToContainer() which is only called
when there is paintInvalidationContainer.
> 
> In next steps for new paint invalidation, I plan to add
PaintInvalidationState::mapObjectRectToAncestor() to include the code below from
1910 to 1922, and remove PaintInvalidationState parameter from this function,
and let PaintInvalidationState::mapObjectRectToAncestor() fallback to this
function if fast mapping is not possible.

Ok. Please add a comment explaining then.

[Xianzhu, 2016-03-11 22:08:25]

https://codereview.chromium.org/1786513002/diff/1/third_party/WebKit/Source/c...
File third_party/WebKit/Source/core/layout/LayoutBox.cpp (right):

https://codereview.chromium.org/1786513002/diff/1/third_party/WebKit/Source/c...
third_party/WebKit/Source/core/layout/LayoutBox.cpp:1904: if (ancestor == this)
{
On 2016/03/11 21:43:06, chrishtr wrote:
> On 2016/03/11 at 21:39:56, Xianzhu wrote:
> > On 2016/03/11 21:17:16, chrishtr wrote:
> > > This seems fragile. Can it just be moved down into
> > > PaintInvalidationState::canMapToContainer?
> > 
> > This code is also needed when there is no paintInvalidationState, so we
can't
> move it into PaintInvalidationState::canMapToContainer() which is only called
> when there is paintInvalidationContainer.
> > 
> > In next steps for new paint invalidation, I plan to add
> PaintInvalidationState::mapObjectRectToAncestor() to include the code below
from
> 1910 to 1922, and remove PaintInvalidationState parameter from this function,
> and let PaintInvalidationState::mapObjectRectToAncestor() fallback to this
> function if fast mapping is not possible.
> 
> Ok. Please add a comment explaining then.

Done.

[chrishtr, 2016-03-11 22:11:44]

https://codereview.chromium.org/1786513002/diff/1/third_party/WebKit/Source/c...
File third_party/WebKit/Source/core/layout/LayoutBox.cpp (right):

https://codereview.chromium.org/1786513002/diff/1/third_party/WebKit/Source/c...
third_party/WebKit/Source/core/layout/LayoutBox.cpp:1904: if (ancestor == this)
{
On 2016/03/11 at 21:39:56, Xianzhu wrote:
> On 2016/03/11 21:17:16, chrishtr wrote:
> > This seems fragile. Can it just be moved down into
> > PaintInvalidationState::canMapToContainer?
> 
> This code is also needed when there is no paintInvalidationState, so we can't
move it into PaintInvalidationState::canMapToContainer() which is only called
when there is paintInvalidationContainer.
> 
> In next steps for new paint invalidation, I plan to add
PaintInvalidationState::mapObjectRectToAncestor() to include the code below from
1910 to 1922, and remove PaintInvalidationState parameter from this function,
and let PaintInvalidationState::mapObjectRectToAncestor() fallback to this
function if fast mapping is not possible.

Actually, hold on a minute. would it be cleaner and just as forward compatible
to leave the lines 1919-1923 you're deleting and implement the
PaintInvalidationContainer logic I suggested?

https://codereview.chromium.org/1786513002/diff/20001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/layout/LayoutBlock.cpp (right):

https://codereview.chromium.org/1786513002/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/layout/LayoutBlock.cpp:359: if
(&paintInvalidationContainerForChild != box &&
paintInvalidationContainerForChild !=
childPaintInvalidationState.paintInvalidationContainer()) {
Explain this in a comment also.

https://codereview.chromium.org/1786513002/diff/20001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/layout/LayoutBox.cpp (right):

https://codereview.chromium.org/1786513002/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/layout/LayoutBox.cpp:1906: // this code is also
needed when there is no PaintInvalidationState.
TODO(wangxianzhu): refactor to fix this code structure.

[Xianzhu, 2016-03-11 22:23:43]

https://codereview.chromium.org/1786513002/diff/1/third_party/WebKit/Source/c...
File third_party/WebKit/Source/core/layout/LayoutBox.cpp (right):

https://codereview.chromium.org/1786513002/diff/1/third_party/WebKit/Source/c...
third_party/WebKit/Source/core/layout/LayoutBox.cpp:1904: if (ancestor == this)
{
On 2016/03/11 22:11:44, chrishtr wrote:
> On 2016/03/11 at 21:39:56, Xianzhu wrote:
> > On 2016/03/11 21:17:16, chrishtr wrote:
> > > This seems fragile. Can it just be moved down into
> > > PaintInvalidationState::canMapToContainer?
> > 
> > This code is also needed when there is no paintInvalidationState, so we
can't
> move it into PaintInvalidationState::canMapToContainer() which is only called
> when there is paintInvalidationContainer.
> > 
> > In next steps for new paint invalidation, I plan to add
> PaintInvalidationState::mapObjectRectToAncestor() to include the code below
from
> 1910 to 1922, and remove PaintInvalidationState parameter from this function,
> and let PaintInvalidationState::mapObjectRectToAncestor() fallback to this
> function if fast mapping is not possible.
> 
> Actually, hold on a minute. would it be cleaner and just as forward compatible
> to leave the lines 1919-1923 you're deleting and implement the
> PaintInvalidationContainer logic I suggested?

I don't quite understand your suggestion. Do you mean to move 'ancestor == this'
condition into canMapToContainer() and let it return false for the condition, or
to move the whole block 1904-1908 into it? The latter is not feasible because we
also need the block for non-paintInvalidationState case.

[Xianzhu, 2016-03-11 22:30:51]

https://codereview.chromium.org/1786513002/diff/20001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/layout/LayoutBlock.cpp (right):

https://codereview.chromium.org/1786513002/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/layout/LayoutBlock.cpp:359: if
(&paintInvalidationContainerForChild != box &&
paintInvalidationContainerForChild !=
childPaintInvalidationState.paintInvalidationContainer()) {
On 2016/03/11 22:11:44, chrishtr wrote:
> Explain this in a comment also.

Done.

https://codereview.chromium.org/1786513002/diff/20001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/layout/LayoutBox.cpp (right):

https://codereview.chromium.org/1786513002/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/layout/LayoutBox.cpp:1906: // this code is also
needed when there is no PaintInvalidationState.
On 2016/03/11 22:11:44, chrishtr wrote:
> TODO(wangxianzhu): refactor to fix this code structure.

Done.

[chrishtr, 2016-03-11 22:36:41]

On 2016/03/11 at 22:23:43, wangxianzhu wrote:
>
https://codereview.chromium.org/1786513002/diff/1/third_party/WebKit/Source/c...
> File third_party/WebKit/Source/core/layout/LayoutBox.cpp (right):
> 
>
https://codereview.chromium.org/1786513002/diff/1/third_party/WebKit/Source/c...
> third_party/WebKit/Source/core/layout/LayoutBox.cpp:1904: if (ancestor ==
this) {
> On 2016/03/11 22:11:44, chrishtr wrote:
> > On 2016/03/11 at 21:39:56, Xianzhu wrote:
> > > On 2016/03/11 21:17:16, chrishtr wrote:
> > > > This seems fragile. Can it just be moved down into
> > > > PaintInvalidationState::canMapToContainer?
> > > 
> > > This code is also needed when there is no paintInvalidationState, so we
can't
> > move it into PaintInvalidationState::canMapToContainer() which is only
called
> > when there is paintInvalidationContainer.
> > > 
> > > In next steps for new paint invalidation, I plan to add
> > PaintInvalidationState::mapObjectRectToAncestor() to include the code below
from
> > 1910 to 1922, and remove PaintInvalidationState parameter from this
function,
> > and let PaintInvalidationState::mapObjectRectToAncestor() fallback to this
> > function if fast mapping is not possible.
> > 
> > Actually, hold on a minute. would it be cleaner and just as forward
compatible
> > to leave the lines 1919-1923 you're deleting and implement the
> > PaintInvalidationContainer logic I suggested?
> 
> I don't quite understand your suggestion. Do you mean to move 'ancestor ==
this' condition into canMapToContainer() and let it return false for the
condition, or to move the whole block 1904-1908 into it? The latter is not
feasible because we also need the block for non-paintInvalidationState case.

The latter, and to leave the code below also, except to put it in an
if(!paintInvalidationState) block.

[Xianzhu, 2016-03-12 00:06:59]

On 2016/03/11 22:36:41, chrishtr wrote:
> On 2016/03/11 at 22:23:43, wangxianzhu wrote:
> >
>
https://codereview.chromium.org/1786513002/diff/1/third_party/WebKit/Source/c...
> > File third_party/WebKit/Source/core/layout/LayoutBox.cpp (right):
> > 
> >
>
https://codereview.chromium.org/1786513002/diff/1/third_party/WebKit/Source/c...
> > third_party/WebKit/Source/core/layout/LayoutBox.cpp:1904: if (ancestor ==
> this) {
> > On 2016/03/11 22:11:44, chrishtr wrote:
> > > On 2016/03/11 at 21:39:56, Xianzhu wrote:
> > > > On 2016/03/11 21:17:16, chrishtr wrote:
> > > > > This seems fragile. Can it just be moved down into
> > > > > PaintInvalidationState::canMapToContainer?
> > > > 
> > > > This code is also needed when there is no paintInvalidationState, so we
> can't
> > > move it into PaintInvalidationState::canMapToContainer() which is only
> called
> > > when there is paintInvalidationContainer.
> > > > 
> > > > In next steps for new paint invalidation, I plan to add
> > > PaintInvalidationState::mapObjectRectToAncestor() to include the code
below
> from
> > > 1910 to 1922, and remove PaintInvalidationState parameter from this
> function,
> > > and let PaintInvalidationState::mapObjectRectToAncestor() fallback to this
> > > function if fast mapping is not possible.
> > > 
> > > Actually, hold on a minute. would it be cleaner and just as forward
> compatible
> > > to leave the lines 1919-1923 you're deleting and implement the
> > > PaintInvalidationContainer logic I suggested?
> > 
> > I don't quite understand your suggestion. Do you mean to move 'ancestor ==
> this' condition into canMapToContainer() and let it return false for the
> condition, or to move the whole block 1904-1908 into it? The latter is not
> feasible because we also need the block for non-paintInvalidationState case.
> 
> The latter, and to leave the code below also, except to put it in an
> if(!paintInvalidationState) block.

How about the new patch set (which extracts fast-path rect mapping code into
PaintInvalidationState::mapObjectRectToAncestor)?

[chrishtr, 2016-03-12 00:31:51]

lgtm

[Xianzhu, 2016-03-12 02:16:58]

The CQ bit was checked by wangxianzhu@chromium.org

[commit-bot: I haz the power, 2016-03-12 02:17:42]

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

[commit-bot: I haz the power, 2016-03-12 04:58:50]

Description was changed from

==========
Fix paint invalidation of paintInvalidationContainer itself

LayoutBlock::invalidatePaintOfSubtreesIfNeeded() has a condition.
paintInvalidationContainerForChild !=
childPaintInvalidationState.paintInvalidationContainer()
to use the ForceHorriblySlowRectMapping path in case that the
PaintInvalidationState fails to get the correct paintInvalidationContainer.

However, the childPaintInvalidationState actually contains the parent
paintInvalidationContainer if the current object itself is a
paintInvalidationContainer so the above condition is false for
out-of-flow-positioned composited objects, and will use the
ForceHorriblySlowRectMapping path for paint invalidation of the subtree.

Also LayoutBox::mapToVisibleRectInAncestorSpace() depends on the fact
that paintInvalidationState contains parent paintInvalidationContainer
if the current object itself is a paintInvalidationContainer. To avoid
the dependency we should check 'ancestor == this' before checking
PaintInvalidationState.

BUG=371485,591199
==========

to

==========
Fix paint invalidation of paintInvalidationContainer itself

LayoutBlock::invalidatePaintOfSubtreesIfNeeded() has a condition.
paintInvalidationContainerForChild !=
childPaintInvalidationState.paintInvalidationContainer()
to use the ForceHorriblySlowRectMapping path in case that the
PaintInvalidationState fails to get the correct paintInvalidationContainer.

However, the childPaintInvalidationState actually contains the parent
paintInvalidationContainer if the current object itself is a
paintInvalidationContainer so the above condition is false for
out-of-flow-positioned composited objects, and will use the
ForceHorriblySlowRectMapping path for paint invalidation of the subtree.

Also LayoutBox::mapToVisibleRectInAncestorSpace() depends on the fact
that paintInvalidationState contains parent paintInvalidationContainer
if the current object itself is a paintInvalidationContainer. To avoid
the dependency we should check 'ancestor == this' before checking
PaintInvalidationState.

BUG=371485,591199
==========

[commit-bot: I haz the power, 2016-03-12 04:58:51]

Committed patchset #4 (id:60001)

[commit-bot: I haz the power, 2016-03-12 05:00:41]

Description was changed from

==========
Fix paint invalidation of paintInvalidationContainer itself

LayoutBlock::invalidatePaintOfSubtreesIfNeeded() has a condition.
paintInvalidationContainerForChild !=
childPaintInvalidationState.paintInvalidationContainer()
to use the ForceHorriblySlowRectMapping path in case that the
PaintInvalidationState fails to get the correct paintInvalidationContainer.

However, the childPaintInvalidationState actually contains the parent
paintInvalidationContainer if the current object itself is a
paintInvalidationContainer so the above condition is false for
out-of-flow-positioned composited objects, and will use the
ForceHorriblySlowRectMapping path for paint invalidation of the subtree.

Also LayoutBox::mapToVisibleRectInAncestorSpace() depends on the fact
that paintInvalidationState contains parent paintInvalidationContainer
if the current object itself is a paintInvalidationContainer. To avoid
the dependency we should check 'ancestor == this' before checking
PaintInvalidationState.

BUG=371485,591199
==========

to

==========
Fix paint invalidation of paintInvalidationContainer itself

LayoutBlock::invalidatePaintOfSubtreesIfNeeded() has a condition.
paintInvalidationContainerForChild !=
childPaintInvalidationState.paintInvalidationContainer()
to use the ForceHorriblySlowRectMapping path in case that the
PaintInvalidationState fails to get the correct paintInvalidationContainer.

However, the childPaintInvalidationState actually contains the parent
paintInvalidationContainer if the current object itself is a
paintInvalidationContainer so the above condition is false for
out-of-flow-positioned composited objects, and will use the
ForceHorriblySlowRectMapping path for paint invalidation of the subtree.

Also LayoutBox::mapToVisibleRectInAncestorSpace() depends on the fact
that paintInvalidationState contains parent paintInvalidationContainer
if the current object itself is a paintInvalidationContainer. To avoid
the dependency we should check 'ancestor == this' before checking
PaintInvalidationState.

BUG=371485,591199

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

[commit-bot: I haz the power, 2016-03-12 05:00:42]

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