Bug 1450991 Retry failed ServiceWorkerRegistrar saves and never wipe registrar data if saving fails. r=asuth
authorBen Kelly <ben@wanderview.com>
Fri, 06 Apr 2018 14:50:38 -0700
changeset 457061 7881465e0c6b5b908158a907ccb8f4673b08ea9a
parent 457060 fe356b5f9440b481ae18792fd1b4d57f87c2d5ca
child 457126 e8f1530940a5c38f01955f8fcb886496ae6ca1ae
push id153
push userfmarier@mozilla.com
push dateTue, 10 Apr 2018 02:28:40 +0000
reviewersasuth
bugs1450991
milestone61.0a1
Bug 1450991 Retry failed ServiceWorkerRegistrar saves and never wipe registrar data if saving fails. r=asuth
dom/serviceworkers/ServiceWorkerRegistrar.cpp
dom/serviceworkers/ServiceWorkerRegistrar.h
--- a/dom/serviceworkers/ServiceWorkerRegistrar.cpp
+++ b/dom/serviceworkers/ServiceWorkerRegistrar.cpp
@@ -784,41 +784,54 @@ ServiceWorkerRegistrar::RegisterServiceW
 }
 
 class ServiceWorkerRegistrarSaveDataRunnable final : public Runnable
 {
 public:
   ServiceWorkerRegistrarSaveDataRunnable()
     : Runnable("dom::ServiceWorkerRegistrarSaveDataRunnable")
     , mEventTarget(GetCurrentThreadEventTarget())
+    , mRetryCounter(0)
   {
     AssertIsOnBackgroundThread();
   }
 
   NS_IMETHOD
   Run() override
   {
     RefPtr<ServiceWorkerRegistrar> service = ServiceWorkerRegistrar::Get();
     MOZ_ASSERT(service);
 
-    service->SaveData();
+    // If the save fails, try again once in case it was a temporary
+    // problem due to virus scanning, etc.
+    static const uint32_t kMaxSaveRetryCount = 1;
+
+    nsresult rv = service->SaveData();
+    if (NS_FAILED(rv) && mRetryCounter < kMaxSaveRetryCount) {
+      rv = GetCurrentThreadEventTarget()->Dispatch(this, NS_DISPATCH_NORMAL);
+      if (NS_SUCCEEDED(rv)) {
+        mRetryCounter += 1;
+        return NS_OK;
+      }
+    }
 
     RefPtr<Runnable> runnable =
       NewRunnableMethod("ServiceWorkerRegistrar::DataSaved",
                         service, &ServiceWorkerRegistrar::DataSaved);
-    nsresult rv = mEventTarget->Dispatch(runnable, NS_DISPATCH_NORMAL);
+    rv = mEventTarget->Dispatch(runnable, NS_DISPATCH_NORMAL);
     if (NS_WARN_IF(NS_FAILED(rv))) {
       return rv;
     }
 
     return NS_OK;
   }
 
 private:
   nsCOMPtr<nsIEventTarget> mEventTarget;
+  uint32_t mRetryCounter;
 };
 
 void
 ServiceWorkerRegistrar::ScheduleSaveData()
 {
   AssertIsOnBackgroundThread();
   MOZ_ASSERT(!mShuttingDown);
 
@@ -840,26 +853,30 @@ void
 ServiceWorkerRegistrar::ShutdownCompleted()
 {
   MOZ_ASSERT(NS_IsMainThread());
 
   DebugOnly<nsresult> rv = GetShutdownPhase()->RemoveBlocker(this);
   MOZ_ASSERT(NS_SUCCEEDED(rv));
 }
 
-void
+nsresult
 ServiceWorkerRegistrar::SaveData()
 {
   MOZ_ASSERT(!NS_IsMainThread());
 
   nsresult rv = WriteData();
   if (NS_FAILED(rv)) {
     NS_WARNING("Failed to write data for the ServiceWorker Registations.");
-    DeleteData();
+    // Don't touch the file or in-memory state.  Writing files can
+    // sometimes fail due to virus scanning, etc.  We should just leave
+    // things as is so the next save operation can pick up any changes
+    // without losing data.
   }
+  return rv;
 }
 
 void
 ServiceWorkerRegistrar::DataSaved()
 {
   AssertIsOnBackgroundThread();
   MOZ_ASSERT(mRunnableCounter);
 
--- a/dom/serviceworkers/ServiceWorkerRegistrar.h
+++ b/dom/serviceworkers/ServiceWorkerRegistrar.h
@@ -58,17 +58,17 @@ public:
   void UnregisterServiceWorker(const mozilla::ipc::PrincipalInfo& aPrincipalInfo,
                                const nsACString& aScope);
   void RemoveAll();
 
 protected:
   // These methods are protected because we test this class using gTest
   // subclassing it.
   void LoadData();
-  void SaveData();
+  nsresult SaveData();
 
   nsresult ReadData();
   nsresult WriteData();
   void DeleteData();
 
   void RegisterServiceWorkerInternal(const ServiceWorkerRegistrationData& aData);
 
   ServiceWorkerRegistrar();