From e02b223f81dfb3cd1380962d41eefb6ab252c433 Mon Sep 17 00:00:00 2001
From: Ben Turner <bent.mozilla@gmail.com>
Date: Mon, 21 Nov 2011 20:18:19 -0800
Subject: [PATCH] Bug 692652 - IndexedDB: Index updating is broken. r=sicking.

--HG--
extra : transplant_source : %92%5BBT%93%BA%3DO%D7%03v%B7%88%01%3AjZ%D0%81%AA
---
 dom/indexedDB/IDBObjectStore.cpp              |  62 ++++--
 dom/indexedDB/IDBTransaction.cpp              |  61 +++---
 dom/indexedDB/IDBTransaction.h                |   9 +-
 dom/indexedDB/Key.h                           |  13 --
 dom/indexedDB/test/Makefile.in                |   2 +
 .../test/test_index_update_delete.html        | 180 ++++++++++++++++++
 .../test/test_unique_index_update.html        |  79 ++++++++
 7 files changed, 340 insertions(+), 66 deletions(-)
 create mode 100644 dom/indexedDB/test/test_index_update_delete.html
 create mode 100644 dom/indexedDB/test/test_unique_index_update.html

diff --git a/dom/indexedDB/IDBObjectStore.cpp b/dom/indexedDB/IDBObjectStore.cpp
index 620d77b9e82..c68001488ae 100644
--- a/dom/indexedDB/IDBObjectStore.cpp
+++ b/dom/indexedDB/IDBObjectStore.cpp
@@ -664,7 +664,6 @@ IDBObjectStore::UpdateIndexes(IDBTransaction* aTransaction,
 #endif
 
   PRUint32 indexCount = aUpdateInfoArray.Length();
-  NS_ASSERTION(indexCount, "Don't call me!");
 
   nsCOMPtr<mozIStorageStatement> stmt;
   nsresult rv;
@@ -702,34 +701,59 @@ IDBObjectStore::UpdateIndexes(IDBTransaction* aTransaction,
 
   NS_ASSERTION(aObjectDataId != LL_MININT, "Bad objectData id!");
 
-  for (PRUint32 indexIndex = 0; indexIndex < indexCount; indexIndex++) {
-    const IndexUpdateInfo& updateInfo = aUpdateInfoArray[indexIndex];
+  NS_NAMED_LITERAL_CSTRING(indexId, "index_id");
+  NS_NAMED_LITERAL_CSTRING(objectDataId, "object_data_id");
+  NS_NAMED_LITERAL_CSTRING(objectDataKey, "object_data_key");
+  NS_NAMED_LITERAL_CSTRING(value, "value");
 
-    stmt = aTransaction->IndexUpdateStatement(updateInfo.info.autoIncrement,
-                                              updateInfo.info.unique,
-                                              aOverwrite);
+  if (aOverwrite) {
+    stmt = aTransaction->IndexDataDeleteStatement(aAutoIncrement, false);
     NS_ENSURE_TRUE(stmt, NS_ERROR_FAILURE);
 
     mozStorageStatementScoper scoper2(stmt);
 
-    rv = stmt->BindInt64ByName(NS_LITERAL_CSTRING("index_id"),
-                               updateInfo.info.id);
+    rv = stmt->BindInt64ByName(objectDataId, aObjectDataId);
     NS_ENSURE_SUCCESS(rv, rv);
 
-    rv = stmt->BindInt64ByName(NS_LITERAL_CSTRING("object_data_id"),
-                               aObjectDataId);
+    rv = stmt->Execute();
+    NS_ENSURE_SUCCESS(rv, rv);
+
+    stmt = aTransaction->IndexDataDeleteStatement(aAutoIncrement, true);
+    NS_ENSURE_TRUE(stmt, NS_ERROR_FAILURE);
+
+    mozStorageStatementScoper scoper3(stmt);
+
+    rv = stmt->BindInt64ByName(objectDataId, aObjectDataId);
+    NS_ENSURE_SUCCESS(rv, rv);
+
+    rv = stmt->Execute();
+    NS_ENSURE_SUCCESS(rv, rv);
+  }
+
+  for (PRUint32 indexIndex = 0; indexIndex < indexCount; indexIndex++) {
+    const IndexUpdateInfo& updateInfo = aUpdateInfoArray[indexIndex];
+
+    NS_ASSERTION(updateInfo.info.autoIncrement == aAutoIncrement, "Huh?!");
+
+    // Insert new values.
+    stmt = aTransaction->IndexDataInsertStatement(aAutoIncrement,
+                                                  updateInfo.info.unique);
+    NS_ENSURE_TRUE(stmt, NS_ERROR_FAILURE);
+
+    mozStorageStatementScoper scoper4(stmt);
+
+    rv = stmt->BindInt64ByName(indexId, updateInfo.info.id);
+    NS_ENSURE_SUCCESS(rv, rv);
+
+    rv = stmt->BindInt64ByName(objectDataId, aObjectDataId);
     NS_ENSURE_SUCCESS(rv, rv);
 
     if (!updateInfo.info.autoIncrement) {
-      rv =
-        aObjectStoreKey.BindToStatement(stmt,
-                                        NS_LITERAL_CSTRING("object_data_key"));
+      rv = aObjectStoreKey.BindToStatement(stmt, objectDataKey);
       NS_ENSURE_SUCCESS(rv, rv);
     }
 
-    rv =
-      updateInfo.value.BindToStatementAllowUnset(stmt,
-                                                 NS_LITERAL_CSTRING("value"));
+    rv = updateInfo.value.BindToStatement(stmt, value);
     NS_ENSURE_SUCCESS(rv, rv);
 
     rv = stmt->Execute();
@@ -1718,7 +1742,7 @@ AddHelper::DoDatabaseWork(mozIStorageConnection* aConnection)
   }
 
   // Update our indexes if needed.
-  if (!mIndexUpdateInfo.IsEmpty()) {
+  if (mOverwrite || !mIndexUpdateInfo.IsEmpty()) {
     PRInt64 objectDataId = autoIncrement ? mKey.ToInteger() : LL_MININT;
     rv = IDBObjectStore::UpdateIndexes(mTransaction, osid, mKey,
                                        autoIncrement, mOverwrite,
@@ -2215,8 +2239,8 @@ CreateIndexHelper::InsertDataFromObjectStore(mozIStorageConnection* aConnection)
   bool hasResult;
   while (NS_SUCCEEDED(stmt->ExecuteStep(&hasResult)) && hasResult) {
     nsCOMPtr<mozIStorageStatement> insertStmt =
-      mTransaction->IndexUpdateStatement(mIndex->IsAutoIncrement(),
-                                         mIndex->IsUnique(), false);
+      mTransaction->IndexDataInsertStatement(mIndex->IsAutoIncrement(),
+                                             mIndex->IsUnique());
     NS_ENSURE_TRUE(insertStmt, NS_ERROR_DOM_INDEXEDDB_UNKNOWN_ERR);
 
     mozStorageStatementScoper scoper2(insertStmt);
diff --git a/dom/indexedDB/IDBTransaction.cpp b/dom/indexedDB/IDBTransaction.cpp
index b4f42fa5551..13d9a90d3c9 100644
--- a/dom/indexedDB/IDBTransaction.cpp
+++ b/dom/indexedDB/IDBTransaction.cpp
@@ -394,28 +394,13 @@ IDBTransaction::AddStatement(bool aCreate,
 }
 
 already_AddRefed<mozIStorageStatement>
-IDBTransaction::IndexUpdateStatement(bool aAutoIncrement,
-                                     bool aUnique,
-                                     bool aOverwrite)
+IDBTransaction::IndexDataInsertStatement(bool aAutoIncrement,
+                                         bool aUnique)
 {
   if (aAutoIncrement) {
     if (aUnique) {
-      if (aOverwrite) {
-        return GetCachedStatement(
-          "INSERT OR REPLACE INTO ai_unique_index_data "
-            "(index_id, ai_object_data_id, value) "
-          "VALUES (:index_id, :object_data_id, :value)"
-        );
-      }
       return GetCachedStatement(
         "INSERT INTO ai_unique_index_data "
-          "(index_id, aI_object_data_id, value) "
-        "VALUES (:index_id, :object_data_id, :value)"
-      );
-    }
-    if (aOverwrite) {
-      return GetCachedStatement(
-        "INSERT OR REPLACE INTO ai_index_data "
           "(index_id, ai_object_data_id, value) "
         "VALUES (:index_id, :object_data_id, :value)"
       );
@@ -427,26 +412,12 @@ IDBTransaction::IndexUpdateStatement(bool aAutoIncrement,
     );
   }
   if (aUnique) {
-    if (aOverwrite) {
-      return GetCachedStatement(
-        "INSERT OR REPLACE INTO unique_index_data "
-          "(index_id, object_data_id, object_data_key, value) "
-        "VALUES (:index_id, :object_data_id, :object_data_key, :value)"
-      );
-    }
     return GetCachedStatement(
       "INSERT INTO unique_index_data "
         "(index_id, object_data_id, object_data_key, value) "
       "VALUES (:index_id, :object_data_id, :object_data_key, :value)"
     );
   }
-  if (aOverwrite) {
-    return GetCachedStatement(
-      "INSERT INTO index_data ("
-        "index_id, object_data_id, object_data_key, value) "
-      "VALUES (:index_id, :object_data_id, :object_data_key, :value)"
-    );
-  }
   return GetCachedStatement(
     "INSERT INTO index_data ("
       "index_id, object_data_id, object_data_key, value) "
@@ -454,6 +425,34 @@ IDBTransaction::IndexUpdateStatement(bool aAutoIncrement,
   );
 }
 
+already_AddRefed<mozIStorageStatement>
+IDBTransaction::IndexDataDeleteStatement(bool aAutoIncrement,
+                                         bool aUnique)
+{
+  if (aAutoIncrement) {
+    if (aUnique) {
+      return GetCachedStatement(
+        "DELETE FROM ai_unique_index_data "
+        "WHERE ai_object_data_id = :object_data_id"
+      );
+    }
+    return GetCachedStatement(
+      "DELETE FROM ai_index_data "
+      "WHERE ai_object_data_id = :object_data_id"
+    );
+  }
+  if (aUnique) {
+    return GetCachedStatement(
+      "DELETE FROM unique_index_data "
+      "WHERE object_data_id = :object_data_id"
+    );
+  }
+  return GetCachedStatement(
+    "DELETE FROM index_data "
+    "WHERE object_data_id = :object_data_id"
+  );
+}
+
 already_AddRefed<mozIStorageStatement>
 IDBTransaction::GetCachedStatement(const nsACString& aQuery)
 {
diff --git a/dom/indexedDB/IDBTransaction.h b/dom/indexedDB/IDBTransaction.h
index e60f9db3e97..73f3b190b1b 100644
--- a/dom/indexedDB/IDBTransaction.h
+++ b/dom/indexedDB/IDBTransaction.h
@@ -118,9 +118,12 @@ public:
                bool aAutoIncrement);
 
   already_AddRefed<mozIStorageStatement>
-  IndexUpdateStatement(bool aAutoIncrement,
-                       bool aUnique,
-                       bool aOverwrite);
+  IndexDataInsertStatement(bool aAutoIncrement,
+                           bool aUnique);
+
+  already_AddRefed<mozIStorageStatement>
+  IndexDataDeleteStatement(bool aAutoIncrement,
+                           bool aUnique);
 
   already_AddRefed<mozIStorageStatement>
   GetCachedStatement(const nsACString& aQuery);
diff --git a/dom/indexedDB/Key.h b/dom/indexedDB/Key.h
index 27f4010dd30..e6476ed5631 100644
--- a/dom/indexedDB/Key.h
+++ b/dom/indexedDB/Key.h
@@ -248,19 +248,6 @@ public:
     return NS_SUCCEEDED(rv) ? NS_OK : NS_ERROR_DOM_INDEXEDDB_UNKNOWN_ERR;
   }
 
-  nsresult BindToStatementAllowUnset(mozIStorageStatement* aStatement,
-                                     const nsACString& aParamName) const
-  {
-    nsresult rv;
-
-    if (IsUnset()) {
-      rv = aStatement->BindStringByName(aParamName, EmptyString());
-      return NS_SUCCEEDED(rv) ? NS_OK : NS_ERROR_DOM_INDEXEDDB_UNKNOWN_ERR;
-    }
-
-    return BindToStatement(aStatement, aParamName);
-  }
-
   nsresult SetFromStatement(mozIStorageStatement* aStatement,
                             PRUint32 aIndex)
   {
diff --git a/dom/indexedDB/test/Makefile.in b/dom/indexedDB/test/Makefile.in
index cefad24c06b..719b315929d 100644
--- a/dom/indexedDB/test/Makefile.in
+++ b/dom/indexedDB/test/Makefile.in
@@ -78,6 +78,7 @@ TEST_FILES = \
   test_index_getAll.html \
   test_index_getAllObjects.html \
   test_index_object_cursors.html \
+  test_index_update_delete.html \
   test_indexes.html \
   test_indexes_bad_values.html \
   test_key_requirements.html \
@@ -108,6 +109,7 @@ TEST_FILES = \
   test_setVersion_abort.html \
   test_setVersion_events.html \
   test_setVersion_exclusion.html \
+  test_unique_index_update.html \
   test_writer_starvation.html \
   third_party_iframe1.html \
   third_party_iframe2.html \
diff --git a/dom/indexedDB/test/test_index_update_delete.html b/dom/indexedDB/test/test_index_update_delete.html
new file mode 100644
index 00000000000..61c6bf795d3
--- /dev/null
+++ b/dom/indexedDB/test/test_index_update_delete.html
@@ -0,0 +1,180 @@
+<!--
+  Any copyright is dedicated to the Public Domain.
+  http://creativecommons.org/publicdomain/zero/1.0/
+-->
+<html>
+<head>
+  <title>Indexed Database Property Test</title>
+
+  <script type="text/javascript" src="/tests/SimpleTest/SimpleTest.js"></script>
+  <link rel="stylesheet" type="text/css" href="/tests/SimpleTest/test.css"/>
+
+  <script type="text/javascript;version=1.7">
+    function testSteps()
+    {
+      let request = mozIndexedDB.open(window.location.pathname, 1);
+      request.onerror = errorHandler;
+      request.onupgradeneeded = grabEventAndContinueHandler;
+      request.onsuccess = grabEventAndContinueHandler;
+
+      let event = yield;
+
+      let db = event.target.result;
+      db.onerror = errorHandler;
+
+      for each (let autoIncrement in [false, true]) {
+        let objectStore =
+          db.createObjectStore(autoIncrement, { keyPath: "id",
+                                                autoIncrement: autoIncrement });
+
+        for (let i = 0; i < 10; i++) {
+          objectStore.add({ id: i, index: i });
+        }
+
+        for each (let unique in [false, true]) {
+          objectStore.createIndex(unique, "index", { unique: unique });
+        }
+
+        for (let i = 10; i < 20; i++) {
+          objectStore.add({ id: i, index: i });
+        }
+      }
+
+      event = yield;
+      is(event.type, "success", "expect a success event");
+
+      for each (let autoIncrement in [false, true]) {
+        let objectStore = db.transaction(autoIncrement)
+                            .objectStore(autoIncrement);
+
+        objectStore.count().onsuccess = grabEventAndContinueHandler;
+        let event = yield;
+
+        is(event.target.result, 20, "Correct number of entries in objectStore");
+
+        let objectStoreCount = event.target.result;
+        let indexCount = event.target.result;
+
+        for each (let unique in [false, true]) {
+          let index = db.transaction(autoIncrement, IDBTransaction.READ_WRITE)
+                        .objectStore(autoIncrement)
+                        .index(unique);
+
+          index.count().onsuccess = grabEventAndContinueHandler;
+          let event = yield;
+
+          is(event.target.result, indexCount,
+             "Correct number of entries in index");
+
+          let modifiedEntry = unique ? 5 : 10;
+          let keyRange = IDBKeyRange.only(modifiedEntry);
+
+          let sawEntry = false;
+          index.openCursor(keyRange).onsuccess = function(event) {
+            let cursor = event.target.result;
+            if (cursor) {
+              sawEntry = true;
+              is(cursor.key, modifiedEntry, "Correct key");
+
+              cursor.value.index = unique ? 30 : 35;
+              cursor.update(cursor.value).onsuccess = function(event) {
+                cursor.continue();
+              }
+            }
+            else {
+              continueToNextStep();
+            }
+          }
+          yield;
+
+          is(sawEntry, true, "Saw entry for key value " + modifiedEntry);
+
+          // Recount index. Shouldn't change.
+          index = db.transaction(autoIncrement, IDBTransaction.READ_WRITE)
+                    .objectStore(autoIncrement)
+                    .index(unique);
+
+          index.count().onsuccess = grabEventAndContinueHandler;
+          event = yield;
+
+          is(event.target.result, indexCount,
+             "Correct number of entries in index");
+
+          modifiedEntry = unique ? 30 : 35;
+          keyRange = IDBKeyRange.only(modifiedEntry);
+
+          sawEntry = false;
+          index.openCursor(keyRange).onsuccess = function(event) {
+            let cursor = event.target.result;
+            if (cursor) {
+              sawEntry = true;
+              is(cursor.key, modifiedEntry, "Correct key");
+
+              delete cursor.value.index;
+              cursor.update(cursor.value).onsuccess = function(event) {
+                indexCount--;
+                cursor.continue();
+              }
+            }
+            else {
+              continueToNextStep();
+            }
+          }
+          yield;
+
+          is(sawEntry, true, "Saw entry for key value " + modifiedEntry);
+
+          // Recount objectStore. Should be unchanged.
+          objectStore = db.transaction(autoIncrement, IDBTransaction.READ_WRITE)
+                          .objectStore(autoIncrement);
+
+          objectStore.count().onsuccess = grabEventAndContinueHandler;
+          event = yield;
+
+          is(event.target.result, objectStoreCount,
+             "Correct number of entries in objectStore");
+
+          // Recount index. Should be one item less.
+          index = objectStore.index(unique);
+
+          index.count().onsuccess = grabEventAndContinueHandler;
+          event = yield;
+
+          is(event.target.result, indexCount,
+             "Correct number of entries in index");
+
+          modifiedEntry = objectStoreCount - 1;
+
+          objectStore.delete(modifiedEntry).onsuccess =
+            grabEventAndContinueHandler;
+          event = yield;
+
+          objectStoreCount--;
+          indexCount--;
+
+          objectStore.count().onsuccess = grabEventAndContinueHandler;
+          event = yield;
+
+          is(event.target.result, objectStoreCount,
+             "Correct number of entries in objectStore");
+
+          index.count().onsuccess = grabEventAndContinueHandler;
+          event = yield;
+
+          is(event.target.result, indexCount,
+             "Correct number of entries in index");
+        }
+      }
+
+      finishTest();
+      yield;
+    }
+
+  </script>
+  <script type="text/javascript;version=1.7" src="helpers.js"></script>
+
+</head>
+
+<body onload="runTest();"></body>
+
+</html>
diff --git a/dom/indexedDB/test/test_unique_index_update.html b/dom/indexedDB/test/test_unique_index_update.html
new file mode 100644
index 00000000000..9172870b6f4
--- /dev/null
+++ b/dom/indexedDB/test/test_unique_index_update.html
@@ -0,0 +1,79 @@
+<!--
+  Any copyright is dedicated to the Public Domain.
+  http://creativecommons.org/publicdomain/zero/1.0/
+-->
+<html>
+<head>
+  <title>Indexed Database Property Test</title>
+
+  <script type="text/javascript" src="/tests/SimpleTest/SimpleTest.js"></script>
+  <link rel="stylesheet" type="text/css" href="/tests/SimpleTest/test.css"/>
+
+  <script type="text/javascript;version=1.7">
+    function testSteps()
+    {
+      let request = mozIndexedDB.open(window.location.pathname, 1);
+      request.onerror = errorHandler;
+      request.onupgradeneeded = grabEventAndContinueHandler;
+      request.onsuccess = grabEventAndContinueHandler;
+
+      let event = yield;
+
+      let db = event.target.result;
+
+      for each (let autoIncrement in [false, true]) {
+        let objectStore =
+          db.createObjectStore(autoIncrement, { keyPath: "id",
+                                                autoIncrement: autoIncrement });
+        objectStore.createIndex("", "index", { unique: true });
+
+        for (let i = 0; i < 10; i++) {
+          objectStore.add({ id: i, index: i });
+        }
+      }
+
+      event = yield;
+      is(event.type, "success", "expect a success event");
+
+      for each (let autoIncrement in [false, true]) {
+        objectStore = db.transaction(autoIncrement, IDBTransaction.READ_WRITE)
+                        .objectStore(autoIncrement);
+
+        request = objectStore.put({ id: 5, index: 6 });
+        request.onsuccess = unexpectedSuccessHandler;
+        request.onerror = new ExpectError(IDBDatabaseException.CONSTRAINT_ERR);
+        event = yield;
+
+        event.preventDefault();
+
+        let keyRange = IDBKeyRange.only(5);
+
+        objectStore.index("").openCursor(keyRange).onsuccess = function(event) {
+          let cursor = event.target.result;
+          ok(cursor, "Must have a cursor here");
+
+          is(cursor.value.index, 5, "Still have the right index value");
+
+          cursor.value.index = 6;
+
+          request = cursor.update(cursor.value);
+          request.onsuccess = unexpectedSuccessHandler;
+          request.onerror =
+            new ExpectError(IDBDatabaseException.CONSTRAINT_ERR);
+        };
+
+        yield;
+      }
+
+      finishTest();
+      yield;
+    }
+
+  </script>
+  <script type="text/javascript;version=1.7" src="helpers.js"></script>
+
+</head>
+
+<body onload="runTest();"></body>
+
+</html>