Remove LayoutBlockFlow overflow invalidation

Remove LayoutBlockFlow overflow invalidation

"Overflow" invalidation was to invalidate changed lines in a
LayoutBlockFlow with inline children. Upper and lower bounds of
changed lines are recorded during layout. The concept was not
properly named, and the implementation is complex.

Now switch to LayoutInline/LayoutText invalidation. Set the owning
inline LayoutObject shouldDoFullPaintInvalidation when a InlineBox
is deleted or changed.

This also improves performance during inline layout for paint
invalidation when many InlineBoxes are deleted.

Under-invalidation checking didn't find any error in layout tests (which
would be pixel mismatches or crashes).

With this patch, we may invalidate bigger rect in case that not all
InlineBoxes changed in a big LayutInline or LayoutText. Previously the
paint invalidation rects cover the changed InlineBoxes only; now
the rects cover the whole LayoutInline and LayoutText containing the
changed InlineBoxes. The case normally happens when some intruding
floating objects change geometry which is not very common.

In other cases we may invalidate smaller rects because we no longer
invalidate the whole width of the LayoutBlockFlow.

We may invalidate more rects than before.

BUG=619630
Committed: https://crrev.com/0e20bd123062a6bc463391c8970b24936983c466
Cr-Commit-Position: refs/heads/master@{#402292}

[Xianzhu, 2016-06-22 22:05:16]

Description was changed from

==========
Remove LayoutBlockFlow overflow invalidation
==========

to

==========
Remove LayoutBlockFlow overflow invalidation

"Overflow" invalidation was to invalidate changed lines in a
LayoutBlockFlow with inline children. Upper and lower bounds of
changed lines are recorded during layout. The concept was not
properly named, and the implementation is complex.

Now switch to LayoutInline/LayoutText invalidation. Set the owning
inline LayoutObject shouldDoFullPaintInvalidation when a InlineBox
is deleted or changed.

This also improves performance during inline layout when many
InlineBoxes are deleted.

BUG=619630
==========

[Xianzhu, 2016-06-22 22:05:57]

Description was changed from

==========
Remove LayoutBlockFlow overflow invalidation

"Overflow" invalidation was to invalidate changed lines in a
LayoutBlockFlow with inline children. Upper and lower bounds of
changed lines are recorded during layout. The concept was not
properly named, and the implementation is complex.

Now switch to LayoutInline/LayoutText invalidation. Set the owning
inline LayoutObject shouldDoFullPaintInvalidation when a InlineBox
is deleted or changed.

This also improves performance during inline layout when many
InlineBoxes are deleted.

BUG=619630
==========

to

==========
Remove LayoutBlockFlow overflow invalidation

"Overflow" invalidation was to invalidate changed lines in a
LayoutBlockFlow with inline children. Upper and lower bounds of
changed lines are recorded during layout. The concept was not
properly named, and the implementation is complex.

Now switch to LayoutInline/LayoutText invalidation. Set the owning
inline LayoutObject shouldDoFullPaintInvalidation when a InlineBox
is deleted or changed.

This also improves performance during inline layout for paint
invalidation when many InlineBoxes are deleted.

BUG=619630
==========

[Xianzhu, 2016-06-24 00:14:27]

Description was changed from

==========
Remove LayoutBlockFlow overflow invalidation

"Overflow" invalidation was to invalidate changed lines in a
LayoutBlockFlow with inline children. Upper and lower bounds of
changed lines are recorded during layout. The concept was not
properly named, and the implementation is complex.

Now switch to LayoutInline/LayoutText invalidation. Set the owning
inline LayoutObject shouldDoFullPaintInvalidation when a InlineBox
is deleted or changed.

This also improves performance during inline layout for paint
invalidation when many InlineBoxes are deleted.

BUG=619630
==========

to

==========
Remove LayoutBlockFlow overflow invalidation

"Overflow" invalidation was to invalidate changed lines in a
LayoutBlockFlow with inline children. Upper and lower bounds of
changed lines are recorded during layout. The concept was not
properly named, and the implementation is complex.

Now switch to LayoutInline/LayoutText invalidation. Set the owning
inline LayoutObject shouldDoFullPaintInvalidation when a InlineBox
is deleted or changed.

This also improves performance during inline layout for paint
invalidation when many InlineBoxes are deleted.

Under-invalidation checking didn't find any error in layout tests.

With this patch, we may invalidate bigger rect in case that not all
InlineBoxes changed in a big LayutInline or LayoutText. Previously the
paint invalidation rects cover the changed InlineBoxes only; now
the rects cover the whole LayoutInline and LayoutText containing the
changed InlineBoxes. The case normally happens when some intruding
floating objects change geometry which is not very common.

In other cases we may invalidate smaller rects because we no longer
invalidate the whole width of the LayoutBlockFlow.

We may invalidate more rects than before.

BUG=619630
==========

[Xianzhu, 2016-06-24 00:15:07]

Description was changed from

==========
Remove LayoutBlockFlow overflow invalidation

"Overflow" invalidation was to invalidate changed lines in a
LayoutBlockFlow with inline children. Upper and lower bounds of
changed lines are recorded during layout. The concept was not
properly named, and the implementation is complex.

Now switch to LayoutInline/LayoutText invalidation. Set the owning
inline LayoutObject shouldDoFullPaintInvalidation when a InlineBox
is deleted or changed.

This also improves performance during inline layout for paint
invalidation when many InlineBoxes are deleted.

Under-invalidation checking didn't find any error in layout tests.

With this patch, we may invalidate bigger rect in case that not all
InlineBoxes changed in a big LayutInline or LayoutText. Previously the
paint invalidation rects cover the changed InlineBoxes only; now
the rects cover the whole LayoutInline and LayoutText containing the
changed InlineBoxes. The case normally happens when some intruding
floating objects change geometry which is not very common.

In other cases we may invalidate smaller rects because we no longer
invalidate the whole width of the LayoutBlockFlow.

We may invalidate more rects than before.

BUG=619630
==========

to

==========
Remove LayoutBlockFlow overflow invalidation

"Overflow" invalidation was to invalidate changed lines in a
LayoutBlockFlow with inline children. Upper and lower bounds of
changed lines are recorded during layout. The concept was not
properly named, and the implementation is complex.

Now switch to LayoutInline/LayoutText invalidation. Set the owning
inline LayoutObject shouldDoFullPaintInvalidation when a InlineBox
is deleted or changed.

This also improves performance during inline layout for paint
invalidation when many InlineBoxes are deleted.

Under-invalidation checking didn't find any error in layout tests (which
would be pixel mismatches or crashes).

