Bug 1398556 - IPCBlobInputStream should call OnInputStreamReady() on the current thread if ::AsyncWait() is called without passing nsIEventTarget, r=smaug
☠☠ backed out by 68008e66e9df ☠ ☠
authorAndrea Marchesini <amarchesini@mozilla.com>
Wed, 13 Sep 2017 15:29:39 +0200
changeset 430183 0140b94180037fb8a02bdbb19760dc9f566c83e9
parent 430182 17637a9cb3db623a41e2dc29a0f56f9a2cefa35f
child 430184 b382ec54d164fde891d5f520fd15654d1d521ba9
push id7761
push userjlund@mozilla.com
push dateFri, 15 Sep 2017 00:19:52 +0000
treeherdermozilla-beta@c38455951db4 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerssmaug
bugs1398556
milestone57.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 1398556 - IPCBlobInputStream should call OnInputStreamReady() on the current thread if ::AsyncWait() is called without passing nsIEventTarget, r=smaug
dom/file/ipc/IPCBlobInputStream.cpp
dom/file/ipc/IPCBlobInputStreamChild.cpp
netwerk/base/nsIFileStreams.idl
--- a/dom/file/ipc/IPCBlobInputStream.cpp
+++ b/dom/file/ipc/IPCBlobInputStream.cpp
@@ -20,30 +20,34 @@ namespace dom {
 
 namespace {
 
 static NS_DEFINE_CID(kStreamTransportServiceCID, NS_STREAMTRANSPORTSERVICE_CID);
 
 class InputStreamCallbackRunnable final : public CancelableRunnable
 {
 public:
+  // Note that the execution can be synchronous in case the event target is
+  // null.
   static void
   Execute(nsIInputStreamCallback* aCallback,
           nsIEventTarget* aEventTarget,
           IPCBlobInputStream* aStream)
   {
+    MOZ_ASSERT(aCallback);
+
     RefPtr<InputStreamCallbackRunnable> runnable =
       new InputStreamCallbackRunnable(aCallback, aStream);
 
     nsCOMPtr<nsIEventTarget> target = aEventTarget;
-    if (!target) {
-      target = NS_GetCurrentThread();
+    if (aEventTarget) {
+      target->Dispatch(runnable, NS_DISPATCH_NORMAL);
+    } else {
+      runnable->Run();
     }
-
-    target->Dispatch(runnable, NS_DISPATCH_NORMAL);
   }
 
   NS_IMETHOD
   Run() override
   {
     mCallback->OnInputStreamReady(mStream);
     mCallback = nullptr;
     mStream = nullptr;
@@ -68,24 +72,23 @@ private:
 class FileMetadataCallbackRunnable final : public CancelableRunnable
 {
 public:
   static void
   Execute(nsIFileMetadataCallback* aCallback,
           nsIEventTarget* aEventTarget,
           IPCBlobInputStream* aStream)
   {
+    MOZ_ASSERT(aCallback);
+    MOZ_ASSERT(aEventTarget);
+
     RefPtr<FileMetadataCallbackRunnable> runnable =
       new FileMetadataCallbackRunnable(aCallback, aStream);
 
     nsCOMPtr<nsIEventTarget> target = aEventTarget;
-    if (!target) {
-      target = NS_GetCurrentThread();
-    }
-
     target->Dispatch(runnable, NS_DISPATCH_NORMAL);
   }
 
   NS_IMETHOD
   Run() override
   {
     mCallback->OnFileMetadataReady(mStream);
     mCallback = nullptr;
@@ -504,16 +507,18 @@ IPCBlobInputStream::OnInputStreamReady(n
   }
 
   nsCOMPtr<nsIInputStreamCallback> callback;
   callback.swap(mInputStreamCallback);
 
   nsCOMPtr<nsIEventTarget> callbackEventTarget;
   callbackEventTarget.swap(mInputStreamCallbackEventTarget);
  
+  // This must be the last operation because the execution of the callback can
+  // be synchronous.
   InputStreamCallbackRunnable::Execute(callback, callbackEventTarget, this);
   return NS_OK;
 }
 
 // nsIIPCSerializableInputStream
 
 void
 IPCBlobInputStream::Serialize(mozilla::ipc::InputStreamParams& aParams,
@@ -542,16 +547,23 @@ IPCBlobInputStream::ExpectedSerializedLe
 }
 
 // nsIAsyncFileMetadata
 
 NS_IMETHODIMP
 IPCBlobInputStream::AsyncWait(nsIFileMetadataCallback* aCallback,
                               nsIEventTarget* aEventTarget)
 {
+  MOZ_ASSERT(!!aCallback != !!aEventTarget);
+
+  // If we have the callback, we must have the event target.
+  if (NS_WARN_IF(!!aCallback != !!aEventTarget)) {
+    return NS_ERROR_FAILURE;
+  }
+
   // See IPCBlobInputStream.h for more information about this state machine.
 
   switch (mState) {
   // First call, we need to retrieve the stream from the parent actor.
   case eInit:
     MOZ_ASSERT(mActor);
 
     mFileMetadataCallback = aCallback;
--- a/dom/file/ipc/IPCBlobInputStreamChild.cpp
+++ b/dom/file/ipc/IPCBlobInputStreamChild.cpp
@@ -272,17 +272,17 @@ IPCBlobInputStreamChild::StreamNeeded(IP
   if (mState == eInactive) {
     return;
   }
 
   MOZ_ASSERT(mStreams.Contains(aStream));
 
   PendingOperation* opt = mPendingOperations.AppendElement();
   opt->mStream = aStream;
-  opt->mEventTarget = aEventTarget ? aEventTarget : NS_GetCurrentThread();
+  opt->mEventTarget = aEventTarget;
 
   if (mState == eActiveMigrating || mState == eInactiveMigrating) {
     // This operation will be continued when the migration is completed.
     return;
   }
 
   MOZ_ASSERT(mState == eActive);
 
@@ -311,17 +311,26 @@ IPCBlobInputStreamChild::RecvStreamReady
     pendingStream = mPendingOperations[0].mStream;
     eventTarget = mPendingOperations[0].mEventTarget;
 
     mPendingOperations.RemoveElementAt(0);
   }
 
   RefPtr<StreamReadyRunnable> runnable =
     new StreamReadyRunnable(pendingStream, stream);
-  eventTarget->Dispatch(runnable, NS_DISPATCH_NORMAL);
+
+  // If IPCBlobInputStream::AsyncWait() has been executed without passing an
+  // event target, we run the callback synchronous because any thread could be
+  // result to be the wrong one. See more in nsIAsyncInputStream::asyncWait
+  // documentation.
+  if (eventTarget) {
+    eventTarget->Dispatch(runnable, NS_DISPATCH_NORMAL);
+  } else {
+    runnable->Run();
+  }
 
   return IPC_OK();
 }
 
 void
 IPCBlobInputStreamChild::Migrated()
 {
   MutexAutoLock lock(mMutex);
--- a/netwerk/base/nsIFileStreams.idl
+++ b/netwerk/base/nsIFileStreams.idl
@@ -217,16 +217,23 @@ interface nsIFileMetadata : nsISupports
     [noscript] PRFileDescPtr getFileDescriptor();
 };
 
 [scriptable, uuid(de15b80b-29ba-4b7f-9220-a3d75b17ae8c)]
 interface nsIAsyncFileMetadata : nsIFileMetadata
 {
     /**
      * Asynchronously wait for the object to be ready.
+     *
+     * @param aCallback The callback will be used when the stream is ready to
+     *                  return File metadata. Use a nullptr to cancel a
+     *                  previous operation.
+     *
+     * @param aEventTarget The event target where aCallback will be executed.
+     *                     If aCallback is passed, aEventTarget cannot be null.
      */
     void asyncWait(in nsIFileMetadataCallback aCallback,
                    in nsIEventTarget aEventTarget);
 };
 
 /**
  * This is a companion interface for nsIAsyncFileMetadata::asyncWait.
  */