fix contents of render_pictures JSON summary

fix contents of render_pictures JSON summary

BUG=skia:2043, skia:2044, skia:1942
Committed: http://code.google.com/p/skia/source/detail?r=14391

[epoger, 2014-04-24 20:51:20]

Ready for review at patchset 2.

Leon, if you don't think you're the right person to review this, let me know...

https://codereview.chromium.org/259703002/diff/20001/tools/tests/render_pictu...
File tools/tests/render_pictures_test.py (right):

https://codereview.chromium.org/259703002/diff/20001/tools/tests/render_pictu...
tools/tests/render_pictures_test.py:130: TODO(epoger): This test generates
undesired results!  The JSON summary
This behavior CHANGED with this CL, but it was already broken.  Now it's just
broken in a different way.

[scroggo, 2014-04-24 21:15:08]

lgtm

https://codereview.chromium.org/259703002/diff/20001/tools/PictureRenderer.cpp
File tools/PictureRenderer.cpp (right):

https://codereview.chromium.org/259703002/diff/20001/tools/PictureRenderer.cp...
tools/PictureRenderer.cpp:326: * Write the canvas to the specified path.
Since this function does a lot more than this (and possibly doesn't even do
this), maybe this should be more verbose?

https://codereview.chromium.org/259703002/diff/20001/tools/PictureRenderer.cp...
tools/PictureRenderer.cpp:334: * @return bool True if the Canvas is written to a
file.
or if outputDir is NULL but we still wrote a summary.

https://codereview.chromium.org/259703002/diff/20001/tools/tests/render_pictu...
File tools/tests/render_pictures_test.py (left):

https://codereview.chromium.org/259703002/diff/20001/tools/tests/render_pictu...
tools/tests/render_pictures_test.py:165: See
https://code.google.com/p/skia/issues/detail?id=2044 ('render_pictures:
I'm glad to know that using the flag "validate" no longer fails! Did that get
fixed in this CL by returning true when there's no writePath?

https://codereview.chromium.org/259703002/diff/20001/tools/tests/render_pictu...
File tools/tests/render_pictures_test.py (right):

https://codereview.chromium.org/259703002/diff/20001/tools/tests/render_pictu...
tools/tests/render_pictures_test.py:134: TODO(epoger): I noticed that when this
is run without --writePath being
Are there bugs for these TODOs? Should they be referenced in the file?

https://codereview.chromium.org/259703002/diff/20001/tools/tests/render_pictu...
tools/tests/render_pictures_test.py:161: # The disk contents are not what they
should be--we should see all the
I suppose this goes with the TODO above?

[epoger, 2014-04-25 15:26:43]

Thanks, Leon.  Made changes according to your comments... I'll go ahead and
commit, since you LGed already.

https://codereview.chromium.org/259703002/diff/20001/tools/PictureRenderer.cpp
File tools/PictureRenderer.cpp (right):

https://codereview.chromium.org/259703002/diff/20001/tools/PictureRenderer.cp...
tools/PictureRenderer.cpp:326: * Write the canvas to the specified path.
On 2014/04/24 21:15:08, scroggo wrote:
> Since this function does a lot more than this (and possibly doesn't even do
> this), maybe this should be more verbose? 

Done.

https://codereview.chromium.org/259703002/diff/20001/tools/PictureRenderer.cp...
tools/PictureRenderer.cpp:334: * @return bool True if the Canvas is written to a
file.
On 2014/04/24 21:15:08, scroggo wrote:
> or if outputDir is NULL but we still wrote a summary.

It's slightly different than that... updated.

https://codereview.chromium.org/259703002/diff/20001/tools/tests/render_pictu...
File tools/tests/render_pictures_test.py (left):

https://codereview.chromium.org/259703002/diff/20001/tools/tests/render_pictu...
tools/tests/render_pictures_test.py:165: See
https://code.google.com/p/skia/issues/detail?id=2044 ('render_pictures:
On 2014/04/24 21:15:08, scroggo wrote:
> I'm glad to know that using the flag "validate" no longer fails! Did that get
> fixed in this CL by returning true when there's no writePath?

Probably so.

This code really should have had unittests from the beginning.  :-(

https://codereview.chromium.org/259703002/diff/20001/tools/tests/render_pictu...
File tools/tests/render_pictures_test.py (right):

https://codereview.chromium.org/259703002/diff/20001/tools/tests/render_pictu...
tools/tests/render_pictures_test.py:134: TODO(epoger): I noticed that when this
is run without --writePath being
On 2014/04/24 21:15:08, scroggo wrote:
> Are there bugs for these TODOs? Should they be referenced in the file?

Now they do, and they are.  Good idea, thanks.

https://codereview.chromium.org/259703002/diff/20001/tools/tests/render_pictu...
tools/tests/render_pictures_test.py:161: # The disk contents are not what they
should be--we should see all the
On 2014/04/24 21:15:08, scroggo wrote:
> I suppose this goes with the TODO above?

Yes.  Removed this redundant comment.

[epoger, 2014-04-25 15:27:16]

The CQ bit was checked by epoger@google.com

[commit-bot: I haz the power, 2014-04-25 15:27:49]

CQ is trying da patch. Follow status at
https://skia-tree-status.appspot.com/cq/epoger@google.com/259703002/40001

[commit-bot: I haz the power, 2014-04-28 13:18:48]

Change committed as 14391

[caryclark, 2014-04-28 14:06:44]

A revert of this CL has been created in
https://codereview.chromium.org/251103004/ by caryclark@google.com.

The reason for reverting is: This appears to have caused regressions such as
this one:
http://108.170.220.120:10117/builders/Perf-Win7-ShuttleA-HD2000-x86-Release/b....