Skip paint offset caching when DisplayItemList is skipping cache

Skip paint offset caching when DisplayItemList is skipping cache

DisplayItemList skips cache when an object may be painted multiple
times in different contexts and the object may have multiple paint
offsets.

Don't cache paint offset when DisplayItemList is skipping cache.

No visible behavior change until https://codereview.chromium.org/1387923002/
which enhances checking of problematic paint offset caching.

BUG=508383
Committed: https://crrev.com/256b50a90dc439782693f8bc614f69dc295d6b01
Cr-Commit-Position: refs/heads/master@{#352632}

[Xianzhu, 2015-10-06 00:54:58]

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

[pdr., 2015-10-06 03:10:25]

LGTM with or without my suggestion.

https://codereview.chromium.org/1389803002/diff/1/third_party/WebKit/Source/c...
File third_party/WebKit/Source/core/paint/BlockPainter.cpp (right):

https://codereview.chromium.org/1389803002/diff/1/third_party/WebKit/Source/c...
third_party/WebKit/Source/core/paint/BlockPainter.cpp:146: if
(RuntimeEnabledFeatures::slimmingPaintOffsetCachingEnabled() &&
m_layoutBlock.childrenInline() &&
!paintInfo.context->displayItemList()->skippingCache()) {
WDYT about renaming skippingCache to skippingCacheAndPaintOffsetChecks()? I
think it's likely we will forget that skipping the cache also skips the paint
offset code.

We could then update the setPreviousPaintOffset calls to
ASSERT(!skippingCacheAndPaintOffsetChecks());

[Xianzhu, 2015-10-06 16:35:23]

https://codereview.chromium.org/1389803002/diff/1/third_party/WebKit/Source/c...
File third_party/WebKit/Source/core/paint/BlockPainter.cpp (right):

https://codereview.chromium.org/1389803002/diff/1/third_party/WebKit/Source/c...
third_party/WebKit/Source/core/paint/BlockPainter.cpp:146: if
(RuntimeEnabledFeatures::slimmingPaintOffsetCachingEnabled() &&
m_layoutBlock.childrenInline() &&
!paintInfo.context->displayItemList()->skippingCache()) {
On 2015/10/06 03:10:25, pdr wrote:
> WDYT about renaming skippingCache to skippingCacheAndPaintOffsetChecks()? I
> think it's likely we will forget that skipping the cache also skips the paint
> offset code.

I think paint offset cache is also a cache that skip-cache should skip :) 

> We could then update the setPreviousPaintOffset calls to
> ASSERT(!skippingCacheAndPaintOffsetChecks());

I'd like LayoutObject no to know such details of DisplayItemList. The current
DisplayItemList::invalidatePaintOffset() assertions are enough to discover such
issues.

[Xianzhu, 2015-10-06 16:35:48]

The CQ bit was checked by wangxianzhu@chromium.org

[Xianzhu, 2015-10-06 16:35:49]

The patchset sent to the CQ was uploaded after l-g-t-m from pdr@chromium.org
Link to the patchset: https://codereview.chromium.org/1389803002/#ps20001
(title: "Rebase on origin/master")

[commit-bot: I haz the power, 2015-10-06 16:36:28]

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

[commit-bot: I haz the power, 2015-10-06 17:41:51]

Committed patchset #2 (id:20001)

[commit-bot: I haz the power, 2015-10-06 17:42:59]

Patchset 2 (id:??) landed as
https://crrev.com/256b50a90dc439782693f8bc614f69dc295d6b01
Cr-Commit-Position: refs/heads/master@{#352632}