Bug 1535384 part 7. Get rid of MOZ_CAN_RUN_SCRIPT_BOUNDARY for filesystem callbacks. r=mccr8

I'm a bit torn between the const-member-and-MOZ_KnownLive approach
here and just taking extra stack refs...

Differential Revision: https://phabricator.services.mozilla.com/D23778

--HG--
extra : moz-landing-system : lando

dom/filesystem/compat/CallbackRunnables.cpp

@@ -33,7 +33,9 @@ EntryCallbackRunnable::EntryCallbackRunnable(FileSystemEntryCallback* aCallback,
 
 NS_IMETHODIMP
 EntryCallbackRunnable::Run() {
-  mCallback->Call(*mEntry);
+  // Both of our strongly held members never change (they're const), so we can
+  // use MOZ_KnownLive on them.
+  MOZ_KnownLive(mCallback)->Call(MOZ_KnownLive(*mEntry));
   return NS_OK;
 }
 
@@ -52,7 +54,8 @@ ErrorCallbackRunnable::ErrorCallbackRunnable(nsIGlobalObject* aGlobalObject,
 NS_IMETHODIMP
 ErrorCallbackRunnable::Run() {
   RefPtr<DOMException> exception = DOMException::Create(mError);
-  mCallback->Call(*exception);
+  // mCallback never changes (it's const) so MOZ_KnownLive is ok.
+  MOZ_KnownLive(mCallback)->Call(*exception);
   return NS_OK;
 }
 
@@ -65,7 +68,8 @@ EmptyEntriesCallbackRunnable::EmptyEntriesCallbackRunnable(
 NS_IMETHODIMP
 EmptyEntriesCallbackRunnable::Run() {
   Sequence<OwningNonNull<FileSystemEntry>> sequence;
-  mCallback->Call(sequence);
+  // mCallback never changes (it's const), so MOZ_KnownLive is ok.
+  MOZ_KnownLive(mCallback)->Call(sequence);
   return NS_OK;
 }
 
@@ -188,7 +192,8 @@ void GetEntryHelper::CompleteOperation(JSObject* aObj) {
 
     RefPtr<FileSystemFileEntry> entry = new FileSystemFileEntry(
         mParentEntry->GetParentObject(), file, mParentEntry, mFileSystem);
-    mSuccessCallback->Call(*entry);
+    // mSuccessCallback never changes (it's const), so MOZ_KnownLive is ok.
+    MOZ_KnownLive(mSuccessCallback)->Call(*entry);
     return;
   }
 
@@ -202,7 +207,8 @@ void GetEntryHelper::CompleteOperation(JSObject* aObj) {
 
   RefPtr<FileSystemDirectoryEntry> entry = new FileSystemDirectoryEntry(
       mParentEntry->GetParentObject(), directory, mParentEntry, mFileSystem);
-  mSuccessCallback->Call(*entry);
+  // mSuccessCallback never changes (it's const), so MOZ_KnownLive is ok.
+  MOZ_KnownLive(mSuccessCallback)->Call(*entry);
 }
 
 void GetEntryHelper::ContinueRunning(JSObject* aObj) {

dom/filesystem/compat/CallbackRunnables.h

@@ -23,12 +23,13 @@ class EntryCallbackRunnable final : public Runnable {
   EntryCallbackRunnable(FileSystemEntryCallback* aCallback,
                         FileSystemEntry* aEntry);
 
-  NS_IMETHOD
-  Run() override;
+  // MOZ_CAN_RUN_SCRIPT_BOUNDARY until Runnable::Run is MOZ_CAN_RUN_SCRIPT.  See
+  // bug 1535398.
+  MOZ_CAN_RUN_SCRIPT_BOUNDARY NS_IMETHOD Run() override;
 
  private:
-  RefPtr<FileSystemEntryCallback> mCallback;
-  RefPtr<FileSystemEntry> mEntry;
+  const RefPtr<FileSystemEntryCallback> mCallback;
+  const RefPtr<FileSystemEntry> mEntry;
 };
 
 class ErrorCallbackRunnable final : public Runnable {
@@ -36,12 +37,13 @@ class ErrorCallbackRunnable final : public Runnable {
   ErrorCallbackRunnable(nsIGlobalObject* aGlobalObject,
                         ErrorCallback* aCallback, nsresult aError);
 
-  NS_IMETHOD
-  Run() override;
+  // MOZ_CAN_RUN_SCRIPT_BOUNDARY until Runnable::Run is MOZ_CAN_RUN_SCRIPT.  See
+  // bug 1535398.
+  MOZ_CAN_RUN_SCRIPT_BOUNDARY NS_IMETHOD Run() override;
 
  private:
   nsCOMPtr<nsIGlobalObject> mGlobal;
-  RefPtr<ErrorCallback> mCallback;
+  const RefPtr<ErrorCallback> mCallback;
   nsresult mError;
 };
 
@@ -49,11 +51,12 @@ class EmptyEntriesCallbackRunnable final : public Runnable {
  public:
   explicit EmptyEntriesCallbackRunnable(FileSystemEntriesCallback* aCallback);
 
-  NS_IMETHOD
-  Run() override;
+  // MOZ_CAN_RUN_SCRIPT_BOUNDARY until Runnable::Run is MOZ_CAN_RUN_SCRIPT.  See
+  // bug 1535398.
+  MOZ_CAN_RUN_SCRIPT_BOUNDARY NS_IMETHOD Run() override;
 
  private:
-  RefPtr<FileSystemEntriesCallback> mCallback;
+  const RefPtr<FileSystemEntriesCallback> mCallback;
 };
 
 class GetEntryHelper final : public PromiseNativeHandler {
@@ -68,6 +71,7 @@ class GetEntryHelper final : public PromiseNativeHandler {
 
   void Run();
 
+  MOZ_CAN_RUN_SCRIPT
   virtual void ResolvedCallback(JSContext* aCx,
                                 JS::Handle<JS::Value> aValue) override;
 
@@ -81,14 +85,14 @@ class GetEntryHelper final : public PromiseNativeHandler {
 
   void ContinueRunning(JSObject* aObj);
 
-  void CompleteOperation(JSObject* aObj);
+  MOZ_CAN_RUN_SCRIPT void CompleteOperation(JSObject* aObj);
 
   RefPtr<FileSystemDirectoryEntry> mParentEntry;
   RefPtr<Directory> mDirectory;
   nsTArray<nsString> mParts;
   RefPtr<FileSystem> mFileSystem;
 
-  RefPtr<FileSystemEntryCallback> mSuccessCallback;
+  const RefPtr<FileSystemEntryCallback> mSuccessCallback;
   RefPtr<ErrorCallback> mErrorCallback;
 
   FileSystemDirectoryEntry::GetInternalType mType;

dom/filesystem/compat/FileSystemDirectoryReader.cpp

@@ -36,6 +36,7 @@ class PromiseHandler final : public PromiseNativeHandler {
     MOZ_ASSERT(aSuccessCallback);
   }
 
+  MOZ_CAN_RUN_SCRIPT
   virtual void ResolvedCallback(JSContext* aCx,
                                 JS::Handle<JS::Value> aValue) override {
     if (NS_WARN_IF(!aValue.isObject())) {
@@ -86,7 +87,8 @@ class PromiseHandler final : public PromiseNativeHandler {
       sequence[i] = entry;
     }
 
-    mSuccessCallback->Call(sequence);
+    // mSuccessCallback never changes (it's const), so MOZ_KnownLive is ok.
+    MOZ_KnownLive(mSuccessCallback)->Call(sequence);
   }
 
   virtual void RejectedCallback(JSContext* aCx,
@@ -106,7 +108,7 @@ class PromiseHandler final : public PromiseNativeHandler {
 
   RefPtr<FileSystemDirectoryEntry> mParentEntry;
   RefPtr<FileSystem> mFileSystem;
-  RefPtr<FileSystemEntriesCallback> mSuccessCallback;
+  const RefPtr<FileSystemEntriesCallback> mSuccessCallback;
   RefPtr<ErrorCallback> mErrorCallback;
 };
 

dom/filesystem/compat/FileSystemFileEntry.cpp

@@ -24,19 +24,21 @@ class FileCallbackRunnable final : public Runnable {
     MOZ_ASSERT(aFile);
   }
 
-  NS_IMETHOD
-  Run() override {
+  // MOZ_CAN_RUN_SCRIPT_BOUNDARY until Runnable::Run is MOZ_CAN_RUN_SCRIPT.  See
+  // bug 1535398.
+  MOZ_CAN_RUN_SCRIPT_BOUNDARY NS_IMETHOD Run() override {
     // Here we clone the File object.
 
     RefPtr<File> file = File::Create(mFile->GetParentObject(), mFile->Impl());
     MOZ_ASSERT(file);
 
-    mCallback->Call(*file);
+    // mCallback never changes (it's const) so MOZ_KnownLive is ok.
+    MOZ_KnownLive(mCallback)->Call(*file);
     return NS_OK;
   }
 
  private:
-  RefPtr<FileCallback> mCallback;
+  const RefPtr<FileCallback> mCallback;
   RefPtr<File> mFile;
 };
 

dom/filesystem/compat/FileSystemRootDirectoryReader.cpp

@@ -24,8 +24,9 @@ class EntriesCallbackRunnable final : public Runnable {
     MOZ_ASSERT(aCallback);
   }
 
-  NS_IMETHOD
-  Run() override {
+  // MOZ_CAN_RUN_SCRIPT_BOUNDARY until Runnable::Run is MOZ_CAN_RUN_SCRIPT.  See
+  // bug 1535398.
+  MOZ_CAN_RUN_SCRIPT_BOUNDARY NS_IMETHOD Run() override {
     Sequence<OwningNonNull<FileSystemEntry>> entries;
     for (uint32_t i = 0; i < mEntries.Length(); ++i) {
       if (!entries.AppendElement(mEntries[i].forget(), fallible)) {
@@ -33,12 +34,13 @@ class EntriesCallbackRunnable final : public Runnable {
       }
     }
 
-    mCallback->Call(entries);
+    // mCallback never changes (it's const), so MOZ_KnownLive is ok.
+    MOZ_KnownLive(mCallback)->Call(entries);
     return NS_OK;
   }
 
  private:
-  RefPtr<FileSystemEntriesCallback> mCallback;
+  const RefPtr<FileSystemEntriesCallback> mCallback;
   Sequence<RefPtr<FileSystemEntry>> mEntries;
 };
 

dom/promise/Promise.cpp

@@ -316,6 +316,7 @@ void Promise::MaybeReject(JSContext* aCx, JS::Handle<JS::Value> aValue) {
 
 enum class NativeHandlerTask : int32_t { Resolve, Reject };
 
+MOZ_CAN_RUN_SCRIPT
 static bool NativeHandlerCallback(JSContext* aCx, unsigned aArgc,
                                   JS::Value* aVp) {
   JS::CallArgs args = CallArgsFromVp(aArgc, aVp);
@@ -334,10 +335,12 @@ static bool NativeHandlerCallback(JSContext* aCx, unsigned aArgc,
   NativeHandlerTask task = static_cast<NativeHandlerTask>(v.toInt32());
 
   if (task == NativeHandlerTask::Resolve) {
-    handler->ResolvedCallback(aCx, args.get(0));
+    // handler is kept alive by "obj" on the stack.
+    MOZ_KnownLive(handler)->ResolvedCallback(aCx, args.get(0));
   } else {
     MOZ_ASSERT(task == NativeHandlerTask::Reject);
-    handler->RejectedCallback(aCx, args.get(0));
+    // handler is kept alive by "obj" on the stack.
+    MOZ_KnownLive(handler)->RejectedCallback(aCx, args.get(0));
   }
 
   return true;
@@ -377,14 +380,18 @@ class PromiseNativeHandlerShim final : public PromiseNativeHandler {
     MOZ_ASSERT(mInner);
   }
 
+  MOZ_CAN_RUN_SCRIPT
   void ResolvedCallback(JSContext* aCx, JS::Handle<JS::Value> aValue) override {
-    mInner->ResolvedCallback(aCx, aValue);
-    mInner = nullptr;
+    RefPtr<PromiseNativeHandler> inner = mInner.forget();
+    inner->ResolvedCallback(aCx, aValue);
+    MOZ_ASSERT(!mInner);
   }
 
+  MOZ_CAN_RUN_SCRIPT
   void RejectedCallback(JSContext* aCx, JS::Handle<JS::Value> aValue) override {
-    mInner->RejectedCallback(aCx, aValue);
-    mInner = nullptr;
+    RefPtr<PromiseNativeHandler> inner = mInner.forget();
+    inner->RejectedCallback(aCx, aValue);
+    MOZ_ASSERT(!mInner);
   }
 
   bool WrapObject(JSContext* aCx, JS::Handle<JSObject*> aGivenProto,

dom/promise/PromiseNativeHandler.h

@@ -23,9 +23,11 @@ class PromiseNativeHandler : public nsISupports {
   virtual ~PromiseNativeHandler() {}
 
  public:
+  MOZ_CAN_RUN_SCRIPT
   virtual void ResolvedCallback(JSContext* aCx,
                                 JS::Handle<JS::Value> aValue) = 0;
 
+  MOZ_CAN_RUN_SCRIPT
   virtual void RejectedCallback(JSContext* aCx,
                                 JS::Handle<JS::Value> aValue) = 0;
 };

dom/webidl/FileSystem.webidl

@@ -11,10 +11,8 @@ dictionary FileSystemFlags {
     boolean exclusive = false;
 };
 
-[MOZ_CAN_RUN_SCRIPT_BOUNDARY]
 callback FileSystemEntryCallback = void (FileSystemEntry entry);
 
-[MOZ_CAN_RUN_SCRIPT_BOUNDARY]
 callback ErrorCallback = void (DOMException err);
 
 interface FileSystem {

dom/webidl/FileSystemDirectoryReader.webidl

@@ -6,7 +6,6 @@
  * https://wicg.github.io/entries-api/#idl-index
  */
 
-[MOZ_CAN_RUN_SCRIPT_BOUNDARY]
 callback FileSystemEntriesCallback = void (sequence<FileSystemEntry> entries);
 
 interface FileSystemDirectoryReader {

dom/webidl/FileSystemFileEntry.webidl

@@ -6,7 +6,6 @@
  * https://wicg.github.io/entries-api/#idl-index
  */
 
-[MOZ_CAN_RUN_SCRIPT_BOUNDARY]
 callback FileCallback = void (File file);
 
 interface FileSystemFileEntry : FileSystemEntry {