Painter side preparation for subtree caching

Painter side preparation for subtree caching

- Add back SubtreeRecorder in BlockPainter::paint(), but initiate
  it only when the block has multiple children or multiple line boxes
  to avoid too many of subtree display items;
- Remove SubtreeRecorder::begin() because we ignore all paintings
  out of interest rect;
- Add SubtreeRecorder::canUseCache() for future real implementation of
  subtree caching.

BUG=410097
TEST=DisplayItemListPaintTest.*

Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=200359

[Xianzhu, 2015-08-11 19:30:02]

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

[chrishtr, 2015-08-11 20:29:16]

https://codereview.chromium.org/1282713003/diff/20001/Source/core/paint/Subtr...
File Source/core/paint/SubtreeRecorder.cpp (right):

https://codereview.chromium.org/1282713003/diff/20001/Source/core/paint/Subtr...
Source/core/paint/SubtreeRecorder.cpp:26: return;
Bail if phase 2 is not on

[Xianzhu, 2015-08-11 20:57:34]

https://codereview.chromium.org/1282713003/diff/20001/Source/core/paint/Subtr...
File Source/core/paint/SubtreeRecorder.cpp (right):

https://codereview.chromium.org/1282713003/diff/20001/Source/core/paint/Subtr...
Source/core/paint/SubtreeRecorder.cpp:26: return;
On 2015/08/11 20:29:15, chrishtr wrote:
> Bail if phase 2 is not on

Done.

[pdr., 2015-08-11 21:08:41]

I like the direction this is going.

https://codereview.chromium.org/1282713003/diff/60001/Source/core/paint/Block...
File Source/core/paint/BlockPainter.cpp (right):

https://codereview.chromium.org/1282713003/diff/60001/Source/core/paint/Block...
Source/core/paint/BlockPainter.cpp:35: if ((m_layoutBlock.childrenInline() &&
m_layoutBlock.lineBoxes() && m_layoutBlock.lineBoxes()->firstLineBox() &&
m_layoutBlock.lineBoxes()->firstLineBox()->nextLineBox())
Can you break this big if statement into a helper function?

https://codereview.chromium.org/1282713003/diff/60001/Source/core/paint/Displ...
File Source/core/paint/DisplayItemListPaintTest.cpp (right):

https://codereview.chromium.org/1282713003/diff/60001/Source/core/paint/Displ...
Source/core/paint/DisplayItemListPaintTest.cpp:52:
RuntimeEnabledFeatures::setSlimmingPaintV2Enabled(m_originalSlimmingPaintV2Enabled);
I think we'll want to pipeline spv2 a bit more.
RuntimeEnabledFeatures::SlimmingPaintInvalidationEnabled?

[Xianzhu, 2015-08-11 21:52:23]

https://codereview.chromium.org/1282713003/diff/60001/Source/core/paint/Block...
File Source/core/paint/BlockPainter.cpp (right):

https://codereview.chromium.org/1282713003/diff/60001/Source/core/paint/Block...
Source/core/paint/BlockPainter.cpp:35: if ((m_layoutBlock.childrenInline() &&
m_layoutBlock.lineBoxes() && m_layoutBlock.lineBoxes()->firstLineBox() &&
m_layoutBlock.lineBoxes()->firstLineBox()->nextLineBox())
On 2015/08/11 21:08:41, pdr wrote:
> Can you break this big if statement into a helper function?

Done.

https://codereview.chromium.org/1282713003/diff/60001/Source/core/paint/Displ...
File Source/core/paint/DisplayItemListPaintTest.cpp (right):

https://codereview.chromium.org/1282713003/diff/60001/Source/core/paint/Displ...
Source/core/paint/DisplayItemListPaintTest.cpp:52:
RuntimeEnabledFeatures::setSlimmingPaintV2Enabled(m_originalSlimmingPaintV2Enabled);
On 2015/08/11 21:08:41, pdr wrote:
> I think we'll want to pipeline spv2 a bit more.
> RuntimeEnabledFeatures::SlimmingPaintInvalidationEnabled?

Many spv2 features seem to have mutual dependencies. For example, paint
invalidation requires no blink compositing[1] which requires cc layerization. It
seems hard to separate them into independent features.

[1] This is mainly for simplication. Paint invalidation v2 would be much
complicated with blink compositing because of the complex composited layer
structure.

[pdr., 2015-08-11 21:53:35]

On 2015/08/11 at 21:52:23, wangxianzhu wrote:
>
https://codereview.chromium.org/1282713003/diff/60001/Source/core/paint/Block...
> File Source/core/paint/BlockPainter.cpp (right):
> 
>
https://codereview.chromium.org/1282713003/diff/60001/Source/core/paint/Block...
> Source/core/paint/BlockPainter.cpp:35: if ((m_layoutBlock.childrenInline() &&
m_layoutBlock.lineBoxes() && m_layoutBlock.lineBoxes()->firstLineBox() &&
m_layoutBlock.lineBoxes()->firstLineBox()->nextLineBox())
> On 2015/08/11 21:08:41, pdr wrote:
> > Can you break this big if statement into a helper function?
> 
> Done.
> 
>
https://codereview.chromium.org/1282713003/diff/60001/Source/core/paint/Displ...
> File Source/core/paint/DisplayItemListPaintTest.cpp (right):
> 
>
https://codereview.chromium.org/1282713003/diff/60001/Source/core/paint/Displ...
> Source/core/paint/DisplayItemListPaintTest.cpp:52:
RuntimeEnabledFeatures::setSlimmingPaintV2Enabled(m_originalSlimmingPaintV2Enabled);
> On 2015/08/11 21:08:41, pdr wrote:
> > I think we'll want to pipeline spv2 a bit more.
> > RuntimeEnabledFeatures::SlimmingPaintInvalidationEnabled?
> 
> Many spv2 features seem to have mutual dependencies. For example, paint
invalidation requires no blink compositing[1] which requires cc layerization. It
seems hard to separate them into independent features.
> 
> [1] This is mainly for simplication. Paint invalidation v2 would be much
complicated with blink compositing because of the complex composited layer
structure.

SGTM, one flag to rule them all!

[chrishtr, 2015-08-11 21:56:09]

The CQ bit was checked by chrishtr@chromium.org

[chrishtr, 2015-08-11 21:56:10]

lgtm

[commit-bot: I haz the power, 2015-08-11 21:56:17]

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

[pdr., 2015-08-11 21:57:26]

On 2015/08/11 at 21:56:17, commit-bot wrote:
> CQ is trying da patch. Follow status at
>  https://chromium-cq-status.appspot.com/patch-status/1282713003/80001
> View timeline at
>  https://chromium-cq-status.appspot.com/patch-timeline/1282713003/80001

For posterity, LGTM as well.

[commit-bot: I haz the power, 2015-08-11 23:20:36]

Committed patchset #5 (id:80001) as
https://src.chromium.org/viewvc/blink?view=rev&revision=200359