Bug 1452244 Avoid empty entries in serviceworker.txt. r=asuth

dom/serviceworkers/ServiceWorkerManager.cpp

@@ -87,6 +87,7 @@
 #include "ServiceWorkerUnregisterJob.h"
 #include "ServiceWorkerUpdateJob.h"
 #include "ServiceWorkerUpdaterChild.h"
+#include "ServiceWorkerUtils.h"
 
 #ifdef PostMessage
 #undef PostMessage
@@ -196,27 +197,28 @@ PopulateRegistrationData(nsIPrincipal* aPrincipal,
 
   aData.scope() = aRegistration->Scope();
 
-  RefPtr<ServiceWorkerInfo> newest = aRegistration->Newest();
-  if (NS_WARN_IF(!newest)) {
+  // TODO: When bug 1426401 is implemented we will need to handle more
+  //       than just the active worker here.
+  RefPtr<ServiceWorkerInfo> active = aRegistration->GetActive();
+  MOZ_ASSERT(active);
+  if (NS_WARN_IF(!active)) {
     return NS_ERROR_FAILURE;
   }
 
-  if (aRegistration->GetActive()) {
-    aData.currentWorkerURL() = aRegistration->GetActive()->ScriptSpec();
-    aData.cacheName() = aRegistration->GetActive()->CacheName();
-    aData.currentWorkerHandlesFetch() = aRegistration->GetActive()->HandlesFetch();
+  aData.currentWorkerURL() = active->ScriptSpec();
+  aData.cacheName() = active->CacheName();
+  aData.currentWorkerHandlesFetch() = active->HandlesFetch();
 
-    aData.currentWorkerInstalledTime() =
-      aRegistration->GetActive()->GetInstalledTime();
-    aData.currentWorkerActivatedTime() =
-      aRegistration->GetActive()->GetActivatedTime();
-  }
+  aData.currentWorkerInstalledTime() = active->GetInstalledTime();
+  aData.currentWorkerActivatedTime() = active->GetActivatedTime();
 
   aData.updateViaCache() =
     static_cast<uint32_t>(aRegistration->GetUpdateViaCache());
 
   aData.lastUpdateTime() = aRegistration->GetLastUpdateTime();
 
+  MOZ_ASSERT(ServiceWorkerRegistrationDataIsValid(aData));
+
   return NS_OK;
 }
 

dom/serviceworkers/ServiceWorkerRegisterJob.cpp

@@ -40,13 +40,8 @@ ServiceWorkerRegisterJob::AsyncExecute()
     bool sameUVC = GetUpdateViaCache() == registration->GetUpdateViaCache();
     registration->SetUpdateViaCache(GetUpdateViaCache());
 
-    // If we are resurrecting an uninstalling registration, then persist
-    // it to disk again.  We preemptively removed it earlier during
-    // unregister so that closing the window by shutting down the browser
-    // results in the registration being gone on restart.
     if (registration->IsPendingUninstall()) {
       registration->ClearPendingUninstall();
-      swm->StoreRegistration(mPrincipal, registration);
       // Its possible that a ready promise is created between when the
       // uninstalling flag is set and when we resurrect the registration
       // here.  In that case we might need to fire the ready promise

dom/serviceworkers/ServiceWorkerRegistrar.cpp

@@ -689,6 +689,12 @@ ServiceWorkerRegistrar::ReadData()
 
   // Copy data over to mData.
   for (uint32_t i = 0; i < tmpData.Length(); ++i) {
+    // Older versions could sometimes write out empty, useless entries.
+    // Prune those here.
+    if (!ServiceWorkerRegistrationDataIsValid(tmpData[i])) {
+      continue;
+    }
+
     bool match = false;
     if (dedupe) {
       MOZ_ASSERT(overwrite);
@@ -779,6 +785,7 @@ ServiceWorkerRegistrar::RegisterServiceWorkerInternal(const ServiceWorkerRegistr
   }
 
   if (!found) {
+    MOZ_ASSERT(ServiceWorkerRegistrationDataIsValid(aData));
     mData.AppendElement(aData);
   }
 }
@@ -956,6 +963,12 @@ ServiceWorkerRegistrar::WriteData()
   }
 
   for (uint32_t i = 0, len = data.Length(); i < len; ++i) {
+    // We have an assertion further up the stack, but as a last
+    // resort avoid writing out broken entries here.
+    if (!ServiceWorkerRegistrationDataIsValid(data[i])) {
+      continue;
+    }
+
     const mozilla::ipc::PrincipalInfo& info = data[i].principal();
 
     MOZ_ASSERT(info.type() == mozilla::ipc::PrincipalInfo::TContentPrincipalInfo);

dom/serviceworkers/ServiceWorkerRegistrationInfo.cpp

@@ -141,6 +141,16 @@ ServiceWorkerRegistrationInfo::SetPendingUninstall()
 void
 ServiceWorkerRegistrationInfo::ClearPendingUninstall()
 {
+  // If we are resurrecting an uninstalling registration, then persist
+  // it to disk again.  We preemptively removed it earlier during
+  // unregister so that closing the window by shutting down the browser
+  // results in the registration being gone on restart.
+  if (mPendingUninstall && mActiveWorker) {
+    RefPtr<ServiceWorkerManager> swm = ServiceWorkerManager::GetInstance();
+    if (swm) {
+      swm->StoreRegistration(mPrincipal, this);
+    }
+  }
   mPendingUninstall = false;
 }
 
@@ -607,12 +617,8 @@ ServiceWorkerRegistrationInfo::TransitionInstallingToWaiting()
 
   NotifyChromeRegistrationListeners();
 
-  RefPtr<ServiceWorkerManager> swm = ServiceWorkerManager::GetInstance();
-  if (!swm) {
-    // browser shutdown began
-    return;
-  }
-  swm->StoreRegistration(mPrincipal, this);
+  // TODO: When bug 1426401 is implemented we will need to call
+  //       StoreRegistration() here to persist the waiting worker.
 }
 
 void

dom/serviceworkers/ServiceWorkerUtils.cpp

@@ -32,5 +32,13 @@ ServiceWorkerParentInterceptEnabled()
   return sEnabled;
 }
 
+bool
+ServiceWorkerRegistrationDataIsValid(const ServiceWorkerRegistrationData& aData)
+{
+  return !aData.scope().IsEmpty() &&
+         !aData.currentWorkerURL().IsEmpty() &&
+         !aData.cacheName().IsEmpty();
+}
+
 } // namespace dom
 } // namespace mozilla

dom/serviceworkers/ServiceWorkerUtils.h

@@ -12,6 +12,7 @@
 namespace mozilla {
 namespace dom {
 
+class ServiceWorkerRegistrationData;
 class ServiceWorkerRegistrationDescriptor;
 
 typedef MozPromise<ServiceWorkerRegistrationDescriptor, nsresult, false>
@@ -20,6 +21,9 @@ typedef MozPromise<ServiceWorkerRegistrationDescriptor, nsresult, false>
 bool
 ServiceWorkerParentInterceptEnabled();
 
+bool
+ServiceWorkerRegistrationDataIsValid(const ServiceWorkerRegistrationData& aData);
+
 } // namespace dom
 } // namespace mozilla