Bug 1632128 - De-virtualize IDBRequest::SetResult(Callback). r=dom-workers-and-storage-reviewers,janv
authorSimon Giesecke <sgiesecke@mozilla.com>
Thu, 14 May 2020 09:42:18 +0000
changeset 593596 b1d098d0edcfa5028d8193db74e3c22dd32ce7ef
parent 593595 e4f2bb3f46359552fae16d0d836f4b9a27541d9a
child 593597 4c55bf108c9b69828419e969612bfe2356c34830
push id13186
push userffxbld-merge
push dateMon, 01 Jun 2020 09:52:46 +0000
treeherdermozilla-beta@3e7c70a1e4a1 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersdom-workers-and-storage-reviewers, janv
bugs1632128
milestone78.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 1632128 - De-virtualize IDBRequest::SetResult(Callback). r=dom-workers-and-storage-reviewers,janv Differential Revision: https://phabricator.services.mozilla.com/D74874
dom/indexedDB/ActorsChild.cpp
dom/indexedDB/IDBRequest.cpp
dom/indexedDB/IDBRequest.h
--- a/dom/indexedDB/ActorsChild.cpp
+++ b/dom/indexedDB/ActorsChild.cpp
@@ -189,17 +189,17 @@ class MOZ_STACK_CLASS AutoSetCurrentTran
 
     if (mThreadLocal) {
       // Reset old value.
       mThreadLocal->SetCurrentTransaction(mPreviousTransaction);
     }
   }
 };
 
