Report repetition count in SkCodec

Report repetition count in SkCodec

Add a new accessor to retrieve the repetition count.

Remove constants (and corresponding copyright) in SkCodecAnimation.
These may make sense for the calling code, but are not needed here.

kRepetitionCountInfinite corresponds to Blink's kAnimationLoopInfinite.
Move cLoopCountNotSeen to private. It is used to determine whether we
still need to parse. Add a new enum to the parse query - only parse
enough to determine the repetition count.

Unlike Chromium, SkGifCodec does not account for deleting the reader
(which SkGifCodec does not do) or failed decodes.

Add a test.

GOLD_TRYBOT_URL= https://gold.skia.org/search?issue=2447863002

Committed: https://skia.googlesource.com/skia/+/e71b1a1496738ebce4a8cac4ffa5ee1413996542

[scroggo_chromium, 2016-10-24 19:23:44]

Description was changed from

==========
Report repetition count in SkCodec::getFrameInfo

Remove constants (and corresponding copyright) in SkCodecAnimation.
These may make sense for the calling code, but are not needed here.

kRepetitionCountInfinite corresponds to Blink's kAnimationLoopInfinite.
Remove cLoopCountNotSeen. From the codec's perspective, this is the
same as 0 - repeat zero times.

Add a test.
==========

to

==========
Report repetition count in SkCodec::getFrameInfo

Remove constants (and corresponding copyright) in SkCodecAnimation.
These may make sense for the calling code, but are not needed here.

kRepetitionCountInfinite corresponds to Blink's kAnimationLoopInfinite.
Remove cLoopCountNotSeen. From the codec's perspective, this is the
same as 0 - repeat zero times.

Add a test.
GOLD_TRYBOT_URL= https://gold.skia.org/search?issue=2447863002
==========

[scroggo_chromium, 2016-10-24 19:24:37]

scroggo@chromium.org changed reviewers:
+ cblume@chromium.org, joostouwerling@google.com

[scroggo_chromium, 2016-10-26 14:38:06]

Description was changed from

==========
Report repetition count in SkCodec::getFrameInfo

Remove constants (and corresponding copyright) in SkCodecAnimation.
These may make sense for the calling code, but are not needed here.

kRepetitionCountInfinite corresponds to Blink's kAnimationLoopInfinite.
Remove cLoopCountNotSeen. From the codec's perspective, this is the
same as 0 - repeat zero times.

Add a test.
GOLD_TRYBOT_URL= https://gold.skia.org/search?issue=2447863002
==========

to

==========
Report repetition count in SkCodec::getFrameInfo

Remove constants (and corresponding copyright) in SkCodecAnimation.
These may make sense for the calling code, but are not needed here.

kRepetitionCountInfinite corresponds to Blink's kAnimationLoopInfinite.
Remove cLoopCountNotSeen. From the codec's perspective, this is the
same as 0 - repeat zero times.

Further, because the repetition count is returned by the same method
that parses the full stream, it should always return the actual
reported repetition count (if present).

Another difference from Chromium - it does not account for deleting
the reader (which SkGifCodec does not do) or failed decodes.

Add a test.
GOLD_TRYBOT_URL= https://gold.skia.org/search?issue=2447863002
==========

[scroggo_chromium, 2016-10-26 14:39:38]

scroggo@chromium.org changed reviewers:
+ cblume@google.com

[scroggo_chromium, 2016-10-26 14:39:38]

cblume@, please take a look. This API makes sense to me, and I think it will
support Chromium's use case, but I want to make sure you agree, since it is a
little different from Chromium's implementation.

[cblume, 2016-10-26 17:45:59]

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

https://codereview.chromium.org/2447863002/diff/1/include/codec/SkCodec.h#new...
include/codec/SkCodec.h:623: std::vector<FrameInfo> getFrameInfo(int*
outRepetitionCount = nullptr) {
I like where this is going.

I am pretty sure I would always get the repetition count when calling this. Is
there a use case where we wouldn't? Maybe because we call this multiple times
when we provide more buffers?

If we would always specify outRepetitionCount (btw I love "out" in the name),
1.) we don't need a default value, but more importantly
2.) maybe we should be returning a struct that contains the vector and the
repetition count

