Fix or workaround several issues of slimmingPaintInvalidation on spv1

Fix or workaround several issues of slimmingPaintInvalidation on spv1

- Fix frame scrollbar paint invalidation (non-root-layer-scrolls only)
- Workaround for multicol contents (crbug.com/648274) and inline
  contents in flipped writing mode (crbug.com/648409).

With this CL, run-webkit-tests --additional-driver-flag=--enable-blink-features=slimmingPaintInvalidation paint/invalidation:
Crashes: 1 (previous: 2)
Failures: 19 (previous: 62)

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

Committed: https://crrev.com/4e39096940590df4c11256e09938957290cbd863
Cr-Commit-Position: refs/heads/master@{#419677}

[Xianzhu, 2016-09-19 21:53:22]

Description was changed from

==========
Fix or workaround several issues of slimmingPaintInvalidation on spv1

- Fix frame scrollbar paint invalidation (non-root-layer-scrolls only)
- Workaround for multicol contents (crbug.com/648274) and inline
  contents in flipped writing mode (crbug.com/648409).

With this CL, run-webkit-tests
--additional-driver-flag=--enable-blink-features=slimmingPaintInvalidation
paint/invalidation:
Crashes: 1
Failures: 19

BUG=646176
==========

to

==========
Fix or workaround several issues of slimmingPaintInvalidation on spv1

- Fix frame scrollbar paint invalidation (non-root-layer-scrolls only)
- Workaround for multicol contents (crbug.com/648274) and inline
  contents in flipped writing mode (crbug.com/648409).

With this CL, run-webkit-tests
--additional-driver-flag=--enable-blink-features=slimmingPaintInvalidation
paint/invalidation:
Crashes: 1
Failures: 19

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

[Xianzhu, 2016-09-19 21:53:26]

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

[Xianzhu, 2016-09-19 21:53:54]

Description was changed from

==========
Fix or workaround several issues of slimmingPaintInvalidation on spv1

- Fix frame scrollbar paint invalidation (non-root-layer-scrolls only)
- Workaround for multicol contents (crbug.com/648274) and inline
  contents in flipped writing mode (crbug.com/648409).

With this CL, run-webkit-tests
--additional-driver-flag=--enable-blink-features=slimmingPaintInvalidation
paint/invalidation:
Crashes: 1
Failures: 19

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

to

==========
Fix or workaround several issues of slimmingPaintInvalidation on spv1

- Fix frame scrollbar paint invalidation (non-root-layer-scrolls only)
- Workaround for multicol contents (crbug.com/648274) and inline
  contents in flipped writing mode (crbug.com/648409).

With this CL, run-webkit-tests
--additional-driver-flag=--enable-blink-features=slimmingPaintInvalidation
paint/invalidation:
Crashes: 1 (previous: 2)
Failures: 19 (previous: 62)

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

[commit-bot: I haz the power, 2016-09-19 21:54:02]

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

[Xianzhu, 2016-09-19 21:54:06]

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

[pdr., 2016-09-19 22:19:15]

https://codereview.chromium.org/2346343003/diff/1/third_party/WebKit/LayoutTe...
File
third_party/WebKit/LayoutTests/FlagExpectations/enable-blink-features=slimmingPaintInvalidation
(right):

https://codereview.chromium.org/2346343003/diff/1/third_party/WebKit/LayoutTe...
third_party/WebKit/LayoutTests/FlagExpectations/enable-blink-features=slimmingPaintInvalidation:15:
paint/invalidation/compositing/iframe-inside-squashed-layer.html [
NeedsRebaseline ]
Do we have flag-specific baselines?

https://codereview.chromium.org/2346343003/diff/1/third_party/WebKit/Source/c...
File third_party/WebKit/Source/core/paint/PaintInvalidator.cpp (right):

https://codereview.chromium.org/2346343003/diff/1/third_party/WebKit/Source/c...
third_party/WebKit/Source/core/paint/PaintInvalidator.cpp:187: if
(object.hasFilterInducingProperty() || object.isLayoutFlowThread() ||
object.hasFlippedBlocksWritingMode())
Can the hasFlippedBlocksWritingMode check be made a little tougher by checking
for inline too? For example, (object.isInline() &&
object.hasFlippedBlocksWritingMode())

https://codereview.chromium.org/2346343003/diff/1/third_party/WebKit/Source/c...
third_party/WebKit/Source/core/paint/PaintInvalidator.cpp:212: // Undo content
clip and scroll before invalidating FrameView scrollbars.
If there is a scroll translation (because the frameview is scrolled), we will
incorrectly use the scrolled transform node for invalidating the scrollbars.
WDYT of splitting out the self and non-self invalidations like we do for
LayoutObjects?

Something like:
void PrePaintTreeWalk::walk(FrameView& frameView, const PrePaintTreeWalkContext&
context)
{
...
    PrePaintTreeWalkContext localContext(context);
    m_propertyTreeBuilder.buildTreeNodesForSelf(frameView,
localContext.treeBuilderContext);
    if (RuntimeEnabledFeatures::slimmingPaintInvalidationEnabled())
        m_paintInvalidator.invalidatePaintForSelfIfNeeded(frameView,
localContext.paintInvalidatorContext);
    m_propertyTreeBuilder.buildTreeNodesForChildren(frameView,
localContext.treeBuilderContext);
...
}

https://codereview.chromium.org/2346343003/diff/1/third_party/WebKit/Source/c...
third_party/WebKit/Source/core/paint/PaintInvalidator.cpp:217:
treeBuilderContext.current.scroll = scroll->parent();
This will be incorrect if the frameview does not scroll (and, therefore, does
not create a scroll node). Do any tests hit this case?

I think we need something like:
treeBuilderContext.current.scroll = frameView.scroll() ? scroll->parent() :
scroll;

[Xianzhu, 2016-09-19 23:37:40]

https://codereview.chromium.org/2346343003/diff/1/third_party/WebKit/LayoutTe...
File
third_party/WebKit/LayoutTests/FlagExpectations/enable-blink-features=slimmingPaintInvalidation
(right):

https://codereview.chromium.org/2346343003/diff/1/third_party/WebKit/LayoutTe...
third_party/WebKit/LayoutTests/FlagExpectations/enable-blink-features=slimmingPaintInvalidation:15:
paint/invalidation/compositing/iframe-inside-squashed-layer.html [
NeedsRebaseline ]
On 2016/09/19 22:19:15, pdr. wrote:
> Do we have flag-specific baselines?

No. This NeedsRebaselines just remind us to rebaseline the tests when enable the
feature by default.

https://codereview.chromium.org/2346343003/diff/1/third_party/WebKit/Source/c...
File third_party/WebKit/Source/core/paint/PaintInvalidator.cpp (right):

https://codereview.chromium.org/2346343003/diff/1/third_party/WebKit/Source/c...
third_party/WebKit/Source/core/paint/PaintInvalidator.cpp:187: if
(object.hasFilterInducingProperty() || object.isLayoutFlowThread() ||
object.hasFlippedBlocksWritingMode())
On 2016/09/19 22:19:15, pdr. wrote:
> Can the hasFlippedBlocksWritingMode check be made a little tougher by checking
> for inline too? For example, (object.isInline() &&
> object.hasFlippedBlocksWritingMode())

This would be complex: we need to check for the flipped blocks writing mode of a
writing-mode container and inline status of a descendant. I will work on this in
this week and hopefully fix the bug soon.

https://codereview.chromium.org/2346343003/diff/1/third_party/WebKit/Source/c...
third_party/WebKit/Source/core/paint/PaintInvalidator.cpp:212: // Undo content
clip and scroll before invalidating FrameView scrollbars.
On 2016/09/19 22:19:15, pdr. wrote:
> If there is a scroll translation (because the frameview is scrolled), we will
> incorrectly use the scrolled transform node for invalidating the scrollbars.
> WDYT of splitting out the self and non-self invalidations like we do for
> LayoutObjects?
> 
> Something like:
> void PrePaintTreeWalk::walk(FrameView& frameView, const
PrePaintTreeWalkContext&
> context)
> {
> ...
>     PrePaintTreeWalkContext localContext(context);
>     m_propertyTreeBuilder.buildTreeNodesForSelf(frameView,
> localContext.treeBuilderContext);
>     if (RuntimeEnabledFeatures::slimmingPaintInvalidationEnabled())
>         m_paintInvalidator.invalidatePaintForSelfIfNeeded(frameView,
> localContext.paintInvalidatorContext);
>     m_propertyTreeBuilder.buildTreeNodesForChildren(frameView,
> localContext.treeBuilderContext);
> ...
> }

Thought of this but chose the current method because
1. it will need less code change when we remove non-root-layer-scrolls code;
2. using the above method we must also split invalidatePaintIfNeeded() into two,
one for scrollbars, one for caret.

https://codereview.chromium.org/2346343003/diff/1/third_party/WebKit/Source/c...
third_party/WebKit/Source/core/paint/PaintInvalidator.cpp:217:
treeBuilderContext.current.scroll = scroll->parent();
On 2016/09/19 22:19:15, pdr. wrote:
> This will be incorrect if the frameview does not scroll (and, therefore, does
> not create a scroll node). Do any tests hit this case?
> 
> I think we need something like:
> treeBuilderContext.current.scroll = frameView.scroll() ? scroll->parent() :
> scroll;

Fixed.

No test hit the bug because if the frameView is not scrollable, there will be no
scrollbars to calculate the paint invalidation rect using the paint properties.

[Xianzhu, 2016-09-19 23:39:45]

https://codereview.chromium.org/2346343003/diff/1/third_party/WebKit/Source/c...
File third_party/WebKit/Source/core/paint/PaintInvalidator.cpp (right):

https://codereview.chromium.org/2346343003/diff/1/third_party/WebKit/Source/c...
third_party/WebKit/Source/core/paint/PaintInvalidator.cpp:187: if
(object.hasFilterInducingProperty() || object.isLayoutFlowThread() ||
object.hasFlippedBlocksWritingMode())
On 2016/09/19 23:37:40, Xianzhu wrote:
> On 2016/09/19 22:19:15, pdr. wrote:
> > Can the hasFlippedBlocksWritingMode check be made a little tougher by
checking
> > for inline too? For example, (object.isInline() &&
> > object.hasFlippedBlocksWritingMode())
> 
> This would be complex: we need to check for the flipped blocks writing mode of
a
> writing-mode container and inline status of a descendant. I will work on this
in
> this week and hopefully fix the bug soon.

P.S. I'm using the workarounds to see how many tests are affected and how many
tests are still failing because of other bugs.

[pdr., 2016-09-19 23:44:47]

On 2016/09/19 at 23:39:45, wangxianzhu wrote:
>
https://codereview.chromium.org/2346343003/diff/1/third_party/WebKit/Source/c...
> File third_party/WebKit/Source/core/paint/PaintInvalidator.cpp (right):
> 
>
https://codereview.chromium.org/2346343003/diff/1/third_party/WebKit/Source/c...
> third_party/WebKit/Source/core/paint/PaintInvalidator.cpp:187: if
(object.hasFilterInducingProperty() || object.isLayoutFlowThread() ||
object.hasFlippedBlocksWritingMode())
> On 2016/09/19 23:37:40, Xianzhu wrote:
> > On 2016/09/19 22:19:15, pdr. wrote:
> > > Can the hasFlippedBlocksWritingMode check be made a little tougher by
checking
> > > for inline too? For example, (object.isInline() &&
> > > object.hasFlippedBlocksWritingMode())
> > 
> > This would be complex: we need to check for the flipped blocks writing mode
of a
> > writing-mode container and inline status of a descendant. I will work on
this in
> > this week and hopefully fix the bug soon.
> 
> P.S. I'm using the workarounds to see how many tests are affected and how many
tests are still failing because of other bugs.

SG, LGTM

I thought we had a bug for SPV2-on-RLS but I couldn't find it. I've filed this
as https://crbug.com/648455.

[Xianzhu, 2016-09-20 02:54:26]

The CQ bit was checked by wangxianzhu@chromium.org

[commit-bot: I haz the power, 2016-09-20 02:54:47]

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-09-20 04:11:34]

