Backed out changeset 6babd3b956aa (bug 1502403) for wpt leak at mozilla::dom::FileReader::ReadFileContent
authorDaniel Varga <dvarga@mozilla.com>
Tue, 30 Oct 2018 12:50:37 +0200
changeset 443474 f2aee8014db6194ee85ec6043263389698a4b1d4
parent 443473 131bc0e561970979dec138a2d65d5b7b4d3d995a
child 443475 9ccb4ba13d95c8e7fadf64479771326ee1fb37b6
push id109384
push userdvarga@mozilla.com
push dateTue, 30 Oct 2018 10:51:09 +0000
treeherdermozilla-inbound@f2aee8014db6 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
bugs1502403
milestone65.0a1
backs out6babd3b956aa5dbbf3d93208c48efc593cec0ce6
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
Backed out changeset 6babd3b956aa (bug 1502403) for wpt leak at mozilla::dom::FileReader::ReadFileContent
dom/file/FileReader.cpp
dom/file/FileReader.h
dom/file/tests/common_fileReader.js
testing/web-platform/meta/FileAPI/reading-data-section/filereader_abort.html.ini
--- a/dom/file/FileReader.cpp
+++ b/dom/file/FileReader.cpp
@@ -83,43 +83,16 @@ public:
   {}
 
   ~FileReaderDecreaseBusyCounter()
   {
     mFileReader->DecreaseBusyCounter();
   }
 };
 
-class FileReader::AsyncWaitRunnable final : public CancelableRunnable
-{
-public:
-  explicit AsyncWaitRunnable(FileReader* aReader)
-    : CancelableRunnable("FileReader::AsyncWaitRunnable")
-    , mReader(aReader)
-  {}
-
-  NS_IMETHOD
-  Run() override
-  {
-    if (mReader) {
-      mReader->InitialAsyncWait();
-    }
-    return NS_OK;
-  }
-
-  void
-  Abort()
-  {
-    mReader = nullptr;
-  }
-
-public:
-  RefPtr<FileReader> mReader;
-};
-
 void
 FileReader::RootResultArrayBuffer()
 {
   mozilla::HoldJSObjects(this);
 }
 
 //FileReader constructors/initializers
 
@@ -468,39 +441,24 @@ FileReader::ReadFileContent(Blob& aBlob,
 
     if (!mFileData) {
       NS_WARNING("Preallocation failed for ReadFileData");
       aRv.Throw(NS_ERROR_OUT_OF_MEMORY);
       return;
     }
   }
 
-  mAsyncWaitRunnable = new AsyncWaitRunnable(this);
-  aRv = NS_DispatchToCurrentThread(mAsyncWaitRunnable);
+  aRv = DoAsyncWait();
   if (NS_WARN_IF(aRv.Failed())) {
     FreeFileData();
     return;
   }
 
   //FileReader should be in loading state here
   mReadyState = LOADING;
-}
-
-void
-FileReader::InitialAsyncWait()
-{
-  mAsyncWaitRunnable = nullptr;
-
-  nsresult rv = DoAsyncWait();
-  if (NS_WARN_IF(NS_FAILED(rv))) {
-    mReadyState = EMPTY;
-    FreeFileData();
-    return;
-  }
-
   DispatchProgressEvent(NS_LITERAL_STRING(LOADSTART_STR));
 }
 
 nsresult
 FileReader::GetAsText(Blob *aBlob,
                       const nsACString &aCharset,
                       const char *aFileData,
                       uint32_t aDataLen,
@@ -811,43 +769,26 @@ FileReader::Abort()
   if (mReadyState == EMPTY || mReadyState == DONE) {
     return;
   }
 
   MOZ_ASSERT(mReadyState == LOADING);
 
   ClearProgressEventTimer();
 
-  if (mAsyncWaitRunnable) {
-    mAsyncWaitRunnable->Abort();
-    mAsyncWaitRunnable = nullptr;
-  }
-
   mReadyState = DONE;
 
   // XXX The spec doesn't say this
   mError = DOMException::Create(NS_ERROR_DOM_ABORT_ERR);
 
   // Revert status and result attributes
   SetDOMStringToNull(mResult);
   mResultArrayBuffer = nullptr;
 
-  if (mAsyncStream) {
-    if (mBusyCount) {
-      // This will abort any pending reading. See nsIAsyncInputStream.idl.
-      mAsyncStream->AsyncWait(/* callback */ nullptr,
-                              /* aFlags*/ 0,
-                              /* aRequestedCount */ 0,
-                              mTarget);
-      DecreaseBusyCounter();
-      MOZ_ASSERT(mBusyCount == 0);
-    }
-    mAsyncStream = nullptr;
-  }
-
+  mAsyncStream = nullptr;
   mBlob = nullptr;
 
   //Clean up memory buffer
   FreeFileData();
 
   // Dispatch the events
   DispatchProgressEvent(NS_LITERAL_STRING(ABORT_STR));
   DispatchProgressEvent(NS_LITERAL_STRING(LOADEND_STR));
@@ -885,21 +826,16 @@ FileReader::DecreaseBusyCounter()
   }
 }
 
 void
 FileReader::Shutdown()
 {
   mReadyState = DONE;
 
-  if (mAsyncWaitRunnable) {
-    mAsyncWaitRunnable->Abort();
-    mAsyncWaitRunnable = nullptr;
-  }
-
   if (mAsyncStream) {
     mAsyncStream->Close();
     mAsyncStream = nullptr;
   }
 
   FreeFileData();
   mResultArrayBuffer = nullptr;
 
--- a/dom/file/FileReader.h
+++ b/dom/file/FileReader.h
@@ -123,18 +123,16 @@ public:
     FILE_AS_BINARY,
     FILE_AS_TEXT,
     FILE_AS_DATAURL
   };
 
   eDataFormat DataFormat() const { return mDataFormat; }
   const nsString& Result() const { return mResult; }
 