[scroggo_chromium, 2016-10-26 18:42:18]

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

https://codereview.chromium.org/2447863002/diff/1/include/codec/SkCodec.h#new...
include/codec/SkCodec.h:623: std::vector<FrameInfo> getFrameInfo(int*
outRepetitionCount = nullptr) {
On 2016/10/26 17:45:59, cblume wrote:
> I like where this is going.
> 
> I am pretty sure I would always get the repetition count when calling this. Is
> there a use case where we wouldn't? Maybe because we call this multiple times
> when we provide more buffers?

(I assume "more buffers" means "more data"?) Yes, I suppose that if the first
call revealed the repetition count, and then you supplied more data, you would
not need to query the repetition count again, since it should not change. On the
other hand, it's probably not very expensive to query it a second time, since
we've already cached the value.

Maybe that justifies reporting a different value for "we have not seen the loop
count" yet, but I think it just means "do not loop" (unless we later change our
minds), which is the same as a zero returned by the decoder.

> 
> If we would always specify outRepetitionCount (btw I love "out" in the name),
> 1.) we don't need a default value, but more importantly
> 2.) maybe we should be returning a struct that contains the vector and the
> repetition count

I guess I just imagine the repetition count to be less interesting than the
FrameInfo. I cannot imagine someone wanting the repetition count without the
FrameInfo, but I *could* imagine someone wanting the FrameInfo without the
repetition count (e.g. because they want to continue to loop regardless).

Another option that fits with Skia conventions - return a boolean (i.e. whether
this image has multiple frames) and use out-parameters for the repetition count
and the vector (either in a single struct or as separate parameters).

I don't think any of these options put restrictions on the client that should be
a problem, so I'll defer to reed@ on what is most appropriate for our API.

[scroggo_chromium, 2016-10-27 20:54:16]

scroggo@chromium.org changed reviewers:
+ reed@google.com

[scroggo_chromium, 2016-10-27 20:54:16]

reed@, can you take a look at the public API in SkCodec.h? Patch set 1 and 2
have slightly different takes on the method.

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

https://codereview.chromium.org/2447863002/diff/1/include/codec/SkCodec.h#new...
include/codec/SkCodec.h:623: std::vector<FrameInfo> getFrameInfo(int*
outRepetitionCount = nullptr) {
On 2016/10/26 18:42:18, scroggo_chromium wrote:
> On 2016/10/26 17:45:59, cblume wrote:
> > I like where this is going.
> > 
> > I am pretty sure I would always get the repetition count when calling this.
Is
> > there a use case where we wouldn't? Maybe because we call this multiple
times
> > when we provide more buffers?
> 
> (I assume "more buffers" means "more data"?) Yes, I suppose that if the first
> call revealed the repetition count, and then you supplied more data, you would
> not need to query the repetition count again, since it should not change. On
the
> other hand, it's probably not very expensive to query it a second time, since
> we've already cached the value.
> 
> Maybe that justifies reporting a different value for "we have not seen the
loop
> count" yet, but I think it just means "do not loop" (unless we later change
our
> minds), which is the same as a zero returned by the decoder.
> 
> > 
> > If we would always specify outRepetitionCount (btw I love "out" in the
name),
> > 1.) we don't need a default value, but more importantly
> > 2.) maybe we should be returning a struct that contains the vector and the
> > repetition count
> 
> I guess I just imagine the repetition count to be less interesting than the
> FrameInfo. I cannot imagine someone wanting the repetition count without the
> FrameInfo, but I *could* imagine someone wanting the FrameInfo without the
> repetition count (e.g. because they want to continue to loop regardless).
> 
> Another option that fits with Skia conventions - return a boolean (i.e.
whether
> this image has multiple frames) and use out-parameters for the repetition
count
> and the vector (either in a single struct or as separate parameters).
> 
> I don't think any of these options put restrictions on the client that should
be
> a problem, so I'll defer to reed@ on what is most appropriate for our API.

patch set 2 returns a bool, and uses multiple out parameters. I kind of like
patch set 1, because I can say:

