Bug 1162088 - patch 2 - ServiceWorkerManager should use OriginAttributes from the principal as scopeKey, r=nsm
authorAndrea Marchesini <amarchesini@mozilla.com>
Wed, 03 Jun 2015 09:44:09 +0100
changeset 279326 58aab1e6a65b2dd684fa70a85c85ba721212ace1
parent 279325 8bcfd31f7d5e5e63aa6a105f13dfcc032ccaea76
child 279327 2901436c9047202f7cc30fd89474e1bd2075294d
push id897
push userjlund@mozilla.com
push dateMon, 14 Sep 2015 18:56:12 +0000
treeherdermozilla-release@9411e2d2b214 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersnsm
bugs1162088
milestone41.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 1162088 - patch 2 - ServiceWorkerManager should use OriginAttributes from the principal as scopeKey, r=nsm
dom/interfaces/base/nsIServiceWorkerManager.idl
dom/workers/ServiceWorkerRegistrar.cpp
dom/workers/ServiceWorkerRegistrar.h
dom/workers/test/gtest/TestReadWrite.cpp
ipc/glue/BackgroundParentImpl.cpp
--- a/dom/interfaces/base/nsIServiceWorkerManager.idl
+++ b/dom/interfaces/base/nsIServiceWorkerManager.idl
@@ -28,17 +28,17 @@ interface nsIServiceWorkerInfo : nsISupp
   readonly attribute DOMString scope;
   readonly attribute DOMString scriptSpec;
   readonly attribute DOMString currentWorkerURL;
 
   readonly attribute DOMString activeCacheName;
   readonly attribute DOMString waitingCacheName;
 };
 
