Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(503)

Unified Diff: third_party/WebKit/Source/platform/graphics/ImageFrameGenerator.cpp

Issue 1484853003: Ganesh: images upload to GPU performance fix (skip copying encoded data) (Closed) Base URL: https://chromium.googlesource.com/chromium/src.git@master
Patch Set: Comment #59 fix. Not using fast malloc as suggested during merge - attempt to fix memory leak intro… Created 5 years ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
Index: third_party/WebKit/Source/platform/graphics/ImageFrameGenerator.cpp
diff --git a/third_party/WebKit/Source/platform/graphics/ImageFrameGenerator.cpp b/third_party/WebKit/Source/platform/graphics/ImageFrameGenerator.cpp
index 0d29bce0f18e6463f7fcbfd94cabe702c6455231..ea742717a742b7b5591360656aaaab3c507cfc28 100644
--- a/third_party/WebKit/Source/platform/graphics/ImageFrameGenerator.cpp
+++ b/third_party/WebKit/Source/platform/graphics/ImageFrameGenerator.cpp
@@ -27,6 +27,7 @@
#include "platform/graphics/ImageFrameGenerator.h"
+#include "SkData.h"
#include "platform/SharedBuffer.h"
#include "platform/TraceEvent.h"
#include "platform/graphics/ImageDecodingStore.h"
@@ -101,6 +102,7 @@ static bool updateYUVComponentSizes(ImageDecoder* decoder, SkISize componentSize
ImageFrameGenerator::ImageFrameGenerator(const SkISize& fullSize, PassRefPtr<SharedBuffer> data, bool allDataReceived, bool isMultiFrame)
: m_fullSize(fullSize)
+ , m_data(adoptRef(new ThreadSafeDataTransport()))
, m_isMultiFrame(isMultiFrame)
, m_decodeFailedAndEmpty(false)
, m_decodeCount(0)
@@ -116,15 +118,54 @@ ImageFrameGenerator::~ImageFrameGenerator()
void ImageFrameGenerator::setData(PassRefPtr<SharedBuffer> data, bool allDataReceived)
{
- m_data.setData(data.get(), allDataReceived);
+ m_data->setData(data.get(), allDataReceived);
}
-void ImageFrameGenerator::copyData(RefPtr<SharedBuffer>* data, bool* allDataReceived)
+static void sharedSkDataReleaseCallback(const void* address, void* context)
{
+ ThreadSafeDataTransport* dataTransport = static_cast<ThreadSafeDataTransport*>(context);
+#if ENABLE(ASSERT)
+ ASSERT(dataTransport);
SharedBuffer* buffer = 0;
- m_data.data(&buffer, allDataReceived);
- if (buffer)
- *data = buffer->copy();
+ bool allDataReceived = false;
+ dataTransport->data(&buffer, &allDataReceived);
+ ASSERT(allDataReceived && buffer && buffer->data() == address);
+#endif
+ // Deref m_data now.
+ dataTransport->deref();
+}
+
+SkData* ImageFrameGenerator::refEncodedData() const
+{
+ // Every SkData instance and SharedBuffer need to keep buffer->data() refcounted.
+ // Both SkData and m_data.m_readBuffer are having separate ref counting implementations.
+ //
+ // SkData's SkData::NewWithProc is designed with similar use case in mind,
+ // unmapping wrapped shmem data once all SkData instances are disposed.
+ // in this case, sharedBufSkDataReleaseProc would just need to deref (since m_data->m_readBuffer
+ // might still hold a reference) and that is happening in sharedBufSkDataReleaseProc.
+ //
+ // Preventing m_data disposal if ImageFrameGenerator is thread safe - SkData could get disposed
+ // in any thread, and SkData would still need to hold the ref to SharedBuffer if ImageFrameGenerator
+ // is deleted. This is why full m_data (ThreadSafeRefCounted) is passed to client - to hold the
+ // reference to SharedBuffer (accompanying returned SkData). By contract in Skia's SkData,
+ // client needs to call SkData unref that would end up in sharedBufSkDataReleaseProc
+ // releasing the ref to ThreadSafeDataTransport (in thread safe way) and underlying SharedBuffer.
+ //
+ // A note about allDataReceived:
+ // SkData is returned only for full image (encoded) data downloaded. This is important since
+ // DeferredImageDecoder::setData is called only once with allDataReceived and after that
+ // m_data->m_readBuffer.data() is not changed. See also RELEASE_ASSERT used in
+ // ThreadSafeDataTransport::data().
+ SharedBuffer* buffer = 0;
+ bool allDataReceived = false;
+ m_data->data(&buffer, &allDataReceived);
+ if (!allDataReceived)
+ return nullptr;
+
+ // While SkData is holding reference to underlying data, prevent disposing m_data and its content.
+ m_data->ref();
+ return SkData::NewWithProc(buffer->data(), buffer->size(), sharedSkDataReleaseCallback, m_data.get());
}
bool ImageFrameGenerator::decodeAndScale(const SkImageInfo& info, size_t index, void* pixels, size_t rowBytes)
@@ -184,7 +225,7 @@ bool ImageFrameGenerator::decodeToYUV(SkISize componentSizes[3], void* planes[3]
SharedBuffer* data = 0;
bool allDataReceived = false;
- m_data.data(&data, &allDataReceived);
+ m_data->data(&data, &allDataReceived);
// FIXME: YUV decoding does not currently support progressive decoding.
ASSERT(allDataReceived);
@@ -290,7 +331,7 @@ bool ImageFrameGenerator::decode(size_t index, ImageDecoder** decoder, SkBitmap*
SharedBuffer* data = 0;
bool allDataReceived = false;
bool newDecoder = false;
- m_data.data(&data, &allDataReceived);
+ m_data->data(&data, &allDataReceived);
// Try to create an ImageDecoder if we are not given one.
if (!*decoder) {
@@ -358,7 +399,7 @@ bool ImageFrameGenerator::getYUVComponentSizes(SkISize componentSizes[3])
SharedBuffer* data = 0;
bool allDataReceived = false;
- m_data.data(&data, &allDataReceived);
+ m_data->data(&data, &allDataReceived);
// FIXME: YUV decoding does not currently support progressive decoding.
if (!allDataReceived)

Powered by Google App Engine
This is Rietveld 408576698