auto frameInfo = codec->getFrameInfo(&repetitionCount)

which saves a little bit of typing. But I don't have a strong preference.

[cblume, 2016-10-28 01:09:42]

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

https://codereview.chromium.org/2447863002/diff/1/include/codec/SkCodec.h#new...
include/codec/SkCodec.h:623: std::vector<FrameInfo> getFrameInfo(int*
outRepetitionCount = nullptr) {
On 2016/10/26 18:42:18, scroggo_chromium wrote:
> On 2016/10/26 17:45:59, cblume wrote:
> > I like where this is going.
> > 
> > I am pretty sure I would always get the repetition count when calling this.
Is
> > there a use case where we wouldn't? Maybe because we call this multiple
times
> > when we provide more buffers?
> 
> (I assume "more buffers" means "more data"?) Yes, I suppose that if the first
> call revealed the repetition count, and then you supplied more data, you would
> not need to query the repetition count again, since it should not change. On
the
> other hand, it's probably not very expensive to query it a second time, since
> we've already cached the value.
> 
> Maybe that justifies reporting a different value for "we have not seen the
loop
> count" yet, but I think it just means "do not loop" (unless we later change
our
> minds), which is the same as a zero returned by the decoder.
> 
> > 
> > If we would always specify outRepetitionCount (btw I love "out" in the
name),
> > 1.) we don't need a default value, but more importantly
> > 2.) maybe we should be returning a struct that contains the vector and the
> > repetition count
> 
> I guess I just imagine the repetition count to be less interesting than the
> FrameInfo. I cannot imagine someone wanting the repetition count without the
> FrameInfo, but I *could* imagine someone wanting the FrameInfo without the
> repetition count (e.g. because they want to continue to loop regardless).
> 
> Another option that fits with Skia conventions - return a boolean (i.e.
whether
> this image has multiple frames) and use out-parameters for the repetition
count
> and the vector (either in a single struct or as separate parameters).
> 
> I don't think any of these options put restrictions on the client that should
be
> a problem, so I'll defer to reed@ on what is most appropriate for our API.

Actually, I may have said this wrong.

I definitely want to know the repetition count at some point. But if I am making
multiple calls to getFrameInfo (as I provide more data), I don't need it each
time. I only really need it once I have provided all the data. I only ever need
to get it once.

In that way, would it be better to keep the concepts separate? The frameInfo is
different from the repetition count. Should they be separate functions?

https://codereview.chromium.org/2447863002/diff/1/include/codec/SkCodec.h#new...
include/codec/SkCodec.h:623: std::vector<FrameInfo> getFrameInfo(int*
outRepetitionCount = nullptr) {
On 2016/10/27 20:54:16, scroggo_chromium wrote:
> On 2016/10/26 18:42:18, scroggo_chromium wrote:
> > On 2016/10/26 17:45:59, cblume wrote:
> > > I like where this is going.
> > > 
> > > I am pretty sure I would always get the repetition count when calling
this.
> Is
> > > there a use case where we wouldn't? Maybe because we call this multiple
> times
> > > when we provide more buffers?
> > 
> > (I assume "more buffers" means "more data"?) Yes, I suppose that if the
first
> > call revealed the repetition count, and then you supplied more data, you
would
> > not need to query the repetition count again, since it should not change. On
> the
> > other hand, it's probably not very expensive to query it a second time,
since
> > we've already cached the value.
> > 
> > Maybe that justifies reporting a different value for "we have not seen the
> loop
> > count" yet, but I think it just means "do not loop" (unless we later change
> our
> > minds), which is the same as a zero returned by the decoder.
> > 
> > > 
> > > If we would always specify outRepetitionCount (btw I love "out" in the
> name),
> > > 1.) we don't need a default value, but more importantly
> > > 2.) maybe we should be returning a struct that contains the vector and the
> > > repetition count
> > 
> > I guess I just imagine the repetition count to be less interesting than the
> > FrameInfo. I cannot imagine someone wanting the repetition count without the
> > FrameInfo, but I *could* imagine someone wanting the FrameInfo without the
> > repetition count (e.g. because they want to continue to loop regardless).
> > 
> > Another option that fits with Skia conventions - return a boolean (i.e.
> whether
> > this image has multiple frames) and use out-parameters for the repetition
> count
> > and the vector (either in a single struct or as separate parameters).
> > 
> > I don't think any of these options put restrictions on the client that
should
> be
> > a problem, so I'll defer to reed@ on what is most appropriate for our API.
> 
> patch set 2 returns a bool, and uses multiple out parameters. I kind of like
> patch set 1, because I can say:
> 
> auto frameInfo = codec->getFrameInfo(&repetitionCount)
> 
> which saves a little bit of typing. But I don't have a strong preference.

