Bug 1149815 - Don't assume that index creation always succeeds, r=janv.
authorBen Turner <bent.mozilla@gmail.com>
Sat, 20 Jun 2015 09:08:26 -0700
changeset 280660 f270dc49a348c10c91d8efa5d349d44bed7ff407
parent 280659 826980ade3b28bc856ca32e2f49b1ec2b7a9effa
child 280661 a9e5be3e74b2f1a9bc4a2943e5ae45a3ebe5940c
push id4932
push userjlund@mozilla.com
push dateMon, 10 Aug 2015 18:23:06 +0000
treeherdermozilla-beta@6dd5a4f5f745 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersjanv
bugs1149815
milestone41.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 1149815 - Don't assume that index creation always succeeds, r=janv.
dom/indexedDB/ActorsParent.cpp
--- a/dom/indexedDB/ActorsParent.cpp
+++ b/dom/indexedDB/ActorsParent.cpp
@@ -5338,21 +5338,20 @@ protected:
                                             const OptionalKeyRange& aKeyRange);
 
   static nsresult
   UpdateIndexValues(DatabaseConnection* aConnection,
                     const int64_t aObjectStoreId,
                     const Key& aObjectStoreKey,
                     const FallibleTArray<IndexDataValue>& aIndexValues);
 
-#ifdef DEBUG
-  static bool
+  static nsresult
   ObjectStoreHasIndexes(DatabaseConnection* aConnection,
-                        const int64_t aObjectStoreId);
-#endif
+                        const int64_t aObjectStoreId,
+                        bool* aHasIndexes);
 
 private:
   template <typename T>
   static nsresult
   GetStructuredCloneReadInfoFromSource(T* aSource,
                                        uint32_t aDataIndex,
                                        uint32_t aFileIdsIndex,
                                        FileManager* aFileManager,
@@ -6936,27 +6935,25 @@ private:
 
 class DeleteObjectStoreOp final
   : public VersionChangeTransactionOp
 {
   friend class VersionChangeTransaction;
 
   const nsRefPtr<FullObjectStoreMetadata> mMetadata;
   const bool mIsLastObjectStore;
-  const bool mObjectStoreHasIndexes;
 
 private:
   // Only created by VersionChangeTransaction.
   DeleteObjectStoreOp(VersionChangeTransaction* aTransaction,
                       FullObjectStoreMetadata* const aMetadata,
                       const bool aIsLastObjectStore)
     : VersionChangeTransactionOp(aTransaction)
     , mMetadata(aMetadata)
     , mIsLastObjectStore(aIsLastObjectStore)
-    , mObjectStoreHasIndexes(aMetadata->HasLiveIndexes())
   {
     MOZ_ASSERT(aMetadata->mCommonMetadata.id());
   }
 
   ~DeleteObjectStoreOp()
   { }
 
   virtual nsresult
@@ -7133,16 +7130,25 @@ protected:
     : TransactionDatabaseOperationBase(aTransaction)
     , mResponseSent(false)
   { }
 
   virtual
   ~NormalTransactionOp()
   { }
 
+  // An overload of DatabaseOperationBase's function that can avoid doing extra
+  // work on non-versionchange transactions.
+  static nsresult
+  ObjectStoreHasIndexes(NormalTransactionOp* aOp,
+                        DatabaseConnection* aConnection,
+                        const int64_t aObjectStoreId,
+                        const bool aMayHaveIndexes,
+                        bool* aHasIndexes);
+
   // Subclasses use this override to set the IPDL response value.
   virtual void
   GetResponse(RequestResponse& aResponse) = 0;
 
 private:
   virtual nsresult
   SendSuccessResult() override;
 
@@ -7174,17 +7180,17 @@ class ObjectStoreAddOrPutRequestOp final
 
   nsRefPtr<FileManager> mFileManager;
 
   Key mResponse;
   const nsCString mGroup;
   const nsCString mOrigin;
   const PersistenceType mPersistenceType;
   const bool mOverwrite;
-  const bool mObjectStoreHasIndexes;
+  const bool mObjectStoreMayHaveIndexes;
 
 private:
   // Only created by TransactionBase.
   ObjectStoreAddOrPutRequestOp(TransactionBase* aTransaction,
                                const RequestParams& aParams);
 
   ~ObjectStoreAddOrPutRequestOp()
   { }
@@ -7292,17 +7298,17 @@ private:
 
 class ObjectStoreDeleteRequestOp final
   : public NormalTransactionOp
 {
   friend class TransactionBase;
 
   const ObjectStoreDeleteParams mParams;
   ObjectStoreDeleteResponse mResponse;
-  const bool mObjectStoreHasIndexes;
+  const bool mObjectStoreMayHaveIndexes;
 
 private:
   ObjectStoreDeleteRequestOp(TransactionBase* aTransaction,
                              const ObjectStoreDeleteParams& aParams);
 
   ~ObjectStoreDeleteRequestOp()
   { }
 
@@ -7318,17 +7324,17 @@ private:
 
 class ObjectStoreClearRequestOp final
   : public NormalTransactionOp
 {
   friend class TransactionBase;
 
   const ObjectStoreClearParams mParams;
   ObjectStoreClearResponse mResponse;
-  const bool mObjectStoreHasIndexes;
+  const bool mObjectStoreMayHaveIndexes;
 
 private:
   ObjectStoreClearRequestOp(TransactionBase* aTransaction,
                             const ObjectStoreClearParams& aParams);
 
   ~ObjectStoreClearRequestOp()
   { }
 
@@ -18058,18 +18064,26 @@ nsresult
 DatabaseOperationBase::DeleteObjectStoreDataTableRowsWithIndexes(
                                               DatabaseConnection* aConnection,
                                               const int64_t aObjectStoreId,
                                               const OptionalKeyRange& aKeyRange)
 {
   MOZ_ASSERT(aConnection);
   aConnection->AssertIsOnConnectionThread();
   MOZ_ASSERT(aObjectStoreId);
-  MOZ_ASSERT(ObjectStoreHasIndexes(aConnection, aObjectStoreId),
-             "Don't use this slow method if there are no indexes!");
+
+#ifdef DEBUG
+  {
+    bool hasIndexes = false;
+    MOZ_ASSERT(NS_SUCCEEDED(
+      ObjectStoreHasIndexes(aConnection, aObjectStoreId, &hasIndexes)));
+    MOZ_ASSERT(hasIndexes,
+               "Don't use this slow method if there are no indexes!");
+  }
+#endif
 
   PROFILER_LABEL("IndexedDB",
                  "DatabaseOperationBase::"
                  "DeleteObjectStoreDataTableRowsWithIndexes",
                  js::ProfileEntry::Category::STORAGE);
 
   const bool singleRowOnly =
     aKeyRange.type() == OptionalKeyRange::TSerializedKeyRange &&
@@ -18267,47 +18281,54 @@ DatabaseOperationBase::UpdateIndexValues
   rv = updateStmt->Execute();
   if (NS_WARN_IF(NS_FAILED(rv))) {
     return rv;
   }
 
   return NS_OK;
 }
 
-#ifdef DEBUG
-
 // static
-bool
+nsresult
 DatabaseOperationBase::ObjectStoreHasIndexes(DatabaseConnection* aConnection,
-                                             const int64_t aObjectStoreId)
+                                             const int64_t aObjectStoreId,
+                                             bool* aHasIndexes)
 {
   MOZ_ASSERT(aConnection);
   aConnection->AssertIsOnConnectionThread();
   MOZ_ASSERT(aObjectStoreId);
+  MOZ_ASSERT(aHasIndexes);
 
   DatabaseConnection::CachedStatement stmt;
 
-  MOZ_ALWAYS_TRUE(NS_SUCCEEDED(
-    aConnection->GetCachedStatement(NS_LITERAL_CSTRING(
-      "SELECT id "
-        "FROM object_store_index "
-        "WHERE object_store_id = :object_store_id;"),
-      &stmt)));
-
-  MOZ_ALWAYS_TRUE(NS_SUCCEEDED(
-    stmt->BindInt64ByName(NS_LITERAL_CSTRING("object_store_id"),
-                          aObjectStoreId)));
+  nsresult rv = aConnection->GetCachedStatement(NS_LITERAL_CSTRING(
+    "SELECT id "
+      "FROM object_store_index "
+      "WHERE object_store_id = :object_store_id "
+      "LIMIT 1;"),
+    &stmt);
+  if (NS_WARN_IF(NS_FAILED(rv))) {
+    return rv;
+  }
+
+  rv = stmt->BindInt64ByName(NS_LITERAL_CSTRING("object_store_id"),
+                             aObjectStoreId);
+  if (NS_WARN_IF(NS_FAILED(rv))) {
+    return rv;
+  }
 
   bool hasResult;
-  MOZ_ALWAYS_TRUE(NS_SUCCEEDED(stmt->ExecuteStep(&hasResult)));
-
-  return hasResult;
-}
-
-#endif // DEBUG
+  rv = stmt->ExecuteStep(&hasResult);
+  if (NS_WARN_IF(NS_FAILED(rv))) {
+    return rv;
+  }
+
+  *aHasIndexes = hasResult;
+  return NS_OK;
+}
 
 NS_IMPL_ISUPPORTS_INHERITED(DatabaseOperationBase,
                             nsRunnable,
                             mozIStorageProgressHandler)
 
 NS_IMETHODIMP
 DatabaseOperationBase::OnProgress(mozIStorageConnection* aConnection,
                                   bool* _retval)
@@ -21630,21 +21651,16 @@ DeleteObjectStoreOp::DoDatabaseWork(Data
         foundOtherObjectStore = true;
       }
     }
 
     MOZ_ASSERT_IF(mIsLastObjectStore,
                   foundThisObjectStore && !foundOtherObjectStore);
     MOZ_ASSERT_IF(!mIsLastObjectStore,
                   foundThisObjectStore && foundOtherObjectStore);
-
-    // Make sure |hasIndexes| is telling the truth.
-    MOZ_ASSERT(mObjectStoreHasIndexes ==
-                 ObjectStoreHasIndexes(aConnection,
-                                       mMetadata->mCommonMetadata.id()));
   }
 #endif
 
   DatabaseConnection::AutoSavepoint autoSave;
   nsresult rv = autoSave.Start(Transaction());
   if (NS_WARN_IF(NS_FAILED(rv))) {
     return rv;
   }
