Bug 916128 - Ensure that image encoding callbacks are released on the main thread. r=khuey
authorStephen Pohl <spohl.mozilla.bugs@gmail.com>
Wed, 16 Oct 2013 22:55:41 -0400
changeset 165867 3b3eb94009e61f23ef573897c015c17a9ead39e8
parent 165866 3d04445e3a92c86885111c36c546e1baf45a5fa5
child 165868 18d72b85134557a949f6392e8ec86a73aa48a83c
push id428
push userbbajaj@mozilla.com
push dateTue, 28 Jan 2014 00:16:25 +0000
treeherdermozilla-release@cd72a7ff3a75 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerskhuey
bugs916128
milestone27.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 916128 - Ensure that image encoding callbacks are released on the main thread. r=khuey
content/canvas/src/ImageEncoder.cpp
--- a/content/canvas/src/ImageEncoder.cpp
+++ b/content/canvas/src/ImageEncoder.cpp
@@ -19,55 +19,71 @@ public:
                         nsIThread* aEncoderThread,
                         FileCallback& aCallback)
     : mImgSize(0)
     , mType()
     , mImgData(nullptr)
     , mScriptContext(aScriptContext)
     , mEncoderThread(aEncoderThread)
     , mCallback(&aCallback)
+    , mFailed(false)
   {}
   virtual ~EncodingCompleteEvent() {}
 
   NS_IMETHOD Run()
   {
     MOZ_ASSERT(NS_IsMainThread());
 
-    nsRefPtr<nsDOMMemoryFile> blob =
-      new nsDOMMemoryFile(mImgData, mImgSize, mType);
+    mozilla::ErrorResult rv;
+
+    if (!mFailed) {
+      nsRefPtr<nsDOMMemoryFile> blob =
+        new nsDOMMemoryFile(mImgData, mImgSize, mType);
 
-    if (mScriptContext) {
-      JSContext* jsContext = mScriptContext->GetNativeContext();
-      if (jsContext) {
-        JS_updateMallocCounter(jsContext, mImgSize);
+      if (mScriptContext) {
+        JSContext* jsContext = mScriptContext->GetNativeContext();
+        if (jsContext) {
+          JS_updateMallocCounter(jsContext, mImgSize);
+        }
       }
+
+      mCallback->Call(blob, rv);
     }
 
-    mozilla::ErrorResult rv;
-    mCallback->Call(blob, rv);
-    NS_ENSURE_SUCCESS(rv.ErrorCode(), rv.ErrorCode());
+    // These members aren't thread-safe. We're making sure that they're being
+    // released on the main thread here. Otherwise, they could be getting
+    // released by EncodingRunnable's destructor on the encoding thread
+    // (bug 916128).
+    mScriptContext = nullptr;
+    mCallback = nullptr;
 
     mEncoderThread->Shutdown();
     return rv.ErrorCode();
   }
 
   void SetMembers(void* aImgData, uint64_t aImgSize, const nsAutoString& aType)
   {
     mImgData = aImgData;
     mImgSize = aImgSize;
     mType = aType;
   }
 
+  void SetFailed()
+  {
+    mFailed = true;
+  }
+
 private:
   uint64_t mImgSize;
   nsAutoString mType;
   void* mImgData;
   nsCOMPtr<nsIScriptContext> mScriptContext;
   nsCOMPtr<nsIThread> mEncoderThread;
   nsRefPtr<FileCallback> mCallback;
+  bool mFailed;
 };
 
 NS_IMPL_ISUPPORTS1(EncodingCompleteEvent, nsIRunnable);
 
 class EncodingRunnable : public nsRunnable
 {
 public:
   NS_DECL_THREADSAFE_ISUPPORTS
@@ -86,17 +102,17 @@ public:
     , mEncoder(aEncoder)
     , mEncodingCompleteEvent(aEncodingCompleteEvent)
     , mFormat(aFormat)
     , mSize(aSize)
     , mUsingCustomOptions(aUsingCustomOptions)
   {}
   virtual ~EncodingRunnable() {}
 
-  NS_IMETHOD Run()
+  nsresult ProcessImageData(uint64_t* aImgSize, void** aImgData)
   {
     nsCOMPtr<nsIInputStream> stream;
     nsresult rv = ImageEncoder::ExtractDataInternal(mType,
                                                     mOptions,
                                                     mImageBuffer,
                                                     mFormat,
                                                     mSize,
                                                     nullptr,
@@ -112,28 +128,43 @@ public:
                                              mFormat,
                                              mSize,
                                              nullptr,
                                              getter_AddRefs(stream),
                                              mEncoder);
     }
     NS_ENSURE_SUCCESS(rv, rv);
 
-    uint64_t imgSize;
-    rv = stream->Available(&imgSize);
+    rv = stream->Available(aImgSize);
     NS_ENSURE_SUCCESS(rv, rv);
-    NS_ENSURE_TRUE(imgSize <= UINT32_MAX, NS_ERROR_FILE_TOO_BIG);
+    NS_ENSURE_TRUE(*aImgSize <= UINT32_MAX, NS_ERROR_FILE_TOO_BIG);
 
-    void* imgData = nullptr;
-    rv = NS_ReadInputStreamToBuffer(stream, &imgData, imgSize);
+    rv = NS_ReadInputStreamToBuffer(stream, aImgData, *aImgSize);
     NS_ENSURE_SUCCESS(rv, rv);
 
-    mEncodingCompleteEvent->SetMembers(imgData, imgSize, mType);
+    return rv;
+  }
+
+  NS_IMETHOD Run()
+  {
+    uint64_t imgSize;
+    void* imgData = nullptr;
+
+    nsresult rv = ProcessImageData(&imgSize, &imgData);
+    if (NS_FAILED(rv)) {
+      mEncodingCompleteEvent->SetFailed();
+    } else {
+      mEncodingCompleteEvent->SetMembers(imgData, imgSize, mType);
+    }
     rv = NS_DispatchToMainThread(mEncodingCompleteEvent, NS_DISPATCH_NORMAL);
-    NS_ENSURE_SUCCESS(rv, rv);
+    if (NS_FAILED(rv)) {
+      // Better to leak than to crash.
+      mEncodingCompleteEvent.forget();
+      return rv;
+    }
 
     return rv;
   }
 
 private:
   nsAutoString mType;
   nsAutoString mOptions;
   nsAutoArrayPtr<uint8_t> mImageBuffer;