patch set 2's solution would allow us to reuse allocated space. However, I don't
expect to be calling this function so many times that this would matter. I agree
that patch set 1 reads more easily and feels more natural. I like it, but I'm
okay with either.

[scroggo_chromium, 2016-10-28 11:49:41]

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

https://codereview.chromium.org/2447863002/diff/1/include/codec/SkCodec.h#new...
include/codec/SkCodec.h:623: std::vector<FrameInfo> getFrameInfo(int*
outRepetitionCount = nullptr) {
On 2016/10/28 01:09:41, cblume wrote:
> On 2016/10/26 18:42:18, scroggo_chromium wrote:
> > On 2016/10/26 17:45:59, cblume wrote:
> > > I like where this is going.
> > > 
> > > I am pretty sure I would always get the repetition count when calling
this.
> Is
> > > there a use case where we wouldn't? Maybe because we call this multiple
> times
> > > when we provide more buffers?
> > 
> > (I assume "more buffers" means "more data"?) Yes, I suppose that if the
first
> > call revealed the repetition count, and then you supplied more data, you
would
> > not need to query the repetition count again, since it should not change. On
> the
> > other hand, it's probably not very expensive to query it a second time,
since
> > we've already cached the value.
> > 
> > Maybe that justifies reporting a different value for "we have not seen the
> loop
> > count" yet, but I think it just means "do not loop" (unless we later change
> our
> > minds), which is the same as a zero returned by the decoder.
> > 
> > > 
> > > If we would always specify outRepetitionCount (btw I love "out" in the
> name),
> > > 1.) we don't need a default value, but more importantly
> > > 2.) maybe we should be returning a struct that contains the vector and the
> > > repetition count
> > 
> > I guess I just imagine the repetition count to be less interesting than the
> > FrameInfo. I cannot imagine someone wanting the repetition count without the
> > FrameInfo, but I *could* imagine someone wanting the FrameInfo without the
> > repetition count (e.g. because they want to continue to loop regardless).
> > 
> > Another option that fits with Skia conventions - return a boolean (i.e.
> whether
> > this image has multiple frames) and use out-parameters for the repetition
> count
> > and the vector (either in a single struct or as separate parameters).
> > 
> > I don't think any of these options put restrictions on the client that
should
> be
> > a problem, so I'll defer to reed@ on what is most appropriate for our API.
> 
> Actually, I may have said this wrong.
> 
> I definitely want to know the repetition count at some point. But if I am
making
> multiple calls to getFrameInfo (as I provide more data), I don't need it each
> time. I only really need it once I have provided all the data. I only ever
need
> to get it once.
> 
> In that way, would it be better to keep the concepts separate? The frameInfo
is
> different from the repetition count. Should they be separate functions?

I still think you always want the FrameInfo when you want the repetition count,
but you never want the repetition count without the FrameInfo. (Maybe the client
will want to find them out at different times architecturally, but logically I
think you need to query them both once you've supplied all the data, since the
final bit of data may have more frames and the count.)

I'm not opposed to splitting them up, but I mainly kept them together because I
felt it was consistent with the decision we made for getFrameInfo. I originally
had one method to query the number of frames, and then other methods to query
the duration and dependent frame information, but we decided to provide a single
query that provided all of the information. Just like the repetition count,
after receiving the information about the first frame, you don't need to query
that again, but we still provide it each time you provide more data and ask for
the frame info.

[cblume, 2016-10-28 17:21:37]

