Repaint on interest rect change

Repaint on interest rect change

This is needed before we enable sync painting. Repaint when interest
rect changed sufficiently.

Also combine GraphicsLayer::paintIfNeeded() and GraphicsLayer::paint().
The former was needed because the caller needed to check the result to
determine if it's safe to commit the new paints. Now this is not needed.
Actually we always paint, and output CachedDisplayItemList when the
whole display item list is cached.

BUG=536999
Committed: https://crrev.com/f118da73a9f15420c1757133de7b89a019186bb3
Cr-Commit-Position: refs/heads/master@{#356979}

[Xianzhu, 2015-10-28 19:55:36]

Description was changed from

==========
Repaint on interest rect change

This is needed before we enable sync painting. Repaint when interest
rect changed sufficiently.

Also combine GraphicsLayer::paintIfNeeded() and GraphicsLayer::paint().
The former was needed because the caller needed to check the result to
determine if it's safe to commit the new paints. Now this is not needed.
Actually we always paint, and output CachedDisplayItemList when the
whole display item list is cached.

BUG=536999
==========

to

==========
Repaint on interest rect change

This is needed before we enable sync painting. Repaint when interest
rect changed sufficiently.

Also combine GraphicsLayer::paintIfNeeded() and GraphicsLayer::paint().
The former was needed because the caller needed to check the result to
determine if it's safe to commit the new paints. Now this is not needed.
Actually we always paint, and output CachedDisplayItemList when the
whole display item list is cached.

BUG=536999
==========

[Xianzhu, 2015-10-28 19:55:45]

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

[chrishtr, 2015-10-28 21:03:54]

https://codereview.chromium.org/1428643004/diff/20001/third_party/WebKit/Sour...
File
third_party/WebKit/Source/core/layout/compositing/CompositedLayerMapping.cpp
(right):

https://codereview.chromium.org/1428643004/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/layout/compositing/CompositedLayerMapping.cpp:2207:
return std::abs(newValue - previousValue) * 16 < baseValue;
> instead of < ?

https://codereview.chromium.org/1428643004/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/layout/compositing/CompositedLayerMapping.cpp:2210:
static bool interestRectChangeNeedsRepaint(const IntRect& newInterestRect, const
IntRect& previousInterestRect)
This is a little different than what DisplayListRecordingSource does. We should
just copy the logic exactly.

Also, rename to interestChangedEnoughToNeedRepaint.

[Xianzhu, 2015-10-29 01:55:09]

https://codereview.chromium.org/1428643004/diff/20001/third_party/WebKit/Sour...
File
third_party/WebKit/Source/core/layout/compositing/CompositedLayerMapping.cpp
(right):

https://codereview.chromium.org/1428643004/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/layout/compositing/CompositedLayerMapping.cpp:2210:
static bool interestRectChangeNeedsRepaint(const IntRect& newInterestRect, const
IntRect& previousInterestRect)
On 2015/10/28 21:03:54, chrishtr wrote:
> This is a little different than what DisplayListRecordingSource does. We
should
> just copy the logic exactly.
> 
> Also, rename to interestChangedEnoughToNeedRepaint.

Done.

[chrishtr, 2015-10-29 16:34:42]

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

[chrishtr, 2015-10-29 16:34:42]

https://codereview.chromium.org/1428643004/diff/40001/third_party/WebKit/Sour...
File
third_party/WebKit/Source/core/layout/compositing/CompositedLayerMappingTest.cpp
(right):

https://codereview.chromium.org/1428643004/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/layout/compositing/CompositedLayerMappingTest.cpp:211:
TEST_F(CompositedLayerMappingTest, InterestRectChangedEnoughToRepaint)
Please copy the exact tests from here:

https://code.google.com/p/chromium/codesearch#chromium/src/cc/playback/displa...

so we know the algorithms are the same.

[Xianzhu, 2015-10-29 17:17:27]

https://codereview.chromium.org/1428643004/diff/40001/third_party/WebKit/Sour...
File
third_party/WebKit/Source/core/layout/compositing/CompositedLayerMappingTest.cpp
(right):

https://codereview.chromium.org/1428643004/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/layout/compositing/CompositedLayerMappingTest.cpp:211:
TEST_F(CompositedLayerMappingTest, InterestRectChangedEnoughToRepaint)
On 2015/10/29 16:34:42, chrishtr wrote:
> Please copy the exact tests from here:
> 
>
https://code.google.com/p/chromium/codesearch#chromium/src/cc/playback/displa...
> 
> so we know the algorithms are the same.

Done.

[chrishtr, 2015-10-29 17:20:26]

Is there any integration test (i.e. layout test) which exercises interest rect
updates? e.g. have content 10000px down and then scroll to it.

[chrishtr, 2015-10-29 17:21:31]

https://codereview.chromium.org/1428643004/diff/60001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/frame/FrameView.h (right):

https://codereview.chromium.org/1428643004/diff/60001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/frame/FrameView.h:235: void
updateAllLifecyclePhases(const IntRect* interestRect = nullptr);
Will there be any non-testing use case for passing an interest rect here?
If not, we should find a way to test without this.

[Xianzhu, 2015-10-29 17:29:43]

On 2015/10/29 17:20:26, chrishtr wrote:
> Is there any integration test (i.e. layout test) which exercises interest rect
> updates? e.g. have content 10000px down and then scroll to it.

Searched for '0000px' under compositing/, fast/scrolling, fast/repaint and
paint/, and found no tests directly testing this. Will add one.

[Xianzhu, 2015-10-29 18:31:28]

https://codereview.chromium.org/1428643004/diff/60001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/frame/FrameView.h (right):

https://codereview.chromium.org/1428643004/diff/60001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/frame/FrameView.h:235: void
updateAllLifecyclePhases(const IntRect* interestRect = nullptr);
On 2015/10/29 17:21:30, chrishtr wrote:
> Will there be any non-testing use case for passing an interest rect here?
> If not, we should find a way to test without this.

Done.

[Xianzhu, 2015-10-29 18:31:45]

On 2015/10/29 17:29:43, Xianzhu wrote:
> On 2015/10/29 17:20:26, chrishtr wrote:
> > Is there any integration test (i.e. layout test) which exercises interest
rect
> > updates? e.g. have content 10000px down and then scroll to it.
> 
> Searched for '0000px' under compositing/, fast/scrolling, fast/repaint and
> paint/, and found no tests directly testing this. Will add one.

Also done.

[chrishtr, 2015-10-29 19:56:51]

https://codereview.chromium.org/1428643004/diff/80001/third_party/WebKit/Sour...
File third_party/WebKit/Source/platform/graphics/GraphicsLayer.h (right):

https://codereview.chromium.org/1428643004/diff/80001/third_party/WebKit/Sour...
third_party/WebKit/Source/platform/graphics/GraphicsLayer.h:248: void
paint(GraphicsContext&, const IntRect* clip) override;
How about:

paint()

paintForTesting(IntRect cullRect)

?

[Xianzhu, 2015-10-29 20:00:46]

https://codereview.chromium.org/1428643004/diff/80001/third_party/WebKit/Sour...
File third_party/WebKit/Source/platform/graphics/GraphicsLayer.h (right):

https://codereview.chromium.org/1428643004/diff/80001/third_party/WebKit/Sour...
third_party/WebKit/Source/platform/graphics/GraphicsLayer.h:248: void
paint(GraphicsContext&, const IntRect* clip) override;
On 2015/10/29 19:56:51, chrishtr wrote:
> How about:
> 
> paint()
> 
> paintForTesting(IntRect cullRect)
> 
> ?

We still need the parameter when it is called from ContentPageDelegate,
especially for the non-synchronized-painting path.

We can do this when we remove the non-sychronized-painting path.

[chrishtr, 2015-10-29 20:02:18]

The CQ bit was checked by chrishtr@chromium.org

[chrishtr, 2015-10-29 20:02:18]

lgtm

[commit-bot: I haz the power, 2015-10-29 20:02:41]

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

[commit-bot: I haz the power, 2015-10-29 21:00:25]

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

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

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_...)

[Xianzhu, 2015-10-29 21:16:53]

The CQ bit was checked by wangxianzhu@chromium.org

[Xianzhu, 2015-10-29 21:16:53]

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/1428643004/#ps100001
(title: " ")

[commit-bot: I haz the power, 2015-10-29 21:17:15]

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

[commit-bot: I haz the power, 2015-10-29 22:43:56]

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

[commit-bot: I haz the power, 2015-10-29 22:43:57]

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

[Xianzhu, 2015-10-29 22:44:46]

The CQ bit was checked by wangxianzhu@chromium.org

[commit-bot: I haz the power, 2015-10-29 22:47:43]

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

[commit-bot: I haz the power, 2015-10-29 23:24:44]

Committed patchset #6 (id:100001)

[commit-bot: I haz the power, 2015-10-29 23:25:33]

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