RESOLVED FIXED 107404
Fix a race condition on SkBitmap::lockPixels()/unlockPixels() for lazy image decoding 
https://bugs.webkit.org/show_bug.cgi?id=107404
Summary Fix a race condition on SkBitmap::lockPixels()/unlockPixels() for lazy image ...
Min Qin
Reported 2013-01-20 14:14:46 PST
Fix a race condition on SkBitmap::lockPixels()/unlockPixels() for lazy image decoding
Attachments
Patch (5.10 KB, patch)
2013-01-20 14:17 PST, Min Qin 		no flags
DetailsFormatted DiffDiff
Patch (5.39 KB, patch)
2013-01-22 10:45 PST, Min Qin 		no flags
DetailsFormatted DiffDiff
Patch (5.39 KB, patch)
2013-01-22 10:52 PST, Min Qin 		no flags
DetailsFormatted DiffDiff
Show Obsolete (2) View All Add attachment proposed patch, testcase, etc.
Min Qin
Comment 1 2013-01-20 14:17:39 PST
Created attachment 183685 [details] Patch
Stephen White
Comment 2 2013-01-21 07:50:56 PST
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 3 2013-01-22 10:03:37 PST
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 4 2013-01-22 10:03:39 PST
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
Comment 5 2013-01-22 10:45:37 PST
Created attachment 184014 [details] Patch
Min Qin
Comment 6 2013-01-22 10:46:36 PST
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 7 2013-01-22 10:47:47 PST
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
Comment 8 2013-01-22 10:48:06 PST
After the fix then lgtm.
Min Qin
Comment 9 2013-01-22 10:52:31 PST
Created attachment 184018 [details] Patch
Min Qin
Comment 10 2013-01-22 10:53:31 PST
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 11 2013-01-22 10:55:58 PST
Comment on attachment 184018 [details] Patch Transitively OK. r=me
WebKit Review Bot
Comment 12 2013-01-22 11:30:26 PST
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 13 2013-01-22 13:02:46 PST
Comment on attachment 184018 [details] Patch Last cq was rejected because of unrelated flakyness. Try again.
WebKit Review Bot
Comment 14 2013-01-22 13:10:36 PST
Comment on attachment 184018 [details] Patch Clearing flags on attachment: 184018 Committed r140454: <http://trac.webkit.org/changeset/140454>
WebKit Review Bot
Comment 15 2013-01-22 13:10:40 PST
All reviewed patches have been landed. Closing bug.
Note You need to log in before you can comment on or make changes to this bug.