Bug 1168606 - Removed duplication between different BackgroundCursorChild::HandleResponse overloads. r=ttung,asuth
authorSimon Giesecke <sgiesecke@mozilla.com>
Tue, 05 Nov 2019 16:46:23 +0000
changeset 500662 61c29e7a541969295ba51ef4de6f5771a04cb987
parent 500661 c6d3254dcd8e3fc253b9620fb9417e6745a61f57
child 500663 bd872bd17560fb20663d4be58c3cb48eb0ce99e3
push id114166
push userapavel@mozilla.com
push dateThu, 07 Nov 2019 10:04:01 +0000
treeherdermozilla-inbound@d271c572a9bc [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersttung, asuth
bugs1168606
milestone72.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 1168606 - Removed duplication between different BackgroundCursorChild::HandleResponse overloads. r=ttung,asuth Differential Revision: https://phabricator.services.mozilla.com/D45674
dom/indexedDB/ActorsChild.cpp
dom/indexedDB/ActorsChild.h
--- a/dom/indexedDB/ActorsChild.cpp
+++ b/dom/indexedDB/ActorsChild.cpp
@@ -3558,16 +3558,34 @@ void BackgroundCursorChild::HandleRespon
   if (!mCursor) {
     nsCOMPtr<nsIRunnable> deleteRunnable = new DelayedActionRunnable(
         this, &BackgroundCursorChild::SendDeleteMeInternal);
     MOZ_ALWAYS_SUCCEEDS(this->GetActorEventTarget()->Dispatch(
         deleteRunnable.forget(), NS_DISPATCH_NORMAL));
   }
 }
 
