Experiment: add flag for finish-recording to return null

Experiment: add flag for finish-recording to return null

BUG=skia:5495
GOLD_TRYBOT_URL= https://gold.skia.org/search?issue=2106843004

Committed: https://skia.googlesource.com/skia/+/41c27e15ec2740850700f1b82038ce0f7a632481

[reed1, 2016-06-29 15:26:51]

Description was changed from

==========
add flag for finish-recording to return null

BUG=skia:
==========

to

==========
add flag for finish-recording to return null

BUG=skia:
GOLD_TRYBOT_URL= https://gold.skia.org/search?issue=2106843004
==========

[reed1, 2016-06-29 15:27:18]

reed@google.com changed reviewers:
+ djsollen@google.com, mtklein@google.com

[reed1, 2016-06-29 15:27:22]

The CQ bit was checked by reed@google.com to run a CQ dry run

[commit-bot: I haz the power, 2016-06-29 15:27:27]

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-06-29 15:48:32]

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

[commit-bot: I haz the power, 2016-06-29 15:48:33]

Dry run: This issue passed the CQ dry run.

[reed1, 2016-06-29 16:06:11]

Description was changed from

==========
add flag for finish-recording to return null

BUG=skia:
GOLD_TRYBOT_URL= https://gold.skia.org/search?issue=2106843004
==========

to

==========
Experiment: add flag for finish-recording to return null

BUG=skia:
GOLD_TRYBOT_URL= https://gold.skia.org/search?issue=2106843004
==========

[reed1, 2016-06-29 16:24:41]

The CQ bit was checked by reed@google.com to run a CQ dry run

[commit-bot: I haz the power, 2016-06-29 16:24:44]

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

[reed1, 2016-06-29 16:26:49]

reed@google.com changed reviewers:
+ liyuqian@google.com

[reed1, 2016-06-29 16:26:49]

liyuqian, try patching this locally and re-time/capture on android

[commit-bot: I haz the power, 2016-06-29 16:33:30]

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

[commit-bot: I haz the power, 2016-06-29 16:33:31]

Dry run: Try jobs failed on following builders:
  Test-Ubuntu-GCC-GCE-CPU-AVX2-x86_64-Release-Shared-Trybot on
