cc: Add base64 encoding to picture

cc: Add base64 encoding to picture

This adds ability to serialize cc::Picture to base64 string
and to initialize the picture back from that serialization.

Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=195361

[tfarina, 2013-04-13 00:49:01]

https://codereview.chromium.org/13884018/diff/1/cc/resources/picture.cc
File cc/resources/picture.cc (right):

https://codereview.chromium.org/13884018/diff/1/cc/resources/picture.cc#newcode9
cc/resources/picture.cc:9: #include "cc/resources/picture.h"
nit: this should be the first include.

https://codereview.chromium.org/13884018/diff/1/cc/resources/picture.cc#newco...
cc/resources/picture.cc:155: void Picture::AsBase64String(std::string* output)
const {
the order of implementation should match with the declaration. i.e. move this
right after GatherPixelRefs().

https://codereview.chromium.org/13884018/diff/1/cc/resources/picture.h
File cc/resources/picture.h (right):

https://codereview.chromium.org/13884018/diff/1/cc/resources/picture.h#newcode65
cc/resources/picture.h:65: private:
nit: add a blank line above.

[vmpstr, 2013-04-16 17:08:39]

>
https://codereview.chromium.org/13884018/diff/1/cc/resources/picture.cc#newcode9
> cc/resources/picture.cc:9: #include "cc/resources/picture.h"
> nit: this should be the first include.

Done.

> 
>
https://codereview.chromium.org/13884018/diff/1/cc/resources/picture.cc#newco...
> cc/resources/picture.cc:155: void Picture::AsBase64String(std::string* output)
> const {
> the order of implementation should match with the declaration. i.e. move this
> right after GatherPixelRefs().
> 

Done.

>
https://codereview.chromium.org/13884018/diff/1/cc/resources/picture.h#newcode65
> cc/resources/picture.h:65: private:
> nit: add a blank line above.

Done.

PTAL.

[nduca, 2013-04-16 22:06:36]

Lets do the plumbing to put this into a TRACE_EVENT_INSTANT('cc-debug', ...)
conditional on --trace-full-frames. An example of this in layer tree host impl,
look for trace full frames and you'll see how its done there.

[nduca, 2013-04-16 22:07:31]

Also, unit test for the picture bits?

[vmpstr, 2013-04-18 19:23:17]

I added a unittest. The approach I took is to rasterize both the original
picture and the picture that was restored from a serialization, and compare the
result. I found that the actual UTF8 strings and bits within SkPicture are not
the same, so I can't really memcmp those objects.

PTAL.

[nduca, 2013-04-18 19:43:48]

https://codereview.chromium.org/13884018/diff/12005/cc/resources/picture.h
File cc/resources/picture.h (right):

https://codereview.chromium.org/13884018/diff/12005/cc/resources/picture.h#ne...
cc/resources/picture.h:64: void SetPictureFromBase64String(const std::string&
encoded);
Does this include set layer_rect and opaque_rect? Should it?

What if the SetPictureFromBase64String fails?

[vmpstr, 2013-04-18 20:22:51]

I added serialization for layer_rect and opaque_rect, since I think we should
just serialize everything we can in cc::Picture.

In case of failure, SkPicture code just silently creates an empty picture, so I
can't distinguish it from a valid empty picture in de-serialization code. I did
add a unittest to make sure that invalid pictures don't draw anything.

[nduca, 2013-04-18 20:40:20]

Can we file a bug against skia then and get that fixed? Its important to have
robust error handling, no?

[vmpstr, 2013-04-18 21:07:05]

On 2013/04/18 20:40:20, nduca wrote:
> Can we file a bug against skia then and get that fixed? Its important to have
> robust error handling, no?

Filed: https://code.google.com/p/skia/issues/detail?id=1255

[vmpstr, 2013-04-18 21:21:27]

Turns out I was just using a wrong API. My bad. I've added a check and fixed
unittests. PTAL.

[nduca, 2013-04-18 21:33:04]

lgtm with nits

https://codereview.chromium.org/13884018/diff/24001/cc/resources/picture.cc
File cc/resources/picture.cc (right):

https://codereview.chromium.org/13884018/diff/24001/cc/resources/picture.cc#n...
cc/resources/picture.cc:60: std::string decoded;
put a cc picture  number in this so that if we ever start putting new fields in
this binary format, we can make the old import fail or handle it.

e.g. push a 1. and read a 1.

[commit-bot: I haz the power, 2013-04-19 17:01:29]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/vmpstr@chromium.org/13884018/26001

[commit-bot: I haz the power, 2013-04-19 17:01:31]

Failed to apply patch for cc/cc_tests.gyp:
While running patch -p1 --forward --force --no-backup-if-mismatch;
  patching file cc/cc_tests.gyp
  Hunk #1 FAILED at 50.
  1 out of 1 hunk FAILED -- saving rejects to file cc/cc_tests.gyp.rej

Patch:       cc/cc_tests.gyp
Index: cc/cc_tests.gyp
diff --git a/cc/cc_tests.gyp b/cc/cc_tests.gyp
index
0406810efe98bb97a4b79d01c498425fbb7ec3d2..927bfd4444ac987ebcba19b166c8cfd81c1b4ef1
100644
--- a/cc/cc_tests.gyp
+++ b/cc/cc_tests.gyp
@@ -50,6 +50,7 @@
       'layers/picture_layer_impl_unittest.cc',
       'resources/picture_layer_tiling_set_unittest.cc',
       'resources/picture_layer_tiling_unittest.cc',
+      'resources/picture_unittest.cc',
       'resources/prioritized_resource_unittest.cc',
       'trees/quad_culler_unittest.cc',
       'base/region_unittest.cc',

[commit-bot: I haz the power, 2013-04-19 17:15:56]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/vmpstr@chromium.org/13884018/28001

[commit-bot: I haz the power, 2013-04-19 20:54:02]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/vmpstr@chromium.org/13884018/22002

[commit-bot: I haz the power, 2013-04-20 01:15:00]

Change committed as 195361