@@ -21707,17 +21723,25 @@ DeleteObjectStoreOp::DoDatabaseWork(Data
       return rv;
     }
 
     rv = stmt->Execute();
     if (NS_WARN_IF(NS_FAILED(rv))) {
       return rv;
     }
   } else {
-    if (mObjectStoreHasIndexes) {
+    bool hasIndexes;
+    rv = ObjectStoreHasIndexes(aConnection,
+                               mMetadata->mCommonMetadata.id(),
+                               &hasIndexes);
+    if (NS_WARN_IF(NS_FAILED(rv))) {
+      return rv;
+    }
+
+    if (hasIndexes) {
       rv = DeleteObjectStoreDataTableRowsWithIndexes(
         aConnection,
         mMetadata->mCommonMetadata.id(),
         void_t());
       if (NS_WARN_IF(NS_FAILED(rv))) {
         return rv;
       }
 
@@ -22785,16 +22809,58 @@ DeleteIndexOp::DoDatabaseWork(DatabaseCo
   rv = autoSave.Commit();
   if (NS_WARN_IF(NS_FAILED(rv))) {
     return rv;
   }
 
   return NS_OK;
 }
 
+// static
+nsresult
+NormalTransactionOp::ObjectStoreHasIndexes(NormalTransactionOp* aOp,
+                                           DatabaseConnection* aConnection,
+                                           const int64_t aObjectStoreId,
+                                           const bool aMayHaveIndexes,
+                                           bool* aHasIndexes)
+{
+  MOZ_ASSERT(aOp);
+  MOZ_ASSERT(aConnection);
+  aConnection->AssertIsOnConnectionThread();
+  MOZ_ASSERT(aObjectStoreId);
+  MOZ_ASSERT(aHasIndexes);
+
+  bool hasIndexes;
+  if (aOp->Transaction()->GetMode() == IDBTransaction::VERSION_CHANGE &&
+      aMayHaveIndexes) {
+    // If this is a version change transaction then mObjectStoreMayHaveIndexes
+    // could be wrong (e.g. if a unique index failed to be created due to a
+    // constraint error). We have to check on this thread by asking the database
+    // directly.
+    nsresult rv =
+      DatabaseOperationBase::ObjectStoreHasIndexes(aConnection,
+                                                   aObjectStoreId,
+                                                   &hasIndexes);
+    if (NS_WARN_IF(NS_FAILED(rv))) {
+      return rv;
+    }
+  } else {
+    MOZ_ASSERT(NS_SUCCEEDED(
+      DatabaseOperationBase::ObjectStoreHasIndexes(aConnection,
+                                                   aObjectStoreId,
+                                                   &hasIndexes)));
+    MOZ_ASSERT(aMayHaveIndexes == hasIndexes);
+
+    hasIndexes = aMayHaveIndexes;
+  }
+
+  *aHasIndexes = hasIndexes;
+  return NS_OK;
+}
+
 nsresult
 NormalTransactionOp::SendSuccessResult()
 {
   AssertIsOnOwningThread();
 
   if (!IsActorDestroyed()) {
     RequestResponse response;
     GetResponse(response);
@@ -22861,37 +22927,48 @@ ObjectStoreAddOrPutRequestOp::ObjectStor
   : NormalTransactionOp(aTransaction)
   , mParams(aParams.type() == RequestParams::TObjectStoreAddParams ?
               aParams.get_ObjectStoreAddParams().commonParams() :
               aParams.get_ObjectStorePutParams().commonParams())
   , mGroup(aTransaction->GetDatabase()->Group())
   , mOrigin(aTransaction->GetDatabase()->Origin())
   , mPersistenceType(aTransaction->GetDatabase()->Type())
   , mOverwrite(aParams.type() == RequestParams::TObjectStorePutParams)
-  , mObjectStoreHasIndexes(false)
+  , mObjectStoreMayHaveIndexes(false)
 {
   MOZ_ASSERT(aParams.type() == RequestParams::TObjectStoreAddParams ||
              aParams.type() == RequestParams::TObjectStorePutParams);
 
   mMetadata =
     aTransaction->GetMetadataForObjectStoreId(mParams.objectStoreId());
   MOZ_ASSERT(mMetadata);
 
-  const_cast<bool&>(mObjectStoreHasIndexes) = mMetadata->HasLiveIndexes();
+  const_cast<bool&>(mObjectStoreMayHaveIndexes) = mMetadata->HasLiveIndexes();
 }
 
 nsresult
 ObjectStoreAddOrPutRequestOp::RemoveOldIndexDataValues(
                                                 DatabaseConnection* aConnection)
 {
   AssertIsOnConnectionThread();
   MOZ_ASSERT(aConnection);
   MOZ_ASSERT(mOverwrite);
   MOZ_ASSERT(!mResponse.IsUnset());
-  MOZ_ASSERT(mObjectStoreHasIndexes);
+
+#ifdef DEBUG
+  {
+    bool hasIndexes = false;
+    MOZ_ASSERT(NS_SUCCEEDED(
+      DatabaseOperationBase::ObjectStoreHasIndexes(aConnection,
+                                                   mParams.objectStoreId(),
+                                                   &hasIndexes)));
+    MOZ_ASSERT(hasIndexes,
+               "Don't use this slow method if there are no indexes!");
+  }
+#endif
 
   DatabaseConnection::CachedStatement indexValuesStmt;
   nsresult rv = aConnection->GetCachedStatement(NS_LITERAL_CSTRING(
     "SELECT index_data_values "
       "FROM object_data "
       "WHERE object_store_id = :object_store_id "
       "AND key = :key;"),
     &indexValuesStmt);
@@ -23093,44 +23170,52 @@ ObjectStoreAddOrPutRequestOp::Init(Trans
 
 nsresult
 ObjectStoreAddOrPutRequestOp::DoDatabaseWork(DatabaseConnection* aConnection)
 {
   MOZ_ASSERT(aConnection);
   aConnection->AssertIsOnConnectionThread();
   MOZ_ASSERT(aConnection->GetStorageConnection());
   MOZ_ASSERT_IF(mFileManager, !mStoredFileInfos.IsEmpty());
-  MOZ_ASSERT(mObjectStoreHasIndexes ==
-               ObjectStoreHasIndexes(aConnection, mParams.objectStoreId()));
 
   PROFILER_LABEL("IndexedDB",
                  "ObjectStoreAddOrPutRequestOp::DoDatabaseWork",
                  js::ProfileEntry::Category::STORAGE);
 
   if (NS_WARN_IF(IndexedDatabaseManager::InLowDiskSpaceMode())) {
     return NS_ERROR_DOM_INDEXEDDB_QUOTA_ERR;
   }
 
   DatabaseConnection::AutoSavepoint autoSave;
   nsresult rv = autoSave.Start(Transaction());
   if (NS_WARN_IF(NS_FAILED(rv))) {
     return rv;
   }
 
+  bool objectStoreHasIndexes;
+  rv = ObjectStoreHasIndexes(this,
+                             aConnection,
+                             mParams.objectStoreId(),
+                             mObjectStoreMayHaveIndexes,
+                             &objectStoreHasIndexes);
+  if (NS_WARN_IF(NS_FAILED(rv))) {
+    return rv;
+  }
+
   // This will be the final key we use.
   Key& key = mResponse;
   key = mParams.key();
 
   const bool keyUnset = key.IsUnset();
   const int64_t osid = mParams.objectStoreId();
   const KeyPath& keyPath = mMetadata->mCommonMetadata.keyPath();
 
   // First delete old index_data_values if we're overwriting something and we
   // have indexes.
-  if (mOverwrite && !keyUnset && mObjectStoreHasIndexes) {
+  if (mOverwrite && !keyUnset && objectStoreHasIndexes) {
     rv = RemoveOldIndexDataValues(aConnection);
     if (NS_WARN_IF(NS_FAILED(rv))) {
       return rv;
     }
   }
 
   // The "|| keyUnset" here is mostly a debugging tool. If a key isn't
   // specified we should never have a collision and so it shouldn't matter
@@ -23774,47 +23859,54 @@ ObjectStoreGetAllKeysRequestOp::GetRespo
   }
 }
 
 ObjectStoreDeleteRequestOp::ObjectStoreDeleteRequestOp(
                                          TransactionBase* aTransaction,
                                          const ObjectStoreDeleteParams& aParams)
   : NormalTransactionOp(aTransaction)
   , mParams(aParams)
-  , mObjectStoreHasIndexes(false)
+  , mObjectStoreMayHaveIndexes(false)
 {
   AssertIsOnBackgroundThread();
   MOZ_ASSERT(aTransaction);
 
   nsRefPtr<FullObjectStoreMetadata> metadata =
     aTransaction->GetMetadataForObjectStoreId(mParams.objectStoreId());
   MOZ_ASSERT(metadata);
 
-  const_cast<bool&>(mObjectStoreHasIndexes) = metadata->HasLiveIndexes();
+  const_cast<bool&>(mObjectStoreMayHaveIndexes) = metadata->HasLiveIndexes();
 }
 
 nsresult
 ObjectStoreDeleteRequestOp::DoDatabaseWork(DatabaseConnection* aConnection)
 {
   MOZ_ASSERT(aConnection);
   aConnection->AssertIsOnConnectionThread();
-  MOZ_ASSERT(mObjectStoreHasIndexes ==
-               ObjectStoreHasIndexes(aConnection, mParams.objectStoreId()));
-
   PROFILER_LABEL("IndexedDB",
                  "ObjectStoreDeleteRequestOp::DoDatabaseWork",
                  js::ProfileEntry::Category::STORAGE);
 
   DatabaseConnection::AutoSavepoint autoSave;
   nsresult rv = autoSave.Start(Transaction());
   if (NS_WARN_IF(NS_FAILED(rv))) {
     return rv;
   }
 
-  if (mObjectStoreHasIndexes) {
+  bool objectStoreHasIndexes;
+  rv = ObjectStoreHasIndexes(this,
+                             aConnection,
+                             mParams.objectStoreId(),
+                             mObjectStoreMayHaveIndexes,
+                             &objectStoreHasIndexes);
+  if (NS_WARN_IF(NS_FAILED(rv))) {
+    return rv;
+  }
+
+  if (objectStoreHasIndexes) {
     rv = DeleteObjectStoreDataTableRowsWithIndexes(aConnection,
                                                    mParams.objectStoreId(),
                                                    mParams.keyRange());
     if (NS_WARN_IF(NS_FAILED(rv))) {
       return rv;
     }
   } else {
     NS_NAMED_LITERAL_CSTRING(objectStoreIdString, "object_store_id");
@@ -23859,47 +23951,55 @@ ObjectStoreDeleteRequestOp::DoDatabaseWo
   return NS_OK;
 }
 
 ObjectStoreClearRequestOp::ObjectStoreClearRequestOp(
                                           TransactionBase* aTransaction,
                                           const ObjectStoreClearParams& aParams)
   : NormalTransactionOp(aTransaction)
   , mParams(aParams)
-  , mObjectStoreHasIndexes(false)
+  , mObjectStoreMayHaveIndexes(false)
 {
   AssertIsOnBackgroundThread();
   MOZ_ASSERT(aTransaction);
 
   nsRefPtr<FullObjectStoreMetadata> metadata =
     aTransaction->GetMetadataForObjectStoreId(mParams.objectStoreId());
   MOZ_ASSERT(metadata);
 
-  const_cast<bool&>(mObjectStoreHasIndexes) = metadata->HasLiveIndexes();
+  const_cast<bool&>(mObjectStoreMayHaveIndexes) = metadata->HasLiveIndexes();
 }
 
 nsresult
 ObjectStoreClearRequestOp::DoDatabaseWork(DatabaseConnection* aConnection)
 {
   MOZ_ASSERT(aConnection);
   aConnection->AssertIsOnConnectionThread();
-  MOZ_ASSERT(mObjectStoreHasIndexes ==
-               ObjectStoreHasIndexes(aConnection, mParams.objectStoreId()));
 
   PROFILER_LABEL("IndexedDB",
                  "ObjectStoreClearRequestOp::DoDatabaseWork",
                  js::ProfileEntry::Category::STORAGE);
 
   DatabaseConnection::AutoSavepoint autoSave;
   nsresult rv = autoSave.Start(Transaction());
   if (NS_WARN_IF(NS_FAILED(rv))) {
     return rv;
   }
 
-  if (mObjectStoreHasIndexes) {
+  bool objectStoreHasIndexes;
+  rv = ObjectStoreHasIndexes(this,
+                             aConnection,
+                             mParams.objectStoreId(),
+                             mObjectStoreMayHaveIndexes,
+                             &objectStoreHasIndexes);
+  if (NS_WARN_IF(NS_FAILED(rv))) {
+    return rv;
+  }
+
+  if (objectStoreHasIndexes) {
     rv = DeleteObjectStoreDataTableRowsWithIndexes(aConnection,
                                                    mParams.objectStoreId(),
                                                    void_t());
     if (NS_WARN_IF(NS_FAILED(rv))) {
       return rv;
     }
   } else {
     DatabaseConnection::CachedStatement stmt;