Enable subsequence caching for spv2

Enable subsequence caching for spv2

When copying subsequences, copy paint chunk properties from cached chunks
to new chunks.

BUG=596983
Committed: https://crrev.com/599b19c6d8dfff6ab923aec291587c5df04741f7
Cr-Commit-Position: refs/heads/master@{#407983}

[Xianzhu, 2016-07-22 20:24:18]

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

[commit-bot: I haz the power, 2016-07-22 20:25:00]

Dry run: 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-22 20:37:07]

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

[commit-bot: I haz the power, 2016-07-22 20:37:08]

Dry run: Try jobs failed on following builders:
  chromeos_daisy_chromium_compile_only_ng on master.tryserver.chromium.linux
(JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/chromeos_daisy_...)
  chromeos_x86-generic_chromium_compile_only_ng on
master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/chromeos_x86-ge...)

[Xianzhu, 2016-07-22 23:31:16]

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

[commit-bot: I haz the power, 2016-07-22 23:31:56]

Dry run: 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-22 23:52:08]

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

[commit-bot: I haz the power, 2016-07-22 23:52:08]

Dry run: Try jobs failed on following builders:
  mac_chromium_compile_dbg_ng on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_comp...)

[Xianzhu, 2016-07-25 19:47:19]

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

[commit-bot: I haz the power, 2016-07-25 19:48:00]

Dry run: 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-25 20:23:12]

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

[commit-bot: I haz the power, 2016-07-25 20:23:12]

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

[Xianzhu, 2016-07-25 21:19:04]

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

[commit-bot: I haz the power, 2016-07-25 21:19:35]

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-25 21:28:01]

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

[pdr., 2016-07-25 21:44:40]

I'm not sure we have good test coverage of this area yet. Can you add some unit
tests for the difficult cases?

[commit-bot: I haz the power, 2016-07-25 22:50:07]

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

[commit-bot: I haz the power, 2016-07-25 22:50:08]

Dry run: This issue passed the CQ dry run.

[Xianzhu, 2016-07-26 00:21:23]

On 2016/07/25 21:44:40, pdr. wrote:
> I'm not sure we have good test coverage of this area yet. Can you add some
unit
> tests for the difficult cases?

I feel that the virtual/spv2 layout tests provide coverage, though I'm not sure
about all the corner cases, and they may not expose all problems because the
current paint invalidation for spv1 creates a lot of over-invalidations for spv2
(e.g. using spv1 paint invalidation we invalidate the whole subtrees of objects
with transform, etc. changed, no matter if the object will be composited.)
Problems will be exposed when we fix these over-invalidation issues, but for now
it's hard to produce the problems with test cases.

In this CL I will let PaintControllerTest also run for spv2, and may add tests
covering cached paint chunk. Also I think the changed code in this CL is also
covered by existing layout tests well.

[pdr., 2016-07-26 16:31:02]

On 2016/07/26 at 00:21:23, wangxianzhu wrote:
> On 2016/07/25 21:44:40, pdr. wrote:

There's still quite a bit of discussion on
https://codereview.chromium.org/2161523002. Do you think we should proceed with
this CL or wait?

[Xianzhu, 2016-07-26 16:49:30]

On 2016/07/26 16:31:02, pdr. wrote:
> On 2016/07/26 at 00:21:23, wangxianzhu wrote:
> > On 2016/07/25 21:44:40, pdr. wrote:
> 
> There's still quite a bit of discussion on
> https://codereview.chromium.org/2161523002. Do you think we should proceed
with
> this CL or wait?

You can proceed to review the code now while I'm adding test cases.

I think enabling this can expose the problems earlier which may help the
discussions.

[pdr., 2016-07-26 16:54:21]

This looks good overall.

https://codereview.chromium.org/2176573004/diff/60001/third_party/WebKit/Sour...
File third_party/WebKit/Source/platform/graphics/paint/PaintArtifact.cpp
(right):

