Invalidate as DisplayItemClient when PaintLayer is setNeedsRepaint

Invalidate as DisplayItemClient when PaintLayer is setNeedsRepaint

This is mainly for PaintController to check cache validaty when copying
cached subsequence.

BUG=609218
Committed: https://crrev.com/72f6d81fceb5bce77d87042b305fe0d3d361e3c7
Cr-Commit-Position: refs/heads/master@{#400606}

[Xianzhu, 2016-06-17 19:16:59]

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

[chrishtr, 2016-06-18 08:42:07]

Isn't needsRepaint supposed to be orthogonal to DisplayItemClient invalidation?
Why isn't it
a bug that these PaintLayers' DisplayItemClients are not already invalidated via
another code path?

[Xianzhu, 2016-06-18 17:00:57]

On 2016/06/18 08:42:07, chrishtr wrote:
> Isn't needsRepaint supposed to be orthogonal to DisplayItemClient
invalidation?
> Why isn't it
> a bug that these PaintLayers' DisplayItemClients are not already invalidated
via
> another code path?

We didn't have another code path to invalidate PaintLayers' DisplayItemClients,
but there wasn't any bug because we always check PaintLayer::needsRepaint before
generating CachedSubsequence. This patch enables PaintController to check
validity of subsequences.

[chrishtr, 2016-06-18 22:13:55]

https://codereview.chromium.org/2071053003/diff/20001/third_party/WebKit/Sour...
File third_party/WebKit/Source/platform/graphics/paint/PaintController.cpp
(left):

https://codereview.chromium.org/2071053003/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/platform/graphics/paint/PaintController.cpp:318:
DCHECK(clientCacheIsValid(newDisplayItem.client()) ||
(RuntimeEnabledFeatures::slimmingPaintInvalidationEnabled() &&
!paintOffsetWasInvalidated(newDisplayItem.client())));
Why is this now irrelevant?

https://codereview.chromium.org/2071053003/diff/20001/third_party/WebKit/Sour...
File third_party/WebKit/Source/platform/graphics/paint/PaintController.cpp
(right):

https://codereview.chromium.org/2071053003/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/platform/graphics/paint/PaintController.cpp:235:
CHECK(clientCacheIsValid(currentIt->client()) || !currentIt->isCacheable());
DCHECK instead?

https://codereview.chromium.org/2071053003/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/platform/graphics/paint/PaintController.cpp:315:
CHECK(clientCacheIsValid(newDisplayItem.client()));
DCHECK?

[Xianzhu, 2016-06-18 23:51:25]

https://codereview.chromium.org/2071053003/diff/20001/third_party/WebKit/Sour...
File third_party/WebKit/Source/platform/graphics/paint/PaintController.cpp
(left):

https://codereview.chromium.org/2071053003/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/platform/graphics/paint/PaintController.cpp:318:
DCHECK(clientCacheIsValid(newDisplayItem.client()) ||
(RuntimeEnabledFeatures::slimmingPaintInvalidationEnabled() &&
!paintOffsetWasInvalidated(newDisplayItem.client())));
On 2016/06/18 22:13:54, chrishtr wrote:
> Why is this now irrelevant?

The removed code is for paint offset caching code which will be reimplemented
with PaintInvalidator.

Rebased on https://codereview.chromium.org/2051333005/ and this change is no
longer in this CL.

https://codereview.chromium.org/2071053003/diff/20001/third_party/WebKit/Sour...
File third_party/WebKit/Source/platform/graphics/paint/PaintController.cpp
(right):

https://codereview.chromium.org/2071053003/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/platform/graphics/paint/PaintController.cpp:235:
CHECK(clientCacheIsValid(currentIt->client()) || !currentIt->isCacheable());
On 2016/06/18 22:13:54, chrishtr wrote:
> DCHECK instead?

Used CHECK because I want this for DisplayItemClient aliveness checking.

Added #if CHECK_DISPLAY_ITEM_CLIENT_ALIVENESS.

https://codereview.chromium.org/2071053003/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/platform/graphics/paint/PaintController.cpp:315:
CHECK(clientCacheIsValid(newDisplayItem.client()));
On 2016/06/18 22:13:54, chrishtr wrote:
> DCHECK?

Ditto.

[chrishtr, 2016-06-19 00:59:46]

The CQ bit was checked by chrishtr@chromium.org

[chrishtr, 2016-06-19 00:59:46]

lgtm

[commit-bot: I haz the power, 2016-06-19 00:59:58]

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

[commit-bot: I haz the power, 2016-06-19 00:59:59]

This CL has an open dependency (Issue 2051333005 Patch 220001). 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-19 02:51:01]

The CQ bit was checked by wangxianzhu@chromium.org

[commit-bot: I haz the power, 2016-06-19 02:51:05]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/2071053003/40001

[commit-bot: I haz the power, 2016-06-19 02:54:58]

Committed patchset #3 (id:40001)

[commit-bot: I haz the power, 2016-06-19 02:56:30]

Description was changed from

==========
Invalidate as DisplayItemClient when PaintLayer is setNeedsRepaint

This is mainly for PaintController to check cache validaty when copying
cached subsequence.

BUG=609218
==========

to

==========
Invalidate as DisplayItemClient when PaintLayer is setNeedsRepaint

This is mainly for PaintController to check cache validaty when copying
cached subsequence.

BUG=609218

Committed: https://crrev.com/72f6d81fceb5bce77d87042b305fe0d3d361e3c7
Cr-Commit-Position: refs/heads/master@{#400606}
==========

[commit-bot: I haz the power, 2016-06-19 02:56:31]

Patchset 3 (id:??) landed as
https://crrev.com/72f6d81fceb5bce77d87042b305fe0d3d361e3c7
Cr-Commit-Position: refs/heads/master@{#400606}

[vasilii, 2016-06-20 10:35:51]

A revert of this CL (patchset #3 id:40001) has been created in
https://codereview.chromium.org/2080683002/ by vasilii@chromium.org.

The reason for reverting is: Based on r400605 that is to be reverted..

[Xianzhu, 2016-06-20 17:03:53]

Note that the revert was not successful.