Add subsetting to SkScanlineDecoder

Add subsetting to SkScanlineDecoder

This CL allows the SkScanlineDecoder to decode partial
scanlines.

This is a first step in efficiently implementing subsetting
in SkScaledCodec.

BUG=skia:4209
Committed: https://skia.googlesource.com/skia/+/fdb47571a3b5e72469b67de44e32ac14d9352ab4

[msarett, 2015-10-07 21:10:50]

Patchset #1 (id:1) has been deleted

[msarett, 2015-10-07 21:10:57]

Patchset #1 (id:20001) has been deleted

[msarett, 2015-10-07 21:11:03]

Patchset #1 (id:40001) has been deleted

[msarett, 2015-10-07 21:46:32]

Patchset #1 (id:60001) has been deleted

[msarett, 2015-10-07 22:08:03]

Patchset #1 (id:80001) has been deleted

[msarett, 2015-10-07 22:15:13]

msarett@google.com changed reviewers:
+ scroggo@google.com

[msarett, 2015-10-07 22:15:14]

Sorry I know you think this is backwards.  The other half will follow shortly.

https://codereview.chromium.org/1390213002/diff/100001/src/codec/SkBmpRLECode...
File src/codec/SkBmpRLECodec.cpp (right):

https://codereview.chromium.org/1390213002/diff/100001/src/codec/SkBmpRLECode...
src/codec/SkBmpRLECodec.cpp:260: // FIXME: Support subsets for scanline decodes.
This is broken.  Will fix later.

https://codereview.chromium.org/1390213002/diff/100001/src/codec/SkCodec.cpp
File src/codec/SkCodec.cpp (right):

