Bug 1255298. Just pass through the JSContext when passing through the NotificationOptions in notification code. r=wchen a=lizzard
authorBoris Zbarsky <bzbarsky@mit.edu>
Thu, 10 Mar 2016 18:07:28 -0500
changeset 323436 cc357481aa2bfe4eac8ec0c4c9a65e832a6b1369
parent 323435 d5911159db9e44bd29aeb44cb8356b00840256ac
child 323437 88b29c784ee17f374c3e99ba41e35886bc8bc9de
push id5913
push userjlund@mozilla.com
push dateMon, 25 Apr 2016 16:57:49 +0000
treeherdermozilla-beta@dcaf0a6fa115 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerswchen, lizzard
bugs1255298
milestone47.0a2
Bug 1255298. Just pass through the JSContext when passing through the NotificationOptions in notification code. r=wchen a=lizzard MozReview-Commit-ID: I3KHxrrDHZo
dom/notification/Notification.cpp
dom/notification/Notification.h
dom/workers/ServiceWorkerRegistration.cpp
--- a/dom/notification/Notification.cpp
+++ b/dom/notification/Notification.cpp
@@ -1044,17 +1044,18 @@ Notification::Constructor(const GlobalOb
   UNWRAP_WORKER_OBJECT(ServiceWorkerGlobalScope, aGlobal.Get(), scope);
   if (scope) {
     aRv.ThrowTypeError<MSG_NOTIFICATION_NO_CONSTRUCTOR_IN_SERVICEWORKER>();
     return nullptr;
   }
 
   nsCOMPtr<nsIGlobalObject> global = do_QueryInterface(aGlobal.GetAsSupports());
   RefPtr<Notification> notification =
-    CreateAndShow(global, aTitle, aOptions, EmptyString(), aRv);
+    CreateAndShow(aGlobal.Context(), global, aTitle, aOptions,
+                  EmptyString(), aRv);
   if (NS_WARN_IF(aRv.Failed())) {
     return nullptr;
   }
 
   // This is be ok since we are on the worker thread where this function will
   // run to completion before the Notification has a chance to go away.
   return notification.forget();
 }
@@ -2577,17 +2578,18 @@ public:
   {
     return mRv;
   }
 
 };
 
 /* static */
 already_AddRefed<Promise>
-Notification::ShowPersistentNotification(nsIGlobalObject *aGlobal,
+Notification::ShowPersistentNotification(JSContext* aCx,
+                                         nsIGlobalObject *aGlobal,
                                          const nsAString& aScope,
                                          const nsAString& aTitle,
                                          const NotificationOptions& aOptions,
                                          ErrorResult& aRv)
 {
   MOZ_ASSERT(aGlobal);
 
   // Validate scope.
@@ -2654,48 +2656,40 @@ Notification::ShowPersistentNotification
   }
 
   // "Otherwise, resolve promise with undefined."
   // The Notification may still not be shown due to other errors, but the spec
   // is not concerned with those.
   p->MaybeResolve(JS::UndefinedHandleValue);
 
   RefPtr<Notification> notification =
-    CreateAndShow(aGlobal, aTitle, aOptions, aScope, aRv);
+    CreateAndShow(aCx, aGlobal, aTitle, aOptions, aScope, aRv);
   if (NS_WARN_IF(aRv.Failed())) {
     return nullptr;
   }
 
   return p.forget();
 }
 
 /* static */ already_AddRefed<Notification>
