Use SkError for a bitmap that could not be decoded.

Use SkError for a bitmap that could not be decoded.

When recreating an SkPicture from an SkStream, use the
new error reporting system to report that an SkBitmap
could not be decoded.

Add a test that the parse error is thrown.

Committed: https://code.google.com/p/skia/source/detail?r=8866

[humper, 2013-04-25 17:07:08]

https://codereview.chromium.org/13892009/diff/1/tests/PictureTest.cpp
File tests/PictureTest.cpp (right):

https://codereview.chromium.org/13892009/diff/1/tests/PictureTest.cpp#newcode405
tests/PictureTest.cpp:405: SkSetErrorCallback(error_callback, NULL);
Is the idea here to silence the built-in default callback?  Maybe if all tests
are going to have to do that this should either get bumped up to the testing
infrastructure classes, or we should extend the error API to have a "shut up"
method?

[reed1, 2013-04-25 17:10:07]

might vote for adding some layer in our testing for for now. perhaps a push/pop
model, where a test can say:

save
clear_proc (or install proc)
restore

So that it doesn't have the side-effect of change the proc for *other* tests...

[humper, 2013-04-25 17:29:47]

That's a good idea; should I take care of that?  I presume this is intended for
use in our testing infrastructure, not so much as a general API.

On 2013/04/25 17:10:07, reed1 wrote:
> might vote for adding some layer in our testing for for now. perhaps a
push/pop
> model, where a test can say:
> 
> save
> clear_proc (or install proc)
> restore
> 
> So that it doesn't have the side-effect of change the proc for *other*
tests...

[reed1, 2013-04-25 17:38:58]

yes, my suggestion is to make this testing-specific, at least for now (easier to
experiment and specialize)

[scroggo, 2013-04-25 17:43:45]

On 2013/04/25 17:38:58, reed1 wrote:
> yes, my suggestion is to make this testing-specific, at least for now (easier
to
> experiment and specialize)

In the meantime, I'll update my call back to actually test to make sure that it
only receives the error I'm expecting.

[humper, 2013-04-25 17:44:54]

That seems fine, a little meta-test if you will.

On 2013/04/25 17:43:45, scroggo wrote:
> On 2013/04/25 17:38:58, reed1 wrote:
> > yes, my suggestion is to make this testing-specific, at least for now
(easier
> to
> > experiment and specialize)
> 
> In the meantime, I'll update my call back to actually test to make sure that
it
> only receives the error I'm expecting.

[humper, 2013-04-25 17:49:54]

How about something simpler where we just set the error callback to the default
(NULL) after a test returns in the Test::run wrapper?  This way a test can't
have any cross-test side effects on the error callbacks?

On 2013/04/25 17:44:54, humper wrote:
> That seems fine, a little meta-test if you will.
> 
> On 2013/04/25 17:43:45, scroggo wrote:
> > On 2013/04/25 17:38:58, reed1 wrote:
> > > yes, my suggestion is to make this testing-specific, at least for now
> (easier
> > to
> > > experiment and specialize)
> > 
> > In the meantime, I'll update my call back to actually test to make sure that
> it
> > only receives the error I'm expecting.

[scroggo, 2013-04-25 18:02:46]

New patch, using a better error callback.

[humper, 2013-04-25 18:24:44]

On 2013/04/25 18:02:46, scroggo wrote:
> New patch, using a better error callback.

lgtm

I'll land something soon so you can remove the clearing of the error callback.

[scroggo, 2013-04-25 18:30:16]

Committed patchset #3 manually as r8866 (presubmit successful).