With this patch, we may invalidate bigger rect in case that not all
InlineBoxes changed in a big LayutInline or LayoutText. Previously the
paint invalidation rects cover the changed InlineBoxes only; now
the rects cover the whole LayoutInline and LayoutText containing the
changed InlineBoxes. The case normally happens when some intruding
floating objects change geometry which is not very common.

In other cases we may invalidate smaller rects because we no longer
invalidate the whole width of the LayoutBlockFlow.

We may invalidate more rects than before.

BUG=619630
==========

[Xianzhu, 2016-06-24 00:30:56]

Patchset #5 (id:80001) has been deleted

[Xianzhu, 2016-06-24 17:12:56]

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

[chrishtr, 2016-06-24 17:18:57]

The larger invalidations might have a performance impact for editing use-cases
where there are lots of small text invalidations, right?
Do we have any performance tests for that?

[Xianzhu, 2016-06-24 19:15:14]

On 2016/06/24 17:18:57, chrishtr wrote:
> The larger invalidations might have a performance impact for editing use-cases
> where there are lots of small text invalidations, right?

Just tested with a contenteditable div with a large LayoutText and many small
LayoutTexts, and found no larger invalidations or more rect invlaidations
because we always fully invalidate the LayoutText with content text changed.

Based on observations so far, the larger invalidations occur only when the
LayoutText or LayoutInline containing the changed InlineBoxes isn't fully
invalidated and doesn't change visual rect. This seems to happen during layout
of floating objects only.

> Do we have any performance tests for that?

We have performance tests covering text layout under PerformanceTests/Layout.
Running the benchmarks on trybots: https://codereview.chromium.org/2093103003.

[Xianzhu, 2016-06-25 05:35:28]

> We have performance tests covering text layout under PerformanceTests/Layout.
> Running the benchmarks on trybots: https://codereview.chromium.org/2093103003.

Perf runs finished. There seem a lot of noise, for example some tests progressed
on some bots, but are not changed or regressed on other bot, but generally there
seem more progressions than regressions.

[chrishtr, 2016-06-27 15:12:42]

https://codereview.chromium.org/2087513003/diff/160001/third_party/WebKit/Sou...
File third_party/WebKit/Source/core/layout/LayoutBlockFlow.cpp (left):

https://codereview.chromium.org/2087513003/diff/160001/third_party/WebKit/Sou...
third_party/WebKit/Source/core/layout/LayoutBlockFlow.cpp:2792: void
LayoutBlockFlow::invalidatePaintForOverflow()
Does the version control history give a hint as to which use cases this code was
optimizing?

