Switch the factory chunk in the skps to storing its size in bytes

This CL is motivated by the desire to make the skpinfo tool work a bit better. The main concern is that the assumptions made w.r.t. written bytes may not be valid for all SkWStream sub-classes.

Committed: http://code.google.com/p/skia/source/detail?r=13673

[bungeman-skia, 2014-03-04 23:01:49]

Why not? lgtm

https://codereview.chromium.org/182733008/diff/20001/include/core/SkStream.h
File include/core/SkStream.h (right):

https://codereview.chromium.org/182733008/diff/20001/include/core/SkStream.h#...
include/core/SkStream.h:220: static int SizeOfPackedUInt(size_t value);
No issue with making this static, but it does force the encoding to be context
free.

[robertphillips, 2014-03-05 14:30:27]

It is a bit odd that the chunk size in bytes will only be useful for the SkFile-
and SkMemory- streams. The SkDynamicMemory - streams won't be able to make use
of it. Additionally, it is somewhat irrelevant for the SkMD5 and SkSHA1 derived
classes. So, in use it should be fine but it is a bit odd. Basically the
contract is, you can use this number if you know your stream is an SkFile- or
SkMemory- flavor.

[robertphillips, 2014-03-05 14:36:31]

One practical upshot of this is that I would've liked to have added a check in
write_factories that the size written actually matched the size computed in
compute_chunk_size. I was unable to do so however, since SkWStream
doesn't/can't, in general, support a bytesWritten virtual method
(SkMemoryWStream & SkDynamicMemoryStream do but not the other derived classes).

[reed1, 2014-03-05 14:39:53]

I just signed off on a diff CL that added bytesWritten() to another wstream
subclass. Time to make that virtual on the baseclass SkWStream?

[mtklein, 2014-03-05 14:59:55]

On 2014/03/05 14:40:28, reed1 wrote:

What beautiful timing: https://codereview.chromium.org/187653003/

[robertphillips, 2014-03-05 16:12:50]

The CQ bit was checked by robertphillips@google.com

[commit-bot: I haz the power, 2014-03-05 16:12:52]

CQ is trying da patch. Follow status at
https://skia-tree-status.appspot.com/cq/robertphillips@google.com/182733008/2...

[commit-bot: I haz the power, 2014-03-05 16:12:55]

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

[commit-bot: I haz the power, 2014-03-05 16:12:56]

Presubmit check for 182733008-20001 failed and returned exit status 1.

Running presubmit commit checks ...

** Presubmit ERRORS **
Since the CL is editing public API, you must have an LGTM from one of:
('reed@chromium.org', 'reed@google.com', 'bsalomon@chromium.org',
'bsalomon@google.com')

[robertphillips, 2014-03-05 16:13:40]

My confusion (regarding SkDynamicMemoryWStream's behavior) has been resolved.
This should work fine.

[reed1, 2014-03-05 17:05:57]

lgtm w/ dox request

https://codereview.chromium.org/182733008/diff/20001/include/core/SkStream.h
File include/core/SkStream.h (right):

https://codereview.chromium.org/182733008/diff/20001/include/core/SkStream.h#...
include/core/SkStream.h:220: static int SizeOfPackedUInt(size_t value);
/**
 * This returns ...
 */

number of bytes it will take to store the value?

[robertphillips, 2014-03-05 17:17:40]

The CQ bit was checked by robertphillips@google.com

[commit-bot: I haz the power, 2014-03-05 17:17:47]

CQ is trying da patch. Follow status at
https://skia-tree-status.appspot.com/cq/robertphillips@google.com/182733008/4...

[robertphillips, 2014-03-05 17:18:24]

https://codereview.chromium.org/182733008/diff/20001/include/core/SkStream.h
File include/core/SkStream.h (right):

https://codereview.chromium.org/182733008/diff/20001/include/core/SkStream.h#...
include/core/SkStream.h:220: static int SizeOfPackedUInt(size_t value);
On 2014/03/05 17:05:57, reed1 wrote:
> /**
>  * This returns ...
>  */
> 
> number of bytes it will take to store the value?

Done.

[commit-bot: I haz the power, 2014-03-05 18:25:23]

Change committed as 13673