On 2016/10/28 11:49:41, scroggo_chromium wrote:
> I still think you always want the FrameInfo when you want the repetition
count,
> but you never want the repetition count without the FrameInfo. (Maybe the
client
> will want to find them out at different times architecturally, but logically I
> think you need to query them both once you've supplied all the data, since the
> final bit of data may have more frames and the count.)
> 
> I'm not opposed to splitting them up, but I mainly kept them together because
I
> felt it was consistent with the decision we made for getFrameInfo. I
originally
> had one method to query the number of frames, and then other methods to query
> the duration and dependent frame information, but we decided to provide a
single
> query that provided all of the information. Just like the repetition count,
> after receiving the information about the first frame, you don't need to query
> that again, but we still provide it each time you provide more data and ask
for
> the frame info.

I agree with you.
I only ever need to query the repetition count once: After I have provided all
of the data.
I will need to query frame info each time I provide data.
So the final chunk of data being provided will query both.

So from a conceptual point of view, you are right -- I will always query frame
info when I query repetition count.

But like you said, I expect them to in slightly different places,
architecturally.
Keeping them together prevents me from separating concerns.

I'm okay if we don't separate them. Just thought I would throw the idea out
there and see if we liked it.

[reed1, 2016-10-31 14:17:01]

api lgtm

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

https://codereview.chromium.org/2447863002/diff/20001/include/codec/SkCodec.h...
include/codec/SkCodec.h:622: *      times an animated image should repeat.
Assuming this can take nullptr for outFrameInfo, then I'm fine with the
additonal out-param

[scroggo, 2016-10-31 19:04:44]

Description was changed from

==========
Report repetition count in SkCodec::getFrameInfo

Remove constants (and corresponding copyright) in SkCodecAnimation.
These may make sense for the calling code, but are not needed here.

kRepetitionCountInfinite corresponds to Blink's kAnimationLoopInfinite.
Remove cLoopCountNotSeen. From the codec's perspective, this is the
same as 0 - repeat zero times.

Further, because the repetition count is returned by the same method
that parses the full stream, it should always return the actual
reported repetition count (if present).

Another difference from Chromium - it does not account for deleting
the reader (which SkGifCodec does not do) or failed decodes.

Add a test.
GOLD_TRYBOT_URL= https://gold.skia.org/search?issue=2447863002
==========

to

==========
Report repetition count in SkCodec::getFrameInfo

Remove constants (and corresponding copyright) in SkCodecAnimation.
These may make sense for the calling code, but are not needed here.

kRepetitionCountInfinite corresponds to Blink's kAnimationLoopInfinite.
Move cLoopCountNotSeen to private. It is used to determine whether we
still need to parse.

Another difference from Chromium - it does not account for deleting
the reader (which SkGifCodec does not do) or failed decodes.

Add a test.

GOLD_TRYBOT_URL= https://gold.skia.org/search?issue=2447863002
==========

[scroggo, 2016-10-31 19:12:27]

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

[scroggo, 2016-10-31 19:12:27]

PTAL at patch sets 4 and 5. 4 tests for a null vector and updates the comment to
be explicit that null is allowed. (It also updates the impl to only parse as
much as is necessary for the repetition count.) 5 uses a separate method.

Another option that I haven't written is to have separate methods but still use
an out parameter. i.e.

bool getFrameInfo(vector<>);
int getRepetitionCount();

Any preference?

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

https://codereview.chromium.org/2447863002/diff/20001/include/codec/SkCodec.h...
include/codec/SkCodec.h:622: *      times an animated image should repeat.
On 2016/10/31 14:17:01, reed1 wrote:
> Assuming this can take nullptr for outFrameInfo, then I'm fine with the
> additonal out-param

I've updated the header, and updated the test to call this method with null
outFrameInfo.

[reed1, 2016-10-31 19:15:36]

I'm fine as is, or using a stand-alone getter.

[joostouwerling, 2016-10-31 20:03:54]

I have a slight preference for patch set 5. The only downside would be an extra
line of code every now and then, whereas the out parameters add more cognitive
overload (for me, at least). An example: which parameters do I need to provide
to get a vector of frame info? codec->getFrameInfo(&frameInfos, nullptr); seems
harder to grasp than: vector<FrameInfo> frameInfos = codec->getFrameInfo();. 