https://codereview.chromium.org/2087513003/diff/160001/third_party/WebKit/Sou...
File third_party/WebKit/Source/core/layout/api/LineLayoutItem.h (left):

https://codereview.chromium.org/2087513003/diff/160001/third_party/WebKit/Sou...
third_party/WebKit/Source/core/layout/api/LineLayoutItem.h:425: return
m_layoutObject->slowSetPaintingLayerNeedsRepaintAndInvalidateDisplayItemClient(displayItemClient,
reason);
needsRepaint is not needed on the containing PaintLayer?

https://codereview.chromium.org/2087513003/diff/160001/third_party/WebKit/Sou...
File third_party/WebKit/Source/core/layout/line/InlineBox.cpp (left):

https://codereview.chromium.org/2087513003/diff/160001/third_party/WebKit/Sou...
third_party/WebKit/Source/core/layout/line/InlineBox.cpp:70:
m_lineLayoutItem.slowSetPaintingLayerNeedsRepaintAndInvalidateDisplayItemClient(*this,
PaintInvalidationFull);
needsRepaint is not needed on the containing PaintLayer?

[Xianzhu, 2016-06-27 17:05:19]

https://codereview.chromium.org/2087513003/diff/160001/third_party/WebKit/Sou...
File third_party/WebKit/Source/core/layout/LayoutBlockFlow.cpp (left):

https://codereview.chromium.org/2087513003/diff/160001/third_party/WebKit/Sou...
third_party/WebKit/Source/core/layout/LayoutBlockFlow.cpp:2792: void
LayoutBlockFlow::invalidatePaintForOverflow()
On 2016/06/27 15:12:42, chrishtr wrote:
> Does the version control history give a hint as to which use cases this code
was
> optimizing?

This was to avoid full inalidation of LayoutBlockFlow with inline contents when
not all of the lines are changed during layout. This was needed before
paint-invalidation-after-layout when we didn't invalidate LayoutInlines and
LayoutTexts and let LayoutBlockFlow handle all paint invalidation of changed
line layout.

Actually I hadn't know how and why of this before several weeks ago, and didn't
mention this in the paint invalidation documentation in
Source/core/paint/README.md.

This is also hard to handle in SPv2 paint invalidation because we'll use visual
rects of display items.

https://codereview.chromium.org/2087513003/diff/160001/third_party/WebKit/Sou...
File third_party/WebKit/Source/core/layout/line/InlineBox.cpp (left):

https://codereview.chromium.org/2087513003/diff/160001/third_party/WebKit/Sou...
third_party/WebKit/Source/core/layout/line/InlineBox.cpp:70:
m_lineLayoutItem.slowSetPaintingLayerNeedsRepaintAndInvalidateDisplayItemClient(*this,
PaintInvalidationFull);
On 2016/06/27 15:12:42, chrishtr wrote:
> needsRepaint is not needed on the containing PaintLayer?

The new code will call setShouldDoFullPaintInvalidation() of the containing
LayoutObject, and we'll invalidate the painting layer during paint invalidation.

[chrishtr, 2016-06-27 17:24:13]

The CQ bit was checked by chrishtr@chromium.org

[chrishtr, 2016-06-27 17:24:14]

lgtm

[commit-bot: I haz the power, 2016-06-27 17:25:11]

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

[commit-bot: I haz the power, 2016-06-27 17:25:12]

This CL has an open dependency (Issue 2092953002 Patch 40001). Please resolve
the dependency and try again.
If you are sure that there is no real dependency, please use one of the options
listed in https://goo.gl/9Es4OR to land the CL.

[Xianzhu, 2016-06-27 17:38:13]

The CQ bit was checked by wangxianzhu@chromium.org

[Xianzhu, 2016-06-27 17:38:13]

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

[commit-bot: I haz the power, 2016-06-27 17:39: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-06-27 19:18:48]

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

[commit-bot: I haz the power, 2016-06-27 19:18:50]

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

[Xianzhu, 2016-06-27 20:06:18]

The CQ bit was checked by wangxianzhu@chromium.org

[Xianzhu, 2016-06-27 20:06:19]

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

[commit-bot: I haz the power, 2016-06-27 20:07:12]

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-06-27 21:36:27]

Description was changed from

==========
Remove LayoutBlockFlow overflow invalidation

"Overflow" invalidation was to invalidate changed lines in a
LayoutBlockFlow with inline children. Upper and lower bounds of
changed lines are recorded during layout. The concept was not
properly named, and the implementation is complex.

