Remove ForceHorriblySlowRectMapping

Remove ForceHorriblySlowRectMapping

Change paint invalidation tree walk to DOM-order instead of containing-
block order for out-of-flow positioned objects so that we can track
paint invalidation container and paint offset for them correctly without
forcing slow mapping if possible.

Move some cases to fast path:
- Absolute-position objects (except the slow-path cases):
  track paint offset etc separately for them.
  See stacked objects for paint invalidation container tracking.
- Stacked objects (including absolute-position objects): track paint
  invalidation container separately for them.

Still use slow path for the following cases (conditions are in
supportsCachedOffsets() and supportsCachedOffsetsForChildren()):
- Absolute-position whose container is above the current paint
  invalidation container for stacked contents;
- Fixed-position objects: normally its paint invalidation container
  is either the LayoutView or below its containing block, so slow-
  path is either not so slow, or fast-path is not feasible;
- Multicolumn, SVGBlock, objects with reflection or filter.

BUG=591199, 598094
Committed: https://crrev.com/bf7dc636b3ed84582375d2cebe613ffaf1c00794
Cr-Commit-Position: refs/heads/master@{#385056}

[Xianzhu, 2016-03-29 23:04:30]

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

[Xianzhu, 2016-03-29 23:05:24]

https://codereview.chromium.org/1833493003/diff/60001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/layout/LayoutBoxModelObject.cpp (left):

https://codereview.chromium.org/1833493003/diff/60001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/layout/LayoutBoxModelObject.cpp:370: //
ASSERT(&newPaintInvalidationState.paintInvalidationContainer() ==
&containerForPaintInvalidation());
It's enough to check this in LayoutObject::invalidatePaintIfNeeded().

[chrishtr, 2016-03-30 03:29:36]

Description was changed from

==========
Remove ForceHorriblySlowRectMapping

Change paint invalidation tree walk to DOM-order instead of containing-
block order for out-of-flow positioned objects so that we can track
paint invalidation container and paint offset for them correctly without
forcing slow mapping if possible.

Move some cases to fast path:
- Absolute-position objects (except the slow-path cases):
  track paint offset etc separately for them.
  See stacked objects for paint invalidation container tracking.
- Stacked objects (including absolute-position objects): track paint
  invalidation container separately for them.

Still use slow path for the following cases (conditions are in
supportsCachedOffsets() and supportsCachedOffsetsForChildren()):
- Absolute-position whose container is above the current paint
  invalidation container for stacked contents;
- Fixed-position objects: normally its paint invalidation container
  is either the LayoutView or below its containing block, so slow-
  path is either not so slow, or fast-path is not feasible;
- Multicolumn, SVGBlock, objects with reflection or filter.

BUG=591199
==========

to

==========
Remove ForceHorriblySlowRectMapping

Change paint invalidation tree walk to DOM-order instead of containing-
block order for out-of-flow positioned objects so that we can track
paint invalidation container and paint offset for them correctly without
forcing slow mapping if possible.

Move some cases to fast path:
- Absolute-position objects (except the slow-path cases):
  track paint offset etc separately for them.
  See stacked objects for paint invalidation container tracking.
- Stacked objects (including absolute-position objects): track paint
  invalidation container separately for them.

Still use slow path for the following cases (conditions are in
supportsCachedOffsets() and supportsCachedOffsetsForChildren()):
- Absolute-position whose container is above the current paint
  invalidation container for stacked contents;
- Fixed-position objects: normally its paint invalidation container
  is either the LayoutView or below its containing block, so slow-
  path is either not so slow, or fast-path is not feasible;
- Multicolumn, SVGBlock, objects with reflection or filter.

BUG=591199,576854
==========

[chrishtr, 2016-03-31 00:15:02]

https://codereview.chromium.org/1833493003/diff/60001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/layout/LayoutObject.cpp (right):

https://codereview.chromium.org/1833493003/diff/60001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/layout/LayoutObject.cpp:1344: for (LayoutObject*
child = slowFirstChild(); child; child = child->nextSibling())
Document the new traversal order and any other pertinent information briefly in
Markdown.

https://codereview.chromium.org/1833493003/diff/60001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/layout/LayoutObject.cpp:2625: return
isLayoutView() ? frame()->ownerLayoutObject() : container();
Delete this method?

