Fix issues of display item client cache flags

Fix issues of display item client cache flags

- Clear just created flag if the client is used in any display item
  or chunk. Don't set a chunk client to validly cached status because
  it doesn't have any validly cached display item (otherwise it should
  have been marked validly cached in the m_newADisplayItemList loop.

- Invalidate the client if it has any display items skipped cache.

Committed: https://crrev.com/080b866a4608e16edf4df4efefc023cbeebc90ca
Cr-Commit-Position: refs/heads/master@{#408251}

[Xianzhu, 2016-07-27 18:59:10]

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

[commit-bot: I haz the power, 2016-07-27 18:59:32]

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

[Xianzhu, 2016-07-27 19:01:04]

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

[Xianzhu, 2016-07-27 19:01:05]

How about this CL that also fixes the skipping-cache issue?

[Xianzhu, 2016-07-27 19:02:46]

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

[commit-bot: I haz the power, 2016-07-27 19:03:19]

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

[chrishtr, 2016-07-27 19:18:34]

Looking good.

https://codereview.chromium.org/2185933003/diff/20001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/paint/PaintControllerPaintTest.cpp (right):

https://codereview.chromium.org/2185933003/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/paint/PaintControllerPaintTest.cpp:70: "<div
id='div' style='width: 200px; height: 200px; opacity: 0.5'>"
Why does this use case cause cache skipping?

[Xianzhu, 2016-07-27 19:25:55]

https://codereview.chromium.org/2185933003/diff/20001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/paint/PaintControllerPaintTest.cpp (right):

https://codereview.chromium.org/2185933003/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/paint/PaintControllerPaintTest.cpp:70: "<div
id='div' style='width: 200px; height: 200px; opacity: 0.5'>"
On 2016/07/27 19:18:34, chrishtr wrote:
> Why does this use case cause cache skipping?

This doesn't test cache skipping, but test the isJustCreated flag of chunk
client.

I don't have idea about testing cache skipping yet, but your compositing change
will cover it.

[chrishtr, 2016-07-27 19:28:25]

https://codereview.chromium.org/2185933003/diff/20001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/paint/PaintControllerPaintTest.cpp (right):

https://codereview.chromium.org/2185933003/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/paint/PaintControllerPaintTest.cpp:70: "<div
id='div' style='width: 200px; height: 200px; opacity: 0.5'>"
On 2016/07/27 at 19:25:55, Xianzhu wrote:
> On 2016/07/27 19:18:34, chrishtr wrote:
> > Why does this use case cause cache skipping?
> 
> This doesn't test cache skipping, but test the isJustCreated flag of chunk
client.
> 
> I don't have idea about testing cache skipping yet, but your compositing
change will cover it.

Why is line 92 indicating that the client cache is not valid then?

As for testing: cache skipping is unittested in PaintControllerTest. I think you
can add an assert that the DisplayItemClient was marked as
uncached, where previously it was not.

[Xianzhu, 2016-07-27 19:43:39]

https://codereview.chromium.org/2185933003/diff/20001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/paint/PaintControllerPaintTest.cpp (right):

https://codereview.chromium.org/2185933003/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/paint/PaintControllerPaintTest.cpp:70: "<div
id='div' style='width: 200px; height: 200px; opacity: 0.5'>"
On 2016/07/27 19:28:25, chrishtr wrote:
> On 2016/07/27 at 19:25:55, Xianzhu wrote:
> > On 2016/07/27 19:18:34, chrishtr wrote:
> > > Why does this use case cause cache skipping?
> > 
> > This doesn't test cache skipping, but test the isJustCreated flag of chunk
> client.
> > 
> > I don't have idea about testing cache skipping yet, but your compositing
> change will cover it.
> 
> Why is line 92 indicating that the client cache is not valid then?

Added comment. A client is validly cached only if it has validly cached display
items (now and no display items skipped cache).

> 
> As for testing: cache skipping is unittested in PaintControllerTest. I think
you
> can add an assert that the DisplayItemClient was marked as
> uncached, where previously it was not.

Added PaintControllerTest.PartialSkipCache.

[chrishtr, 2016-07-27 20:14:57]

lgtm

https://codereview.chromium.org/2185933003/diff/40001/third_party/WebKit/Sour...
File third_party/WebKit/Source/platform/graphics/paint/PaintControllerTest.cpp
(right):

https://codereview.chromium.org/2185933003/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/platform/graphics/paint/PaintControllerTest.cpp:858:
EXPECT_FALSE(getPaintController().clientCacheIsValid(content));
Add a comment that it's not valid because at least one of its display items is
not cached.

Also add an expectation on the client that it has a PaintInvalidationFull
PaintInvalidationReason at this point.

[Xianzhu, 2016-07-27 20:40:34]

https://codereview.chromium.org/2185933003/diff/40001/third_party/WebKit/Sour...
File third_party/WebKit/Source/platform/graphics/paint/PaintControllerTest.cpp
(right):

https://codereview.chromium.org/2185933003/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/platform/graphics/paint/PaintControllerTest.cpp:858:
EXPECT_FALSE(getPaintController().clientCacheIsValid(content));
On 2016/07/27 20:14:57, chrishtr wrote:
> Add a comment that it's not valid because at least one of its display items is
> not cached.
> 

Done.

> Also add an expectation on the client that it has a PaintInvalidationFull
> PaintInvalidationReason at this point.

Done.

[Xianzhu, 2016-07-27 20:40:42]

The CQ bit was checked by wangxianzhu@chromium.org

[Xianzhu, 2016-07-27 20:40:42]

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/2185933003/#ps60001
(title: "Tests")

[commit-bot: I haz the power, 2016-07-27 20:40:53]

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-07-27 22:04:57]

Committed patchset #4 (id:60001)

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

Description was changed from

==========
Fix issues of display item client cache flags

- Clear just created flag if the client is used in any display item
  or chunk. Don't set a chunk client to validly cached status because
  it doesn't have any validly cached display item (otherwise it should
  have been marked validly cached in the m_newADisplayItemList loop.

- Invalidate the client if it has any display items skipped cache.
==========

to

==========
Fix issues of display item client cache flags

- Clear just created flag if the client is used in any display item
  or chunk. Don't set a chunk client to validly cached status because
  it doesn't have any validly cached display item (otherwise it should
  have been marked validly cached in the m_newADisplayItemList loop.

- Invalidate the client if it has any display items skipped cache.

Committed: https://crrev.com/080b866a4608e16edf4df4efefc023cbeebc90ca
Cr-Commit-Position: refs/heads/master@{#408251}
==========

[commit-bot: I haz the power, 2016-07-27 22:06:09]

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