-Notification::CreateAndShow(nsIGlobalObject* aGlobal,
+Notification::CreateAndShow(JSContext* aCx,
+                            nsIGlobalObject* aGlobal,
                             const nsAString& aTitle,
                             const NotificationOptions& aOptions,
                             const nsAString& aScope,
                             ErrorResult& aRv)
 {
   MOZ_ASSERT(aGlobal);
 
-  AutoJSAPI jsapi;
-  if (NS_WARN_IF(!jsapi.Init(aGlobal)))
-  {
-    aRv.Throw(NS_ERROR_DOM_ABORT_ERR);
-    return nullptr;
-  }
-
-  JSContext* cx = jsapi.cx();
-
   RefPtr<Notification> notification = CreateInternal(aGlobal, EmptyString(),
-                                                       aTitle, aOptions);
+                                                     aTitle, aOptions);
 
   // Make a structured clone of the aOptions.mData object
-  JS::Rooted<JS::Value> data(cx, aOptions.mData);
-  notification->InitFromJSVal(cx, data, aRv);
+  JS::Rooted<JS::Value> data(aCx, aOptions.mData);
+  notification->InitFromJSVal(aCx, data, aRv);
   if (NS_WARN_IF(aRv.Failed())) {
     return nullptr;
   }
 
   notification->SetScope(aScope);
 
   auto ref = MakeUnique<NotificationRef>(notification);
   if (NS_WARN_IF(!ref->Initialized())) {
--- a/dom/notification/Notification.h
+++ b/dom/notification/Notification.h
@@ -254,18 +254,23 @@ public:
 
   static already_AddRefed<Promise> WorkerGet(workers::WorkerPrivate* aWorkerPrivate,
                                              const GetNotificationOptions& aFilter,
                                              const nsAString& aScope,
                                              ErrorResult& aRv);
 
   // Notification implementation of
   // ServiceWorkerRegistration.showNotification.
+  //
+  //
+  // Note that aCx may not be in the compartment of aGlobal, but aOptions will
+  // have its JS things in the compartment of aCx.
   static already_AddRefed<Promise>
-  ShowPersistentNotification(nsIGlobalObject* aGlobal,
+  ShowPersistentNotification(JSContext* aCx,
+                             nsIGlobalObject* aGlobal,
                              const nsAString& aScope,
                              const nsAString& aTitle,
                              const NotificationOptions& aOptions,
                              ErrorResult& aRv);
 
   void Close();
 
   nsPIDOMWindowInner* GetParentObject()
@@ -412,18 +417,22 @@ protected:
 
 private:
   virtual ~Notification();
 
   // Creates a Notification and shows it. Returns a reference to the
   // Notification if result is NS_OK. The lifetime of this Notification is tied
   // to an underlying NotificationRef. Do not hold a non-stack raw pointer to
   // it. Be careful about thread safety if acquiring a strong reference.
+  //
+  // Note that aCx may not be in the compartment of aGlobal, but aOptions will
+  // have its JS things in the compartment of aCx.
   static already_AddRefed<Notification>
-  CreateAndShow(nsIGlobalObject* aGlobal,
+  CreateAndShow(JSContext* aCx,
+                nsIGlobalObject* aGlobal,
                 const nsAString& aTitle,
                 const NotificationOptions& aOptions,
                 const nsAString& aScope,
                 ErrorResult& aRv);
 
   nsIPrincipal* GetPrincipal();
 
   nsresult PersistNotification();
--- a/dom/workers/ServiceWorkerRegistration.cpp
+++ b/dom/workers/ServiceWorkerRegistration.cpp
@@ -724,18 +724,18 @@ ServiceWorkerRegistrationMainThread::Sho
   RefPtr<workers::ServiceWorker> worker = GetActive();
   if (!worker) {
     aRv.ThrowTypeError<MSG_NO_ACTIVE_WORKER>(mScope);
     return nullptr;
   }
 
   nsCOMPtr<nsIGlobalObject> global = do_QueryInterface(window);
   RefPtr<Promise> p =
-    Notification::ShowPersistentNotification(global,
-                                             mScope, aTitle, aOptions, aRv);
+    Notification::ShowPersistentNotification(aCx, global, mScope, aTitle,
+                                             aOptions, aRv);
   if (NS_WARN_IF(aRv.Failed())) {
     return nullptr;
   }
 
   return p.forget();
 }
 
 already_AddRefed<Promise>
@@ -1207,17 +1207,17 @@ ServiceWorkerRegistrationWorkerThread::S
                                                         const NotificationOptions& aOptions,
                                                         ErrorResult& aRv)
 {
   // Until Bug 1131324 exposes ServiceWorkerContainer on workers,
   // ShowPersistentNotification() checks for valid active worker while it is
   // also verifying scope so that we block the worker on the main thread only
   // once.
   RefPtr<Promise> p =
-    Notification::ShowPersistentNotification(mWorkerPrivate->GlobalScope(),
+    Notification::ShowPersistentNotification(aCx, mWorkerPrivate->GlobalScope(),
                                              mScope, aTitle, aOptions, aRv);
   if (NS_WARN_IF(aRv.Failed())) {
     return nullptr;
   }
 
   return p.forget();
 }