-[scriptable, builtinclass, uuid(bfb913ad-177d-49ae-bf62-efd32ca73424)]
+[scriptable, builtinclass, uuid(d130fcbd-1afe-4dd9-b70d-08a4b2373af5)]
 interface nsIServiceWorkerManager : nsISupports
 {
   /**
    * Registers a ServiceWorker with script loaded from `aScriptURI` to act as
    * the ServiceWorker for aScope.  Requires a valid entry settings object on
    * the stack. This means you must call this from content code 'within'
    * a window.
    *
--- a/dom/workers/ServiceWorkerRegistrar.cpp
+++ b/dom/workers/ServiceWorkerRegistrar.cpp
@@ -162,33 +162,36 @@ ServiceWorkerRegistrar::RegisterServiceW
       mData.AppendElement(aData);
     }
   }
 
   ScheduleSaveData();
 }
 
 void
-ServiceWorkerRegistrar::UnregisterServiceWorker(const nsACString& aScope)
+ServiceWorkerRegistrar::UnregisterServiceWorker(
+                                            const PrincipalInfo& aPrincipalInfo,
+                                            const nsACString& aScope)
 {
   AssertIsOnBackgroundThread();
 
   if (mShuttingDown) {
     NS_WARNING("Failed to unregister a serviceWorker during shutting down.");
     return;
   }
 
   bool deleted = false;
 
   {
     MonitorAutoLock lock(mMonitor);
     MOZ_ASSERT(mDataLoaded);
 
     for (uint32_t i = 0; i < mData.Length(); ++i) {
-      if (mData[i].scope() == aScope) {
+      if (mData[i].principal() == aPrincipalInfo &&
+          mData[i].scope() == aScope) {
         mData.RemoveElementAt(i);
         deleted = true;
         break;
       }
     }
   }
 
   if (deleted) {
@@ -276,44 +279,33 @@ ServiceWorkerRegistrar::ReadData()
       return rv;                                      \
     }                                                 \
     if (NS_WARN_IF(!hasMoreLines)) {                  \
       return NS_ERROR_FAILURE;                        \
     }
 
     GET_LINE(line);
 
-    if (line.EqualsLiteral(SERVICEWORKERREGISTRAR_SYSTEM_PRINCIPAL)) {
-      entry->principal() = mozilla::ipc::SystemPrincipalInfo();
-    } else {
-      if (!line.EqualsLiteral(SERVICEWORKERREGISTRAR_CONTENT_PRINCIPAL)) {
-        return NS_ERROR_FAILURE;
-      }
+    uint32_t appId = line.ToInteger(&rv);
+    if (NS_WARN_IF(NS_FAILED(rv))) {
+      return rv;
+    }
 
-      GET_LINE(line);
-
-      uint32_t appId = line.ToInteger(&rv);
-      if (NS_WARN_IF(NS_FAILED(rv))) {
-        return rv;
-      }
+    GET_LINE(line);
 
-      GET_LINE(line);
-
-      if (!line.EqualsLiteral(SERVICEWORKERREGISTRAR_TRUE) &&
-          !line.EqualsLiteral(SERVICEWORKERREGISTRAR_FALSE)) {
-        return NS_ERROR_FAILURE;
-      }
+    if (!line.EqualsLiteral(SERVICEWORKERREGISTRAR_TRUE) &&
+        !line.EqualsLiteral(SERVICEWORKERREGISTRAR_FALSE)) {
+      return NS_ERROR_FAILURE;
+    }
 
-      bool isInBrowserElement = line.EqualsLiteral(SERVICEWORKERREGISTRAR_TRUE);
+    bool isInBrowserElement = line.EqualsLiteral(SERVICEWORKERREGISTRAR_TRUE);
 
-      GET_LINE(line);
-      entry->principal() =
-        mozilla::ipc::ContentPrincipalInfo(appId, isInBrowserElement,
-                                           line);
-    }
+    GET_LINE(line);
+    entry->principal() =
+      mozilla::ipc::ContentPrincipalInfo(appId, isInBrowserElement, line);
 
     GET_LINE(entry->scope());
     GET_LINE(entry->scriptSpec());
     GET_LINE(entry->currentWorkerURL());
 
     nsAutoCString cacheName;
     GET_LINE(cacheName);
     CopyUTF8toUTF16(cacheName, entry->activeCacheName());
@@ -513,39 +505,33 @@ ServiceWorkerRegistrar::WriteData()
 
   if (count != buffer.Length()) {
     return NS_ERROR_UNEXPECTED;
   }
 
   for (uint32_t i = 0, len = data.Length(); i < len; ++i) {
     const mozilla::ipc::PrincipalInfo& info = data[i].principal();
 
-    if (info.type() == mozilla::ipc::PrincipalInfo::TSystemPrincipalInfo) {
-      buffer.AssignLiteral(SERVICEWORKERREGISTRAR_SYSTEM_PRINCIPAL);
-    } else {
-      MOZ_ASSERT(info.type() == mozilla::ipc::PrincipalInfo::TContentPrincipalInfo);
+    MOZ_ASSERT(info.type() == mozilla::ipc::PrincipalInfo::TContentPrincipalInfo);
 
-      const mozilla::ipc::ContentPrincipalInfo& cInfo =
-        info.get_ContentPrincipalInfo();
+    const mozilla::ipc::ContentPrincipalInfo& cInfo =
+      info.get_ContentPrincipalInfo();
 
-      buffer.AssignLiteral(SERVICEWORKERREGISTRAR_CONTENT_PRINCIPAL);
-      buffer.Append('\n');
+    buffer.Truncate();
+    buffer.AppendInt(cInfo.appId());
+    buffer.Append('\n');
 
-      buffer.AppendInt(cInfo.appId());
-      buffer.Append('\n');
+    if (cInfo.isInBrowserElement()) {
+      buffer.AppendLiteral(SERVICEWORKERREGISTRAR_TRUE);
+    } else {
+      buffer.AppendLiteral(SERVICEWORKERREGISTRAR_FALSE);
+    }
 
-      if (cInfo.isInBrowserElement()) {
-        buffer.AppendLiteral(SERVICEWORKERREGISTRAR_TRUE);
-      } else {
-        buffer.AppendLiteral(SERVICEWORKERREGISTRAR_FALSE);
-      }
-
-      buffer.Append('\n');
-      buffer.Append(cInfo.spec());
-    }
+    buffer.Append('\n');
+    buffer.Append(cInfo.spec());
 
     buffer.Append('\n');
 
     buffer.Append(data[i].scope());
     buffer.Append('\n');
 
     buffer.Append(data[i].scriptSpec());
     buffer.Append('\n');
--- a/dom/workers/ServiceWorkerRegistrar.h
+++ b/dom/workers/ServiceWorkerRegistrar.h
@@ -15,23 +15,25 @@
 #include "nsString.h"
 #include "nsTArray.h"
 
 #define SERVICEWORKERREGISTRAR_FILE "serviceworker.txt"
 #define SERVICEWORKERREGISTRAR_VERSION "1"
 #define SERVICEWORKERREGISTRAR_TERMINATOR "#"
 #define SERVICEWORKERREGISTRAR_TRUE "true"
 #define SERVICEWORKERREGISTRAR_FALSE "false"
-#define SERVICEWORKERREGISTRAR_SYSTEM_PRINCIPAL "system"
-#define SERVICEWORKERREGISTRAR_CONTENT_PRINCIPAL "content"
-
 
 class nsIFile;
 
 namespace mozilla {
+
+namespace ipc {
+class PrincipalInfo;
+}
+
 namespace dom {
 
 class ServiceWorkerRegistrationData;
 
 class ServiceWorkerRegistrar : public nsIObserver
 {
   friend class ServiceWorkerRegistrarSaveDataRunnable;
 
@@ -45,17 +47,18 @@ public:
 
   void DataSaved();
 
   static already_AddRefed<ServiceWorkerRegistrar> Get();
 
   void GetRegistrations(nsTArray<ServiceWorkerRegistrationData>& aValues);
 
   void RegisterServiceWorker(const ServiceWorkerRegistrationData& aData);
-  void UnregisterServiceWorker(const nsACString& aScope);
+  void UnregisterServiceWorker(const mozilla::ipc::PrincipalInfo& aPrincipalInfo,
+                               const nsACString& aScope);
 
 protected:
   // These methods are protected because we test this class using gTest
   // subclassing it.
   void LoadData();
   void SaveData();
 
   nsresult ReadData();
--- a/dom/workers/test/gtest/TestReadWrite.cpp
+++ b/dom/workers/test/gtest/TestReadWrite.cpp
@@ -135,74 +135,59 @@ TEST(ServiceWorkerRegistrar, TestWrongVe
   const nsTArray<ServiceWorkerRegistrationData>& data = swr->TestGetData();
   ASSERT_EQ((uint32_t)0, data.Length()) << "No data should be found in an empty file";
 }
 
 TEST(ServiceWorkerRegistrar, TestReadData)
 {
   nsAutoCString buffer(SERVICEWORKERREGISTRAR_VERSION "\n");
 
-  buffer.Append(SERVICEWORKERREGISTRAR_SYSTEM_PRINCIPAL "\n");
-  buffer.Append("scope 0\nscriptSpec 0\ncurrentWorkerURL 0\nactiveCache 0\nwaitingCache 0\n");
+  buffer.Append("123\n" SERVICEWORKERREGISTRAR_TRUE "\n");
+  buffer.Append("spec 0\nscope 0\nscriptSpec 0\ncurrentWorkerURL 0\nactiveCache 0\nwaitingCache 0\n");
   buffer.Append(SERVICEWORKERREGISTRAR_TERMINATOR "\n");
 
-  buffer.Append(SERVICEWORKERREGISTRAR_CONTENT_PRINCIPAL "\n");
-  buffer.Append("123\n" SERVICEWORKERREGISTRAR_TRUE "\n");
+  buffer.Append("0\n" SERVICEWORKERREGISTRAR_FALSE "\n");
   buffer.Append("spec 1\nscope 1\nscriptSpec 1\ncurrentWorkerURL 1\nactiveCache 1\nwaitingCache 1\n");
   buffer.Append(SERVICEWORKERREGISTRAR_TERMINATOR "\n");
 
-  buffer.Append(SERVICEWORKERREGISTRAR_CONTENT_PRINCIPAL "\n");
-  buffer.Append("0\n" SERVICEWORKERREGISTRAR_FALSE "\n");
-  buffer.Append("spec 2\nscope 2\nscriptSpec 2\ncurrentWorkerURL 2\nactiveCache 2\nwaitingCache 2\n");
-  buffer.Append(SERVICEWORKERREGISTRAR_TERMINATOR "\n");
-
   ASSERT_TRUE(CreateFile(buffer)) << "CreateFile should not fail";
 
   nsRefPtr<ServiceWorkerRegistrarTest> swr = new ServiceWorkerRegistrarTest;
 
   nsresult rv = swr->TestReadData();
   ASSERT_EQ(NS_OK, rv) << "ReadData() should not fail";
 
   const nsTArray<ServiceWorkerRegistrationData>& data = swr->TestGetData();
-  ASSERT_EQ((uint32_t)3, data.Length()) << "4 entries should be found";
+  ASSERT_EQ((uint32_t)2, data.Length()) << "4 entries should be found";
 
   const mozilla::ipc::PrincipalInfo& info0 = data[0].principal();
-  ASSERT_EQ(info0.type(), mozilla::ipc::PrincipalInfo::TSystemPrincipalInfo) << "First principal must be system";
+  ASSERT_EQ(info0.type(), mozilla::ipc::PrincipalInfo::TContentPrincipalInfo) << "First principal must be content";
+  const mozilla::ipc::ContentPrincipalInfo& cInfo0 = data[0].principal();
 
+  ASSERT_EQ((uint32_t)123, cInfo0.appId());
+  ASSERT_EQ((uint32_t)true, cInfo0.isInBrowserElement());
+  ASSERT_STREQ("spec 0", cInfo0.spec().get());
   ASSERT_STREQ("scope 0", data[0].scope().get());
   ASSERT_STREQ("scriptSpec 0", data[0].scriptSpec().get());
   ASSERT_STREQ("currentWorkerURL 0", data[0].currentWorkerURL().get());
   ASSERT_STREQ("activeCache 0", NS_ConvertUTF16toUTF8(data[0].activeCacheName()).get());
   ASSERT_STREQ("waitingCache 0", NS_ConvertUTF16toUTF8(data[0].waitingCacheName()).get());
 
   const mozilla::ipc::PrincipalInfo& info1 = data[1].principal();
   ASSERT_EQ(info1.type(), mozilla::ipc::PrincipalInfo::TContentPrincipalInfo) << "First principal must be content";
   const mozilla::ipc::ContentPrincipalInfo& cInfo1 = data[1].principal();
 
-  ASSERT_EQ((uint32_t)123, cInfo1.appId());
-  ASSERT_EQ((uint32_t)true, cInfo1.isInBrowserElement());
+  ASSERT_EQ((uint32_t)0, cInfo1.appId());
+  ASSERT_EQ((uint32_t)false, cInfo1.isInBrowserElement());
   ASSERT_STREQ("spec 1", cInfo1.spec().get());
   ASSERT_STREQ("scope 1", data[1].scope().get());
   ASSERT_STREQ("scriptSpec 1", data[1].scriptSpec().get());
   ASSERT_STREQ("currentWorkerURL 1", data[1].currentWorkerURL().get());
   ASSERT_STREQ("activeCache 1", NS_ConvertUTF16toUTF8(data[1].activeCacheName()).get());
   ASSERT_STREQ("waitingCache 1", NS_ConvertUTF16toUTF8(data[1].waitingCacheName()).get());
-
-  const mozilla::ipc::PrincipalInfo& info2 = data[2].principal();
-  ASSERT_EQ(info2.type(), mozilla::ipc::PrincipalInfo::TContentPrincipalInfo) << "First principal must be content";
-  const mozilla::ipc::ContentPrincipalInfo& cInfo2 = data[2].principal();
-
-  ASSERT_EQ((uint32_t)0, cInfo2.appId());
-  ASSERT_EQ((uint32_t)false, cInfo2.isInBrowserElement());
-  ASSERT_STREQ("spec 2", cInfo2.spec().get());
-  ASSERT_STREQ("scope 2", data[2].scope().get());
-  ASSERT_STREQ("scriptSpec 2", data[2].scriptSpec().get());
-  ASSERT_STREQ("currentWorkerURL 2", data[2].currentWorkerURL().get());
-  ASSERT_STREQ("activeCache 2", NS_ConvertUTF16toUTF8(data[2].activeCacheName()).get());
-  ASSERT_STREQ("waitingCache 2", NS_ConvertUTF16toUTF8(data[2].waitingCacheName()).get());
 }
 
 TEST(ServiceWorkerRegistrar, TestDeleteData)
 {
   ASSERT_TRUE(CreateFile(nsAutoCString("Foobar"))) << "CreateFile should not fail";
 
   nsRefPtr<ServiceWorkerRegistrarTest> swr = new ServiceWorkerRegistrarTest;
 
@@ -222,24 +207,19 @@ TEST(ServiceWorkerRegistrar, TestWriteDa
   {
     nsRefPtr<ServiceWorkerRegistrarTest> swr = new ServiceWorkerRegistrarTest;
 
     nsTArray<ServiceWorkerRegistrationData>& data = swr->TestGetData();
 
     for (int i = 0; i < 10; ++i) {
       ServiceWorkerRegistrationData* d = data.AppendElement();
 
-      if ((i % 2) == 0) {
-        d->principal() = mozilla::ipc::SystemPrincipalInfo();
-      } else if ((i % 2) == 1) {
-        nsAutoCString spec;
-        spec.AppendPrintf("spec write %d", i);
-        d->principal() = mozilla::ipc::ContentPrincipalInfo(i, i % 2, spec);
-      }
-
+      nsAutoCString spec;
+      spec.AppendPrintf("spec write %d", i);
+      d->principal() = mozilla::ipc::ContentPrincipalInfo(i, i % 2, spec);
       d->scope().AppendPrintf("scope write %d", i);
       d->scriptSpec().AppendPrintf("scriptSpec write %d", i);
       d->currentWorkerURL().AppendPrintf("currentWorkerURL write %d", i);
       d->activeCacheName().AppendPrintf("activeCacheName write %d", i);
       d->waitingCacheName().AppendPrintf("waitingCacheName write %d", i);
     }
 
     nsresult rv = swr->TestWriteData();
@@ -252,28 +232,24 @@ TEST(ServiceWorkerRegistrar, TestWriteDa
   ASSERT_EQ(NS_OK, rv) << "ReadData() should not fail";
 
   const nsTArray<ServiceWorkerRegistrationData>& data = swr->TestGetData();
   ASSERT_EQ((uint32_t)10, data.Length()) << "10 entries should be found";
 
   for (int i = 0; i < 10; ++i) {
     nsAutoCString test;
 
-    if ((i % 2) == 0) {
-      ASSERT_EQ(data[i].principal().type(), mozilla::ipc::PrincipalInfo::TSystemPrincipalInfo);
-    } else if ((i % 2) == 1) {
-      ASSERT_EQ(data[i].principal().type(), mozilla::ipc::PrincipalInfo::TContentPrincipalInfo);
-      const mozilla::ipc::ContentPrincipalInfo& cInfo = data[i].principal();
+    ASSERT_EQ(data[i].principal().type(), mozilla::ipc::PrincipalInfo::TContentPrincipalInfo);
+    const mozilla::ipc::ContentPrincipalInfo& cInfo = data[i].principal();
 
-      ASSERT_EQ((uint32_t)i, cInfo.appId());
-      ASSERT_EQ((uint32_t)(i %2), cInfo.isInBrowserElement());
+    ASSERT_EQ((uint32_t)i, cInfo.appId());
+    ASSERT_EQ((uint32_t)(i %2), cInfo.isInBrowserElement());
 
-      test.AppendPrintf("spec write %d", i);
-      ASSERT_STREQ(test.get(), cInfo.spec().get());
-    }
+    test.AppendPrintf("spec write %d", i);
+    ASSERT_STREQ(test.get(), cInfo.spec().get());
 
     test.Truncate();
     test.AppendPrintf("scope write %d", i);
     ASSERT_STREQ(test.get(), data[i].scope().get());
 
     test.Truncate();
     test.AppendPrintf("scriptSpec write %d", i);
     ASSERT_STREQ(test.get(), data[i].scriptSpec().get());
--- a/ipc/glue/BackgroundParentImpl.cpp
+++ b/ipc/glue/BackgroundParentImpl.cpp
@@ -509,38 +509,42 @@ public:
 
 private:
   ServiceWorkerRegistrationData mData;
 };
 
 class UnregisterServiceWorkerCallback final : public nsRunnable
 {
 public:
-  explicit UnregisterServiceWorkerCallback(const nsString& aScope)
-    : mScope(aScope)
+  UnregisterServiceWorkerCallback(const PrincipalInfo& aPrincipalInfo,
+                                  const nsString& aScope)
+    : mPrincipalInfo(aPrincipalInfo)
+    , mScope(aScope)
   {
     AssertIsInMainProcess();
     AssertIsOnBackgroundThread();
   }
 
   NS_IMETHODIMP
   Run()
   {
     AssertIsInMainProcess();
     AssertIsOnBackgroundThread();
 
     nsRefPtr<dom::ServiceWorkerRegistrar> service =
       dom::ServiceWorkerRegistrar::Get();
     MOZ_ASSERT(service);
 
-    service->UnregisterServiceWorker(NS_ConvertUTF16toUTF8(mScope));
+    service->UnregisterServiceWorker(mPrincipalInfo,
+                                     NS_ConvertUTF16toUTF8(mScope));
     return NS_OK;
   }
 
 private:
+  const PrincipalInfo mPrincipalInfo;
   nsString mScope;
 };
 
 class CheckPrincipalWithCallbackRunnable final : public nsRunnable
 {
 public:
   CheckPrincipalWithCallbackRunnable(already_AddRefed<ContentParent> aParent,
                                      const PrincipalInfo& aPrincipalInfo,
@@ -592,17 +596,18 @@ BackgroundParentImpl::RecvRegisterServic
                                      const ServiceWorkerRegistrationData& aData)
 {
   AssertIsInMainProcess();
   AssertIsOnBackgroundThread();
 
   // Basic validation.
   if (aData.scope().IsEmpty() ||
       aData.scriptSpec().IsEmpty() ||
-      aData.principal().type() == PrincipalInfo::TNullPrincipalInfo) {
+      aData.principal().type() == PrincipalInfo::TNullPrincipalInfo ||
+      aData.principal().type() == PrincipalInfo::TSystemPrincipalInfo) {
     return false;
   }
 
   nsRefPtr<RegisterServiceWorkerCallback> callback =
     new RegisterServiceWorkerCallback(aData);
 
   nsRefPtr<ContentParent> parent = BackgroundParent::GetContentParent(this);
 
@@ -626,22 +631,23 @@ BackgroundParentImpl::RecvUnregisterServ
                                             const PrincipalInfo& aPrincipalInfo,
                                             const nsString& aScope)
 {
   AssertIsInMainProcess();
   AssertIsOnBackgroundThread();
 
   // Basic validation.
   if (aScope.IsEmpty() ||
-      aPrincipalInfo.type() == PrincipalInfo::TNullPrincipalInfo) {
+      aPrincipalInfo.type() == PrincipalInfo::TNullPrincipalInfo ||
+      aPrincipalInfo.type() == PrincipalInfo::TSystemPrincipalInfo) {
     return false;
   }
 
   nsRefPtr<UnregisterServiceWorkerCallback> callback =
-    new UnregisterServiceWorkerCallback(aScope);
+    new UnregisterServiceWorkerCallback(aPrincipalInfo, aScope);
 
   nsRefPtr<ContentParent> parent = BackgroundParent::GetContentParent(this);
 
   // If the ContentParent is null we are dealing with a same-process actor.
   if (!parent) {
     callback->Run();
     return true;
   }