Fix a race condition on SkBitmap::lockPixels()/unlockPixels() for lazy image decoding

[Min Qin]

Fix a race condition on SkBitmap::lockPixels()/unlockPixels() for lazy image decoding

[Min Qin]

Created attachment 183685 [details]
Patch

[Stephen White]

Comment on attachment 183685 [details]
Patch

This looks OK to me, but I'll wait for hclam to take a look.

[Hin-Chung Lam]

Comment on attachment 183685 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=183685&action=review

> Source/WebCore/platform/graphics/chromium/ImageDecodingStore.cpp:135
> +        image->bitmap().lockPixels();

Make this call after line 147 so you don't need to take lock here.

[Hin-Chung Lam]

Comment on attachment 183685 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=183685&action=review

> Source/WebCore/platform/graphics/chromium/ImageDecodingStore.cpp:135
> +        image->bitmap().lockPixels();

Make this call after line 147 so you don't need to take lock here.

[Min Qin]

Created attachment 184014 [details]
Patch

[Min Qin]

Comment on attachment 183685 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=183685&action=review

>>> Source/WebCore/platform/graphics/chromium/ImageDecodingStore.cpp:135
>>> +        image->bitmap().lockPixels();
>> 
>> Make this call after line 147 so you don't need to take lock here.
> 
> Make this call after line 147 so you don't need to take lock here.

Done

[Hin-Chung Lam]

Comment on attachment 184014 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=184014&action=review

> Source/WebCore/platform/graphics/chromium/ImageDecodingStore.cpp:143
> +    image->bitmap().lockPixels();

image is NULL at this point. So this should be cachedImage->bitmap().lockPixels().

[Hin-Chung Lam]

After the fix then lgtm.

[Min Qin]

Created attachment 184018 [details]
Patch

[Min Qin]

Comment on attachment 184014 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=184014&action=review

>> Source/WebCore/platform/graphics/chromium/ImageDecodingStore.cpp:143
>> +    image->bitmap().lockPixels();
> 
> image is NULL at this point. So this should be cachedImage->bitmap().lockPixels().

ah...Didn't notice that CreateAndUse() will take the OwnPtr away. Fixed.

[Stephen White]

Comment on attachment 184018 [details]
Patch

Transitively OK.  r=me

[WebKit Review Bot]

Comment on attachment 184018 [details]
Patch

Rejecting attachment 184018 [details] from commit-queue.

New failing tests:
inspector-protocol/debugger-terminate-dedicated-worker-while-paused.html
Full output: http://queues.webkit.org/results/16063133

[Xianzhu Wang]

Comment on attachment 184018 [details]
Patch

Last cq was rejected because of unrelated flakyness. Try again.

[WebKit Review Bot]

Comment on attachment 184018 [details]
Patch

Clearing flags on attachment: 184018

Committed r140454: <http://trac.webkit.org/changeset/140454>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.