+template <typename... Args>
+void BackgroundCursorChild::HandleIndividualCursorResponse(Args&&... aArgs) {
+  if (mCursor) {
+    if (mCursor->IsContinueCalled()) {
+      mCursor->Reset(std::forward<Args>(aArgs)...);
+    } else {
+      mCachedResponses.emplace_back(std::forward<Args>(aArgs)...);
+    }
+  } else {
+    // TODO: This looks particularly dangerous to me. Why do we need to
+    // have an extra newCursor of type RefPtr? Why can't we directly
+    // assign to mCursor? Why is mCursor not a RefPtr?
+    RefPtr<IDBCursor> newCursor =
+        IDBCursor::Create(this, std::forward<Args>(aArgs)...);
+    mCursor = newCursor;
+  }
+}
+
 template <typename T, typename Func>
 void BackgroundCursorChild::HandleMultipleCursorResponses(
     const nsTArray<T>& aResponses, const Func& aHandleRecord) {
   AssertIsOnOwningThread();
   MOZ_ASSERT(mRequest);
   MOZ_ASSERT(mTransaction);
   MOZ_ASSERT(!mStrongRequest);
   MOZ_ASSERT(!mStrongCursor);
@@ -3623,121 +3641,57 @@ void BackgroundCursorChild::HandleRespon
   AssertIsOnOwningThread();
   MOZ_ASSERT(mObjectStore);
 
   HandleMultipleCursorResponses(
       aResponses, [this](ObjectStoreCursorResponse& response) {
         // TODO: Maybe move the deserialization of the clone-read-info into the
         // cursor, so that it is only done for records actually accessed, which
         // might not be the case for all cached records.
-        auto cloneReadInfo =
-            PrepareCloneReadInfo(std::move(response.cloneInfo()));
-
-        // TODO: the structure of the rest of this function is the same for all
-        // cursor responses, and it can be unified with a template function,
-        // only the arguments to IDBCursor::Reset, IDBCursor::Create and the
-        // fields for the cached response differ.
-        RefPtr<IDBCursor> newCursor;
-
-        if (mCursor) {
-          if (mCursor->IsContinueCalled()) {
-            mCursor->Reset(std::move(response.key()), std::move(cloneReadInfo));
-          } else {
-            mCachedResponses.emplace_back(std::move(response.key()),
-                                          std::move(cloneReadInfo));
-          }
-        } else {
-          newCursor = IDBCursor::Create(this, std::move(response.key()),
-                                        std::move(cloneReadInfo));
-          mCursor = newCursor;
-        }
+        HandleIndividualCursorResponse(
+            std::move(response.key()),
+            PrepareCloneReadInfo(std::move(response.cloneInfo())));
       });
 }
 
 void BackgroundCursorChild::HandleResponse(
     const nsTArray<ObjectStoreKeyCursorResponse>& aResponses) {
   AssertIsOnOwningThread();
   MOZ_ASSERT(mObjectStore);
 
   HandleMultipleCursorResponses(
       aResponses, [this](ObjectStoreKeyCursorResponse& response) {
-        RefPtr<IDBCursor> newCursor;
-
-        if (mCursor) {
-          if (mCursor->IsContinueCalled()) {
-            mCursor->Reset(std::move(response.key()));
-          } else {
-            mCachedResponses.emplace_back(std::move(response.key()));
-          }
-        } else {
-          newCursor = IDBCursor::Create(this, std::move(response.key()));
-          mCursor = newCursor;
-        }
+        HandleIndividualCursorResponse(std::move(response.key()));
       });
 }
 
 void BackgroundCursorChild::HandleResponse(
     const nsTArray<IndexCursorResponse>& aResponses) {
   AssertIsOnOwningThread();
   MOZ_ASSERT(mIndex);
 
-  HandleMultipleCursorResponses(aResponses, [this](
-                                                IndexCursorResponse& response) {
-    auto cloneReadInfo = PrepareCloneReadInfo(std::move(response.cloneInfo()));
-
-    RefPtr<IDBCursor> newCursor;
-
-    if (mCursor) {
-      if (mCursor->IsContinueCalled()) {
-        mCursor->Reset(std::move(response.key()), std::move(response.sortKey()),
-                       std::move(response.objectKey()),
-                       std::move(cloneReadInfo));
-      } else {
-        mCachedResponses.emplace_back(
+  HandleMultipleCursorResponses(
+      aResponses, [this](IndexCursorResponse& response) {
+        HandleIndividualCursorResponse(
             std::move(response.key()), std::move(response.sortKey()),
-            std::move(response.objectKey()), std::move(cloneReadInfo));
-      }
-    } else {
-      // TODO: This looks particularly dangerous to me. Why do we need to
-      // have an extra newCursor of type RefPtr? Why can't we directly
-      // assign to mCursor? Why is mCursor not a RefPtr? (A similar
-      // construct exists in the other HandleResponse overloads).
-      newCursor = IDBCursor::Create(
-          this, std::move(response.key()), std::move(response.sortKey()),
-          std::move(response.objectKey()), std::move(cloneReadInfo));
-      mCursor = newCursor;
-    }
-  });
+            std::move(response.objectKey()),
+            PrepareCloneReadInfo(std::move(response.cloneInfo())));
+      });
 }
 
 void BackgroundCursorChild::HandleResponse(
     const nsTArray<IndexKeyCursorResponse>& aResponses) {
   AssertIsOnOwningThread();
   MOZ_ASSERT(mIndex);
 
   HandleMultipleCursorResponses(
       aResponses, [this](IndexKeyCursorResponse& response) {
-        RefPtr<IDBCursor> newCursor;
-
-        if (mCursor) {
-          if (mCursor->IsContinueCalled()) {
-            mCursor->Reset(std::move(response.key()),
-                           std::move(response.sortKey()),
-                           std::move(response.objectKey()));
-          } else {
-            mCachedResponses.emplace_back(std::move(response.key()),
-                                          std::move(response.sortKey()),
-                                          std::move(response.objectKey()));
-          }
-        } else {
-          newCursor = IDBCursor::Create(this, std::move(response.key()),
-                                        std::move(response.sortKey()),
-                                        std::move(response.objectKey()));
-          mCursor = newCursor;
-        }
+        HandleIndividualCursorResponse(std::move(response.key()),
+                                       std::move(response.sortKey()),
+                                       std::move(response.objectKey()));
       });
 }
 
 void BackgroundCursorChild::ActorDestroy(ActorDestroyReason aWhy) {
   AssertIsOnOwningThread();
   MOZ_ASSERT_IF(aWhy == Deletion, !mStrongRequest);
   MOZ_ASSERT_IF(aWhy == Deletion, !mStrongCursor);
 
--- a/dom/indexedDB/ActorsChild.h
+++ b/dom/indexedDB/ActorsChild.h
@@ -730,16 +730,19 @@ class BackgroundCursorChild final : publ
       SerializedStructuredCloneReadInfo&& aCloneInfo) const;
 
   template <typename T, typename Func>
   void HandleMultipleCursorResponses(const nsTArray<T>& aResponses,
                                      const Func& aHandleRecord);
 
   void HandleResponse(const nsTArray<IndexKeyCursorResponse>& aResponses);
 
+  template <typename... Args>
+  void HandleIndividualCursorResponse(Args&&... aArgs);
+
   // IPDL methods are only called by IPDL.
   virtual void ActorDestroy(ActorDestroyReason aWhy) override;
 
   virtual mozilla::ipc::IPCResult RecvResponse(
       const CursorResponse& aResponse) override;
 
  public:
   // Force callers to use SendContinueInternal.