https://codereview.chromium.org/2176573004/diff/60001/third_party/WebKit/Sour...
third_party/WebKit/Source/platform/graphics/paint/PaintArtifact.cpp:80: static
bool compareChunkAndIndex(const PaintChunk& chunk, unsigned index)
Should all of these unsigned be size_t (here, and in PaintChunk)?

[pdr., 2016-07-26 16:58:23]

The trybot for spv2 is still being set up (crbug.com/601275) so we have to check
manually. Does this pass any more tests or fail any new ones when run with
--additional-driver-flag=--enable-slimming-paint-v2?

run-webkit-tests --additional-driver-flag=--enable-slimming-paint-v2
LayoutTests/fast LayoutTests/compositing LayoutTests/svg

[Xianzhu, 2016-07-26 17:01:43]

https://codereview.chromium.org/2176573004/diff/60001/third_party/WebKit/Sour...
File third_party/WebKit/Source/platform/graphics/paint/PaintArtifact.cpp
(right):

https://codereview.chromium.org/2176573004/diff/60001/third_party/WebKit/Sour...
third_party/WebKit/Source/platform/graphics/paint/PaintArtifact.cpp:80: static
bool compareChunkAndIndex(const PaintChunk& chunk, unsigned index)
On 2016/07/26 16:54:21, pdr. wrote:
> Should all of these unsigned be size_t (here, and in PaintChunk)?

Do you know what was the reason that we chose unsigned in PaintChunk? One reason
I guess is to limit size of chunks on 64-bit systems. Wdyt?

[pdr., 2016-07-26 17:15:58]

On 2016/07/26 at 17:01:43, wangxianzhu wrote:
>
https://codereview.chromium.org/2176573004/diff/60001/third_party/WebKit/Sour...
> File third_party/WebKit/Source/platform/graphics/paint/PaintArtifact.cpp
(right):
> 
>
https://codereview.chromium.org/2176573004/diff/60001/third_party/WebKit/Sour...
> third_party/WebKit/Source/platform/graphics/paint/PaintArtifact.cpp:80: static
bool compareChunkAndIndex(const PaintChunk& chunk, unsigned index)
> On 2016/07/26 16:54:21, pdr. wrote:
> > Should all of these unsigned be size_t (here, and in PaintChunk)?
> 
> Do you know what was the reason that we chose unsigned in PaintChunk? One
reason I guess is to limit size of chunks on 64-bit systems. Wdyt?

