Commit d2b53317 authored by Khushal's avatar Khushal Committed by Commit Bot

blink/images: Ensure thread-safe access to ImageDecoder frames.

ImageFrameGenerator ensures that accessing the cached ImageDecoders is
thread-safe using a mutex, locked for the duration of
ImageFrameGenerator::TryToResumeDecode. However, the bitmap returned by
this method holds a reference to the pixel buffer which can still be
mutated by the ImageDecoder. This can introduce a race between copying
the pixel buffer to the caller's memory and another thread writing to
it to decode a subsequent frame.

Ensure the above is avoided by holding the mutex for the duration of
the decode and copying the pixel buffer to the caller's memory if
necessary.

R=chrishtr@chromium.org
BUG=840946

Cq-Include-Trybots: master.tryserver.blink:linux_trusty_blink_rel;master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2
Change-Id: I110c54232385b5d335001f221a480371968f9551
Reviewed-on: https://chromium-review.googlesource.com/1050619
Commit-Queue: Khushal <khushalsagar@chromium.org>
Reviewed-by: default avatarChris Harrelson <chrishtr@chromium.org>
Cr-Commit-Position: refs/heads/master@{#557059}
parent 5ff8f8ed
...@@ -151,6 +151,9 @@ bool ImageFrameGenerator::DecodeAndScale( ...@@ -151,6 +151,9 @@ bool ImageFrameGenerator::DecodeAndScale(
TRACE_EVENT1("blink", "ImageFrameGenerator::decodeAndScale", "frame index", TRACE_EVENT1("blink", "ImageFrameGenerator::decodeAndScale", "frame index",
static_cast<int>(index)); static_cast<int>(index));
// Lock the mutex, so only one thread can use the decoder at once.
MutexLocker lock(decode_mutex_);
// This implementation does not support arbitrary scaling so check the // This implementation does not support arbitrary scaling so check the
// requested size. // requested size.
SkISize scaled_size = SkISize::Make(info.width(), info.height()); SkISize scaled_size = SkISize::Make(info.width(), info.height());
...@@ -226,13 +229,14 @@ SkBitmap ImageFrameGenerator::TryToResumeDecode( ...@@ -226,13 +229,14 @@ SkBitmap ImageFrameGenerator::TryToResumeDecode(
const SkISize& scaled_size, const SkISize& scaled_size,
SkBitmap::Allocator& allocator, SkBitmap::Allocator& allocator,
ImageDecoder::AlphaOption alpha_option) { ImageDecoder::AlphaOption alpha_option) {
#if DCHECK_IS_ON()
DCHECK(decode_mutex_.Locked());
#endif
TRACE_EVENT1("blink", "ImageFrameGenerator::tryToResumeDecode", "frame index", TRACE_EVENT1("blink", "ImageFrameGenerator::tryToResumeDecode", "frame index",
static_cast<int>(index)); static_cast<int>(index));
ImageDecoder* decoder = nullptr; ImageDecoder* decoder = nullptr;
// Lock the mutex, so only one thread can use the decoder at once.
MutexLocker lock(decode_mutex_);
const bool resume_decoding = ImageDecodingStore::Instance().LockDecoder( const bool resume_decoding = ImageDecodingStore::Instance().LockDecoder(
this, scaled_size, alpha_option, &decoder); this, scaled_size, alpha_option, &decoder);
DCHECK(!resume_decoding || decoder); DCHECK(!resume_decoding || decoder);
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment