Bug 930611 - Introduce WorkerType rather than boolean SharedWorker check. r=bent

dom/workers/RuntimeService.cpp

@@ -2060,7 +2060,7 @@ RuntimeService::CreateSharedWorker(JSContext* aCx, nsPIDOMWindow* aWindow,
   if (!workerPrivate) {
     nsRefPtr<WorkerPrivate> newWorkerPrivate =
       WorkerPrivate::Create(aCx, JS::NullPtr(), nullptr, aScriptURL, false,
-                            true, aName, &loadInfo);
+                            WorkerPrivate::WorkerTypeShared, aName, &loadInfo);
     NS_ENSURE_TRUE(newWorkerPrivate, NS_ERROR_FAILURE);
 
     if (!RegisterWorker(aCx, newWorkerPrivate)) {

dom/workers/Worker.cpp

@@ -94,7 +94,7 @@ public:
 
   static JSObject*
   Create(JSContext* aCx, WorkerPrivate* aParentObj, const nsAString& aScriptURL,
-         bool aIsChromeWorker, bool aIsSharedWorker,
+         bool aIsChromeWorker, WorkerPrivate::WorkerType aWorkerType,
          const nsAString& aSharedWorkerName);
 
 protected:
@@ -128,7 +128,8 @@ protected:
     }
 
     JS::Rooted<JSObject*> obj(aCx,
-      Create(aCx, parent, scriptURL, aIsChromeWorker, false, EmptyString()));
+      Create(aCx, parent, scriptURL, aIsChromeWorker,
+             WorkerPrivate::WorkerTypeDedicated, EmptyString()));
     if (!obj) {
       return false;
     }