https://codereview.chromium.org/1833493003/diff/80001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/layout/PaintInvalidationState.cpp (right):

https://codereview.chromium.org/1833493003/diff/80001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/layout/PaintInvalidationState.cpp:110: //
m_paintInvalidationContainerForStackedContents doesn't cross frame boundary.
Why?

https://codereview.chromium.org/1833493003/diff/80001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/layout/PaintInvalidationState.cpp:115: &&
m_paintInvalidationContainer != m_paintInvalidationContainerForStackedContents)
{
Can you explain in detail what this logic is doing (on the code review to
start)? It's hard to grok.

[Xianzhu, 2016-03-31 01:41:28]

https://codereview.chromium.org/1833493003/diff/60001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/layout/LayoutObject.cpp (right):

https://codereview.chromium.org/1833493003/diff/60001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/layout/LayoutObject.cpp:1344: for (LayoutObject*
child = slowFirstChild(); child; child = child->nextSibling())
On 2016/03/31 00:15:02, chrishtr wrote:
> Document the new traversal order and any other pertinent information briefly
in
> Markdown.

Done.

https://codereview.chromium.org/1833493003/diff/60001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/layout/LayoutObject.cpp:2625: return
isLayoutView() ? frame()->ownerLayoutObject() : container();
On 2016/03/31 00:15:02, chrishtr wrote:
> Delete this method?

Done.

https://codereview.chromium.org/1833493003/diff/80001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/layout/PaintInvalidationState.cpp (right):

https://codereview.chromium.org/1833493003/diff/80001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/layout/PaintInvalidationState.cpp:110: //
m_paintInvalidationContainerForStackedContents doesn't cross frame boundary.
On 2016/03/31 00:15:02, chrishtr wrote:
> Why?

Added explanation as comment:

// m_paintInvalidationContainerForStackedContents is only for stacked
descendants in its own frame,
// because it doesn't establish stacking context for stacked contents in
sub-frames.
// Contents stacked in the root stacking context in this frame should use this
frame's paintInvalidationContainer.