master.client.skia (JOB_FAILED,
http://build.chromium.org/p/client.skia/builders/Test-Ubuntu-GCC-GCE-CPU-AVX2...)

[reed1, 2016-06-29 17:51:12]

The CQ bit was checked by reed@google.com to run a CQ dry run

[commit-bot: I haz the power, 2016-06-29 17:51:23]

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

[reed1, 2016-06-29 18:05:48]

reed@google.com changed reviewers:
+ msarett@google.com

[commit-bot: I haz the power, 2016-06-29 18:11:22]

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

[commit-bot: I haz the power, 2016-06-29 18:11:28]

Dry run: This issue passed the CQ dry run.

[mtklein, 2016-06-29 19:44:20]

https://codereview.chromium.org/2106843004/diff/40001/src/core/SkPictureRecor...
File src/core/SkPictureRecorder.cpp (right):

https://codereview.chromium.org/2106843004/diff/40001/src/core/SkPictureRecor...
src/core/SkPictureRecorder.cpp:139: // Should we call optimize first, in case
that makes us empty?
(You've done this now.)

https://codereview.chromium.org/2106843004/diff/40001/tests/PictureTest.cpp
File tests/PictureTest.cpp (right):

https://codereview.chromium.org/2106843004/diff/40001/tests/PictureTest.cpp#n...
tests/PictureTest.cpp:1483: REPORTER_ASSERT(r, pic.get());
We should also be able to assert pic->approximateOpCount() == 0 here if that
makes things clearer.

[reed1, 2016-07-06 13:55:09]

https://codereview.chromium.org/2106843004/diff/40001/src/core/SkPictureRecor...
File src/core/SkPictureRecorder.cpp (right):

https://codereview.chromium.org/2106843004/diff/40001/src/core/SkPictureRecor...
src/core/SkPictureRecorder.cpp:139: // Should we call optimize first, in case
that makes us empty?
On 2016/06/29 19:44:19, mtklein wrote:
> (You've done this now.)

Done.

https://codereview.chromium.org/2106843004/diff/40001/tests/PictureTest.cpp
File tests/PictureTest.cpp (right):

https://codereview.chromium.org/2106843004/diff/40001/tests/PictureTest.cpp#n...
tests/PictureTest.cpp:1483: REPORTER_ASSERT(r, pic.get());
On 2016/06/29 19:44:19, mtklein wrote:
> We should also be able to assert pic->approximateOpCount() == 0 here if that
> makes things clearer.

Done.

[reed1, 2016-07-06 13:55:13]

The CQ bit was checked by reed@google.com to run a CQ dry run

[commit-bot: I haz the power, 2016-07-06 13:55:21]

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-06 14:21:42]

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

[commit-bot: I haz the power, 2016-07-06 14:21:43]

Dry run: This issue passed the CQ dry run.

[reed1, 2016-07-06 15:31:43]

ptal

[liyuqian, 2016-07-06 15:39:03]

https://codereview.chromium.org/2106843004/diff/60001/include/core/SkPictureR...
File include/core/SkPictureRecorder.h (right):

https://codereview.chromium.org/2106843004/diff/60001/include/core/SkPictureR...
include/core/SkPictureRecorder.h:40: kPlaybackDrawPicture_RecordFlag     = 1 <<
1,
Just curious: is 1 << 1 faster than 0x02? (In case not, what's the reason of
making this change? Easier to read?)

[reed1, 2016-07-06 15:56:42]

On 2016/07/06 15:39:03, liyuqian wrote:
>
https://codereview.chromium.org/2106843004/diff/60001/include/core/SkPictureR...
> File include/core/SkPictureRecorder.h (right):
> 
>
https://codereview.chromium.org/2106843004/diff/60001/include/core/SkPictureR...
> include/core/SkPictureRecorder.h:40: kPlaybackDrawPicture_RecordFlag     = 1
<<
> 1,
> Just curious: is 1 << 1 faster than 0x02? (In case not, what's the reason of
> making this change? Easier to read?)

Easier to read, esp. as we add more flags, this makes it clearer that each flag
is just 1 bit, and that we are moving up in bit-number.

[mtklein, 2016-07-06 15:58:18]

lgtm

[reed1, 2016-07-06 16:28:07]

Description was changed from

==========
Experiment: add flag for finish-recording to return null

BUG=skia:
GOLD_TRYBOT_URL= https://gold.skia.org/search?issue=2106843004
==========

to

==========
Experiment: add flag for finish-recording to return null

BUG=skia:5495
GOLD_TRYBOT_URL= https://gold.skia.org/search?issue=2106843004
==========

[reed1, 2016-07-06 16:28:14]

The CQ bit was checked by reed@google.com

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

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-06 16:29:23]

Description was changed from

==========
Experiment: add flag for finish-recording to return null

BUG=skia:5495
GOLD_TRYBOT_URL= https://gold.skia.org/search?issue=2106843004
==========

to

==========
Experiment: add flag for finish-recording to return null

BUG=skia:5495
GOLD_TRYBOT_URL= https://gold.skia.org/search?issue=2106843004

Committed:
https://skia.googlesource.com/skia/+/41c27e15ec2740850700f1b82038ce0f7a632481
==========

[commit-bot: I haz the power, 2016-07-06 16:29:25]

Committed patchset #4 (id:60001) as
https://skia.googlesource.com/skia/+/41c27e15ec2740850700f1b82038ce0f7a632481

[commit-bot: I haz the power, 2016-07-06 16:29:43]

CQ bit was unchecked.