Bug 1330984 Handle mProfileDir being cleared in ServiceWorkerRegistrar. r=baku a=jcristau
authorBen Kelly <ben@wanderview.com>
Tue, 17 Jan 2017 08:00:48 -0800
changeset 353614 412036d99f78afc1e68c1d7088a3ce1fa275405e
parent 353613 2356029b0bb5085b468d0c6a1acc58e8c664fb12
child 353615 f083e5c687ff775488c807bea1f1cec756ea0f56
push id6795
push userjlund@mozilla.com
push dateMon, 23 Jan 2017 14:19:46 +0000
treeherdermozilla-esr52@76101b503191 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersbaku, jcristau
bugs1330984
milestone52.0a2
Bug 1330984 Handle mProfileDir being cleared in ServiceWorkerRegistrar. r=baku a=jcristau
dom/workers/ServiceWorkerRegistrar.cpp
dom/workers/ServiceWorkerRegistrar.h
dom/workers/test/gtest/TestReadWrite.cpp
--- a/dom/workers/ServiceWorkerRegistrar.cpp
+++ b/dom/workers/ServiceWorkerRegistrar.cpp
@@ -103,42 +103,40 @@ ServiceWorkerRegistrar::~ServiceWorkerRe
 
 void
 ServiceWorkerRegistrar::GetRegistrations(
                                nsTArray<ServiceWorkerRegistrationData>& aValues)
 {
   MOZ_ASSERT(NS_IsMainThread());
   MOZ_ASSERT(aValues.IsEmpty());
 
+  MonitorAutoLock lock(mMonitor);
+
   // If we don't have the profile directory, profile is not started yet (and
   // probably we are in a utest).
   if (!mProfileDir) {
     return;
   }
 
   // We care just about the first execution because this can be blocked by
   // loading data from disk.
   static bool firstTime = true;
   TimeStamp startTime;
 
   if (firstTime) {
     startTime = TimeStamp::NowLoRes();
   }
 
-  {
-    MonitorAutoLock lock(mMonitor);
+  // Waiting for data loaded.
+  mMonitor.AssertCurrentThreadOwns();
+  while (!mDataLoaded) {
+    mMonitor.Wait();
+  }
 
-    // Waiting for data loaded.
-    mMonitor.AssertCurrentThreadOwns();
-    while (!mDataLoaded) {
-      mMonitor.Wait();
-    }
-
-    aValues.AppendElements(mData);
-  }
+  aValues.AppendElements(mData);
 
   if (firstTime) {
     firstTime = false;
     Telemetry::AccumulateTimeDelta(
       Telemetry::SERVICE_WORKER_REGISTRATION_LOADING,
       startTime);
   }
 }