@@ -561,10 +562,13 @@ Worker::GetInstancePrivate(JSContext* aCx, JSObject* aObj,
 JSObject*
 Worker::Create(JSContext* aCx, WorkerPrivate* aParent,
                const nsAString& aScriptURL, bool aIsChromeWorker,
-               bool aIsSharedWorker, const nsAString& aSharedWorkerName)
+               WorkerPrivate::WorkerType aWorkerType,
+               const nsAString& aSharedWorkerName)
 {
-  MOZ_ASSERT_IF(aIsSharedWorker, !aSharedWorkerName.IsVoid());
-  MOZ_ASSERT_IF(!aIsSharedWorker, aSharedWorkerName.IsEmpty());
+  MOZ_ASSERT_IF(aWorkerType == WorkerPrivate::WorkerTypeShared,
+                !aSharedWorkerName.IsVoid());
+  MOZ_ASSERT_IF(aWorkerType != WorkerPrivate::WorkerTypeShared,
+                aSharedWorkerName.IsEmpty());
 
   RuntimeService* runtimeService;
   if (aParent) {
@@ -593,7 +597,7 @@ Worker::Create(JSContext* aCx, WorkerPrivate* aParent,
 
   nsRefPtr<WorkerPrivate> worker =
     WorkerPrivate::Create(aCx, obj, aParent, aScriptURL, aIsChromeWorker,
-                          aIsSharedWorker, aSharedWorkerName);
+                          aWorkerType, aSharedWorkerName);
   if (!worker) {
     // It'd be better if we could avoid allocating the JSObject until after we
     // make sure we have a WorkerPrivate, but failing that we should at least

dom/workers/WorkerPrivate.cpp

@@ -2022,7 +2022,7 @@ WorkerPrivateParent<Derived>::WorkerPrivateParent(
                                      WorkerPrivate* aParent,
                                      const nsAString& aScriptURL,
                                      bool aIsChromeWorker,
-                                             bool aIsSharedWorker,
+                                     WorkerType aWorkerType,
                                              const nsAString& aSharedWorkerName,
                                              LoadInfo& aLoadInfo)
 : EventTarget(aParent ? aCx : nullptr), mMutex("WorkerPrivateParent Mutex"),
@@ -2032,11 +2032,11 @@ WorkerPrivateParent<Derived>::WorkerPrivateParent(
   mSharedWorkerName(aSharedWorkerName), mBusyCount(0), mMessagePortSerial(0),
   mParentStatus(Pending), mJSObjectRooted(false), mParentSuspended(false),
   mIsChromeWorker(aIsChromeWorker), mMainThreadObjectsForgotten(false),
-  mIsSharedWorker(aIsSharedWorker)
+  mWorkerType(aWorkerType)
 {
   MOZ_COUNT_CTOR(mozilla::dom::workers::WorkerPrivateParent);
-  MOZ_ASSERT_IF(aIsSharedWorker, !aObject && !aSharedWorkerName.IsVoid());
-  MOZ_ASSERT_IF(!aIsSharedWorker, aObject && aSharedWorkerName.IsEmpty());
+  MOZ_ASSERT_IF(IsSharedWorker(), !aObject && !aSharedWorkerName.IsVoid());
+  MOZ_ASSERT_IF(!IsSharedWorker(), aObject && aSharedWorkerName.IsEmpty());
 
   if (aLoadInfo.mWindow) {
     NS_ASSERTION(aLoadInfo.mWindow->IsInnerWindow(),
@@ -2056,7 +2056,7 @@ WorkerPrivateParent<Derived>::WorkerPrivateParent(
     RuntimeService::GetDefaultJSSettings(mJSSettings);
   }
 
-  if (!aIsSharedWorker) {
+  if (IsDedicatedWorker()) {
     SetIsDOMBinding();
     SetWrapper(aObject);
   }
@@ -2312,7 +2312,7 @@ WorkerPrivateParent<Derived>::Resume(JSContext* aCx, nsPIDOMWindow* aWindow)
   // could post new messages before we run those that have been queued.
   if (!mQueuedRunnables.IsEmpty()) {
     AssertIsOnMainThread();
-    MOZ_ASSERT(!IsSharedWorker());
+    MOZ_ASSERT(IsDedicatedWorker());
 
     nsTArray<nsRefPtr<WorkerRunnable> > runnables;
     mQueuedRunnables.SwapElements(runnables);
@@ -2341,7 +2341,7 @@ WorkerPrivateParent<Derived>::SynchronizeAndResume(
 {
   AssertIsOnMainThread();
   MOZ_ASSERT(!GetParent());
-  MOZ_ASSERT_IF(!IsSharedWorker(), mParentSuspended);
+  MOZ_ASSERT_IF(IsDedicatedWorker(), mParentSuspended);
 
   // NB: There may be pending unqueued messages.  If we resume here we will
   // execute those messages out of order.  Instead we post an event to the
@@ -3194,11 +3194,11 @@ WorkerPrivateParent<Derived>::ParentJSContext() const
 WorkerPrivate::WorkerPrivate(JSContext* aCx, JS::HandleObject aObject,
                              WorkerPrivate* aParent,
                              const nsAString& aScriptURL,
-                             bool aIsChromeWorker, bool aIsSharedWorker,
+                             bool aIsChromeWorker, WorkerType aWorkerType,
                              const nsAString& aSharedWorkerName,
                              LoadInfo& aLoadInfo)
 : WorkerPrivateParent<WorkerPrivate>(aCx, aObject, aParent, aScriptURL,
-                                     aIsChromeWorker, aIsSharedWorker,
+                                     aIsChromeWorker, aWorkerType,
                                      aSharedWorkerName, aLoadInfo),
   mJSContext(nullptr), mErrorHandlerRecursionCount(0), mNextTimeoutId(1),
   mStatus(Pending), mSuspended(false), mTimerRunning(false),
@@ -3207,8 +3207,8 @@ WorkerPrivate::WorkerPrivate(JSContext* aCx, JS::HandleObject aObject,
   mBlockedForMemoryReporter(false)
 {
   MOZ_COUNT_CTOR(mozilla::dom::workers::WorkerPrivate);
-  MOZ_ASSERT_IF(aIsSharedWorker, !aObject && !aSharedWorkerName.IsVoid());
-  MOZ_ASSERT_IF(!aIsSharedWorker, aObject && aSharedWorkerName.IsEmpty());
+  MOZ_ASSERT_IF(IsSharedWorker(), !aObject && !aSharedWorkerName.IsVoid());
+  MOZ_ASSERT_IF(!IsSharedWorker(), aObject && aSharedWorkerName.IsEmpty());
 }
 
 WorkerPrivate::~WorkerPrivate()
@@ -3220,7 +3220,7 @@ WorkerPrivate::~WorkerPrivate()
 already_AddRefed<WorkerPrivate>
 WorkerPrivate::Create(JSContext* aCx, JS::HandleObject aObject,
                       WorkerPrivate* aParent, const nsAString& aScriptURL,
-                      bool aIsChromeWorker, bool aIsSharedWorker,
+                      bool aIsChromeWorker, WorkerType aWorkerType,
                       const nsAString& aSharedWorkerName, LoadInfo* aLoadInfo)
 {
   if (aParent) {
@@ -3229,8 +3229,10 @@ WorkerPrivate::Create(JSContext* aCx, JS::HandleObject aObject,
     AssertIsOnMainThread();
   }
 
-  MOZ_ASSERT_IF(aIsSharedWorker, !aObject && !aSharedWorkerName.IsVoid());
-  MOZ_ASSERT_IF(!aIsSharedWorker, aObject && aSharedWorkerName.IsEmpty());
+  MOZ_ASSERT_IF(aWorkerType == WorkerTypeShared,
+                !aObject && !aSharedWorkerName.IsVoid());
+  MOZ_ASSERT_IF(aWorkerType != WorkerTypeShared,
+                aObject && aSharedWorkerName.IsEmpty());
 
   mozilla::Maybe<LoadInfo> stackLoadInfo;
   if (!aLoadInfo) {
@@ -3248,7 +3250,7 @@ WorkerPrivate::Create(JSContext* aCx, JS::HandleObject aObject,
 
   nsRefPtr<WorkerPrivate> worker =
     new WorkerPrivate(aCx, aObject, aParent, aScriptURL, aIsChromeWorker,
-                      aIsSharedWorker, aSharedWorkerName, *aLoadInfo);
+                      aWorkerType, aSharedWorkerName, *aLoadInfo);
 
   nsRefPtr<CompileScriptRunnable> compiler = new CompileScriptRunnable(worker);
   if (!compiler->Dispatch(aCx)) {

dom/workers/WorkerPrivate.h

@@ -292,6 +292,12 @@ public:
     }
   };
 
+  enum WorkerType
+  {
+    WorkerTypeDedicated,
+    WorkerTypeShared
+  };
+
 protected:
   typedef mozilla::ErrorResult ErrorResult;
 
@@ -328,12 +334,12 @@ private:
   bool mParentSuspended;
   bool mIsChromeWorker;
   bool mMainThreadObjectsForgotten;
-  bool mIsSharedWorker;
+  WorkerType mWorkerType;
 
 protected:
   WorkerPrivateParent(JSContext* aCx, JS::HandleObject aObject,
                       WorkerPrivate* aParent, const nsAString& aScriptURL,
-                      bool aIsChromeWorker, bool aIsSharedWorker,
+                      bool aIsChromeWorker, WorkerType aWorkerType,
                       const nsAString& aSharedWorkerName, LoadInfo& aLoadInfo);
 
   ~WorkerPrivateParent();
@@ -666,16 +672,24 @@ public:
     aSettings = mJSSettings;
   }
 
+  // The ability to be a chrome worker is orthogonal to the type of
+  // worker [Dedicated|Shared].
   bool
   IsChromeWorker() const
   {
     return mIsChromeWorker;
   }
 
+  bool
+  IsDedicatedWorker() const
+  {
+    return mWorkerType == WorkerTypeDedicated;
+  }
+
   bool
   IsSharedWorker() const
   {
-    return mIsSharedWorker;
+    return mWorkerType == WorkerTypeShared;
   }
 
   const nsString&
@@ -797,7 +811,7 @@ public:
   static already_AddRefed<WorkerPrivate>
   Create(JSContext* aCx, JS::HandleObject aObject, WorkerPrivate* aParent,
          const nsAString& aScriptURL, bool aIsChromeWorker,
-         bool aIsSharedWorker, const nsAString& aSharedWorkerName,
+         WorkerType aWorkerType, const nsAString& aSharedWorkerName,
          LoadInfo* aLoadInfo = nullptr);
 
   static nsresult
@@ -1020,7 +1034,7 @@ public:
 private:
   WorkerPrivate(JSContext* aCx, JS::HandleObject aObject,
                 WorkerPrivate* aParent, const nsAString& aScriptURL,
-                bool aIsChromeWorker, bool aIsSharedWorker,
+                bool aIsChromeWorker, WorkerType aWorkerType,
                 const nsAString& aSharedWorkerName, LoadInfo& aLoadInfo);
 
   bool

dom/workers/WorkerScope.cpp

@@ -1402,9 +1402,14 @@ CreateGlobalScope(JSContext* aCx)
   WorkerPrivate* worker = GetWorkerPrivateFromContext(aCx);
   MOZ_ASSERT(worker);
 
-  const JSClass* classPtr = worker->IsSharedWorker() ?
-                            SharedWorkerGlobalScope::Class() :
-                            DedicatedWorkerGlobalScope::Class();
+  const JSClass* classPtr;
+  if (worker->IsDedicatedWorker()) {
+    classPtr = DedicatedWorkerGlobalScope::Class();
+  } else if (worker->IsSharedWorker()) {
+    classPtr = SharedWorkerGlobalScope::Class();
+  } else {
+    MOZ_CRASH("Bad type");
+  }
 
   JS::CompartmentOptions options;
   if (worker->IsChromeWorker()) {
@@ -1421,12 +1426,16 @@ CreateGlobalScope(JSContext* aCx)
   JSAutoCompartment ac(aCx, global);
 
   // Make the private slots now so that all our instance checks succeed.
-  if (worker->IsSharedWorker()) {
+  if (worker->IsDedicatedWorker()) {
+    if (!DedicatedWorkerGlobalScope::InitPrivate(aCx, global, worker)) {
+      return nullptr;
+    }
+  } else if (worker->IsSharedWorker()) {
     if (!SharedWorkerGlobalScope::InitPrivate(aCx, global, worker)) {
       return nullptr;
-  }
-  } else if (!DedicatedWorkerGlobalScope::InitPrivate(aCx, global, worker)) {
-    return nullptr;
+    }
+  } else {
+    MOZ_CRASH("Bad type");
   }
 
   // Proto chain should be:
@@ -1447,10 +1456,15 @@ CreateGlobalScope(JSContext* aCx)
     return nullptr;
   }
 
-  JS::Rooted<JSObject*> finalScopeProto(aCx,
-    worker->IsSharedWorker() ?
-    SharedWorkerGlobalScope::InitClass(aCx, global, scopeProto) :
-    DedicatedWorkerGlobalScope::InitClass(aCx, global, scopeProto));
+  JS::Rooted<JSObject*> finalScopeProto(aCx);
+  if (worker->IsDedicatedWorker()) {
+    finalScopeProto = DedicatedWorkerGlobalScope::InitClass(aCx, global, scopeProto);
+  } else if (worker->IsSharedWorker()) {
+    finalScopeProto = SharedWorkerGlobalScope::InitClass(aCx, global, scopeProto);
+  } else {
+    MOZ_CRASH("Bad type");
+  }
+
   if (!finalScopeProto) {
     return nullptr;
   }