Bug 1552428 - LSNG: Datastore::PrivateBrowsingClear uses wrong delta for usage updating; r=asuth draft
authorJan Varga <jan.varga@gmail.com>
Fri, 17 May 2019 10:30:19 +0200
changeset 2008611 b4cb17cc1280ed17b454d5bd10a756868b765fd9
parent 2008610 3ce1b3eb8cdf0eec839042d648280dfcafaabaf1
child 2008612 6861192ac08a38c5c39f63de3856e7b2cb137dca
push id363926
push userjvarga@mozilla.com
push dateSat, 18 May 2019 08:19:45 +0000
treeherdertry@e89baae86dd1 [default view] [failures only]
reviewersasuth
bugs1552428
milestone68.0a1
Bug 1552428 - LSNG: Datastore::PrivateBrowsingClear uses wrong delta for usage updating; r=asuth Differential Revision: https://phabricator.services.mozilla.com/D31605
dom/localstorage/ActorsParent.cpp
--- a/dom/localstorage/ActorsParent.cpp
+++ b/dom/localstorage/ActorsParent.cpp
@@ -1673,19 +1673,17 @@ class Datastore final
   DatastoreWriteOptimizer mWriteOptimizer;
   const nsCString mOrigin;
   const uint32_t mPrivateBrowsingId;
   int64_t mUsage;
   int64_t mUpdateBatchUsage;
   int64_t mSizeOfKeys;
   int64_t mSizeOfItems;
   bool mClosed;
-#ifdef DEBUG
   bool mInUpdateBatch;
