Bug 1542779 - Use callback interfaces for JSWindowActor callbacks, r=jdai
authorNika Layzell <nika@thelayzells.com>
Mon, 15 Apr 2019 15:49:02 +0000
changeset 469518 f17ab3584d22
parent 469517 fcd532097b93
child 469519 7d9507696389
push id35873
push userccoroiu@mozilla.com
push dateMon, 15 Apr 2019 21:36:26 +0000
treeherdermozilla-central@b8f49a14c458 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersjdai
bugs1542779
milestone68.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 1542779 - Use callback interfaces for JSWindowActor callbacks, r=jdai This should not have any major behaviour changes, with the following exceptions: 1. The method for receiving messages from IPC is called `receiveMessage` rather than `recvAsyncMessage`. This is more consistent with existing code, so should be OK. 2. Exceptions will be correctly reported when thrown within a callback. Differential Revision: https://phabricator.services.mozilla.com/D26547
dom/chrome-webidl/JSWindowActor.webidl
dom/ipc/JSWindowActorService.cpp
toolkit/actors/TestChild.jsm
toolkit/actors/TestParent.jsm
--- a/dom/chrome-webidl/JSWindowActor.webidl
+++ b/dom/chrome-webidl/JSWindowActor.webidl
@@ -1,14 +1,16 @@
 /* -*- Mode: C++; tab-width: 8; indent-tabs-mode: nil; c-basic-offset: 2 -*- */
 /* vim: set ts=8 sts=2 et sw=2 tw=80: */
 /* This Source Code Form is subject to the terms of the Mozilla Public
  * License, v. 2.0. If a copy of the MPL was not distributed with this
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
+interface nsISupports;
+
 [NoInterfaceObject]
 interface JSWindowActor {
   [Throws]
   void sendAsyncMessage(DOMString messageName,
                         optional any obj,
                         optional any transfers);
 };
 
@@ -17,9 +19,18 @@ interface JSWindowActorParent {
   readonly attribute WindowGlobalParent manager;
 };
 JSWindowActorParent implements JSWindowActor;
 
 [ChromeOnly, ChromeConstructor]
 interface JSWindowActorChild {
   readonly attribute WindowGlobalChild manager;
 };
-JSWindowActorChild implements JSWindowActor;
\ No newline at end of file
+JSWindowActorChild implements JSWindowActor;
+
+// WebIDL callback interface version of the nsIObserver interface for use when
+// calling the observe method on JSWindowActors.
+//
+// NOTE: This isn't marked as ChromeOnly, as it has no interface object, and
+// thus cannot be conditionally exposed.
+callback interface MozObserverCallback {
+  void observe(nsISupports subject, ByteString topic, DOMString? data);
+};
--- a/dom/ipc/JSWindowActorService.cpp
+++ b/dom/ipc/JSWindowActorService.cpp
@@ -2,91 +2,33 @@
 
 /* vim: set ts=8 sts=2 et sw=2 tw=80: */
 /* This Source Code Form is subject to the terms of the Mozilla Public
  * License, v. 2.0. If a copy of the MPL was not distributed with this
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
 #include "mozilla/dom/JSWindowActorService.h"
 #include "mozilla/dom/ChromeUtilsBinding.h"
+#include "mozilla/dom/EventListenerBinding.h"
 #include "mozilla/dom/EventTargetBinding.h"
 #include "mozilla/dom/EventTarget.h"
+#include "mozilla/dom/JSWindowActorBinding.h"
 #include "mozilla/dom/PContent.h"
 #include "mozilla/StaticPtr.h"
 #include "mozJSComponentLoader.h"
 #include "mozilla/extensions/WebExtensionContentScript.h"
 #include "mozilla/Logging.h"
 
 namespace mozilla {
 namespace dom {
 namespace {
 StaticRefPtr<JSWindowActorService> gJSWindowActorService;
 }
 
 /**
- * Helper for calling a named method on a JS Window Actor object with a single
- * parameter.
- *
- * It will do the following:
- *  1. Enter the actor object's compartment.
- *  2. Convert the given parameter into a JS parameter with ToJSValue.
- *  3. Call the named method, passing the single parameter.
- *  4. Place the return value in aRetVal.
- *
- * If an error occurs during this process, this method clears any pending
- * exceptions, and returns a nsresult.
- */
-template <typename T>
-nsresult CallJSActorMethod(nsWrapperCache* aActor, const char* aName,
-                           T& aNativeArg, JS::MutableHandleValue aRetVal) {
-  // FIXME(nika): We should avoid atomizing and interning the |aName| strings
-  // every time we do this call. Given the limited set of possible IDs, it would
-  // be better to cache the `jsid` values.
-
-  aRetVal.setUndefined();
-
-  // Get the wrapper for our actor. If we don't have a wrapper, the target
-  // method won't be defined on it. so there's no reason to continue.
-  JS::Rooted<JSObject*> actor(RootingCx(), aActor->GetWrapper());
-  if (NS_WARN_IF(!actor)) {
-    return NS_ERROR_NOT_IMPLEMENTED;
-  }
-
-  // Enter the realm of our actor object to begin running script.
-  AutoEntryScript aes(actor, "CallJSActorMethod");
-  JSContext* cx = aes.cx();
-  JSAutoRealm ar(cx, actor);
-
-  // Get the method we want to call, and produce NS_ERROR_NOT_IMPLEMENTED if
-  // it is not present.
-  JS::Rooted<JS::Value> func(cx);
-  if (NS_WARN_IF(!JS_GetProperty(cx, actor, aName, &func) ||
-                 func.isPrimitive())) {
-    JS_ClearPendingException(cx);
-    return NS_ERROR_NOT_IMPLEMENTED;
-  }
-
-  // Convert the native argument to a JS value.
-  JS::Rooted<JS::Value> argv(cx);
-  if (NS_WARN_IF(!ToJSValue(cx, aNativeArg, &argv))) {
-    JS_ClearPendingException(cx);
-    return NS_ERROR_FAILURE;
-  }
-
-  // Call our method.
-  if (NS_WARN_IF(!JS_CallFunctionValue(cx, actor, func,
-                                       JS::HandleValueArray(argv), aRetVal))) {
-    JS_ClearPendingException(cx);
-    return NS_ERROR_FAILURE;
-  }
-
-  return NS_OK;
-}
-
-/**
  * Object corresponding to a single actor protocol. This object acts as an
  * Event listener for the actor which is called for events which would
  * trigger actor creation.
  *
  * This object also can act as a carrier for methods and other state related to
  * a single protocol managed by the JSWindowActorService.
  */
 class JSWindowActorProtocol final : public nsIObserver,