https://codereview.chromium.org/1833493003/diff/80001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/layout/PaintInvalidationState.cpp:115: &&
m_paintInvalidationContainer != m_paintInvalidationContainerForStackedContents)
{
On 2016/03/31 00:15:02, chrishtr wrote:
> Can you explain in detail what this logic is doing (on the code review to
> start)? It's hard to grok.

Added explanation as comment:
// The current object is stacked, so we should use
m_paintInvalidationContainerForStackedContents as its
// paint invalidation container on which the current object is painted.

// We are changing paintInvalidationContainer to
m_paintInvalidationContainerForStackedContents. Must disable
// cached offsets because we didn't track paint offset from
m_paintInvalidationContainerForStackedContents.

Also described the logic of paintInvalidationContainer in
Source/core/paint/README.md.

[chrishtr, 2016-03-31 21:55:00]

https://codereview.chromium.org/1833493003/diff/100001/third_party/WebKit/Sou...
File third_party/WebKit/Source/core/paint/README.md (right):

https://codereview.chromium.org/1833493003/diff/100001/third_party/WebKit/Sou...
third_party/WebKit/Source/core/paint/README.md:88: *   Paint invalidation
container: an ancestor (crossing frame boundaries) of the current object
"stacked object": positioned objects, non-auto-z-index-objects, and objects
creating stacking contexts. This is represented in the code by
ComputedStyle::isStacked().

"Paint container": the parent of an object for painting, as defined by <link to
css2.1 spec for painting>. For regular objects, this is the parent in the DOM.
For stacked objects, it's the containing stacking context-inducing object.

"paint container chain": the chain of paint ancestors between an element and the
root of the page

"compositing container": an implementation detail of Blink, which uses
PaintLayers to represent some layout objects. It is the ancestor along the paint
ancestor chain which has a PaintLayer. Implemented in
PaintLayer::compositingContainer(). Think of it as skipping intermediate normal
objects and going directly to the containing stacked object.

"compositing container chain": same as paint invalidation chain, but for
compositing container.

"Paint invalidation container": the nearest object on the compositing container
chain which is composited.

https://codereview.chromium.org/1833493003/diff/100001/third_party/WebKit/Sou...
third_party/WebKit/Source/core/paint/README.md:90: two paint invalidation
containers for stacked contents and normal flow contents separately:
Add:

Since as indicated by the definitions above, the paint invalidation container
for stacked objects can differ from normal objects, we have to track both
separately. Here is an example:

<div style="overflow: scroll">
  <div id=A style="position: absolute"></div>
  <div id=B style="position: absolute"></div>
</div>

If the scroller is composited (for high-DPI screens for example), it is the
paint invalidation container for div B, but not for A.

https://codereview.chromium.org/1833493003/diff/100001/third_party/WebKit/Sou...
third_party/WebKit/Source/core/paint/README.md:100: Because locations of
absolute-position objects are determined by their containing blocks,
"Because locations of objects are determined by their containing blocks, and the
containing block for absolute-position elements differs from non-absolute, ..."

[trchen, 2016-03-31 22:00:38]

https://codereview.chromium.org/1833493003/diff/100001/third_party/WebKit/Sou...
File third_party/WebKit/Source/core/paint/README.md (right):

https://codereview.chromium.org/1833493003/diff/100001/third_party/WebKit/Sou...
third_party/WebKit/Source/core/paint/README.md:90: two paint invalidation
containers for stacked contents and normal flow contents separately:
On 2016/03/31 21:54:59, chrishtr wrote:
> Add:
> 
> Since as indicated by the definitions above, the paint invalidation container
> for stacked objects can differ from normal objects, we have to track both
> separately. Here is an example:
> 
> <div style="overflow: scroll">
>   <div id=A style="position: absolute"></div>
>   <div id=B style="position: absolute"></div>
> </div>
> 
> If the scroller is composited (for high-DPI screens for example), it is the
> paint invalidation container for div B, but not for A.

It this example correct? It looks to me both A and B will paint into the root
layer, or potentially get composited for overlap reason.

[chrishtr, 2016-03-31 22:06:05]

https://codereview.chromium.org/1833493003/diff/100001/third_party/WebKit/Sou...
File third_party/WebKit/Source/core/layout/PaintInvalidationState.h (right):

https://codereview.chromium.org/1833493003/diff/100001/third_party/WebKit/Sou...
third_party/WebKit/Source/core/layout/PaintInvalidationState.h:119: // It is the
nearest ancestor composited object which establishes a stacking context.
Refer to README.md for details on how stacked contents' paint invalidation
containers differ.

https://codereview.chromium.org/1833493003/diff/100001/third_party/WebKit/Sou...
File third_party/WebKit/Source/core/paint/README.md (right):

https://codereview.chromium.org/1833493003/diff/100001/third_party/WebKit/Sou...
third_party/WebKit/Source/core/paint/README.md:90: two paint invalidation
containers for stacked contents and normal flow contents separately:
On 2016/03/31 at 22:00:38, trchen wrote:
> On 2016/03/31 21:54:59, chrishtr wrote:
> > Add:
> > 
> > Since as indicated by the definitions above, the paint invalidation
container
> > for stacked objects can differ from normal objects, we have to track both
> > separately. Here is an example:
> > 
> > <div style="overflow: scroll">
> >   <div id=A style="position: absolute"></div>
> >   <div id=B style="position: absolute"></div>
> > </div>
> > 
> > If the scroller is composited (for high-DPI screens for example), it is the
> > paint invalidation container for div B, but not for A.
> 
> It this example correct? It looks to me both A and B will paint into the root
layer, or potentially get composited for overlap reason.

Sorry, forgot to remove position:absolute on B.

[chrishtr, 2016-03-31 22:32:11]

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

https://codereview.chromium.org/1833493003/diff/100001/third_party/WebKit/Sou...
third_party/WebKit/Source/core/layout/PaintInvalidationState.cpp:106: if
(m_currentObject != m_containerForAbsolutePosition)
Can this slow path be avoided by adjusting m_paintOffsetForAbsolutePosition to
be relative to
the current stacking context? Is that just subtracting m_paintOffset from it?

https://codereview.chromium.org/1833493003/diff/100001/third_party/WebKit/Sou...
third_party/WebKit/Source/core/layout/PaintInvalidationState.cpp:113:
m_paintInvalidationContainerForStackedContents = m_paintInvalidationContainer;
Does the m_cachedOffsetsForAbsolutePositionEnabled = false slow path from above
also apply here?

https://codereview.chromium.org/1833493003/diff/100001/third_party/WebKit/Sou...
third_party/WebKit/Source/core/layout/PaintInvalidationState.cpp:192: if
(container.isInFlowPositioned() && container.isLayoutInline())
What is this code for? Why a special case?

[Xianzhu, 2016-04-01 16:07:56]

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

https://codereview.chromium.org/1833493003/diff/100001/third_party/WebKit/Sou...
third_party/WebKit/Source/core/layout/PaintInvalidationState.cpp:106: if
(m_currentObject != m_containerForAbsolutePosition)
On 2016/03/31 22:32:11, chrishtr wrote:
> Can this slow path be avoided by adjusting m_paintOffsetForAbsolutePosition to
> be relative to
> the current stacking context? Is that just subtracting m_paintOffset from it?

Good idea. Done.

https://codereview.chromium.org/1833493003/diff/100001/third_party/WebKit/Sou...
third_party/WebKit/Source/core/layout/PaintInvalidationState.cpp:113:
m_paintInvalidationContainerForStackedContents = m_paintInvalidationContainer;
On 2016/03/31 22:32:11, chrishtr wrote:
> Does the m_cachedOffsetsForAbsolutePositionEnabled = false slow path from
above
> also apply here?

No, because LayoutView resets both stacking context and containing block for
absolute-position objects.

https://codereview.chromium.org/1833493003/diff/100001/third_party/WebKit/Sou...
third_party/WebKit/Source/core/layout/PaintInvalidationState.cpp:192: if
(container.isInFlowPositioned() && container.isLayoutInline())
On 2016/03/31 22:32:11, chrishtr wrote:
> What is this code for? Why a special case?

This is for
<span style="position: relative">
  <div style="position: absolute"></div>
</span>

The code had been here before my fast-path-to-PaintInvalidationState change, and
I removed it because I mistakenly thought it were incorrect (see
crbug.com/598094 and the blink-dev thread). Removing it didn't break things
because LayoutBlock forced slow-path for the case.

https://codereview.chromium.org/1833493003/diff/100001/third_party/WebKit/Sou...
File third_party/WebKit/Source/core/layout/PaintInvalidationState.h (right):

https://codereview.chromium.org/1833493003/diff/100001/third_party/WebKit/Sou...
third_party/WebKit/Source/core/layout/PaintInvalidationState.h:119: // It is the
nearest ancestor composited object which establishes a stacking context.
On 2016/03/31 22:06:05, chrishtr wrote:
> Refer to README.md for details on how stacked contents' paint invalidation
> containers differ.

Done.

https://codereview.chromium.org/1833493003/diff/100001/third_party/WebKit/Sou...
File third_party/WebKit/Source/core/paint/README.md (right):

https://codereview.chromium.org/1833493003/diff/100001/third_party/WebKit/Sou...
third_party/WebKit/Source/core/paint/README.md:88: *   Paint invalidation
container: an ancestor (crossing frame boundaries) of the current object
On 2016/03/31 21:54:59, chrishtr wrote:
> "stacked object": positioned objects, non-auto-z-index-objects, and objects
> creating stacking contexts. This is represented in the code by
> ComputedStyle::isStacked().
> 
> "Paint container": the parent of an object for painting, as defined by <link
to
> css2.1 spec for painting>. For regular objects, this is the parent in the DOM.
> For stacked objects, it's the containing stacking context-inducing object.
> 
> "paint container chain": the chain of paint ancestors between an element and
the
> root of the page
> 
> "compositing container": an implementation detail of Blink, which uses
> PaintLayers to represent some layout objects. It is the ancestor along the
paint
> ancestor chain which has a PaintLayer. Implemented in
> PaintLayer::compositingContainer(). Think of it as skipping intermediate
normal
> objects and going directly to the containing stacked object.
> 
> "compositing container chain": same as paint invalidation chain, but for
> compositing container.
> 
> "Paint invalidation container": the nearest object on the compositing
container
> chain which is composited.

Done. We already have a whole chapter for "stacked object" in the documentation.

https://codereview.chromium.org/1833493003/diff/100001/third_party/WebKit/Sou...
third_party/WebKit/Source/core/paint/README.md:90: two paint invalidation
containers for stacked contents and normal flow contents separately:
On 2016/03/31 21:54:59, chrishtr wrote:
> Add:
> 
> Since as indicated by the definitions above, the paint invalidation container
> for stacked objects can differ from normal objects, we have to track both
> separately. Here is an example:
> 
> <div style="overflow: scroll">
>   <div id=A style="position: absolute"></div>
>   <div id=B style="position: absolute"></div>
> </div>
> 
> If the scroller is composited (for high-DPI screens for example), it is the
> paint invalidation container for div B, but not for A.

Done.

https://codereview.chromium.org/1833493003/diff/100001/third_party/WebKit/Sou...
third_party/WebKit/Source/core/paint/README.md:100: Because locations of
absolute-position objects are determined by their containing blocks,
On 2016/03/31 21:54:59, chrishtr wrote:
> "Because locations of objects are determined by their containing blocks, and
the
> containing block for absolute-position elements differs from non-absolute,
..."

Done.

[chrishtr, 2016-04-01 16:38:13]

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

https://codereview.chromium.org/1833493003/diff/140001/third_party/WebKit/Sou...
third_party/WebKit/Source/core/layout/PaintInvalidationState.cpp:35: if
(object.styleRef().position() == FixedPosition)
Is there a bug for this?

https://codereview.chromium.org/1833493003/diff/140001/third_party/WebKit/Sou...
third_party/WebKit/Source/core/layout/PaintInvalidationState.cpp:162: // We are
changing paintInvalidationContainer to
m_paintInvalidationContainerForStackedContents. Must disable
Why can't we use the same approach here as line 147?

https://codereview.chromium.org/1833493003/diff/140001/third_party/WebKit/Sou...
File third_party/WebKit/Source/core/paint/README.md (right):

https://codereview.chromium.org/1833493003/diff/140001/third_party/WebKit/Sou...
third_party/WebKit/Source/core/paint/README.md:71: root of the page
Nit: add period.

https://codereview.chromium.org/1833493003/diff/140001/third_party/WebKit/Sou...
third_party/WebKit/Source/core/paint/README.md:132: we will fallback to
slow-path using `LayoutObject::localToAncestorPoint()` or
Nit: s/fallback/fall back/

https://codereview.chromium.org/1833493003/diff/140001/third_party/WebKit/Sou...
third_party/WebKit/Source/core/paint/README.md:150: *   We also stop to track
paint offset and clipping rect for absolute-position objects when
This one is obsolete in the latest patch set, right?

[Xianzhu, 2016-04-01 17:07:22]

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

https://codereview.chromium.org/1833493003/diff/140001/third_party/WebKit/Sou...
third_party/WebKit/Source/core/layout/PaintInvalidationState.cpp:35: if
(object.styleRef().position() == FixedPosition)
On 2016/04/01 16:38:13, chrishtr wrote:
> Is there a bug for this?

I would not treat this as a bug, but a trade-off between performance and paint
invalidation accuracy.

When we meet a fixed-position object, we use slow-path to get its paintOffset,
then enable fast-path of its descendants. In this way, we can't get accurate
clip rect from ancestor of the fixed-position object, so just assume no clip.

Clip on fixed-position object is rare, in the following cases:
- the fixed-position object's container is a LayoutView which is not a
paintInvalidationContainer and clipped by something in the owner document;
- the fixed-position object's container is an object with transform and clip.

It's possible that we use a similar technique for absolute-position and their
descendants: even if the absolute-position object itself needs slow-path offset
calculation, we can use fast-path for its descendants. Would like to explore
this way in later CLs.

https://codereview.chromium.org/1833493003/diff/140001/third_party/WebKit/Sou...
third_party/WebKit/Source/core/layout/PaintInvalidationState.cpp:162: // We are
changing paintInvalidationContainer to
m_paintInvalidationContainerForStackedContents. Must disable
On 2016/04/01 16:38:13, chrishtr wrote:
> Why can't we use the same approach here as line 147?

This is because we don't track the offset from
m_paintInvalidationContainerForStackedContents to m_paintInvalidationContainer.
It's different from the case at line 147 in which the new
m_paintInvalidationContainerForStackedContents is the current object and we know
the offset from the original paintInvalidationContainer to it.

However, there are possible optimization opportunities:
- Like what we do for fixed-position, calculate the paint offset in slow-path
and enable fast-path for descendants;
or
- Track offset between the two paintInvalidationContainers.

Would like to explore the above opportunities in later CLs.

https://codereview.chromium.org/1833493003/diff/140001/third_party/WebKit/Sou...
third_party/WebKit/Source/core/layout/PaintInvalidationState.cpp:266:
m_cachedOffsetsForAbsolutePositionEnabled = false;
There are also optimization opportunities here like line 162.

https://codereview.chromium.org/1833493003/diff/140001/third_party/WebKit/Sou...
File third_party/WebKit/Source/core/paint/README.md (right):

https://codereview.chromium.org/1833493003/diff/140001/third_party/WebKit/Sou...
third_party/WebKit/Source/core/paint/README.md:71: root of the page
On 2016/04/01 16:38:13, chrishtr wrote:
> Nit: add period.

Done.

https://codereview.chromium.org/1833493003/diff/140001/third_party/WebKit/Sou...
third_party/WebKit/Source/core/paint/README.md:132: we will fallback to
slow-path using `LayoutObject::localToAncestorPoint()` or
On 2016/04/01 16:38:13, chrishtr wrote:
> Nit: s/fallback/fall back/

Done.

https://codereview.chromium.org/1833493003/diff/140001/third_party/WebKit/Sou...
third_party/WebKit/Source/core/paint/README.md:150: *   We also stop to track
paint offset and clipping rect for absolute-position objects when
On 2016/04/01 16:38:13, chrishtr wrote:
> This one is obsolete in the latest patch set, right?

The case before 'or' is still valid. Removed the latter case.

[chrishtr, 2016-04-01 17:10:36]

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

https://codereview.chromium.org/1833493003/diff/140001/third_party/WebKit/Sou...
third_party/WebKit/Source/core/layout/PaintInvalidationState.cpp:35: if
(object.styleRef().position() == FixedPosition)
On 2016/04/01 at 17:07:21, Xianzhu wrote:
> On 2016/04/01 16:38:13, chrishtr wrote:
> > Is there a bug for this?
> 
> I would not treat this as a bug, but a trade-off between performance and paint
invalidation accuracy.
> 
> When we meet a fixed-position object, we use slow-path to get its paintOffset,
then enable fast-path of its descendants. In this way, we can't get accurate
clip rect from ancestor of the fixed-position object, so just assume no clip.
> 
> Clip on fixed-position object is rare, in the following cases:
> - the fixed-position object's container is a LayoutView which is not a
paintInvalidationContainer and clipped by something in the owner document;
> - the fixed-position object's container is an object with transform and clip.
> 
> It's possible that we use a similar technique for absolute-position and their
descendants: even if the absolute-position object itself needs slow-path offset
calculation, we can use fast-path for its descendants. Would like to explore
this way in later CLs.

Ok. Please document this in the code.

[chrishtr, 2016-04-01 17:12:16]

lgtm

LGTM after comment update. Great CL!

[Xianzhu, 2016-04-01 17:23:10]

On 2016/04/01 17:10:36, chrishtr wrote:
>
https://codereview.chromium.org/1833493003/diff/140001/third_party/WebKit/Sou...
> File third_party/WebKit/Source/core/layout/PaintInvalidationState.cpp (right):
> 
>
https://codereview.chromium.org/1833493003/diff/140001/third_party/WebKit/Sou...
> third_party/WebKit/Source/core/layout/PaintInvalidationState.cpp:35: if
> (object.styleRef().position() == FixedPosition)
> On 2016/04/01 at 17:07:21, Xianzhu wrote:
> > On 2016/04/01 16:38:13, chrishtr wrote:
> > > Is there a bug for this?
> > 
> > I would not treat this as a bug, but a trade-off between performance and
paint
> invalidation accuracy.
> > 
> > When we meet a fixed-position object, we use slow-path to get its
paintOffset,
> then enable fast-path of its descendants. In this way, we can't get accurate
> clip rect from ancestor of the fixed-position object, so just assume no clip.
> > 
> > Clip on fixed-position object is rare, in the following cases:
> > - the fixed-position object's container is a LayoutView which is not a
> paintInvalidationContainer and clipped by something in the owner document;
> > - the fixed-position object's container is an object with transform and
clip.
> > 
> > It's possible that we use a similar technique for absolute-position and
their
> descendants: even if the absolute-position object itself needs slow-path
offset
> calculation, we can use fast-path for its descendants. Would like to explore
> this way in later CLs.
> 
> Ok. Please document this in the code.

Done.

[Xianzhu, 2016-04-01 17:23:19]

The CQ bit was checked by wangxianzhu@chromium.org

[Xianzhu, 2016-04-01 17:23:20]

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

[Xianzhu, 2016-04-01 17:23:34]

The CQ bit was unchecked by wangxianzhu@chromium.org

[Xianzhu, 2016-04-04 19:50:21]

Description was changed from

==========
Remove ForceHorriblySlowRectMapping

Change paint invalidation tree walk to DOM-order instead of containing-
block order for out-of-flow positioned objects so that we can track
paint invalidation container and paint offset for them correctly without
forcing slow mapping if possible.

Move some cases to fast path:
- Absolute-position objects (except the slow-path cases):
  track paint offset etc separately for them.
  See stacked objects for paint invalidation container tracking.
- Stacked objects (including absolute-position objects): track paint
  invalidation container separately for them.

Still use slow path for the following cases (conditions are in
supportsCachedOffsets() and supportsCachedOffsetsForChildren()):
- Absolute-position whose container is above the current paint
  invalidation container for stacked contents;
- Fixed-position objects: normally its paint invalidation container
  is either the LayoutView or below its containing block, so slow-
  path is either not so slow, or fast-path is not feasible;
- Multicolumn, SVGBlock, objects with reflection or filter.

BUG=591199,576854
==========

to

==========
Remove ForceHorriblySlowRectMapping

Change paint invalidation tree walk to DOM-order instead of containing-
block order for out-of-flow positioned objects so that we can track
paint invalidation container and paint offset for them correctly without
forcing slow mapping if possible.

Move some cases to fast path:
- Absolute-position objects (except the slow-path cases):
  track paint offset etc separately for them.
  See stacked objects for paint invalidation container tracking.
- Stacked objects (including absolute-position objects): track paint
  invalidation container separately for them.

Still use slow path for the following cases (conditions are in
supportsCachedOffsets() and supportsCachedOffsetsForChildren()):
- Absolute-position whose container is above the current paint
  invalidation container for stacked contents;
- Fixed-position objects: normally its paint invalidation container
  is either the LayoutView or below its containing block, so slow-
  path is either not so slow, or fast-path is not feasible;
- Multicolumn, SVGBlock, objects with reflection or filter.

BUG=591199,598094
==========

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

The CQ bit was checked by wangxianzhu@chromium.org

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

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

[Xianzhu, 2016-04-04 19:53:42]

We have got two UMA reports of Blink.PaintInvalidation.UpdateTime on Mac and
Windows canary.

[commit-bot: I haz the power, 2016-04-04 23:53:04]

Description was changed from

==========
Remove ForceHorriblySlowRectMapping

Change paint invalidation tree walk to DOM-order instead of containing-
block order for out-of-flow positioned objects so that we can track
paint invalidation container and paint offset for them correctly without
forcing slow mapping if possible.

Move some cases to fast path:
- Absolute-position objects (except the slow-path cases):
  track paint offset etc separately for them.
  See stacked objects for paint invalidation container tracking.
- Stacked objects (including absolute-position objects): track paint
  invalidation container separately for them.

Still use slow path for the following cases (conditions are in
supportsCachedOffsets() and supportsCachedOffsetsForChildren()):
- Absolute-position whose container is above the current paint
  invalidation container for stacked contents;
- Fixed-position objects: normally its paint invalidation container
  is either the LayoutView or below its containing block, so slow-
  path is either not so slow, or fast-path is not feasible;
- Multicolumn, SVGBlock, objects with reflection or filter.

BUG=591199,598094
==========

to

==========
Remove ForceHorriblySlowRectMapping

Change paint invalidation tree walk to DOM-order instead of containing-
block order for out-of-flow positioned objects so that we can track
paint invalidation container and paint offset for them correctly without
forcing slow mapping if possible.

Move some cases to fast path:
- Absolute-position objects (except the slow-path cases):
  track paint offset etc separately for them.
  See stacked objects for paint invalidation container tracking.
- Stacked objects (including absolute-position objects): track paint
  invalidation container separately for them.

Still use slow path for the following cases (conditions are in
supportsCachedOffsets() and supportsCachedOffsetsForChildren()):
- Absolute-position whose container is above the current paint
  invalidation container for stacked contents;
- Fixed-position objects: normally its paint invalidation container
  is either the LayoutView or below its containing block, so slow-
  path is either not so slow, or fast-path is not feasible;
- Multicolumn, SVGBlock, objects with reflection or filter.

BUG=591199,598094
==========

[commit-bot: I haz the power, 2016-04-04 23:53:06]

Committed patchset #10 (id:180001)

[commit-bot: I haz the power, 2016-04-04 23:54:03]

Description was changed from

==========
Remove ForceHorriblySlowRectMapping

Change paint invalidation tree walk to DOM-order instead of containing-
block order for out-of-flow positioned objects so that we can track
paint invalidation container and paint offset for them correctly without
forcing slow mapping if possible.

Move some cases to fast path:
- Absolute-position objects (except the slow-path cases):
  track paint offset etc separately for them.
  See stacked objects for paint invalidation container tracking.
- Stacked objects (including absolute-position objects): track paint
  invalidation container separately for them.

Still use slow path for the following cases (conditions are in
supportsCachedOffsets() and supportsCachedOffsetsForChildren()):
- Absolute-position whose container is above the current paint
  invalidation container for stacked contents;
- Fixed-position objects: normally its paint invalidation container
  is either the LayoutView or below its containing block, so slow-
  path is either not so slow, or fast-path is not feasible;
- Multicolumn, SVGBlock, objects with reflection or filter.

BUG=591199,598094
==========

to

==========
Remove ForceHorriblySlowRectMapping

Change paint invalidation tree walk to DOM-order instead of containing-
block order for out-of-flow positioned objects so that we can track
paint invalidation container and paint offset for them correctly without
forcing slow mapping if possible.

Move some cases to fast path:
- Absolute-position objects (except the slow-path cases):
  track paint offset etc separately for them.
  See stacked objects for paint invalidation container tracking.
- Stacked objects (including absolute-position objects): track paint
  invalidation container separately for them.

Still use slow path for the following cases (conditions are in
supportsCachedOffsets() and supportsCachedOffsetsForChildren()):
- Absolute-position whose container is above the current paint
  invalidation container for stacked contents;
- Fixed-position objects: normally its paint invalidation container
  is either the LayoutView or below its containing block, so slow-
  path is either not so slow, or fast-path is not feasible;
- Multicolumn, SVGBlock, objects with reflection or filter.

BUG=591199,598094

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

[commit-bot: I haz the power, 2016-04-04 23:54:04]

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

[leviw_travelin_and_unemployed, 2016-04-04 23:56:11]

leviw@chromium.org changed reviewers:
+ leviw@chromium.org

[leviw_travelin_and_unemployed, 2016-04-04 23:56:12]

https://codereview.chromium.org/1833493003/diff/180001/third_party/WebKit/Sou...
File third_party/WebKit/Source/core/layout/LayoutObject.cpp (right):

https://codereview.chromium.org/1833493003/diff/180001/third_party/WebKit/Sou...
third_party/WebKit/Source/core/layout/LayoutObject.cpp:3419: for (LayoutObject*
container = this->parentCrossingFrameBoundaries(); container &&
!container->shouldCheckForPaintInvalidationRegardlessOfPaintInvalidationState();
container = container->parentCrossingFrameBoundaries())
Nit: s/container/parent/

[leviw_travelin_and_unemployed, 2016-04-04 23:57:52]

Also: Holy crap! It's dead!! My least favorite code to add, but favorite to
name!

Awesome work :)

[Xianzhu, 2016-04-05 00:01:39]

https://codereview.chromium.org/1833493003/diff/180001/third_party/WebKit/Sou...
File third_party/WebKit/Source/core/layout/LayoutObject.cpp (right):

https://codereview.chromium.org/1833493003/diff/180001/third_party/WebKit/Sou...
third_party/WebKit/Source/core/layout/LayoutObject.cpp:3419: for (LayoutObject*
container = this->parentCrossingFrameBoundaries(); container &&
!container->shouldCheckForPaintInvalidationRegardlessOfPaintInvalidationState();
container = container->parentCrossingFrameBoundaries())
On 2016/04/04 23:56:12, leviw wrote:
> Nit: s/container/parent/

Will upload a follow-up CL for this.