@@ -267,25 +265,32 @@ ServiceWorkerRegistrar::LoadData()
 }
 
 nsresult
 ServiceWorkerRegistrar::ReadData()
 {
   // We cannot assert about the correct thread because normally this method
   // runs on a IO thread, but in gTests we call it from the main-thread.
 
-  MOZ_ASSERT(mProfileDir);
+  nsCOMPtr<nsIFile> file;
+
+  {
+    MonitorAutoLock lock(mMonitor);
 
-  nsCOMPtr<nsIFile> file;
-  nsresult rv = mProfileDir->Clone(getter_AddRefs(file));
-  if (NS_WARN_IF(NS_FAILED(rv))) {
-    return rv;
+    if (!mProfileDir) {
+      return NS_ERROR_FAILURE;
+    }
+
+    nsresult rv = mProfileDir->Clone(getter_AddRefs(file));
+    if (NS_WARN_IF(NS_FAILED(rv))) {
+      return rv;
+    }
   }
 
-  rv = file->Append(NS_LITERAL_STRING(SERVICEWORKERREGISTRAR_FILE));
+  nsresult rv = file->Append(NS_LITERAL_STRING(SERVICEWORKERREGISTRAR_FILE));
   if (NS_WARN_IF(NS_FAILED(rv))) {
     return rv;
   }
 
   bool exists;
   rv = file->Exists(&exists);
   if (NS_WARN_IF(NS_FAILED(rv))) {
     return rv;
@@ -471,30 +476,33 @@ ServiceWorkerRegistrar::ReadData()
 }
 
 void
 ServiceWorkerRegistrar::DeleteData()
 {
   // We cannot assert about the correct thread because normally this method
   // runs on a IO thread, but in gTests we call it from the main-thread.
 
-  MOZ_ASSERT(mProfileDir);
+  nsCOMPtr<nsIFile> file;
 
   {
     MonitorAutoLock lock(mMonitor);
     mData.Clear();
+
+    if (!mProfileDir) {
+      return;
+    }
+
+    nsresult rv = mProfileDir->Clone(getter_AddRefs(file));
+    if (NS_WARN_IF(NS_FAILED(rv))) {
+      return;
+    }
   }
 
-  nsCOMPtr<nsIFile> file;
-  nsresult rv = mProfileDir->Clone(getter_AddRefs(file));
-  if (NS_WARN_IF(NS_FAILED(rv))) {
-    return;
-  }
-
-  rv = file->Append(NS_LITERAL_STRING(SERVICEWORKERREGISTRAR_FILE));
+  nsresult rv = file->Append(NS_LITERAL_STRING(SERVICEWORKERREGISTRAR_FILE));
   if (NS_WARN_IF(NS_FAILED(rv))) {
     return;
   }
 
   rv = file->Remove(false);
   if (rv == NS_ERROR_FILE_NOT_FOUND) {
     return;
   }
@@ -633,25 +641,32 @@ ServiceWorkerRegistrar::IsSupportedVersi
 }
 
 nsresult
 ServiceWorkerRegistrar::WriteData()
 {
   // We cannot assert about the correct thread because normally this method
   // runs on a IO thread, but in gTests we call it from the main-thread.
 
-  MOZ_ASSERT(mProfileDir);
+  nsCOMPtr<nsIFile> file;
+
+  {
+    MonitorAutoLock lock(mMonitor);
 
-  nsCOMPtr<nsIFile> file;
-  nsresult rv = mProfileDir->Clone(getter_AddRefs(file));
-  if (NS_WARN_IF(NS_FAILED(rv))) {
-    return rv;
+    if (!mProfileDir) {
+      return NS_ERROR_FAILURE;
+    }
+
+    nsresult rv = mProfileDir->Clone(getter_AddRefs(file));
+    if (NS_WARN_IF(NS_FAILED(rv))) {
+      return rv;
+    }
   }
 
-  rv = file->Append(NS_LITERAL_STRING(SERVICEWORKERREGISTRAR_FILE));
+  nsresult rv = file->Append(NS_LITERAL_STRING(SERVICEWORKERREGISTRAR_FILE));
   if (NS_WARN_IF(NS_FAILED(rv))) {
     return rv;
   }
 
   // We need a lock to take a snapshot of the data.
   nsTArray<ServiceWorkerRegistrationData> data;
   {
     MonitorAutoLock lock(mMonitor);
@@ -725,17 +740,19 @@ ServiceWorkerRegistrar::WriteData()
 
   return NS_OK;
 }
 
 void
 ServiceWorkerRegistrar::ProfileStarted()
 {
   MOZ_ASSERT(NS_IsMainThread());
-  MOZ_ASSERT(!mProfileDir);
+
+  MonitorAutoLock lock(mMonitor);
+  MOZ_DIAGNOSTIC_ASSERT(!mProfileDir);
 
   nsresult rv = NS_GetSpecialDirectory(NS_APP_USER_PROFILE_50_DIR,
                                        getter_AddRefs(mProfileDir));
   if (NS_WARN_IF(NS_FAILED(rv))) {
     return;
   }
 
   nsCOMPtr<nsIEventTarget> target =
@@ -750,16 +767,18 @@ ServiceWorkerRegistrar::ProfileStarted()
   }
 }
 
 void
 ServiceWorkerRegistrar::ProfileStopped()
 {
   MOZ_ASSERT(NS_IsMainThread());
 
+  MonitorAutoLock lock(mMonitor);
+
   if (!mProfileDir) {
     nsresult rv = NS_GetSpecialDirectory(NS_APP_USER_PROFILE_50_DIR,
                                          getter_AddRefs(mProfileDir));
     if (NS_WARN_IF(NS_FAILED(rv))) {
       return;
     }
   }
 
--- a/dom/workers/ServiceWorkerRegistrar.h
+++ b/dom/workers/ServiceWorkerRegistrar.h
@@ -79,23 +79,23 @@ private:
   void ShutdownCompleted();
   void MaybeScheduleShutdownCompleted();
 
   bool IsSupportedVersion(const nsACString& aVersion) const;
 
   mozilla::Monitor mMonitor;
 
 protected:
-  // mData and mDataLoaded are protected by mMonitor.
+  // protected by mMonitor.
+  nsCOMPtr<nsIFile> mProfileDir;
   nsTArray<ServiceWorkerRegistrationData> mData;
   bool mDataLoaded;
 
+  // PBackground thread only
   bool mShuttingDown;
   bool* mShutdownCompleteFlag;
   uint32_t mRunnableCounter;
-
-  nsCOMPtr<nsIFile> mProfileDir;
 };
 
 } // namespace dom
 } // namespace mozilla
 
 #endif // mozilla_dom_workers_ServiceWorkerRegistrar_h
--- a/dom/workers/test/gtest/TestReadWrite.cpp
+++ b/dom/workers/test/gtest/TestReadWrite.cpp
@@ -21,19 +21,18 @@ using namespace mozilla::ipc;
 
 class ServiceWorkerRegistrarTest : public ServiceWorkerRegistrar
 {
 public:
   ServiceWorkerRegistrarTest()
   {
     nsresult rv = NS_GetSpecialDirectory(NS_APP_USER_PROFILE_50_DIR,
                                        getter_AddRefs(mProfileDir));
-    if (NS_WARN_IF(NS_FAILED(rv))) {
-      return;
-    }
+    MOZ_DIAGNOSTIC_ASSERT(NS_SUCCEEDED(rv));
+    MOZ_DIAGNOSTIC_ASSERT(mProfileDir);
   }
 
   nsresult TestReadData() { return ReadData(); }
   nsresult TestWriteData() { return WriteData(); }
   void TestDeleteData() { DeleteData(); }
 
   void TestRegisterServiceWorker(const ServiceWorkerRegistrationData& aData)
   {