-  void InitialAsyncWait();
-
 private:
   virtual ~FileReader();
 
   // This must be in sync with dom/webidl/FileReader.webidl
   enum eReadyState {
     EMPTY = 0,
     LOADING = 1,
     DONE = 2
@@ -204,20 +202,16 @@ private:
   // This is set if FileReader is created on workers, but it is null if the
   // worker is shutting down. The null value is checked in ReadFileContent()
   // before starting any reading.
   RefPtr<WeakWorkerRef> mWeakWorkerRef;
 
   // This value is set when the reading starts in order to keep the worker alive
   // during the process.
   RefPtr<StrongWorkerRef> mStrongWorkerRef;
-
-  // Runnable to start the reading asynchronous.
-  class AsyncWaitRunnable;
-  RefPtr<AsyncWaitRunnable> mAsyncWaitRunnable;
 };
 
 NS_DEFINE_STATIC_IID_ACCESSOR(FileReader, FILEREADER_ID)
 
 } // dom namespace
 } // mozilla namespace
 
 #endif // mozilla_dom_FileReader_h
--- a/dom/file/tests/common_fileReader.js
+++ b/dom/file/tests/common_fileReader.js
@@ -252,17 +252,17 @@ function test_readAsText(blob, text) {
 
     r.addEventListener("load", () => { onloadHasRun = true });
     r.addEventListener("loadstart", () => { onloadStartHasRun = true });
 
     r.readAsText(blob);
 
     is(r.readyState, FileReader.LOADING, "correct loading text readyState");
     is(onloadHasRun, false, "text loading must be async");
-    is(onloadStartHasRun, false, "text loadstart should fire async");
+    is(onloadStartHasRun, true, "text loadstart should fire sync");
   });
 }
 
 function test_readAsBinaryString(blob, text) {
   return new Promise(resolve => {
     let onloadHasRun = false;
     let onloadStartHasRun = false;
 
@@ -275,17 +275,17 @@ function test_readAsBinaryString(blob, t
     r.readAsBinaryString(blob);
 
     r.onload = event => {
       loadEventHandler_string(event, resolve, r, text, text.length, "readAsBinaryString");
     }
 
     is(r.readyState, FileReader.LOADING, "correct loading binary readyState");
     is(onloadHasRun, false, "binary loading must be async");
-    is(onloadStartHasRun, false, "binary loadstart should fire async");
+    is(onloadStartHasRun, true, "binary loadstart should fire sync");
   });
 }
 
 function test_readAsArrayBuffer(blob, text) {
   return new Promise(resolve => {
     let onloadHasRun = false;
     let onloadStartHasRun = false;
 
@@ -298,17 +298,17 @@ function test_readAsArrayBuffer(blob, te
     r.readAsArrayBuffer(blob);
 
     r.onload = event => {
       loadEventHandler_arrayBuffer(event, resolve, r, text, "readAsArrayBuffer");
     }
 
     is(r.readyState, FileReader.LOADING, "correct loading arrayBuffer readyState");
     is(onloadHasRun, false, "arrayBuffer loading must be async");
-    is(onloadStartHasRun, false, "arrayBuffer loadstart should fire sync");
+    is(onloadStartHasRun, true, "arrayBuffer loadstart should fire sync");
   });
 }
 
 // Test a variety of encodings, and make sure they work properly
 function test_readAsTextWithEncoding(blob, text, length, charset) {
   return new Promise(resolve => {
     let r = new FileReader();
     r.onload = event => {
new file mode 100644
--- /dev/null
+++ b/testing/web-platform/meta/FileAPI/reading-data-section/filereader_abort.html.ini
@@ -0,0 +1,4 @@
+[filereader_abort.html]
+  [Aborting after read]
+    expected: FAIL
+