Ganesh: images upload to GPU performance fix (skip copying encoded data)

third_party/WebKit/Source/platform/graphics/ImageFrameGenerator.cpp

 /*
  * Copyright (C) 2012 Google Inc. All rights reserved.
  *
  * Redistribution and use in source and binary forms, with or without
  * modification, are permitted provided that the following conditions
  * are met:
  * 1. Redistributions of source code must retain the above copyright
  *    notice, this list of conditions and the following disclaimer.
  * 2. Redistributions in binary form must reproduce the above copyright
  *    notice, this list of conditions and the following disclaimer in the
@@ skipping 99 matching lines @@
 ImageFrameGenerator::~ImageFrameGenerator()
 {
     ImageDecodingStore::instance().removeCacheIndexedByGenerator(this);
 }
 
 void ImageFrameGenerator::setData(PassRefPtr<SharedBuffer> data, bool allDataReceived)
 {
     m_data.setData(data.get(), allDataReceived);
 }
 
-void ImageFrameGenerator::copyData(RefPtr<SharedBuffer>* data, bool* allDataReceived)
+static void sharedBufSkDataReleaseProc(const void* addr, void* ctx)
+{
+    RefPtr<SharedBuffer>* pbuffer = static_cast<RefPtr<SharedBuffer>*>(ctx);
+    ASSERT(pbuffer && (*pbuffer)->data() == addr);
+    delete pbuffer;

[Stephen White, 2015/12/02 02:44:54]

It might be clearer to pass a bare pointer though the callback, and use an
explicit ref() at the call site and derefIfNotNull() here, and forget about
allocating/deleting the RefPtr. Would that work? (I don't normally recommend
using bare ptrs in Blink, but this callback situation is unusual.)

+}
+
+SkData* ImageFrameGenerator::refSkData()
 {
     SharedBuffer* buffer = 0;
-    m_data.data(&buffer, allDataReceived);
-    if (buffer)
-        *data = buffer->copy();
+    bool allDataReceived = false;
+    m_data.data(&buffer, &allDataReceived);

[chrishtr, 2015/12/01 21:56:45]

Please change ThreadSafeDataTransport::data to return a PassRefPtr, and change
the name to something like getData(). That would make this CL easier to read and
more obviously correct.

[Stephen White, 2015/12/02 02:50:39]

Even if we fix that, we still need something to pass through the callback. I've
recommended a bare pointer above, since I find this RefPtr* really odd, but
maybe that's going against your suggestion here.

[aleksandar.stojiljkovic, 2015/12/02 11:04:16]

Thanks, both chrishtr and you have a point here about readability - looks
cleaner now without allocating RefPtr on heap to do deref.

+    if (!allDataReceived)
+        return nullptr;
+
+    RefPtr<SharedBuffer>* pbuffer = new RefPtr<SharedBuffer>(buffer);
+    return SkData::NewWithProc(buffer->data(), buffer->size(), sharedBufSkDataReleaseProc, pbuffer);
 }
 
 bool ImageFrameGenerator::decodeAndScale(const SkImageInfo& info, size_t index, void* pixels, size_t rowBytes)
 {
     // This method is called to populate a discardable memory owned by Skia.
 
     // Prevents concurrent decode or scale operations on the same image data.
     MutexLocker lock(m_decodeMutex);
 
     // This implementation does not support scaling so check the requested size.
@@ skipping 237 matching lines @@
 
     // Setting a dummy ImagePlanes object signals to the decoder that we want to do YUV decoding.
     decoder->setData(data, allDataReceived);
     OwnPtr<ImagePlanes> dummyImagePlanes = adoptPtr(new ImagePlanes);
     decoder->setImagePlanes(dummyImagePlanes.release());
 
     return updateYUVComponentSizes(decoder.get(), componentSizes, ImageDecoder::SizeForMemoryAllocation);
 }
 
 } // namespace blink