Bug 1286798 - Part 30: Preserve key order when sending items to a content process; r=asuth
authorJan Varga <jan.varga@gmail.com>
Thu, 29 Nov 2018 21:48:51 +0100
changeset 508028 e2b5bee9812c4b8f0f2344eb848d85696886151d
parent 508027 70ba8b2410f833ec27655fb3cb6f69f7bb01fbcb
child 508029 bc288ab2655c2cf6d2404fd204b7afa1d67262d2
push id1905
push userffxbld-merge
push dateMon, 21 Jan 2019 12:33:13 +0000
treeherdermozilla-release@c2fca1944d8c [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersasuth
bugs1286798
milestone65.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 1286798 - Part 30: Preserve key order when sending items to a content process; r=asuth Keys needs to be redundantly stored in an array, so we can construct identical hashtable of values in a content process.
dom/localstorage/ActorsParent.cpp
dom/localstorage/test/unit/test_snapshotting.js
dom/localstorage/test/unit/xpcshell.ini
--- a/dom/localstorage/ActorsParent.cpp
+++ b/dom/localstorage/ActorsParent.cpp
@@ -1025,16 +1025,19 @@ class Datastore final
   RefPtr<QuotaObject> mQuotaObject;
   nsCOMPtr<nsITimer> mAutoCommitTimer;
   nsCOMPtr<nsIRunnable> mCompleteCallback;
   nsTHashtable<nsPtrHashKey<PrepareDatastoreOp>> mPrepareDatastoreOps;
   nsTHashtable<nsPtrHashKey<PreparedDatastore>> mPreparedDatastores;
   nsTHashtable<nsPtrHashKey<Database>> mDatabases;
   nsTHashtable<nsPtrHashKey<Database>> mActiveDatabases;
   nsDataHashtable<nsStringHashKey, nsString> mValues;
+  nsDataHashtable<nsStringHashKey, uint32_t> mUpdateBatchRemovals;
+  nsTArray<nsString> mUpdateBatchAppends;
+  nsTArray<nsString> mKeys;
   nsTArray<int64_t> mPendingUsageDeltas;
   const nsCString mOrigin;
   const uint32_t mPrivateBrowsingId;
   int64_t mUsage;
   int64_t mUpdateBatchUsage;
   bool mClosed;
 #ifdef DEBUG
   bool mInUpdateBatch;
@@ -1043,17 +1046,18 @@ class Datastore final
 public:
   // Created by PrepareDatastoreOp.
   Datastore(const nsACString& aOrigin,
             uint32_t aPrivateBrowsingId,
             int64_t aUsage,
             already_AddRefed<DirectoryLock>&& aDirectoryLock,
             already_AddRefed<Connection>&& aConnection,
             already_AddRefed<QuotaObject>&& aQuotaObject,
-            nsDataHashtable<nsStringHashKey, nsString>& aValues);
+            nsDataHashtable<nsStringHashKey, nsString>& aValues,
+            nsTArray<nsString>& aKeys);
 
   const nsCString&
   Origin() const
   {
     return mOrigin;
   }
 
   uint32_t
@@ -1613,16 +1617,17 @@ class PrepareDatastoreOp
   nsCOMPtr<nsIEventTarget> mMainEventTarget;
   RefPtr<PrepareDatastoreOp> mDelayedOp;
   RefPtr<DirectoryLock> mDirectoryLock;
   RefPtr<Connection> mConnection;
   RefPtr<Datastore> mDatastore;
   nsAutoPtr<ArchivedOriginInfo> mArchivedOriginInfo;
   LoadDataOp* mLoadDataOp;
   nsDataHashtable<nsStringHashKey, nsString> mValues;
+  nsTArray<nsString> mKeys;
   const LSRequestPrepareDatastoreParams mParams;
   nsCString mSuffix;
   nsCString mGroup;
   nsCString mMainThreadOrigin;
   nsCString mOrigin;
   nsString mDatabaseFilePath;
   uint32_t mPrivateBrowsingId;
   int64_t mUsage;
@@ -3046,32 +3051,34 @@ ClearOp::DoDatastoreWork()
  ******************************************************************************/
 
 Datastore::Datastore(const nsACString& aOrigin,
                      uint32_t aPrivateBrowsingId,
                      int64_t aUsage,
                      already_AddRefed<DirectoryLock>&& aDirectoryLock,
                      already_AddRefed<Connection>&& aConnection,
                      already_AddRefed<QuotaObject>&& aQuotaObject,
-                     nsDataHashtable<nsStringHashKey, nsString>& aValues)
+                     nsDataHashtable<nsStringHashKey, nsString>& aValues,
+                     nsTArray<nsString>& aKeys)
   : mDirectoryLock(std::move(aDirectoryLock))
   , mConnection(std::move(aConnection))
   , mQuotaObject(std::move(aQuotaObject))
   , mOrigin(aOrigin)
   , mPrivateBrowsingId(aPrivateBrowsingId)
   , mUsage(aUsage)
   , mUpdateBatchUsage(-1)
   , mClosed(false)
 #ifdef DEBUG
   , mInUpdateBatch(false)
 #endif
 {
   AssertIsOnBackgroundThread();
 
   mValues.SwapElements(aValues);
+  mKeys.SwapElements(aKeys);
 }
 
 Datastore::~Datastore()
 {
   AssertIsOnBackgroundThread();
   MOZ_ASSERT(mClosed);
 }
 
