Need to repaint its ::first-line background when a block moves.

Need to repaint its ::first-line background when a block moves.

Everything else pertaining to ::first-line is painted by the InlineBox objects
established by DOM nodes (text, inline SPAN, whatever), so they get invalidated
when their LayoutObjects get invalidated, but ::first-line background is
special and is painted by RootInlineBox, which wasn't properly invalidated
along with the rest.

BUG=537638
R=chrishtr@chromium.org,eae@chromium.org,wangxianzhu@chromium.org

Committed: https://crrev.com/c2c4f22ffab452a9225239bfc86da699af342386
Cr-Commit-Position: refs/heads/master@{#366396}

[mstensho (USE GERRIT), 2015-12-17 13:56:40]

Description was changed from

==========
Need to repaint its ::first-line background when a block moves.

Everything else pertaining to ::first-line is painted by the InlineBox objects
established by DOM nodes (text, inline SPAN, whatever), so they get invalidated
when their LayoutObjects get invalidated, but ::first-line background is
special and is painted by RootInlineBox, which wasn't properly invalidated
along with the rest.

BUG=537638
R=leviw@chromium.org,wangxianzhu@chromium.org
==========

to

==========
Need to repaint its ::first-line background when a block moves.

Everything else pertaining to ::first-line is painted by the InlineBox objects
established by DOM nodes (text, inline SPAN, whatever), so they get invalidated
when their LayoutObjects get invalidated, but ::first-line background is
special and is painted by RootInlineBox, which wasn't properly invalidated
along with the rest.

BUG=537638
R=eae@chromium.org,wangxianzhu@chromium.org
==========

[mstensho (USE GERRIT), 2015-12-17 13:56:40]

mstensho@opera.com changed reviewers:
+ eae@chromium.org
- leviw@chromium.org

[mstensho (USE GERRIT), 2015-12-17 14:01:48]

Description was changed from

==========
Need to repaint its ::first-line background when a block moves.

Everything else pertaining to ::first-line is painted by the InlineBox objects
established by DOM nodes (text, inline SPAN, whatever), so they get invalidated
when their LayoutObjects get invalidated, but ::first-line background is
special and is painted by RootInlineBox, which wasn't properly invalidated
along with the rest.

BUG=537638
R=eae@chromium.org,wangxianzhu@chromium.org
==========

to

==========
Need to repaint its ::first-line background when a block moves.

Everything else pertaining to ::first-line is painted by the InlineBox objects
established by DOM nodes (text, inline SPAN, whatever), so they get invalidated
when their LayoutObjects get invalidated, but ::first-line background is
special and is painted by RootInlineBox, which wasn't properly invalidated
along with the rest.

BUG=537638
R=chrishtr@chromium.org,eae@chromium.org,wangxianzhu@chromium.org
==========

[mstensho (USE GERRIT), 2015-12-17 14:01:48]

mstensho@opera.com changed reviewers:
+ chrishtr@chromium.org

[Xianzhu, 2015-12-18 07:01:20]

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

https://codereview.chromium.org/1536663004/diff/1/third_party/WebKit/Source/c...
third_party/WebKit/Source/core/layout/LayoutBlockFlowLine.cpp:2091:
invalidateDisplayItemClient(*line);
Will this always invalidate the first line, even if there is no first-line
style?

[mstensho (USE GERRIT), 2015-12-18 10:33:04]

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

https://codereview.chromium.org/1536663004/diff/1/third_party/WebKit/Source/c...
third_party/WebKit/Source/core/layout/LayoutBlockFlowLine.cpp:2091:
invalidateDisplayItemClient(*line);
On 2015/12/18 07:01:20, Xianzhu wrote:
> Will this always invalidate the first line, even if there is no first-line
> style?

Yeah, looks like it. That wasn't my intention.

[mstensho (USE GERRIT), 2015-12-18 13:11:11]

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

https://codereview.chromium.org/1536663004/diff/1/third_party/WebKit/Source/c...
third_party/WebKit/Source/core/layout/LayoutBlockFlowLine.cpp:2091:
invalidateDisplayItemClient(*line);
On 2015/12/18 10:33:04, mstensho wrote:
> On 2015/12/18 07:01:20, Xianzhu wrote:
> > Will this always invalidate the first line, even if there is no first-line
> > style?
> 
> Yeah, looks like it. That wasn't my intention.

Fixed it. Not sure if it's worth it, though. Could it generally be more
expensive to figure out that you have no ::first-line that applies, than to just
invalidate it (which would also involve rebaselining a bunch of tests)? What do
you think?

[Xianzhu, 2015-12-18 17:24:54]

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

https://codereview.chromium.org/1536663004/diff/1/third_party/WebKit/Source/c...
third_party/WebKit/Source/core/layout/LayoutBlockFlowLine.cpp:2082:
PaintInvalidationReason
LayoutBlockFlow::invalidatePaintIfNeeded(PaintInvalidationState&
paintInvalidationState, const LayoutBoxModelObject& paintInvalidationContainer)
This will miss the LayoutButton case, but I think it's fine to leave this corner
case as-is which will be automatically fixed when we fix crbug.com/226252.

https://codereview.chromium.org/1536663004/diff/1/third_party/WebKit/Source/c...
third_party/WebKit/Source/core/layout/LayoutBlockFlowLine.cpp:2091:
invalidateDisplayItemClient(*line);
On 2015/12/18 13:11:11, mstensho wrote:
> On 2015/12/18 10:33:04, mstensho wrote:
> > On 2015/12/18 07:01:20, Xianzhu wrote:
> > > Will this always invalidate the first line, even if there is no first-line
> > > style?
> > 
> > Yeah, looks like it. That wasn't my intention.
> 
> Fixed it. Not sure if it's worth it, though. Could it generally be more
> expensive to figure out that you have no ::first-line that applies, than to
just
> invalidate it (which would also involve rebaselining a bunch of tests)? What
do
> you think?

Hmm enclosingFirstLineStyleBlock() seems not cheap (especially there is no
first-line style) and it would be tested for all LayoutBlockFlows. Normally a
RootInlineBox's painting is just one drawRect which is cheap. This invalidation
also won't cause more rasterization invalidation. I think we can use this
always-invalidation version.

Patch Set 1 lgtm.

[mstensho (USE GERRIT), 2015-12-21 10:00:24]

Patchset #2 (id:20001) has been deleted

[mstensho (USE GERRIT), 2015-12-21 10:54:10]

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

https://codereview.chromium.org/1536663004/diff/1/third_party/WebKit/Source/c...
third_party/WebKit/Source/core/layout/LayoutBlockFlowLine.cpp:2091:
invalidateDisplayItemClient(*line);
On 2015/12/18 17:24:54, Xianzhu wrote:
> On 2015/12/18 13:11:11, mstensho wrote:
> > On 2015/12/18 10:33:04, mstensho wrote:
> > > On 2015/12/18 07:01:20, Xianzhu wrote:
> > > > Will this always invalidate the first line, even if there is no
first-line
> > > > style?
> > > 
> > > Yeah, looks like it. That wasn't my intention.
> > 
> > Fixed it. Not sure if it's worth it, though. Could it generally be more
> > expensive to figure out that you have no ::first-line that applies, than to
> just
> > invalidate it (which would also involve rebaselining a bunch of tests)? What
> do
> > you think?
> 
> Hmm enclosingFirstLineStyleBlock() seems not cheap (especially there is no
> first-line style) and it would be tested for all LayoutBlockFlows. Normally a
> RootInlineBox's painting is just one drawRect which is cheap. This
invalidation
> also won't cause more rasterization invalidation. I think we can use this
> always-invalidation version.
> 
> Patch Set 1 lgtm.

Done. Added a comment too.

[mstensho (USE GERRIT), 2015-12-21 10:54:20]

The CQ bit was checked by mstensho@opera.com

[mstensho (USE GERRIT), 2015-12-21 10:54:20]

The patchset sent to the CQ was uploaded after l-g-t-m from
wangxianzhu@chromium.org
Link to the patchset: https://codereview.chromium.org/1536663004/#ps80001
(title: "Need to rebaseline a bunch of tests because of this CL.")

[commit-bot: I haz the power, 2015-12-21 10:54:29]

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

[commit-bot: I haz the power, 2015-12-21 12:19:49]

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

[commit-bot: I haz the power, 2015-12-21 12:19:50]

Try jobs failed on following builders:
  win_chromium_rel_ng on tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_rel_...)