Now switch to LayoutInline/LayoutText invalidation. Set the owning
inline LayoutObject shouldDoFullPaintInvalidation when a InlineBox
is deleted or changed.

This also improves performance during inline layout for paint
invalidation when many InlineBoxes are deleted.

Under-invalidation checking didn't find any error in layout tests (which
would be pixel mismatches or crashes).

With this patch, we may invalidate bigger rect in case that not all
InlineBoxes changed in a big LayutInline or LayoutText. Previously the
paint invalidation rects cover the changed InlineBoxes only; now
the rects cover the whole LayoutInline and LayoutText containing the
changed InlineBoxes. The case normally happens when some intruding
floating objects change geometry which is not very common.

In other cases we may invalidate smaller rects because we no longer
invalidate the whole width of the LayoutBlockFlow.

We may invalidate more rects than before.

BUG=619630
==========

to

==========
Remove LayoutBlockFlow overflow invalidation

"Overflow" invalidation was to invalidate changed lines in a
LayoutBlockFlow with inline children. Upper and lower bounds of
changed lines are recorded during layout. The concept was not
properly named, and the implementation is complex.

Now switch to LayoutInline/LayoutText invalidation. Set the owning
inline LayoutObject shouldDoFullPaintInvalidation when a InlineBox
is deleted or changed.

This also improves performance during inline layout for paint
invalidation when many InlineBoxes are deleted.

Under-invalidation checking didn't find any error in layout tests (which
would be pixel mismatches or crashes).

With this patch, we may invalidate bigger rect in case that not all
InlineBoxes changed in a big LayutInline or LayoutText. Previously the
paint invalidation rects cover the changed InlineBoxes only; now
the rects cover the whole LayoutInline and LayoutText containing the
changed InlineBoxes. The case normally happens when some intruding
floating objects change geometry which is not very common.

In other cases we may invalidate smaller rects because we no longer
invalidate the whole width of the LayoutBlockFlow.

We may invalidate more rects than before.

BUG=619630
==========

[commit-bot: I haz the power, 2016-06-27 21:36:28]

Committed patchset #10 (id:200001)

[commit-bot: I haz the power, 2016-06-27 21:38:38]

Description was changed from

==========
Remove LayoutBlockFlow overflow invalidation

"Overflow" invalidation was to invalidate changed lines in a
LayoutBlockFlow with inline children. Upper and lower bounds of
changed lines are recorded during layout. The concept was not
properly named, and the implementation is complex.

Now switch to LayoutInline/LayoutText invalidation. Set the owning
inline LayoutObject shouldDoFullPaintInvalidation when a InlineBox
is deleted or changed.

This also improves performance during inline layout for paint
invalidation when many InlineBoxes are deleted.

Under-invalidation checking didn't find any error in layout tests (which
would be pixel mismatches or crashes).

With this patch, we may invalidate bigger rect in case that not all
InlineBoxes changed in a big LayutInline or LayoutText. Previously the
paint invalidation rects cover the changed InlineBoxes only; now
the rects cover the whole LayoutInline and LayoutText containing the
changed InlineBoxes. The case normally happens when some intruding
floating objects change geometry which is not very common.

In other cases we may invalidate smaller rects because we no longer
invalidate the whole width of the LayoutBlockFlow.

We may invalidate more rects than before.

BUG=619630
==========

to

==========
Remove LayoutBlockFlow overflow invalidation

"Overflow" invalidation was to invalidate changed lines in a
LayoutBlockFlow with inline children. Upper and lower bounds of
changed lines are recorded during layout. The concept was not
properly named, and the implementation is complex.

Now switch to LayoutInline/LayoutText invalidation. Set the owning
inline LayoutObject shouldDoFullPaintInvalidation when a InlineBox
is deleted or changed.

This also improves performance during inline layout for paint
invalidation when many InlineBoxes are deleted.

Under-invalidation checking didn't find any error in layout tests (which
would be pixel mismatches or crashes).

With this patch, we may invalidate bigger rect in case that not all
InlineBoxes changed in a big LayutInline or LayoutText. Previously the
paint invalidation rects cover the changed InlineBoxes only; now
the rects cover the whole LayoutInline and LayoutText containing the
changed InlineBoxes. The case normally happens when some intruding
floating objects change geometry which is not very common.

In other cases we may invalidate smaller rects because we no longer
invalidate the whole width of the LayoutBlockFlow.

We may invalidate more rects than before.

BUG=619630

Committed: https://crrev.com/0e20bd123062a6bc463391c8970b24936983c466
Cr-Commit-Position: refs/heads/master@{#402292}
==========

[commit-bot: I haz the power, 2016-06-27 21:38:40]

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