-class MOZ_STACK_CLASS ResultHelper final : public IDBRequest::ResultCallback {
+class MOZ_STACK_CLASS ResultHelper final {
   const RefPtr<IDBRequest> mRequest;
   const AutoSetCurrentTransaction mAutoTransaction;
   const SafeRefPtr<IDBTransaction> mTransaction;
 
   mozilla::Variant<
       IDBDatabase*, IDBCursor*, IDBMutableFile*, StructuredCloneReadInfoChild*,
       nsTArray<StructuredCloneReadInfoChild>*, const Key*, const nsTArray<Key>*,
       const uint64_t*, const JS::Handle<JS::Value>*>
@@ -212,18 +212,18 @@ class MOZ_STACK_CLASS ResultHelper final
       : mRequest(aRequest),
         mAutoTransaction(aTransaction ? SomeRef(*aTransaction) : Nothing()),
         mTransaction(std::move(aTransaction)),
         mResult(aResult) {
     MOZ_ASSERT(aRequest);
     MOZ_ASSERT(aResult);
   }
 
-  virtual nsresult GetResult(JSContext* aCx,
-                             JS::MutableHandle<JS::Value> aResult) override {
+  nsresult operator()(JSContext* aCx,
+                      JS::MutableHandle<JS::Value> aResult) const {
     MOZ_ASSERT(aCx);
     MOZ_ASSERT(mRequest);
 
     return mResult.match(
         [aCx, aResult](auto* aPtr) { return GetResult(aCx, aPtr, aResult); });
   }
 
   void DispatchSuccessEvent(RefPtr<Event> aEvent = nullptr);
@@ -610,17 +610,17 @@ void ResultHelper::DispatchSuccessEvent(
   }
 
   if (!aEvent) {
     aEvent = CreateGenericEvent(mRequest, nsDependentString(kSuccessEventType),
                                 eDoesNotBubble, eNotCancelable);
   }
   MOZ_ASSERT(aEvent);
 
-  mRequest->SetResultCallback(this);
+  mRequest->SetResult(*this);
 
   if (mTransaction && mTransaction->IsInactive()) {
     mTransaction->TransitionToActive();
   }
 
   if (mTransaction) {
     IDB_LOG_MARK_CHILD_TRANSACTION_REQUEST(
         "Firing %s event", "%s", mTransaction->LoggingSerialNumber(),
--- a/dom/indexedDB/IDBRequest.cpp
+++ b/dom/indexedDB/IDBRequest.cpp
@@ -259,70 +259,16 @@ void IDBRequest::GetResult(JS::MutableHa
   if (!mHaveResultOrErrorCode) {
     aRv.Throw(NS_ERROR_DOM_INVALID_STATE_ERR);
     return;
   }
 
   aResult.set(mResultVal);
 }
 
-// XXX This function should be renamed, it doesn't set the callback, but uses
-// the callback to set the result. In addition, the ResultCallback class can be
-// removed, a functor can just be passed instead. The same should be done for
-// IDBFileRequest::SetResultCallback.
-void IDBRequest::SetResultCallback(ResultCallback* aCallback) {
-  AssertIsOnOwningThread();
-  MOZ_ASSERT(aCallback);
-  MOZ_ASSERT(!mHaveResultOrErrorCode);
-  MOZ_ASSERT(mResultVal.isUndefined());
-  MOZ_ASSERT(!mError);
-
-  // Already disconnected from the owner.
-  if (!GetOwnerGlobal()) {
-    SetError(NS_ERROR_DOM_INDEXEDDB_UNKNOWN_ERR);
-    return;
-  }
-
-  // See this global is still valid.
-  if (NS_WARN_IF(NS_FAILED(CheckCurrentGlobalCorrectness()))) {
-    SetError(NS_ERROR_DOM_INDEXEDDB_UNKNOWN_ERR);
-    return;
-  }
-
-  AutoJSAPI autoJS;
-  if (!autoJS.Init(GetOwnerGlobal())) {
-    IDB_WARNING("Failed to initialize AutoJSAPI!");
-    SetError(NS_ERROR_DOM_INDEXEDDB_UNKNOWN_ERR);
-    return;
-  }
-
-  JSContext* cx = autoJS.cx();
-
-  JS::Rooted<JS::Value> result(cx);
-  nsresult rv = aCallback->GetResult(cx, &result);
-  if (NS_WARN_IF(NS_FAILED(rv))) {
-    // This can only fail if the structured clone contains a mutable file
-    // and the child is not in the main thread and main process.
-    // In that case CreateAndWrapMutableFile() returns false which shows up
-    // as NS_ERROR_DOM_DATA_CLONE_ERR here.
-    MOZ_ASSERT(rv == NS_ERROR_DOM_DATA_CLONE_ERR);
-
-    // We are not setting a result or an error object here since we want to
-    // throw an exception when the 'result' property is being touched.
-    return;
-  }
-
-  mError = nullptr;
-
-  mResultVal = result;
-  mozilla::HoldJSObjects(this);
-
-  mHaveResultOrErrorCode = true;
-}
-
 DOMException* IDBRequest::GetError(ErrorResult& aRv) {
   AssertIsOnOwningThread();
 
   if (!mHaveResultOrErrorCode) {
     aRv.Throw(NS_ERROR_DOM_INVALID_STATE_ERR);
     return nullptr;
   }
 
--- a/dom/indexedDB/IDBRequest.h
+++ b/dom/indexedDB/IDBRequest.h
@@ -8,16 +8,17 @@
 #define mozilla_dom_idbrequest_h__
 
 #include "js/RootingAPI.h"
 #include "mozilla/Attributes.h"
 #include "mozilla/EventForwards.h"
 #include "mozilla/dom/IDBRequestBinding.h"
 #include "mozilla/DOMEventTargetHelper.h"
 #include "nsCycleCollectionParticipant.h"
+#include "ReportInternalError.h"
 #include "SafeRefPtr.h"
 
 #define PRIVATE_IDBREQUEST_IID                       \
   {                                                  \
     0xe68901e5, 0x1d50, 0x4ee9, {                    \
       0xaf, 0x49, 0x90, 0x99, 0x4a, 0xff, 0xc8, 0x39 \
     }                                                \
   }
@@ -58,18 +59,16 @@ class IDBRequest : public DOMEventTarget
   nsString mFilename;
   uint64_t mLoggingSerialNumber;
   nsresult mErrorCode;
   uint32_t mLineNo;
   uint32_t mColumn;
   bool mHaveResultOrErrorCode;
 
  public:
-  class ResultCallback;
-
   [[nodiscard]] static RefPtr<IDBRequest> Create(
       JSContext* aCx, IDBDatabase* aDatabase,
       SafeRefPtr<IDBTransaction> aTransaction);
 
   [[nodiscard]] static RefPtr<IDBRequest> Create(
       JSContext* aCx, IDBObjectStore* aSource, IDBDatabase* aDatabase,
       SafeRefPtr<IDBTransaction> aTransaction);
 
@@ -85,17 +84,65 @@ class IDBRequest : public DOMEventTarget
   // EventTarget
   void GetEventTargetParent(EventChainPreVisitor& aVisitor) override;
 
   void GetSource(
       Nullable<OwningIDBObjectStoreOrIDBIndexOrIDBCursor>& aSource) const;
 
   void Reset();
 
-  void SetResultCallback(ResultCallback* aCallback);
+  template <typename ResultCallback>
+  void SetResult(const ResultCallback& aCallback) {
+    AssertIsOnOwningThread();
+    MOZ_ASSERT(!mHaveResultOrErrorCode);
+    MOZ_ASSERT(mResultVal.isUndefined());
+    MOZ_ASSERT(!mError);
+
+    // Already disconnected from the owner.
+    if (!GetOwnerGlobal()) {
+      SetError(NS_ERROR_DOM_INDEXEDDB_UNKNOWN_ERR);
+      return;
+    }
+
+    // See this global is still valid.
+    if (NS_WARN_IF(NS_FAILED(CheckCurrentGlobalCorrectness()))) {
+      SetError(NS_ERROR_DOM_INDEXEDDB_UNKNOWN_ERR);
+      return;
+    }
+
+    AutoJSAPI autoJS;
+    if (!autoJS.Init(GetOwnerGlobal())) {
+      IDB_WARNING("Failed to initialize AutoJSAPI!");
+      SetError(NS_ERROR_DOM_INDEXEDDB_UNKNOWN_ERR);
+      return;
+    }
+
+    JSContext* cx = autoJS.cx();
+
+    JS::Rooted<JS::Value> result(cx);
+    nsresult rv = aCallback(cx, &result);
+    if (NS_WARN_IF(NS_FAILED(rv))) {
+      // This can only fail if the structured clone contains a mutable file
+      // and the child is not in the main thread and main process.
+      // In that case CreateAndWrapMutableFile() returns false which shows up
+      // as NS_ERROR_DOM_DATA_CLONE_ERR here.
+      MOZ_ASSERT(rv == NS_ERROR_DOM_DATA_CLONE_ERR);
+
+      // We are not setting a result or an error object here since we want to
+      // throw an exception when the 'result' property is being touched.
+      return;
+    }
+
+    mError = nullptr;
+
+    mResultVal = result;
+    mozilla::HoldJSObjects(this);
+
+    mHaveResultOrErrorCode = true;
+  }
 
   void SetError(nsresult aRv);
 
   nsresult GetErrorCode() const
 #ifdef DEBUG
       ;
 #else
   {
@@ -183,25 +230,16 @@ class IDBRequest : public DOMEventTarget
   explicit IDBRequest(nsIGlobalObject* aGlobal);
   ~IDBRequest();
 
   void InitMembers();
 
   void ConstructResult();
 };
 
-class NS_NO_VTABLE IDBRequest::ResultCallback {
- public:
-  virtual nsresult GetResult(JSContext* aCx,
-                             JS::MutableHandle<JS::Value> aResult) = 0;
-
- protected:
-  ResultCallback() = default;
-};
-
 class IDBOpenDBRequest final : public IDBRequest {
   // Only touched on the owning thread.
   SafeRefPtr<IDBFactory> mFactory;
 
   RefPtr<StrongWorkerRef> mWorkerRef;
 
   const bool mFileHandleDisabled;
   bool mIncreasedActiveDatabaseCount;