Looks like it was added in https://codereview.chromium.org/1379883003 but I
don't see any explanation. I think it's better to unify all of these to be the
same size (but I'm okay doing that in a followup if you want).

[Xianzhu, 2016-07-26 18:33:42]

On 2016/07/26 16:58:23, pdr. wrote:
> The trybot for spv2 is still being set up (crbug.com/601275) so we have to
check
> manually. Does this pass any more tests or fail any new ones when run with
> --additional-driver-flag=--enable-slimming-paint-v2?
> 
> run-webkit-tests --additional-driver-flag=--enable-slimming-paint-v2
> LayoutTests/fast LayoutTests/compositing LayoutTests/svg

The results are similar for master and with this CL:

master (release):

Expected to crash, but passed: (95)
Expected to fail, but passed: (342)
Regressions: Unexpected text-only failures (18)
Regressions: Unexpected image-only failures (82)
Regressions: Unexpected crashes (1)
Regressions: Unexpected timeouts (1)
Regressions: Unexpected image and text failures (2)

This CL (release):

Expected to crash, but passed: (95) ==
Expected to fail, but passed: (344) +2
Regressions: Unexpected text-only failures (18) ==
Regressions: Unexpected image-only failures (82) ==
Regressions: Unexpected crashes (1) ==
Regressions: Unexpected timeouts (1) ==
Regressions: Unexpected image and text failures (2) ==

master (debug):
Expected to crash, but passed: (15)
Expected to fail, but passed: (339)
Regressions: Unexpected text-only failures (42)
Regressions: Unexpected image-only failures (35)
Regressions: Unexpected crashes (95)
Regressions: Unexpected timeouts (1)
Regressions: Unexpected image and text failures (3)

This CL (debug):
Expected to crash, but passed: (15) ==
Expected to fail, but passed: (338) -1
Regressions: Unexpected text-only failures (41) -1
Regressions: Unexpected image-only failures (37) +2
Regressions: Unexpected crashes (97) +2
Regressions: Unexpected timeouts (1) ==
Regressions: Unexpected image and text failures (3) ==

[pdr., 2016-07-26 18:36:55]

On 2016/07/26 at 18:33:42, wangxianzhu wrote:
> On 2016/07/26 16:58:23, pdr. wrote:
> > The trybot for spv2 is still being set up (crbug.com/601275) so we have to
check
> > manually. Does this pass any more tests or fail any new ones when run with
> > --additional-driver-flag=--enable-slimming-paint-v2?
> > 
> > run-webkit-tests --additional-driver-flag=--enable-slimming-paint-v2
> > LayoutTests/fast LayoutTests/compositing LayoutTests/svg
> 
> The results are similar for master and with this CL:
> 
> master (release):
> 
> Expected to crash, but passed: (95)
> Expected to fail, but passed: (342)
> Regressions: Unexpected text-only failures (18)
> Regressions: Unexpected image-only failures (82)
> Regressions: Unexpected crashes (1)
> Regressions: Unexpected timeouts (1)
> Regressions: Unexpected image and text failures (2)
> 
> This CL (release):
> 
> Expected to crash, but passed: (95) ==
> Expected to fail, but passed: (344) +2
> Regressions: Unexpected text-only failures (18) ==
> Regressions: Unexpected image-only failures (82) ==
> Regressions: Unexpected crashes (1) ==
> Regressions: Unexpected timeouts (1) ==
> Regressions: Unexpected image and text failures (2) ==
> 
> master (debug):
> Expected to crash, but passed: (15)
> Expected to fail, but passed: (339)
> Regressions: Unexpected text-only failures (42)
> Regressions: Unexpected image-only failures (35)
> Regressions: Unexpected crashes (95)
> Regressions: Unexpected timeouts (1)
> Regressions: Unexpected image and text failures (3)
> 
> This CL (debug):
> Expected to crash, but passed: (15) ==
> Expected to fail, but passed: (338) -1
> Regressions: Unexpected text-only failures (41) -1
> Regressions: Unexpected image-only failures (37) +2
> Regressions: Unexpected crashes (97) +2
> Regressions: Unexpected timeouts (1) ==
> Regressions: Unexpected image and text failures (3) ==

Thanks, LGTM

[Xianzhu, 2016-07-26 18:53:54]

The CQ bit was checked by wangxianzhu@chromium.org

[Xianzhu, 2016-07-26 18:53:55]

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/2176573004/#ps80001
(title: "Spv2 configuration for PaintLayerPainterTest, unsigned->size_t etc.")

[commit-bot: I haz the power, 2016-07-26 18:54:38]

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-26 22:13:03]

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

[commit-bot: I haz the power, 2016-07-26 22:13:04]

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

[Xianzhu, 2016-07-26 22:50:05]

The CQ bit was checked by wangxianzhu@chromium.org

[commit-bot: I haz the power, 2016-07-26 22:50:45]

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 00:14:05]

Committed patchset #5 (id:80001)

[commit-bot: I haz the power, 2016-07-27 00:16:47]

Description was changed from

==========
Enable subsequence caching for spv2

When copying subsequences, copy paint chunk properties from cached chunks
to new chunks.

BUG=596983
==========

to

==========
Enable subsequence caching for spv2

When copying subsequences, copy paint chunk properties from cached chunks
to new chunks.

BUG=596983

Committed: https://crrev.com/599b19c6d8dfff6ab923aec291587c5df04741f7
Cr-Commit-Position: refs/heads/master@{#407983}
==========

[commit-bot: I haz the power, 2016-07-27 00:16:48]

Patchset 5 (id:??) landed as
https://crrev.com/599b19c6d8dfff6ab923aec291587c5df04741f7
Cr-Commit-Position: refs/heads/master@{#407983}