But it is not a strong preference and if the out-parameters is a common Skia
approach, then PS4 seems fine.

[cblume, 2016-10-31 22:01:32]

non-owner lgtm on all of them. Whatever you choose, I'm happy with. :)

[scroggo, 2016-11-01 14:06:23]

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

[commit-bot: I haz the power, 2016-11-01 14:06:25]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[scroggo, 2016-11-01 14:08:08]

Description was changed from

==========
Report repetition count in SkCodec::getFrameInfo

Remove constants (and corresponding copyright) in SkCodecAnimation.
These may make sense for the calling code, but are not needed here.

kRepetitionCountInfinite corresponds to Blink's kAnimationLoopInfinite.
Move cLoopCountNotSeen to private. It is used to determine whether we
still need to parse.

Another difference from Chromium - it does not account for deleting
the reader (which SkGifCodec does not do) or failed decodes.

Add a test.

GOLD_TRYBOT_URL= https://gold.skia.org/search?issue=2447863002
==========

to

==========
Report repetition count in SkCodec

Add a new accessor to retrieve the repetition count.

Remove constants (and corresponding copyright) in SkCodecAnimation.
These may make sense for the calling code, but are not needed here.

kRepetitionCountInfinite corresponds to Blink's kAnimationLoopInfinite.
Move cLoopCountNotSeen to private. It is used to determine whether we
still need to parse. Add a new enum to the parse query - only parse
enough to determine the repetition count.

Unlike Chromium, SkGifCodec does not account for deleting the reader
(which SkGifCodec does not do) or failed decodes.

Add a test.

GOLD_TRYBOT_URL= https://gold.skia.org/search?issue=2447863002
==========

[commit-bot: I haz the power, 2016-11-01 14:26:56]

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

[commit-bot: I haz the power, 2016-11-01 14:26:57]

Dry run: This issue passed the CQ dry run.

[scroggo, 2016-11-01 14:55:01]

The CQ bit was checked by scroggo@google.com

[scroggo, 2016-11-01 14:55:01]

The patchset sent to the CQ was uploaded after l-g-t-m from reed@google.com,
cblume@chromium.org
Link to the patchset: https://codereview.chromium.org/2447863002/#ps120001
(title: "Make colorTables.gif use a different loop count")

[commit-bot: I haz the power, 2016-11-01 14:55:04]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-11-01 15:28:31]

Description was changed from

==========
Report repetition count in SkCodec

Add a new accessor to retrieve the repetition count.

Remove constants (and corresponding copyright) in SkCodecAnimation.
These may make sense for the calling code, but are not needed here.

kRepetitionCountInfinite corresponds to Blink's kAnimationLoopInfinite.
Move cLoopCountNotSeen to private. It is used to determine whether we
still need to parse. Add a new enum to the parse query - only parse
enough to determine the repetition count.

Unlike Chromium, SkGifCodec does not account for deleting the reader
(which SkGifCodec does not do) or failed decodes.

Add a test.

GOLD_TRYBOT_URL= https://gold.skia.org/search?issue=2447863002
==========

to

==========
Report repetition count in SkCodec

Add a new accessor to retrieve the repetition count.

Remove constants (and corresponding copyright) in SkCodecAnimation.
These may make sense for the calling code, but are not needed here.

kRepetitionCountInfinite corresponds to Blink's kAnimationLoopInfinite.
Move cLoopCountNotSeen to private. It is used to determine whether we
still need to parse. Add a new enum to the parse query - only parse
enough to determine the repetition count.

Unlike Chromium, SkGifCodec does not account for deleting the reader
(which SkGifCodec does not do) or failed decodes.

Add a test.

GOLD_TRYBOT_URL= https://gold.skia.org/search?issue=2447863002

Committed:
https://skia.googlesource.com/skia/+/e71b1a1496738ebce4a8cac4ffa5ee1413996542
==========

[commit-bot: I haz the power, 2016-11-01 15:28:32]

Committed patchset #7 (id:120001) as
https://skia.googlesource.com/skia/+/e71b1a1496738ebce4a8cac4ffa5ee1413996542