Description was changed from

==========
Fix or workaround several issues of slimmingPaintInvalidation on spv1

- Fix frame scrollbar paint invalidation (non-root-layer-scrolls only)
- Workaround for multicol contents (crbug.com/648274) and inline
  contents in flipped writing mode (crbug.com/648409).

With this CL, run-webkit-tests
--additional-driver-flag=--enable-blink-features=slimmingPaintInvalidation
paint/invalidation:
Crashes: 1 (previous: 2)
Failures: 19 (previous: 62)

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

to

==========
Fix or workaround several issues of slimmingPaintInvalidation on spv1

- Fix frame scrollbar paint invalidation (non-root-layer-scrolls only)
- Workaround for multicol contents (crbug.com/648274) and inline
  contents in flipped writing mode (crbug.com/648409).

With this CL, run-webkit-tests
--additional-driver-flag=--enable-blink-features=slimmingPaintInvalidation
paint/invalidation:
Crashes: 1 (previous: 2)
Failures: 19 (previous: 62)

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

[commit-bot: I haz the power, 2016-09-20 04:11:35]

Committed patchset #2 (id:20001)

[commit-bot: I haz the power, 2016-09-20 04:14:15]

Description was changed from

==========
Fix or workaround several issues of slimmingPaintInvalidation on spv1

- Fix frame scrollbar paint invalidation (non-root-layer-scrolls only)
- Workaround for multicol contents (crbug.com/648274) and inline
  contents in flipped writing mode (crbug.com/648409).

With this CL, run-webkit-tests
--additional-driver-flag=--enable-blink-features=slimmingPaintInvalidation
paint/invalidation:
Crashes: 1 (previous: 2)
Failures: 19 (previous: 62)

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

to

==========
Fix or workaround several issues of slimmingPaintInvalidation on spv1

- Fix frame scrollbar paint invalidation (non-root-layer-scrolls only)
- Workaround for multicol contents (crbug.com/648274) and inline
  contents in flipped writing mode (crbug.com/648409).

With this CL, run-webkit-tests
--additional-driver-flag=--enable-blink-features=slimmingPaintInvalidation
paint/invalidation:
Crashes: 1 (previous: 2)
Failures: 19 (previous: 62)

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

Committed: https://crrev.com/4e39096940590df4c11256e09938957290cbd863
Cr-Commit-Position: refs/heads/master@{#419677}
==========

[commit-bot: I haz the power, 2016-09-20 04:14:16]

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