Bug 1168606 - Extract functions for accessing sort key into Cursor. r=ytausky,asuth
authorSimon Giesecke <sgiesecke@mozilla.com>
Tue, 05 Nov 2019 16:46:03 +0000
changeset 500622 9eeb9fd126bc068a35cba011ff06e25f1908ab01
parent 500621 1abd3e7f9df46911581ba653270bc8a8864374d8
child 500623 c6d3254dcd8e3fc253b9620fb9417e6745a61f57
push id36768
push usershindli@mozilla.com
push dateTue, 05 Nov 2019 22:07:34 +0000
treeherdermozilla-central@e96c1ca93d25 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersytausky, 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 - Extract functions for accessing sort key into Cursor. r=ytausky,asuth Depends on D44443 Differential Revision: https://phabricator.services.mozilla.com/D45308
dom/indexedDB/ActorsParent.cpp
--- a/dom/indexedDB/ActorsParent.cpp
+++ b/dom/indexedDB/ActorsParent.cpp
@@ -7678,16 +7678,19 @@ class Cursor final : public PBackgroundI
   mozilla::ipc::IPCResult RecvContinue(
       const CursorRequestParams& aParams, const Key& aCurrentKey,
       const Key& aCurrentObjectStoreKey) override;
 
   bool IsLocaleAware() const { return !mLocale.IsEmpty(); }
 
   void SetOptionalKeyRange(const Maybe<SerializedKeyRange>& aOptionalKeyRange,
                            bool* aOpen);