-#endif
 
  public:
   // Created by PrepareDatastoreOp.
   Datastore(const nsACString& aOrigin, uint32_t aPrivateBrowsingId,
             int64_t aUsage, int64_t aSizeOfKeys, int64_t aSizeOfItems,
             already_AddRefed<DirectoryLock>&& aDirectoryLock,
             already_AddRefed<Connection>&& aConnection,
             already_AddRefed<QuotaObject>&& aQuotaObject,
@@ -1754,18 +1752,16 @@ class Datastore final
    */
   void SetItem(Database* aDatabase, const nsString& aKey,
                const LSValue& aValue);
 
   void RemoveItem(Database* aDatabase, const nsString& aKey);
 
   void Clear(Database* aDatabase);
 
-  void PrivateBrowsingClear();
-
   void BeginUpdateBatch(int64_t aSnapshotInitialUsage);
 
   int64_t EndUpdateBatch(int64_t aSnapshotPeakUsage);
 
   int64_t GetUsage() const { return mUsage; }
 
   int64_t RequestUpdateUsage(int64_t aRequestedSize, int64_t aMinSize);
 
@@ -1790,18 +1786,16 @@ class Datastore final
   void MaybeClose();
 
   void ConnectionClosedCallback();
 
   void CleanupMetadata();
 
   void NotifySnapshots(Database* aDatabase, const nsAString& aKey,
                        const LSValue& aOldValue, bool aAffectsOrder);
-
-  void MarkSnapshotsDirty();
 };
 
 class PreparedDatastore {
   RefPtr<Datastore> mDatastore;
   nsCOMPtr<nsITimer> mTimer;
   const Maybe<ContentParentId> mContentParentId;
   // Strings share buffers if possible, so it's not a problem to duplicate the
   // origin here.
@@ -3651,17 +3645,17 @@ bool RecvLSClearPrivateBrowsing() {
   AssertIsOnBackgroundThread();
 
   if (gDatastores) {
     for (auto iter = gDatastores->ConstIter(); !iter.Done(); iter.Next()) {
       Datastore* datastore = iter.Data();
       MOZ_ASSERT(datastore);
 
       if (datastore->PrivateBrowsingId()) {
-        datastore->PrivateBrowsingClear();
+        datastore->Clear(nullptr);
       }
     }
   }
 
   return true;
 }
 
 namespace localstorage {
@@ -4776,22 +4770,18 @@ Datastore::Datastore(const nsACString& a
       mConnection(std::move(aConnection)),
       mQuotaObject(std::move(aQuotaObject)),
       mOrigin(aOrigin),
       mPrivateBrowsingId(aPrivateBrowsingId),
       mUsage(aUsage),
       mUpdateBatchUsage(-1),
       mSizeOfKeys(aSizeOfKeys),
       mSizeOfItems(aSizeOfItems),
-      mClosed(false)
-#ifdef DEBUG
-      ,
-      mInUpdateBatch(false)
-#endif
-{
+      mClosed(false),
+      mInUpdateBatch(false) {
   AssertIsOnBackgroundThread();
 
   mValues.SwapElements(aValues);
   mOrderedItems.SwapElements(aOrderedItems);
 }
 
 Datastore::~Datastore() {
   AssertIsOnBackgroundThread();
@@ -5242,84 +5232,66 @@ void Datastore::RemoveItem(Database* aDa
     if (IsPersistent()) {
       mConnection->RemoveItem(aKey, delta);
     }
   }
 }
 
 void Datastore::Clear(Database* aDatabase) {
   AssertIsOnBackgroundThread();
-  MOZ_ASSERT(aDatabase);
   MOZ_ASSERT(!mClosed);
-  MOZ_ASSERT(mInUpdateBatch);
 
   if (mValues.Count()) {
     int64_t delta = 0;
     for (auto iter = mValues.ConstIter(); !iter.Done(); iter.Next()) {
       const nsAString& key = iter.Key();
       const LSValue& value = iter.Data();
 
       delta += -static_cast<int64_t>(key.Length()) -
                static_cast<int64_t>(value.UTF16Length());
 
       NotifySnapshots(aDatabase, key, value, /* aAffectsOrder */ true);
     }
 
     mValues.Clear();
 
-    mWriteOptimizer.Truncate();
-
-    mUpdateBatchUsage += delta;
+    if (mInUpdateBatch) {
+      mWriteOptimizer.Truncate();
+
+      mUpdateBatchUsage += delta;
+    } else {
+      mOrderedItems.Clear();
+
+      DebugOnly<bool> ok = UpdateUsage(delta);
+      MOZ_ASSERT(ok);
+    }
 
     mSizeOfKeys = 0;
     mSizeOfItems = 0;
 
     if (IsPersistent()) {
       mConnection->Clear(delta);
     }
   }
 }
 
-void Datastore::PrivateBrowsingClear() {
-  AssertIsOnBackgroundThread();
-  MOZ_ASSERT(mPrivateBrowsingId);
-  MOZ_ASSERT(!mClosed);
-  MOZ_ASSERT(!mInUpdateBatch);
-
-  if (mValues.Count()) {
-    MarkSnapshotsDirty();
-
-    mValues.Clear();
-
-    mOrderedItems.Clear();
-
-    DebugOnly<bool> ok = UpdateUsage(-mSizeOfItems);
-    MOZ_ASSERT(ok);
-
-    mSizeOfKeys = 0;
-    mSizeOfItems = 0;
-  }
-}
-
 void Datastore::BeginUpdateBatch(int64_t aSnapshotInitialUsage) {
   AssertIsOnBackgroundThread();
   MOZ_ASSERT(aSnapshotInitialUsage >= 0);
   MOZ_ASSERT(!mClosed);
   MOZ_ASSERT(mUpdateBatchUsage == -1);
   MOZ_ASSERT(!mInUpdateBatch);
 
   mUpdateBatchUsage = aSnapshotInitialUsage;
 
   if (IsPersistent()) {
     mConnection->BeginUpdateBatch();
   }
 
-#ifdef DEBUG
   mInUpdateBatch = true;
-#endif
 }
 
 int64_t Datastore::EndUpdateBatch(int64_t aSnapshotPeakUsage) {
   AssertIsOnBackgroundThread();
   MOZ_ASSERT(!mClosed);
   MOZ_ASSERT(mInUpdateBatch);
 
   mWriteOptimizer.ApplyAndReset(mOrderedItems);
@@ -5347,19 +5319,17 @@ int64_t Datastore::EndUpdateBatch(int64_
 
   int64_t result = mUpdateBatchUsage;
   mUpdateBatchUsage = -1;
 
   if (IsPersistent()) {
     mConnection->EndUpdateBatch();
   }
 
-#ifdef DEBUG
   mInUpdateBatch = false;
-#endif
 
   return result;
 }
 
 int64_t Datastore::RequestUpdateUsage(int64_t aRequestedSize,
                                       int64_t aMinSize) {
   AssertIsOnBackgroundThread();
   MOZ_ASSERT(aRequestedSize > 0);
@@ -5532,44 +5502,33 @@ void Datastore::CleanupMetadata() {
   if (!gDatastores->Count()) {
     gDatastores = nullptr;
   }
 }
 
 void Datastore::NotifySnapshots(Database* aDatabase, const nsAString& aKey,
                                 const LSValue& aOldValue, bool aAffectsOrder) {
   AssertIsOnBackgroundThread();
-  MOZ_ASSERT(aDatabase);
 
   for (auto iter = mDatabases.ConstIter(); !iter.Done(); iter.Next()) {
     Database* database = iter.Get()->GetKey();
+
+    MOZ_ASSERT(database);
+
     if (database == aDatabase) {
       continue;
     }
 
     Snapshot* snapshot = database->GetSnapshot();
     if (snapshot) {
       snapshot->SaveItem(aKey, aOldValue, aAffectsOrder);
     }
   }
 }
 
-void Datastore::MarkSnapshotsDirty() {
-  AssertIsOnBackgroundThread();
-
-  for (auto iter = mDatabases.ConstIter(); !iter.Done(); iter.Next()) {
-    Database* database = iter.Get()->GetKey();
-
-    Snapshot* snapshot = database->GetSnapshot();
-    if (snapshot) {
-      snapshot->MarkDirty();
-    }
-  }
-}
-
 /*******************************************************************************
  * PreparedDatastore
  ******************************************************************************/
 
 void PreparedDatastore::Destroy() {
   AssertIsOnBackgroundThread();
   MOZ_ASSERT(gPreparedDatastores);
   MOZ_ASSERT(gPreparedDatastores->Get(mDatastoreId));