Commit 34971fa9 authored by lukasza's avatar lukasza Committed by Commit bot

Allocate ExternalMemoryAllocator on the stack.

The change is desirable because:

- Ref-counting doesn't result in memory safety here, because
  ExternalMemoryAllocator wraps a raw pointer to short-lived
  memory.  Ref-counting cannot extend the lifetime of the
  memory this pointer points to.

- No consumers of SkBitmap::Allocator currently use ref-counting.
  If SkBitmap::Allocator was designed today, it probably wouldn't
  use SkRefCnt as a base class.

- Blink-to-Chromium style conversion will rename |ref| to |Ref| in WTF,
  but won't do it outside of Blink/WTF (e.g. in Skia), so RefPtr should
  only be used with Blink/WTF types going forward (i.e. Skia types
  should be wrapped in smart pointers provided by Skia).

More discussion is in an older code review:
https://codereview.chromium.org/1880993003/#msg10

BUG=641014

Review-Url: https://codereview.chromium.org/2326473005
Cr-Commit-Position: refs/heads/master@{#418657}
parent 7671698c
......@@ -124,15 +124,18 @@ bool ImageFrameGenerator::decodeAndScale(SegmentReader* data, bool allDataReceiv
TRACE_EVENT1("blink", "ImageFrameGenerator::decodeAndScale", "frame index", static_cast<int>(index));
RefPtr<ExternalMemoryAllocator> externalAllocator = adoptRef(new ExternalMemoryAllocator(info, pixels, rowBytes));
// This implementation does not support scaling so check the requested size.
SkISize scaledSize = SkISize::Make(info.width(), info.height());
ASSERT(m_fullSize == scaledSize);
// TODO (scroggo): Convert tryToResumeDecode() and decode() to take a
// sk_sp<SkBitmap::Allocator> instead of a bare pointer.
SkBitmap bitmap = tryToResumeDecode(data, allDataReceived, index, scaledSize, externalAllocator.get());
// It is okay to allocate ref-counted ExternalMemoryAllocator on the stack,
// because 1) it contains references to memory that will be invalid after
// returning (i.e. a pointer to |pixels|) and therefore 2) should not live
// longer than the call to the current method.
ExternalMemoryAllocator externalAllocator(info, pixels, rowBytes);
SkBitmap bitmap = tryToResumeDecode(data, allDataReceived, index, scaledSize, &externalAllocator);
DCHECK(externalAllocator.unique()); // Verify we have the only ref-count.
if (bitmap.isNull())
return false;
......
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