Bug 1330984 Handle mProfileDir being cleared in ServiceWorkerRegistrar. r=baku
authorBen Kelly <ben@wanderview.com>
Mon, 16 Jan 2017 14:30:12 -0500
changeset 329609 d794907ebb1561a92cbeaf0745d7071b6de2b626
parent 329608 0878ddbf3dd461b10f044a3ec0783a63eeb88355
child 329610 08866fd222c6e37c7c02a0c7a8229121e554b393
push id85745
push userbkelly@mozilla.com
push dateMon, 16 Jan 2017 19:30:18 +0000
treeherdermozilla-inbound@d794907ebb15 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersbaku
bugs1330984
milestone53.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 1330984 Handle mProfileDir being cleared in ServiceWorkerRegistrar. r=baku
dom/workers/ServiceWorkerRegistrar.cpp
dom/workers/ServiceWorkerRegistrar.h
dom/workers/test/gtest/TestReadWrite.cpp
--- a/dom/workers/ServiceWorkerRegistrar.cpp
+++ b/dom/workers/ServiceWorkerRegistrar.cpp
@@ -105,42 +105,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);
   }
 }
@@ -269,25 +267,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;
@@ -562,30 +567,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;
   }
@@ -724,25 +732,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);
@@ -830,17 +845,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 =
@@ -855,16 +872,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)
   {