+
+  const Key& GetSortKey() const;
+  Key* GetModifiableSortKey();
 };
 
 class Cursor::CursorOpBase : public TransactionDatabaseOperationBase {
  protected:
   RefPtr<Cursor> mCursor;
   nsTArray<FallibleTArray<StructuredCloneFile>> mFiles;
 
   CursorResponse mResponse;
@@ -15130,17 +15133,17 @@ bool Cursor::VerifyRequestParams(const C
 #endif
 
   if (NS_WARN_IF(mObjectStoreMetadata->mDeleted) ||
       (mIndexMetadata && NS_WARN_IF(mIndexMetadata->mDeleted))) {
     ASSERT_UNLESS_FUZZING();
     return false;
   }
 
-  const Key& sortKey = IsLocaleAware() ? mLocaleAwarePosition : mPosition;
+  const Key& sortKey = GetSortKey();
 
   switch (aParams.type()) {
     case CursorRequestParams::TContinueParams: {
       const Key& key = aParams.get_ContinueParams().key();
       if (!key.IsUnset()) {
         switch (mDirection) {
           case IDBCursor::NEXT:
           case IDBCursor::NEXT_UNIQUE:
@@ -15417,16 +15420,22 @@ mozilla::ipc::IPCResult Cursor::RecvCont
   }
 
   continueOp->DispatchToConnectionPool();
   mCurrentlyRunningOp = continueOp;
 
   return IPC_OK();
 }
 
+const Key& Cursor::GetSortKey() const {
+  return IsLocaleAware() ? mLocaleAwarePosition : mPosition;
+}
+
+Key* Cursor::GetModifiableSortKey() { return &const_cast<Key&>(GetSortKey()); }
+
 /*******************************************************************************
  * FileManager
  ******************************************************************************/
 
 FileManager::FileManager(PersistenceType aPersistenceType,
                          const nsACString& aGroup, const nsACString& aOrigin,
                          const nsAString& aDatabaseName, bool aEnforcingQuota)
     : mPersistenceType(aPersistenceType),
@@ -25753,18 +25762,20 @@ void Cursor::CursorOpBase::Cleanup() {
 #endif
 
   TransactionDatabaseOperationBase::Cleanup();
 }
 
 nsresult Cursor::CursorOpBase::PopulateResponseFromStatement(
     mozIStorageStatement* const aStmt, const bool aInitializeResponse) {
   Transaction()->AssertIsOnConnectionThread();
-  MOZ_ASSERT(aInitializeResponse ==
-             (mResponse.type() == CursorResponse::T__None));
+  MOZ_ASSERT_IF(aInitializeResponse,
+                mResponse.type() == CursorResponse::T__None);
+  MOZ_ASSERT_IF(!aInitializeResponse,
+                mResponse.type() != CursorResponse::T__None);
   MOZ_ASSERT_IF(
       mFiles.IsEmpty() &&
           (mResponse.type() ==
                CursorResponse::TArrayOfObjectStoreCursorResponse ||
            mResponse.type() == CursorResponse::TArrayOfIndexCursorResponse),
       aInitializeResponse);
 
   nsresult rv = mCursor->mPosition.SetFromStatement(aStmt, 0);
@@ -25901,20 +25912,17 @@ nsresult Cursor::CursorOpBase::PopulateR
 nsresult Cursor::CursorOpBase::PopulateExtraResponses(
     mozIStorageStatement* const aStmt, const uint32_t aMaxExtraCount,
     const nsCString& aOperation) {
   AssertIsOnConnectionThread();
 
   // For unique cursors, we need to skip records with the same key. The SQL
   // queries currently do not filter these out.
   Key previousKey =
-      IsUnique(mCursor->mDirection)
-          ? (mCursor->IsLocaleAware() ? mCursor->mLocaleAwarePosition
-                                      : mCursor->mPosition)
-          : Key{};
+      IsUnique(mCursor->mDirection) ? mCursor->GetSortKey() : Key{};
 
   uint32_t extraCount = 0;
   do {
     bool hasResult;
     nsresult rv = aStmt->ExecuteStep(&hasResult);
     if (NS_WARN_IF(NS_FAILED(rv))) {
       // In case of a failure on one step, do not attempt to execute further
       // steps, but use the results already populated.
@@ -25926,19 +25934,17 @@ nsresult Cursor::CursorOpBase::PopulateE
       // TODO: For the first result, in case the cursor reaches the end
       // prematurely, mCursor's key members are unset. Should we do this here as
       // well?
 
       break;
     }
 
     if (IsUnique(mCursor->mDirection)) {
-      const auto& currentKey = mCursor->IsLocaleAware()
-                                   ? mCursor->mLocaleAwarePosition
-                                   : mCursor->mPosition;
+      const auto& currentKey = mCursor->GetSortKey();
       const bool sameKey = previousKey == currentKey;
       previousKey = currentKey;
       if (sameKey) {
         continue;
       }
     }
 
     // TODO: Similar to the call to ExecuteStep above, it would be better not to
@@ -26178,17 +26184,17 @@ nsresult Cursor::OpenOp::DoObjectStoreDa
   // Now we need to make the query for ContinueOp.
   PrepareKeyConditionClauses(kStmtParamNameKey, directionClause, queryStart);
 
   // The degree to which extra responses on OpenOp can actually be used depends
   // on the parameters of subsequent ContinueOp operations, see also comment in
   // ContinueOp::DoDatabaseWork.
   //
   // TODO: We should somehow evaluate the effects of this. Maybe use a smaller
-  // extra count that for ContinueOp?
+  // extra count than for ContinueOp?
   //
   // TODO: If this is done here, do this in the other Do*DatabaseWork functions
   // as well (or move this to DoDatabaseWork).
 
   return PopulateExtraResponses(&*stmt, mCursor->mMaxExtraCount,
                                 NS_LITERAL_CSTRING("OpenOp"));
 }
 
@@ -26606,18 +26612,17 @@ nsresult Cursor::ContinueOp::DoDatabaseW
 
   bool hasContinueKey = false;
   bool hasContinuePrimaryKey = false;
   // TODO: the name 'currentKey' for this variable is confusing, as this is only
   // the current key if !hasContinueKey. It is however, always a bound for the
   // key/position in the operation's result. Maybe rename to
   // targetKey/targetPosition (which is also not exact, as it might imply that
   // the result always has this key).
-  Key& currentKey = mCursor->IsLocaleAware() ? mCursor->mLocaleAwarePosition
-                                             : mCursor->mPosition;
+  Key& currentKey = *mCursor->GetModifiableSortKey();
 
   IDB_LOG_MARK_PARENT_TRANSACTION_REQUEST(
       "PRELOAD: ContinueOp: currentKey == %s", "currentKey",
       IDB_LOG_ID_STRING(mBackgroundChildLoggingId),
       mTransactionLoggingSerialNumber, mLoggingSerialNumber,
       currentKey.GetBuffer().get());
 
   switch (mParams.type()) {