@@ -3269,20 +3276,24 @@ Datastore::NoteInactiveDatabase(Database
 
     mPendingUsageDeltas.Clear();
   }
 }
 
 void
 Datastore::GetItemInfos(nsTArray<LSItemInfo>* aItemInfos)
 {
-  for (auto iter = mValues.ConstIter(); !iter.Done(); iter.Next()) {
+  for (auto key : mKeys) {
+    nsString value;
+    DebugOnly<bool> hasValue = mValues.Get(key, &value);
+    MOZ_ASSERT(hasValue);
+
     LSItemInfo* itemInfo = aItemInfos->AppendElement();
-    itemInfo->key() = iter.Key();
-    itemInfo->value() = iter.Data();
+    itemInfo->key() = key;
+    itemInfo->value() = value;
   }
 }
 
 void
 Datastore::GetItem(const nsString& aKey, nsString& aValue) const
 {
   AssertIsOnBackgroundThread();
   MOZ_ASSERT(!mClosed);
@@ -3308,16 +3319,18 @@ Datastore::SetItem(Database* aDatabase,
   GetItem(aKey, oldValue);
 
   if (oldValue != aValue || oldValue.IsVoid() != aValue.IsVoid()) {
     int64_t delta = static_cast<int64_t>(aValue.Length()) -
                     static_cast<int64_t>(oldValue.Length());
 
     if (oldValue.IsVoid()) {
       delta += static_cast<int64_t>(aKey.Length());
+
+      mUpdateBatchAppends.AppendElement(aKey);
     }
 
     mUpdateBatchUsage += delta;
 
     mValues.Put(aKey, aValue);
 
     if (IsPersistent()) {
       EnsureTransaction();
@@ -3340,16 +3353,23 @@ Datastore::RemoveItem(Database* aDatabas
   MOZ_ASSERT(aDatabase);
   MOZ_ASSERT(!mClosed);
   MOZ_ASSERT(mInUpdateBatch);
 
   nsString oldValue;
   GetItem(aKey, oldValue);
 
   if (!oldValue.IsVoid()) {
+    auto entry = mUpdateBatchRemovals.LookupForAdd(aKey);
+    if (entry) {
+      entry.Data()++;
+    } else {
+      entry.OrInsert([]() { return 1; });
+    }
+
     int64_t delta = -(static_cast<int64_t>(aKey.Length()) +
                       static_cast<int64_t>(oldValue.Length()));
 
     mUpdateBatchUsage += delta;
 
     mValues.Remove(aKey);
 
     if (IsPersistent()) {
@@ -3368,27 +3388,31 @@ Datastore::Clear(Database* aDatabase,
                  const nsString& aDocumentURI)
 {
   AssertIsOnBackgroundThread();
   MOZ_ASSERT(aDatabase);
   MOZ_ASSERT(!mClosed);
   MOZ_ASSERT(mInUpdateBatch);
 
   if (mValues.Count()) {
+    mUpdateBatchRemovals.Clear();
+    mUpdateBatchAppends.Clear();
+
     for (auto iter = mValues.ConstIter(); !iter.Done(); iter.Next()) {
       const nsAString& key = iter.Key();
       const nsAString& value = iter.Data();
 
       int64_t delta = -(static_cast<int64_t>(key.Length()) +
                         static_cast<int64_t>(value.Length()));
 
       mUpdateBatchUsage += delta;
     }
 
     mValues.Clear();
+    mKeys.Clear();
 
     if (IsPersistent()) {
       EnsureTransaction();
 
       RefPtr<ClearOp> op = new ClearOp(mConnection);
       mConnection->Dispatch(op);
     }
   }
@@ -3407,16 +3431,17 @@ Datastore::PrivateBrowsingClear()
   MOZ_ASSERT(mPrivateBrowsingId);
   MOZ_ASSERT(!mClosed);
 
   if (mValues.Count()) {
     DebugOnly<bool> ok = UpdateUsage(-mUsage);
     MOZ_ASSERT(ok);
 
     mValues.Clear();
+    mKeys.Clear();
   }
 }
 
 void
 Datastore::BeginUpdateBatch(int64_t aSnapshotInitialUsage)
 {
   AssertIsOnBackgroundThread();
   MOZ_ASSERT(aSnapshotInitialUsage >= 0);
@@ -3434,16 +3459,37 @@ Datastore::BeginUpdateBatch(int64_t aSna
 void
 Datastore::EndUpdateBatch(int64_t aSnapshotPeakUsage)
 {
   AssertIsOnBackgroundThread();
   MOZ_ASSERT(aSnapshotPeakUsage >= 0);
   MOZ_ASSERT(!mClosed);
   MOZ_ASSERT(mInUpdateBatch);
 
+  if (mUpdateBatchAppends.Length()) {
+    mKeys.AppendElements(std::move(mUpdateBatchAppends));
+  }
+
+  if (mUpdateBatchRemovals.Count()) {
+    RefPtr<Datastore> self = this;
+
+    mKeys.RemoveElementsBy([self](const nsString& aKey) {
+      if (auto entry = self->mUpdateBatchRemovals.Lookup(aKey)) {
+        if (--entry.Data() == 0) {
+          entry.Remove();
+        }
+        return true;
+      }
+      return false;
+    });
+  }
+
+  MOZ_ASSERT(mUpdateBatchAppends.Length() == 0);
+  MOZ_ASSERT(mUpdateBatchRemovals.Count() == 0);
+
   int64_t delta = mUpdateBatchUsage - aSnapshotPeakUsage;
 
   if (mActiveDatabases.Count()) {
     // We can't apply deltas while other databases are still active.
     // The final delta must be zero or negative, but individual deltas can be
     // positive. A positive delta can't be applied asynchronously since there's
     // no way to fire the quota exceeded error event.
 
@@ -5080,17 +5126,18 @@ PrepareDatastoreOp::GetResponse(LSReques
     }
 
     mDatastore = new Datastore(mOrigin,
                                mPrivateBrowsingId,
                                mUsage,
                                mDirectoryLock.forget(),
                                mConnection.forget(),
                                quotaObject.forget(),
-                               mValues);
+                               mValues,
+                               mKeys);
 
     mDatastore->NoteLivePrepareDatastoreOp(this);
 
     if (!gDatastores) {
       gDatastores = new DatastoreHashtable();
     }
 
     MOZ_ASSERT(!gDatastores->Get(mOrigin));
@@ -5290,16 +5337,17 @@ LoadDataOp::DoDatastoreWork()
 
     nsString value;
     rv = stmt->GetString(1, value);
     if (NS_WARN_IF(NS_FAILED(rv))) {
       return rv;
     }
 
     mPrepareDatastoreOp->mValues.Put(key, value);
+    mPrepareDatastoreOp->mKeys.AppendElement(key);
 #ifdef DEBUG
     mPrepareDatastoreOp->mDEBUGUsage += key.Length() + value.Length();
 #endif
   }
   if (NS_WARN_IF(NS_FAILED(rv))) {
     return rv;
   }
 
new file mode 100644
--- /dev/null
+++ b/dom/localstorage/test/unit/test_snapshotting.js
@@ -0,0 +1,50 @@
+/**
+ * Any copyright is dedicated to the Public Domain.
+ * http://creativecommons.org/publicdomain/zero/1.0/
+ */
+
+var testGenerator = testSteps();
+
+function* testSteps()
+{
+  const url = "http://example.com";
+
+  const items = [
+    { key: "key1", value: "value1" },
+    { key: "key2", value: "value2" },
+    { key: "key3", value: "value3" },
+    { key: "key4", value: "value4" },
+    { key: "key5", value: "value5" }
+  ];
+
+  info("Getting storage");
+
+  let storage = getLocalStorage(getPrincipal(url));
+
+  info("Adding data");
+
+  for (let item of items) {
+    storage.setItem(item.key, item.value);
+  }
+
+  info("Saving key order");
+
+  let keys = [];
+  for (let i = 0; i < items.length; i++) {
+    keys.push(storage.key(i));
+  }
+
+  // Let the internal snapshot finish by returning to the event loop.
+  continueToNextStep();
+  yield undefined;
+
+  is(storage.length, items.length, "Correct length");
+
+  info("Verifying key order");
+
+  for (let i = 0; i < items.length; i++) {
+    is(storage.key(i), keys[i], "Correct key");
+  }
+
+  finishTest();
+}
--- a/dom/localstorage/test/unit/xpcshell.ini
+++ b/dom/localstorage/test/unit/xpcshell.ini
@@ -7,8 +7,9 @@ head = head.js
 support-files =
   archive_profile.zip
   migration_profile.zip
 
 [test_archive.js]
 [test_eviction.js]
 [test_groupLimit.js]
 [test_migration.js]
+[test_snapshotting.js]