add --readJsonSummaryPath to render_pictures

add --readJsonSummaryPath to render_pictures

BUG=skia:1942
closing in favor of https://codereview.chromium.org/273783004/

[epoger, 2014-05-07 03:15:16]

Eric- PTAL at patchset 5.

https://codereview.chromium.org/261313004/diff/80001/gm/gm_expectations.h
File gm/gm_expectations.h (left):

https://codereview.chromium.org/261313004/diff/80001/gm/gm_expectations.h#old...
gm/gm_expectations.h:219: * Read as many bytes as possible (up to maxBytes) from
the stream into
Moved into src/utils/SkDataUtils, where it can be used by both gm and tools.

https://codereview.chromium.org/261313004/diff/80001/src/utils/SkDataUtils.h
File src/utils/SkDataUtils.h (right):

https://codereview.chromium.org/261313004/diff/80001/src/utils/SkDataUtils.h#...
src/utils/SkDataUtils.h:17: * EPOGER: Before committing, add unittests for these
methods.
The "EPOGER" notes within the code are things I intend to fix before committing,
but I wanted to get your feedback on the CL as a whole first...

https://codereview.chromium.org/261313004/diff/80001/tools/PictureRenderer.cpp
File tools/PictureRenderer.cpp (left):

https://codereview.chromium.org/261313004/diff/80001/tools/PictureRenderer.cp...
tools/PictureRenderer.cpp:52: * TODO(epoger): Make constant strings consistent
instead of mixing hypenated and camel-caps.
moved to image_expectations

[borenet, 2014-05-07 18:36:52]

Seems right, but would be easier to read if the moving of chunks of code and the
 addition of the flag happened in different CLs (or even just different patch
sets).

https://codereview.chromium.org/261313004/diff/80001/src/utils/SkDataUtils.h
File src/utils/SkDataUtils.h (right):

https://codereview.chromium.org/261313004/diff/80001/src/utils/SkDataUtils.h#...
src/utils/SkDataUtils.h:17: * EPOGER: Before committing, add unittests for these
methods.
On 2014/05/07 03:15:16, epoger wrote:
> The "EPOGER" notes within the code are things I intend to fix before
committing,
> but I wanted to get your feedback on the CL as a whole first...

SGTM

[epoger, 2014-05-07 19:41:08]

On 2014/05/07 18:36:52, borenet wrote:
> Seems right, but would be easier to read if the moving of chunks of code and
the
>  addition of the flag happened in different CLs (or even just different patch
> sets).

Makes sense.  I'll try to err more in that direction next time.

In this case, do you think it's worth 45 minutes for me to refactor it into two
different CLs, and whatever time it would take for you to review them
separately?  I'm fine either doing that or not.  Please let me know what you
think is best.

> On 2014/05/07 03:15:16, epoger wrote:
> > The "EPOGER" notes within the code are things I intend to fix before
> committing,
> > but I wanted to get your feedback on the CL as a whole first...
> 
> SGTM

Cool; I'll move onto those things after resolving the above issue.

[borenet, 2014-05-07 19:58:34]

On 2014/05/07 19:41:08, epoger wrote:
> On 2014/05/07 18:36:52, borenet wrote:
> > Seems right, but would be easier to read if the moving of chunks of code and
> the
> >  addition of the flag happened in different CLs (or even just different
patch
> > sets).
> 
> Makes sense.  I'll try to err more in that direction next time.
> 
> In this case, do you think it's worth 45 minutes for me to refactor it into
two
> different CLs, and whatever time it would take for you to review them
> separately?  I'm fine either doing that or not.  Please let me know what you
> think is best.
> 

At this point, probably not.  Maybe just a couple of comments where the moved
blocks differ, if they do?  That way, I'll be sure not to miss those.

> > On 2014/05/07 03:15:16, epoger wrote:
> > > The "EPOGER" notes within the code are things I intend to fix before
> > committing,
> > > but I wanted to get your feedback on the CL as a whole first...
> > 
> > SGTM
> 
> Cool; I'll move onto those things after resolving the above issue.

[epoger, 2014-05-07 22:12:48]

> > In this case, do you think it's worth 45 minutes for me to refactor it into
> two
> > different CLs, and whatever time it would take for you to review them
> > separately?  I'm fine either doing that or not.  Please let me know what you
> > think is best.
> > 
> 
> At this point, probably not.  Maybe just a couple of comments where the moved
> blocks differ, if they do?  That way, I'll be sure not to miss those.

I started looking at the diff blocks, but then my eyes crossed up and I decided
I would be better off splitting the CL.

Sent the refactor one to you as https://codereview.chromium.org/273703006/
('extract some common code from PictureRenderer').