@@ -295,19 +237,23 @@ NS_IMETHODIMP JSWindowActorProtocol::Han
 
     // Don't raise an error if creation of our actor was vetoed.
     if (rv == NS_ERROR_NOT_AVAILABLE) {
       return NS_OK;
     }
     return rv;
   }
 
-  // Call the "handleEvent" method on our actor.
-  JS::Rooted<JS::Value> dummy(RootingCx());
-  return CallJSActorMethod(actor, "handleEvent", aEvent, &dummy);
+  // Build our event listener & call it.
+  JS::Rooted<JSObject*> global(RootingCx(),
+                               JS::GetNonCCWObjectGlobal(actor->GetWrapper()));
+  RefPtr<EventListener> eventListener =
+      new EventListener(actor->GetWrapper(), global, nullptr, nullptr);
+  eventListener->HandleEvent(*aEvent, "JSWindowActorProtocol::HandleEvent");
+  return NS_OK;
 }
 
 NS_IMETHODIMP JSWindowActorProtocol::Observe(nsISupports* aSubject,
                                              const char* aTopic,
                                              const char16_t* aData) {
   nsCOMPtr<nsPIDOMWindowInner> inner = do_QueryInterface(aSubject);
   if (NS_WARN_IF(!inner)) {
     return NS_ERROR_FAILURE;
@@ -326,54 +272,23 @@ NS_IMETHODIMP JSWindowActorProtocol::Obs
 
     // Don't raise an error if creation of our actor was vetoed.
     if (rv == NS_ERROR_NOT_AVAILABLE) {
       return NS_OK;
     }
     return rv;
   }
 
-  // Get the wrapper for our actor. If we don't have a wrapper, the target
-  // method won't be defined on it. so there's no reason to continue.
-  JS::Rooted<JSObject*> obj(RootingCx(), actor->GetWrapper());
-  if (NS_WARN_IF(!obj)) {
-    return NS_ERROR_NOT_IMPLEMENTED;
-  }
-
-  // Enter the realm of our actor object to begin running script.
-  AutoEntryScript aes(obj, "JSWindowActorProtocol::Observe");
-  JSContext* cx = aes.cx();
-  JSAutoRealm ar(cx, obj);
-
-  JS::AutoValueArray<3> argv(cx);
-  if (NS_WARN_IF(
-          !ToJSValue(cx, aSubject, argv[0]) ||
-          !NonVoidByteStringToJsval(cx, nsDependentCString(aTopic), argv[1]))) {
-    JS_ClearPendingException(cx);
-    return NS_ERROR_FAILURE;
-  }
-
-  // aData is an optional parameter.
-  if (aData) {
-    if (NS_WARN_IF(!ToJSValue(cx, nsDependentString(aData), argv[2]))) {
-      JS_ClearPendingException(cx);
-      return NS_ERROR_FAILURE;
-    }
-  } else {
-    argv[2].setNull();
-  }
-
-  // Call the "observe" method on our actor.
-  JS::Rooted<JS::Value> dummy(cx);
-  if (NS_WARN_IF(!JS_CallFunctionName(cx, obj, "observe",
-                                      JS::HandleValueArray(argv), &dummy))) {
-    JS_ClearPendingException(cx);
-    return NS_ERROR_FAILURE;
-  }
-
+  // Build a observer callback.
+  JS::Rooted<JSObject*> global(RootingCx(),
+                               JS::GetNonCCWObjectGlobal(actor->GetWrapper()));
+  RefPtr<MozObserverCallback> observerCallback =
+      new MozObserverCallback(actor->GetWrapper(), global, nullptr, nullptr);
+  observerCallback->Observe(aSubject, nsDependentCString(aTopic),
+                            aData ? nsDependentString(aData) : VoidString());
   return NS_OK;
 }
 
 void JSWindowActorProtocol::RegisterListenersFor(EventTarget* aRoot) {
   EventListenerManager* elm = aRoot->GetOrCreateListenerManager();
 
   for (auto& event : mChild.mEvents) {
     elm->AddEventListenerByType(EventListenerHolder(this), event.mName,
@@ -652,42 +567,37 @@ void JSWindowActorService::ReceiveMessag
   RootedDictionary<ReceiveMessageArgument> argument(cx);
   argument.mObjects = JS_NewPlainObject(cx);
   argument.mTarget = aTarget;
   argument.mName = aMessageName;
   argument.mData = json;
   argument.mJson = json;
   argument.mSync = false;
 
-  JS::RootedValue argv(cx);
-  if (NS_WARN_IF(!ToJSValue(cx, argument, &argv))) {
-    return;
-  }
+  JS::Rooted<JSObject*> global(cx, JS::GetNonCCWObjectGlobal(aObj));
+  RefPtr<MessageListener> messageListener =
+      new MessageListener(aObj, global, nullptr, nullptr);
 
-  // Now that we have finished, call the recvAsyncMessage callback.
-  JS::RootedValue dummy(cx);
-  if (NS_WARN_IF(!JS_CallFunctionName(cx, aObj, "recvAsyncMessage",
-                                      JS::HandleValueArray(argv), &dummy))) {
-    JS_ClearPendingException(cx);
-    return;
-  }
+  JS::Rooted<JS::Value> dummy(cx);
+  messageListener->ReceiveMessage(argument, &dummy,
+                                  "JSWindowActorService::ReceiveMessage");
 }
 
 void JSWindowActorService::RegisterWindowRoot(EventTarget* aRoot) {
   MOZ_ASSERT(!mRoots.Contains(aRoot));
   mRoots.AppendElement(aRoot);
 
   // Register event listeners on the newly added Window Root.
   for (auto iter = mDescriptors.Iter(); !iter.Done(); iter.Next()) {
     iter.Data()->RegisterListenersFor(aRoot);
   }
 }
 
-/* static */ void JSWindowActorService::UnregisterWindowRoot(
-    EventTarget* aRoot) {
+/* static */
+void JSWindowActorService::UnregisterWindowRoot(EventTarget* aRoot) {
   if (gJSWindowActorService) {
     // NOTE: No need to unregister listeners here, as the root is going away.
     gJSWindowActorService->mRoots.RemoveElement(aRoot);
   }
 }
 
 }  // namespace dom
 }  // namespace mozilla
--- a/toolkit/actors/TestChild.jsm
+++ b/toolkit/actors/TestChild.jsm
@@ -6,17 +6,17 @@
 
 var EXPORTED_SYMBOLS = ["TestChild"];
 
 class TestChild extends JSWindowActorChild {
   constructor() {
      super();
   }
 
-  recvAsyncMessage(aMessage) {
+  receiveMessage(aMessage) {
     switch (aMessage.name) {
       case "toChild":
         aMessage.data.toChild = true;
         this.sendAsyncMessage("toParent", aMessage.data);
         break;
       case "done":
         this.done(aMessage.data);
         break;
--- a/toolkit/actors/TestParent.jsm
+++ b/toolkit/actors/TestParent.jsm
@@ -9,17 +9,17 @@ const {Services} = ChromeUtils.import("r
 var EXPORTED_SYMBOLS = ["TestParent"];
 
 class TestParent extends JSWindowActorParent {
   constructor() {
     super();
     this.wrappedJSObject = this;
   }
 
-  recvAsyncMessage(aMessage) {
+  receiveMessage(aMessage) {
     switch (aMessage.name) {
       case "init":
         aMessage.data.initial =  true;
         this.sendAsyncMessage("toChild", aMessage.data);
         break;
       case "toParent":
         aMessage.data.toParent = true;
         this.sendAsyncMessage("done", aMessage.data);