[mstensho (USE GERRIT), 2015-12-21 13:00:03]

The CQ bit was checked by mstensho@opera.com

[mstensho (USE GERRIT), 2015-12-21 13:00:03]

The patchset sent to the CQ was uploaded after l-g-t-m from
wangxianzhu@chromium.org
Link to the patchset: https://codereview.chromium.org/1536663004/#ps140001
(title: "Two tests even need manual rebaseline, since they already existed in
TestExpectations.")

[commit-bot: I haz the power, 2015-12-21 13:00:07]

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

[commit-bot: I haz the power, 2015-12-21 16:23:52]

Description was changed from

==========
Need to repaint its ::first-line background when a block moves.

Everything else pertaining to ::first-line is painted by the InlineBox objects
established by DOM nodes (text, inline SPAN, whatever), so they get invalidated
when their LayoutObjects get invalidated, but ::first-line background is
special and is painted by RootInlineBox, which wasn't properly invalidated
along with the rest.

BUG=537638
R=chrishtr@chromium.org,eae@chromium.org,wangxianzhu@chromium.org
==========

to

==========
Need to repaint its ::first-line background when a block moves.

Everything else pertaining to ::first-line is painted by the InlineBox objects
established by DOM nodes (text, inline SPAN, whatever), so they get invalidated
when their LayoutObjects get invalidated, but ::first-line background is
special and is painted by RootInlineBox, which wasn't properly invalidated
along with the rest.

BUG=537638
R=chrishtr@chromium.org,eae@chromium.org,wangxianzhu@chromium.org
==========

[commit-bot: I haz the power, 2015-12-21 16:23:53]

Committed patchset #7 (id:140001)

[commit-bot: I haz the power, 2015-12-21 16:24:54]

Description was changed from

==========
Need to repaint its ::first-line background when a block moves.

Everything else pertaining to ::first-line is painted by the InlineBox objects
established by DOM nodes (text, inline SPAN, whatever), so they get invalidated
when their LayoutObjects get invalidated, but ::first-line background is
special and is painted by RootInlineBox, which wasn't properly invalidated
along with the rest.

BUG=537638
R=chrishtr@chromium.org,eae@chromium.org,wangxianzhu@chromium.org
==========

to

==========
Need to repaint its ::first-line background when a block moves.

Everything else pertaining to ::first-line is painted by the InlineBox objects
established by DOM nodes (text, inline SPAN, whatever), so they get invalidated
when their LayoutObjects get invalidated, but ::first-line background is
special and is painted by RootInlineBox, which wasn't properly invalidated
along with the rest.

BUG=537638
R=chrishtr@chromium.org,eae@chromium.org,wangxianzhu@chromium.org

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

[commit-bot: I haz the power, 2015-12-21 16:24:55]

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