Bug 1647378 - stop using mStartupWriteInitiated in the gtest for startupcache, r?dthayer draft
authorGijs Kruitbosch <gijskruitbosch@gmail.com>
Tue, 23 Jun 2020 23:11:34 +0000
changeset 2999927 57a19a826dd619dfd8bc2fe55da855ec3b015cdb
parent 2997042 b1146cce5053bf690a8b19f8da7ca97b1f37438d
child 2999928 79e108be96f445be060cecb2614e4a7c0be63336
push id558581
push userreviewbot
push dateTue, 23 Jun 2020 23:11:58 +0000
treeherdertry@79e108be96f4 [default view] [failures only]
reviewersdthayer
bugs1647378
milestone79.0a1
Bug 1647378 - stop using mStartupWriteInitiated in the gtest for startupcache, r?dthayer Summary: Test Plan: Reviewers: dthayer Subscribers: Bug #: 1647378 Differential Diff: PHID-DIFF-57y5fcnvqfcccqb5j7wk
startupcache/StartupCache.cpp
startupcache/StartupCache.h
--- a/startupcache/StartupCache.cpp
+++ b/startupcache/StartupCache.cpp
@@ -143,17 +143,16 @@ bool StartupCache::gIgnoreDiskCache;
 bool StartupCache::gFoundDiskCacheOnInit;
 
 NS_IMPL_ISUPPORTS(StartupCache, nsIMemoryReporter)
 
 StartupCache::StartupCache()
     : mTableLock("StartupCache::mTableLock"),
       mDirty(false),
       mWrittenOnce(false),
-      mStartupWriteInitiated(false),
       mCurTableReferenced(false),
       mRequestedCount(0),
       mCacheEntriesBaseOffset(0),
       mPrefetchThread(nullptr) {}
 
 StartupCache::~StartupCache() { UnregisterWeakMemoryReporter(this); }
 
 nsresult StartupCache::Init() {
@@ -488,17 +487,16 @@ size_t StartupCache::HeapSizeOfIncluding
 /**
  * WriteToDisk writes the cache out to disk. Callers of WriteToDisk need to call
  * WaitOnWriteComplete to make sure there isn't a write
  * happening on another thread
  */
 Result<Ok, nsresult> StartupCache::WriteToDisk() {
   mTableLock.AssertCurrentThreadOwns();
 
-  mStartupWriteInitiated = true;
   if (!mDirty || mWrittenOnce) {
     return Ok();
   }
 
   if (!mFile) {
     return Err(NS_ERROR_UNEXPECTED);
   }
 
@@ -656,17 +654,16 @@ void StartupCache::EnsureShutdownWriteCo
   if (!mTableLock.TryLock()) {
     // Uh oh, we're writing away from the main thread. Wait to gain the lock,
     // to ensure the write completes.
     mTableLock.Lock();
   } else {
     // We got the lock. Keep the following in sync with
     // MaybeWriteOffMainThread:
     WaitOnPrefetchThread();
-    mStartupWriteInitiated = false;
     mDirty = true;
     mCacheData.reset();
     // Most of this should be redundant given MaybeWriteOffMainThread should
     // have run before now.
 
     auto writeResult = WriteToDisk();
     Unused << NS_WARN_IF(writeResult.isErr());
     // We've had the lock, and `WriteToDisk()` sets mWrittenOnce and mDirty
@@ -735,17 +732,16 @@ void StartupCache::MaybeWriteOffMainThre
   }
 
   if (mCacheData.initialized() && !ShouldCompactCache()) {
     return;
   }
 
   // Keep this code in sync with EnsureShutdownWriteComplete.
   WaitOnPrefetchThread();
-  mStartupWriteInitiated = false;
   mDirty = true;
   mCacheData.reset();
 
   RefPtr<StartupCache> self = this;
   nsCOMPtr<nsIRunnable> runnable =
       NS_NewRunnableFunction("StartupCache::Write", [self]() mutable {
         MutexAutoLock unlock(self->mTableLock);
         auto result = self->WriteToDisk();
@@ -801,17 +797,16 @@ nsresult StartupCache::ResetStartupWrite
   // Wait for 60 seconds, then write out the cache.
   mTimer->InitWithNamedFuncCallback(StartupCache::WriteTimeout, this, 60000,
                                     nsITimer::TYPE_ONE_SHOT,
                                     "StartupCache::WriteTimeout");
   return NS_OK;
 }
 
 nsresult StartupCache::ResetStartupWriteTimer() {
-  mStartupWriteInitiated = false;
   mDirty = true;
   nsresult rv = NS_OK;
   if (!mTimer)
     mTimer = NS_NewTimer();
   else
     rv = mTimer->Cancel();
   NS_ENSURE_SUCCESS(rv, rv);
   // Wait for 60 seconds, then write out the cache.
@@ -819,28 +814,17 @@ nsresult StartupCache::ResetStartupWrite
                                     nsITimer::TYPE_ONE_SHOT,
                                     "StartupCache::WriteTimeout");
   return NS_OK;
 }
 
 // Used only in tests:
 bool StartupCache::StartupWriteComplete() {
   // Need to have written to disk and not added new things since;
-  // if one of those is not the case, return immediately.
-  if (!mStartupWriteInitiated || mDirty) {
-    return false;
-  }
-  // Ensure we can grab the lock, ie we're not
-  // writing to disk on another thread right now.
-  // XXXgijs is there a more idiomatic way of doing this?
-  if (!mTableLock.TryLock()) {
-    return false;
-  }
-  mTableLock.Unlock();
-  return mStartupWriteInitiated && !mDirty;
+  return !mDirty && mWrittenOnce;
 }
 
 // StartupCacheDebugOutputStream implementation
 #ifdef DEBUG
 NS_IMPL_ISUPPORTS(StartupCacheDebugOutputStream, nsIObjectOutputStream,
                   nsIBinaryOutputStream, nsIOutputStream)
 
 bool StartupCacheDebugOutputStream::CheckReferences(nsISupports* aObject) {
--- a/startupcache/StartupCache.h
+++ b/startupcache/StartupCache.h
@@ -219,17 +219,16 @@ class StartupCache : public nsIMemoryRep
   Mutex mTableLock;
 
   nsCOMPtr<nsIObserverService> mObserverService;
   RefPtr<StartupCacheListener> mListener;
   nsCOMPtr<nsITimer> mTimer;
 
   Atomic<bool> mDirty;
   Atomic<bool> mWrittenOnce;
-  bool mStartupWriteInitiated;
   bool mCurTableReferenced;
   uint32_t mRequestedCount;
   size_t mCacheEntriesBaseOffset;
 
   static StaticRefPtr<StartupCache> gStartupCache;
   static bool gShutdownInitiated;
   static bool gIgnoreDiskCache;
   static bool gFoundDiskCacheOnInit;