https://codereview.chromium.org/1390213002/diff/100001/src/codec/SkCodec.cpp#...
src/codec/SkCodec.cpp:259: if (options->fSubset->top() != 0 ||
options->fSubset->height() != dstInfo.height()) {
Alternatively we can just ignore the y-coordinates of the subset.  But I think
it might be better to be sure that the client knows that they are responsible
for subsetting in the y-dimension.

https://codereview.chromium.org/1390213002/diff/100001/src/codec/SkCodec.cpp#...
src/codec/SkCodec.cpp:270: fOptions = *options;
SkJpegCodec might need to call getSampler() in startScanlineDecode().
We cannot call getSampler() until fDstInfo and fOptions are set.

I know you had a reason for not setting these fields until the end of this
function, but I can't remember what it was...

[scroggo, 2015-10-08 20:16:08]

I still consider this an optimization that need not come first.

I am pleased at how simple the change is though - the codecs themselves know
very little about subsetting, which is good.

https://codereview.chromium.org/1390213002/diff/100001/src/codec/SkCodec.cpp
File src/codec/SkCodec.cpp (right):

https://codereview.chromium.org/1390213002/diff/100001/src/codec/SkCodec.cpp#...
src/codec/SkCodec.cpp:270: fOptions = *options;
On 2015/10/07 22:15:14, msarett wrote:
> I know you had a reason for not setting these fields until the end of this
> function, but I can't remember what it was...

The reason is because I did not want to update them until we knew that
onStartScanlineDecode succeeded. This way we do not have some dstInfo and
options that perhaps did not work. It does not go all the way though - we could
be even more defensive about this and reset them at the beginning (like we do
with fCurrScanline).

> SkJpegCodec might need to call getSampler() in startScanlineDecode().
> We cannot call getSampler() until fDstInfo and fOptions are set.

Do you mean in onStartScanlineDecode? That function receives dstInfo and options
as parameters, so it seems like you can just read those.

https://codereview.chromium.org/1390213002/diff/120001/dm/DMSrcSink.cpp
File dm/DMSrcSink.cpp (right):

https://codereview.chromium.org/1390213002/diff/120001/dm/DMSrcSink.cpp#newco...
dm/DMSrcSink.cpp:436: SkBitmap bitmap;
Why did you change this name? The comment above no longer applies, and you have
shadowed the variable "bitmap" in an outer scope (the fact that I had to look up
100 lines is evidence this function is too big...).

https://codereview.chromium.org/1390213002/diff/120001/src/codec/SkCodec.cpp
File src/codec/SkCodec.cpp (right):

https://codereview.chromium.org/1390213002/diff/120001/src/codec/SkCodec.cpp#...
src/codec/SkCodec.cpp:114: SkISize SkCodec::getScaledDimensions(float
desiredScale) const {
This appears to be moved here unchanged. What was the motivation for it? It
seems like an independent CL (unless something else depends on it?).

https://codereview.chromium.org/1390213002/diff/120001/src/codec/SkCodec.cpp#...
src/codec/SkCodec.cpp:134: SkIRect dimensions  =
SkIRect::MakeSize(this->getInfo().dimensions());
This check seems unnecessary for subclasses which do not support subsets at all.

https://codereview.chromium.org/1390213002/diff/120001/src/codec/SkCodec.cpp#...
src/codec/SkCodec.cpp:257: // We only support subsetting in the x-dimension for
scanline decoder.
We haven't really talked about how scanline decoding should handle fSubset. It
seems it will be different between getPixels and startScanlineDecode. We should
document that in the header.

https://codereview.chromium.org/1390213002/diff/120001/src/codec/SkCodec.cpp#...
src/codec/SkCodec.cpp:260: return kUnimplemented;
Maybe this should be kInvalidInput?

https://codereview.chromium.org/1390213002/diff/120001/src/codec/SkCodecPriv.h
File src/codec/SkCodecPriv.h (right):

https://codereview.chromium.org/1390213002/diff/120001/src/codec/SkCodecPriv....
src/codec/SkCodecPriv.h:34: inline float get_scale_from_sample_size(uint32_t
sampleSize) { return 1.0f / (float) sampleSize; }
This change (moving this function here, removing SkCodecTools, and including
SkCodecPriv where you previously included SkCodecTools) seems like it should be
its own change. It doesn't appear to be newly used by anything else here.

That said, I'm not sure it's the right thing to do. Not sure if dm etc should
rely on src/codec. (I think we do rely on other private includes, so it's at
least no worse than we already do.)

https://codereview.chromium.org/1390213002/diff/120001/src/codec/SkCodec_libg...
File src/codec/SkCodec_libgif.cpp (right):

https://codereview.chromium.org/1390213002/diff/120001/src/codec/SkCodec_libg...
src/codec/SkCodec_libgif.cpp:128: DGifCloseFile(gif, nullptr);
This change deserves to be a separate CL. It makes it harder to find the
interesting pieces of this CL.

https://codereview.chromium.org/1390213002/diff/120001/src/codec/SkCodec_libg...
File src/codec/SkCodec_libgif.h (right):

https://codereview.chromium.org/1390213002/diff/120001/src/codec/SkCodec_libg...
src/codec/SkCodec_libgif.h:132: * @param zeroInit Indicates if destination
memory is zero initialized.
This comment is out of date.

https://codereview.chromium.org/1390213002/diff/120001/src/codec/SkCodec_libp...
File src/codec/SkCodec_libpng.h (left):

https://codereview.chromium.org/1390213002/diff/120001/src/codec/SkCodec_libp...
src/codec/SkCodec_libpng.h:21: class SkScanlineDecoder;
Yuck! Didn't realize I missed these...

https://codereview.chromium.org/1390213002/diff/120001/src/codec/SkJpegCodec.cpp
File src/codec/SkJpegCodec.cpp (right):

https://codereview.chromium.org/1390213002/diff/120001/src/codec/SkJpegCodec....
src/codec/SkJpegCodec.cpp:445: this->getSampler(true);
As mentioned elsewhere, you don't need to have the options stored in
this->options() in order to do this. You can have a helper function that takes
SkImageInfo and options, which can be called here or in getSampler.

https://codereview.chromium.org/1390213002/diff/120001/src/codec/SkSwizzler.cpp
File src/codec/SkSwizzler.cpp (left):

https://codereview.chromium.org/1390213002/diff/120001/src/codec/SkSwizzler.c...
src/codec/SkSwizzler.cpp:236: memcpy(dst, src, dstWidth);
Is the assumption here that dstWidth is smaller when we have an offset, so this
is safe?

https://codereview.chromium.org/1390213002/diff/120001/src/codec/SkSwizzler.cpp
File src/codec/SkSwizzler.cpp (right):

https://codereview.chromium.org/1390213002/diff/120001/src/codec/SkSwizzler.c...
src/codec/SkSwizzler.cpp:707: SkSwizzler::ResultAlpha SkSwizzler::swizzle(void*
dst, const uint8_t* SK_RESTRICT src) {
Why did this move?

https://codereview.chromium.org/1390213002/diff/120001/src/codec/SkSwizzler.c...
src/codec/SkSwizzler.cpp:716: fX0 = fSrcOffset + get_start_coord(sampleX);
Any reason this is the opposite order of mask swizzler?

[msarett, 2015-10-09 19:58:18]

Patchset #4 (id:160001) has been deleted

[msarett, 2015-10-09 20:01:32]

Patchset #4 (id:180001) has been deleted

[msarett, 2015-10-09 20:03:44]

I think that this CL is a lot easier to looks at now that I've removed most of
the excess.  There's even still some spacing stuff etc. that I should have
restored after I decided not to touch that code...

I will try to be more conscious of that in the future.

BTW is this going to need an API review?  All I've added is a comment.

https://codereview.chromium.org/1390213002/diff/100001/src/codec/SkCodec.cpp
File src/codec/SkCodec.cpp (right):

https://codereview.chromium.org/1390213002/diff/100001/src/codec/SkCodec.cpp#...
src/codec/SkCodec.cpp:270: fOptions = *options;
On 2015/10/08 20:16:07, scroggo wrote:
> On 2015/10/07 22:15:14, msarett wrote:
> > I know you had a reason for not setting these fields until the end of this
> > function, but I can't remember what it was...
> 
> The reason is because I did not want to update them until we knew that
> onStartScanlineDecode succeeded. This way we do not have some dstInfo and
> options that perhaps did not work. It does not go all the way though - we
could
> be even more defensive about this and reset them at the beginning (like we do
> with fCurrScanline).
> 
> > SkJpegCodec might need to call getSampler() in startScanlineDecode().
> > We cannot call getSampler() until fDstInfo and fOptions are set.
> 
> Do you mean in onStartScanlineDecode? That function receives dstInfo and
options
> as parameters, so it seems like you can just read those.

Acknowledged.

https://codereview.chromium.org/1390213002/diff/120001/dm/DMSrcSink.cpp
File dm/DMSrcSink.cpp (right):

https://codereview.chromium.org/1390213002/diff/120001/dm/DMSrcSink.cpp#newco...
dm/DMSrcSink.cpp:436: SkBitmap bitmap;
On 2015/10/08 20:16:07, scroggo wrote:
> Why did you change this name? The comment above no longer applies, and you
have
> shadowed the variable "bitmap" in an outer scope (the fact that I had to look
up
> 100 lines is evidence this function is too big...).

Can't remember.  Changing it back sorry.

https://codereview.chromium.org/1390213002/diff/120001/src/codec/SkCodec.cpp
File src/codec/SkCodec.cpp (right):

https://codereview.chromium.org/1390213002/diff/120001/src/codec/SkCodec.cpp#...
src/codec/SkCodec.cpp:114: SkISize SkCodec::getScaledDimensions(float
desiredScale) const {
On 2015/10/08 20:16:07, scroggo wrote:
> This appears to be moved here unchanged. What was the motivation for it? It
> seems like an independent CL (unless something else depends on it?).

There's no good reason, except that it helped me to see everything in one place.

All it does is add clutter to this CL, moving it back.

https://codereview.chromium.org/1390213002/diff/120001/src/codec/SkCodec.cpp#...
src/codec/SkCodec.cpp:134: SkIRect dimensions  =
SkIRect::MakeSize(this->getInfo().dimensions());
On 2015/10/08 20:16:07, scroggo wrote:
> This check seems unnecessary for subclasses which do not support subsets at
all.

Agreed.  I think my motivation was to refactor shared code, but now I'm noticing
that it's not even shared in this CL.

https://codereview.chromium.org/1390213002/diff/120001/src/codec/SkCodec.cpp#...
src/codec/SkCodec.cpp:257: // We only support subsetting in the x-dimension for
scanline decoder.
On 2015/10/08 20:16:07, scroggo wrote:
> We haven't really talked about how scanline decoding should handle fSubset. It
> seems it will be different between getPixels and startScanlineDecode. We
should
> document that in the header.

Will add some documentation.

This check is kind of a stop-gap.  It will most likely be refactored again in
the next CL.

https://codereview.chromium.org/1390213002/diff/120001/src/codec/SkCodec.cpp#...
src/codec/SkCodec.cpp:260: return kUnimplemented;
On 2015/10/08 20:16:07, scroggo wrote:
> Maybe this should be kInvalidInput?

Done.

https://codereview.chromium.org/1390213002/diff/120001/src/codec/SkCodecPriv.h
File src/codec/SkCodecPriv.h (right):

https://codereview.chromium.org/1390213002/diff/120001/src/codec/SkCodecPriv....
src/codec/SkCodecPriv.h:34: inline float get_scale_from_sample_size(uint32_t
sampleSize) { return 1.0f / (float) sampleSize; }
On 2015/10/08 20:16:07, scroggo wrote:
> This change (moving this function here, removing SkCodecTools, and including
> SkCodecPriv where you previously included SkCodecTools) seems like it should
be
> its own change. It doesn't appear to be newly used by anything else here.
> 
> That said, I'm not sure it's the right thing to do. Not sure if dm etc should
> rely on src/codec. (I think we do rely on other private includes, so it's at
> least no worse than we already do.)

I'll remove this from the change.

Right now, get_scale_from_sample_size is used in dm and nanobench.  My thought
process was that anything that is used by more than one of dm, nanobench, tools
should go in SkCodecPriv.h but maybe that is wrong.

https://codereview.chromium.org/1390213002/diff/120001/src/codec/SkCodec_libg...
File src/codec/SkCodec_libgif.cpp (right):

https://codereview.chromium.org/1390213002/diff/120001/src/codec/SkCodec_libg...
src/codec/SkCodec_libgif.cpp:128: DGifCloseFile(gif, nullptr);
On 2015/10/08 20:16:07, scroggo wrote:
> This change deserves to be a separate CL. It makes it harder to find the
> interesting pieces of this CL.

Removing.

https://codereview.chromium.org/1390213002/diff/120001/src/codec/SkCodec_libg...
File src/codec/SkCodec_libgif.h (right):

https://codereview.chromium.org/1390213002/diff/120001/src/codec/SkCodec_libg...
src/codec/SkCodec_libgif.h:132: * @param zeroInit Indicates if destination
memory is zero initialized.
On 2015/10/08 20:16:07, scroggo wrote:
> This comment is out of date.

Done.

https://codereview.chromium.org/1390213002/diff/120001/src/codec/SkCodec_libp...
File src/codec/SkCodec_libpng.h (left):

https://codereview.chromium.org/1390213002/diff/120001/src/codec/SkCodec_libp...
src/codec/SkCodec_libpng.h:21: class SkScanlineDecoder;
On 2015/10/08 20:16:08, scroggo wrote:
> Yuck! Didn't realize I missed these...

Acknowledged.

https://codereview.chromium.org/1390213002/diff/120001/src/codec/SkJpegCodec.cpp
File src/codec/SkJpegCodec.cpp (right):

https://codereview.chromium.org/1390213002/diff/120001/src/codec/SkJpegCodec....
src/codec/SkJpegCodec.cpp:445: this->getSampler(true);
On 2015/10/08 20:16:08, scroggo wrote:
> As mentioned elsewhere, you don't need to have the options stored in
> this->options() in order to do this. You can have a helper function that takes
> SkImageInfo and options, which can be called here or in getSampler.

Yes, there is a better way to do this.

https://codereview.chromium.org/1390213002/diff/120001/src/codec/SkSwizzler.cpp
File src/codec/SkSwizzler.cpp (left):

https://codereview.chromium.org/1390213002/diff/120001/src/codec/SkSwizzler.c...
src/codec/SkSwizzler.cpp:236: memcpy(dst, src, dstWidth);
On 2015/10/08 20:16:08, scroggo wrote:
> Is the assumption here that dstWidth is smaller when we have an offset, so
this
> is safe?

Sorry I meant to respond to your original comment on this.

Yes we are relying on the subset being valid.  We check this in SkCodec.cpp.

It used to be the case that every time offset did not equal zero, it meant that
we were sampling.  Now offset can be nonzero because we are subsetting, but we
can still use a memcpy if we are not sampling.

https://codereview.chromium.org/1390213002/diff/120001/src/codec/SkSwizzler.cpp
File src/codec/SkSwizzler.cpp (right):

https://codereview.chromium.org/1390213002/diff/120001/src/codec/SkSwizzler.c...
src/codec/SkSwizzler.cpp:707: SkSwizzler::ResultAlpha SkSwizzler::swizzle(void*
dst, const uint8_t* SK_RESTRICT src) {
On 2015/10/08 20:16:08, scroggo wrote:
> Why did this move?

I think it moved on a rebase, I'll put it back.

https://codereview.chromium.org/1390213002/diff/120001/src/codec/SkSwizzler.c...
src/codec/SkSwizzler.cpp:716: fX0 = fSrcOffset + get_start_coord(sampleX);
On 2015/10/08 20:16:08, scroggo wrote:
> Any reason this is the opposite order of mask swizzler?

No I'll reverse it.

[scroggo, 2015-10-09 20:18:40]

Can you also update nanobench to do partial scanlines? You'll need the fixes to
the subset benches in crrev.com/1396113002

Otherwise lgtm

On 2015/10/09 20:03:44, msarett wrote:
> I think that this CL is a lot easier to looks at now that I've removed most of
> the excess.  There's even still some spacing stuff etc. that I should have
> restored after I decided not to touch that code...
> 
> I will try to be more conscious of that in the future.
> 
> BTW is this going to need an API review?  All I've added is a comment.

Yes.

[msarett, 2015-10-09 20:53:28]

msarett@google.com changed reviewers:
+ reed@google.com

[msarett, 2015-10-09 21:23:53]

The CQ bit was checked by msarett@google.com to run a CQ dry run

[commit-bot: I haz the power, 2015-10-09 21:24:04]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1390213002/220001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1390213002/220001

[commit-bot: I haz the power, 2015-10-09 21:25:55]

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

[commit-bot: I haz the power, 2015-10-09 21:25:55]

Dry run: Try jobs failed on following builders:
  Build-Win-MSVC-x86_64-Debug-Trybot on client.skia.compile (JOB_FAILED,
http://build.chromium.org/p/client.skia.compile/builders/Build-Win-MSVC-x86_6...)

[msarett, 2015-10-09 21:30:50]

Patchset #5 (id:220001) has been deleted

[msarett, 2015-10-09 21:32:08]

The CQ bit was checked by msarett@google.com to run a CQ dry run

[commit-bot: I haz the power, 2015-10-09 21:32:18]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1390213002/240001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1390213002/240001

[commit-bot: I haz the power, 2015-10-09 21:38:48]

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

[commit-bot: I haz the power, 2015-10-09 21:38:49]

Dry run: This issue passed the CQ dry run.

[msarett, 2015-10-09 21:40:50]

Leon can you take another look?

[scroggo, 2015-10-12 16:11:20]

Included are a couple of small comments on your last patch set. I occasionally
forget that clicking "reply" just leaves all my drafts sitting there, for no one
to read :(

https://codereview.chromium.org/1390213002/diff/200001/include/codec/SkCodec.h
File include/codec/SkCodec.h (right):

https://codereview.chromium.org/1390213002/diff/200001/include/codec/SkCodec....
include/codec/SkCodec.h:177: *  The above description applies to getPixels() but
the usage is
I think it would be better to say something like:

In getPixels, ... (explanation for how it's used in getPixels)

In scanline decoding,  ..

https://codereview.chromium.org/1390213002/diff/200001/include/codec/SkCodec....
include/codec/SkCodec.h:276: *  kIndex8_SkColorType and uses the default
options.
Why this change?

https://codereview.chromium.org/1390213002/diff/240001/bench/subset/SubsetSin...
File bench/subset/SubsetSingleBench.cpp (right):

https://codereview.chromium.org/1390213002/diff/240001/bench/subset/SubsetSin...
bench/subset/SubsetSingleBench.cpp:87: SkDEBUGCODE(int lines = )
codec->getScanlines(bitmap.getPixels(), fSubsetHeight,
What would happen if you just declared lines as a uint32_t? Will it do that
conversion for you, making the following cast unnecessary?

https://codereview.chromium.org/1390213002/diff/240001/bench/subset/SubsetSin...
bench/subset/SubsetSingleBench.cpp:89: SkASSERT((uint32_t) lines ==
fSubsetHeight);
Oh, this is interesting. If I understand correctly, we no longer need to
differentiate between top down and none, since we write directly into the dst
memory. Does this also support the other orderings?

https://codereview.chromium.org/1390213002/diff/240001/bench/subset/SubsetTra...
File bench/subset/SubsetTranslateBench.cpp (right):

https://codereview.chromium.org/1390213002/diff/240001/bench/subset/SubsetTra...
bench/subset/SubsetTranslateBench.cpp:108: SkDEBUGCODE(int lines =)
codec->getScanlines(bitmap.getPixels(),
Similar to the other one, can we declare this as a uint32_t, and avoid the cast?

[msarett, 2015-10-12 17:13:56]

Mike, would you mind taking a look at the API?

Leon, I've responded to your latest comments.

https://codereview.chromium.org/1390213002/diff/200001/include/codec/SkCodec.h
File include/codec/SkCodec.h (right):

https://codereview.chromium.org/1390213002/diff/200001/include/codec/SkCodec....
include/codec/SkCodec.h:177: *  The above description applies to getPixels() but
the usage is
On 2015/10/12 16:11:20, scroggo wrote:
> I think it would be better to say something like:
> 
> In getPixels, ... (explanation for how it's used in getPixels)
> 
> In scanline decoding,  ..

Done.

https://codereview.chromium.org/1390213002/diff/200001/include/codec/SkCodec....
include/codec/SkCodec.h:276: *  kIndex8_SkColorType and uses the default
options.
On 2015/10/12 16:11:20, scroggo wrote:
> Why this change?

I changed this comment when I changed the API, and then I unchanged it, but it
looks like I didn't unchange it properly.

Will fix.

https://codereview.chromium.org/1390213002/diff/240001/bench/subset/SubsetSin...
File bench/subset/SubsetSingleBench.cpp (right):

https://codereview.chromium.org/1390213002/diff/240001/bench/subset/SubsetSin...
bench/subset/SubsetSingleBench.cpp:87: SkDEBUGCODE(int lines = )
codec->getScanlines(bitmap.getPixels(), fSubsetHeight,
On 2015/10/12 16:11:20, scroggo wrote:
> What would happen if you just declared lines as a uint32_t? Will it do that
> conversion for you, making the following cast unnecessary?

Yes that will.

https://codereview.chromium.org/1390213002/diff/240001/bench/subset/SubsetSin...
bench/subset/SubsetSingleBench.cpp:89: SkASSERT((uint32_t) lines ==
fSubsetHeight);
On 2015/10/12 16:11:20, scroggo wrote:
> Oh, this is interesting. If I understand correctly, we no longer need to
> differentiate between top down and none, since we write directly into the dst
> memory. Does this also support the other orderings?

It doesn't work for kOutOfOrder.  I'll add a check preventing us from running
this on kOutOfOrder.

https://codereview.chromium.org/1390213002/diff/240001/bench/subset/SubsetTra...
File bench/subset/SubsetTranslateBench.cpp (right):

https://codereview.chromium.org/1390213002/diff/240001/bench/subset/SubsetTra...
bench/subset/SubsetTranslateBench.cpp:108: SkDEBUGCODE(int lines =)
codec->getScanlines(bitmap.getPixels(),
On 2015/10/12 16:11:20, scroggo wrote:
> Similar to the other one, can we declare this as a uint32_t, and avoid the
cast?

Yes.

[reed1, 2015-10-13 19:40:47]

lgtm

[msarett, 2015-10-13 19:42:12]

The CQ bit was checked by msarett@google.com

[msarett, 2015-10-13 19:42:12]

The patchset sent to the CQ was uploaded after l-g-t-m from scroggo@google.com
Link to the patchset: https://codereview.chromium.org/1390213002/#ps260001
(title: "Response to comments")

[commit-bot: I haz the power, 2015-10-13 19:42:44]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1390213002/260001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1390213002/260001

[commit-bot: I haz the power, 2015-10-13 19:50:20]

Committed patchset #6 (id:260001) as
https://skia.googlesource.com/skia/+/fdb47571a3b5e72469b67de44e32ac14d9352ab4