Bug 1541557: Part 1 - Use correct globals for JSWindowActors not in the shared JSM global. r=nika
☠☠ backed out by f9bf5e4b0b4f ☠ ☠
authorKris Maglione <maglione.k@gmail.com>
Wed, 12 Jun 2019 16:06:40 -0700
changeset 540535 61fa2745733f3631488a3ecccc144823683b7b6d
parent 540534 d2ee912c518913c33e4c63b5a5eddf6e10b0c9f4
child 540536 158a4000c44b9b17a7935340db79431d544fb556
push id11529
push userarchaeopteryx@coole-files.de
push dateThu, 04 Jul 2019 15:22:33 +0000
treeherdermozilla-beta@ebb510a784b8 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersnika
bugs1541557
milestone69.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 1541557: Part 1 - Use correct globals for JSWindowActors not in the shared JSM global. r=nika The current JSWindowActor code assumes that all actors will be created in the shared JSM global. This has a few problems: 1) For actors in other scopes, it enters the wrong compartment before decoding messages, which leads to a compartment checker error when trying to call message handlers. That could be fixed by just wrapping the result for the caller, but that would lead to other problems. Aside from the efficiency concerns of having to deal with cross-compartment wrappers, SpecialPowers in particular would not be able to pass the resulting objects to unprivileged scopes, since only SpecialPowers compartments have permissive CCWs enabled. 2) It also leads to the prototype objects for the actor instances being created in the shared JSM scope, even when the actors themselves are defined in other compartments. Again, aside from CCW efficiency issues, this prevents the SpecialPowers instances from being accessed by the unprivileged scopes that they're exposed to, since the prototype objects live in privileged scopes which don't have permissive CCWs enabled. This patch changes child actors to always create their objects in the global of their constructors. The parent objects are still created in the shared JSM global, but they now wrap values for the appropriate compartment before trying to call message handlers. Differential Revision: https://phabricator.services.mozilla.com/D35051
dom/ipc/JSWindowActor.cpp
dom/ipc/JSWindowActor.h
dom/ipc/JSWindowActorChild.cpp
dom/ipc/JSWindowActorChild.h
dom/ipc/JSWindowActorParent.cpp
dom/ipc/JSWindowActorParent.h
--- a/dom/ipc/JSWindowActor.cpp
+++ b/dom/ipc/JSWindowActor.cpp
@@ -3,16 +3,17 @@
 /* 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/JSWindowActor.h"
 #include "mozilla/dom/JSWindowActorBinding.h"
 #include "mozilla/dom/MessageManagerBinding.h"
 #include "mozilla/dom/PWindowGlobal.h"
+#include "mozilla/dom/Promise.h"
 
 namespace mozilla {
 namespace dom {
 
 NS_INTERFACE_MAP_BEGIN_CYCLE_COLLECTION(JSWindowActor)
   NS_WRAPPERCACHE_INTERFACE_MAP_ENTRY
   NS_INTERFACE_MAP_ENTRY(nsISupports)
 NS_INTERFACE_MAP_END
@@ -30,30 +31,26 @@ NS_IMPL_CYCLE_COLLECTION_UNLINK_END
 NS_IMPL_CYCLE_COLLECTION_TRAVERSE_BEGIN(JSWindowActor)
   NS_IMPL_CYCLE_COLLECTION_TRAVERSE(mPendingQueries)
 NS_IMPL_CYCLE_COLLECTION_TRAVERSE_END
 
 NS_IMPL_CYCLE_COLLECTION_TRACE_WRAPPERCACHE(JSWindowActor)
 
 JSWindowActor::JSWindowActor() : mNextQueryId(0) {}
 
-nsIGlobalObject* JSWindowActor::GetParentObject() const {
-  return xpc::NativeGlobal(xpc::PrivilegedJunkScope());
-}
-
 void JSWindowActor::StartDestroy() {
   DestroyCallback(DestroyCallbackFunction::WillDestroy);
 }
 
 void JSWindowActor::AfterDestroy() {
   DestroyCallback(DestroyCallbackFunction::DidDestroy);
 }
 
 void JSWindowActor::DestroyCallback(DestroyCallbackFunction callback) {
-  AutoEntryScript aes(xpc::PrivilegedJunkScope(),
+  AutoEntryScript aes(GetParentObject(),
                       "JSWindowActor destroy callback");
   JSContext* cx = aes.cx();
   MozActorDestroyCallbacks callbacksHolder;
   NS_ENSURE_TRUE_VOID(GetWrapper());
   JS::Rooted<JS::Value> val(cx, JS::ObjectValue(*GetWrapper()));
   if (NS_WARN_IF(!callbacksHolder.Init(cx, val))) {
     return;
   }
@@ -135,17 +132,17 @@ already_AddRefed<Promise> JSWindowActor:
   mPendingQueries.Put(meta.queryId(), promise);
 
   SendRawMessage(meta, std::move(data), aRv);
   return promise.forget();
 }
 
 void JSWindowActor::ReceiveRawMessage(const JSWindowActorMessageMeta& aMetadata,
                                       ipc::StructuredCloneData&& aData) {
-  AutoEntryScript aes(xpc::PrivilegedJunkScope(),
+  AutoEntryScript aes(GetParentObject(),
                       "JSWindowActor message handler");
   JSContext* cx = aes.cx();
 
   // Read the message into a JS object from IPC.
   ErrorResult error;
   JS::Rooted<JS::Value> data(cx);
   aData.Read(cx, &data, error);
   if (NS_WARN_IF(error.Failed())) {
@@ -230,18 +227,25 @@ void JSWindowActor::ReceiveQueryReply(JS
 
   RefPtr<Promise> promise;
   if (NS_WARN_IF(!mPendingQueries.Remove(aMetadata.queryId(),
                                          getter_AddRefs(promise)))) {
     aRv.Throw(NS_ERROR_DOM_INVALID_STATE_ERR);
     return;
   }
 
+  JSAutoRealm ar(aCx, promise->PromiseObj());
+  JS::RootedValue data(aCx, aData);
+  if (NS_WARN_IF(!JS_WrapValue(aCx, &data))) {
+    aRv.Throw(NS_ERROR_FAILURE);
+    return;
+  }
+
   if (aMetadata.kind() == JSWindowActorMessageKind::QueryResolve) {
-    promise->MaybeResolve(aCx, aData);
+    promise->MaybeResolve(aCx, data);
   } else {
     promise->MaybeReject(NS_ERROR_DOM_OPERATION_ERR);
   }
 }
 
 // Native handler for our generated promise which is used to handle Queries and
 // send the reply when their promises have been resolved.
 JSWindowActor::QueryHandler::QueryHandler(
--- a/dom/ipc/JSWindowActor.h
+++ b/dom/ipc/JSWindowActor.h
@@ -51,17 +51,17 @@ class JSWindowActor : public nsISupports
                                       const nsAString& aMessageName,
                                       JS::Handle<JS::Value> aObj,
                                       JS::Handle<JS::Value> aTransfers,
                                       ErrorResult& aRv);
 
   void ReceiveRawMessage(const JSWindowActorMessageMeta& aMetadata,
                          ipc::StructuredCloneData&& aData);
 
-  nsIGlobalObject* GetParentObject() const;
+  virtual nsIGlobalObject* GetParentObject() const = 0;
 
   void RejectPendingQueries();
 
  protected:
   // Send the message described by the structured clone data |aData|, and the
   // message metadata |aMetadata|. The underlying transport should call the
   // |ReceiveMessage| method on the other side asynchronously.
   virtual void SendRawMessage(const JSWindowActorMessageMeta& aMetadata,
--- a/dom/ipc/JSWindowActorChild.cpp
+++ b/dom/ipc/JSWindowActorChild.cpp
@@ -12,16 +12,20 @@
 #include "mozilla/dom/WindowProxyHolder.h"
 #include "mozilla/dom/MessageManagerBinding.h"
 #include "mozilla/dom/BrowsingContext.h"
 #include "nsGlobalWindowInner.h"
 
 namespace mozilla {
 namespace dom {
 
+JSWindowActorChild::JSWindowActorChild(nsIGlobalObject* aGlobal)
+    : mGlobal(aGlobal ? aGlobal
+                      : xpc::NativeGlobal(xpc::PrivilegedJunkScope())) {}
+
 JSWindowActorChild::~JSWindowActorChild() { MOZ_ASSERT(!mManager); }
 
 JSObject* JSWindowActorChild::WrapObject(JSContext* aCx,
                                          JS::Handle<JSObject*> aGivenProto) {
   return JSWindowActorChild_Binding::Wrap(aCx, this, aGivenProto);
 }
 
 WindowGlobalChild* JSWindowActorChild::GetManager() const { return mManager; }
--- a/dom/ipc/JSWindowActorChild.h
+++ b/dom/ipc/JSWindowActorChild.h
@@ -32,22 +32,27 @@ namespace mozilla {
 namespace dom {
 
 class JSWindowActorChild final : public JSWindowActor {
  public:
   NS_DECL_ISUPPORTS_INHERITED
   NS_DECL_CYCLE_COLLECTION_SCRIPT_HOLDER_CLASS_INHERITED(JSWindowActorChild,
                                                          JSWindowActor)
 
+  explicit JSWindowActorChild(nsIGlobalObject* aGlobal = nullptr);
+
+  nsIGlobalObject* GetParentObject() const override { return mGlobal; }
+
   JSObject* WrapObject(JSContext* aCx,
                        JS::Handle<JSObject*> aGivenProto) override;
 
   static already_AddRefed<JSWindowActorChild> Constructor(GlobalObject& aGlobal,
                                                           ErrorResult& aRv) {
-    return MakeAndAddRef<JSWindowActorChild>();
+    nsCOMPtr<nsIGlobalObject> global(do_QueryInterface(aGlobal.GetAsSupports()));
+    return MakeAndAddRef<JSWindowActorChild>(global);
   }
 
   WindowGlobalChild* GetManager() const;
   void Init(const nsAString& aName, WindowGlobalChild* aManager);
   void StartDestroy();
   void AfterDestroy();
   Document* GetDocument(ErrorResult& aRv);
   BrowsingContext* GetBrowsingContext(ErrorResult& aRv);
@@ -59,14 +64,16 @@ class JSWindowActorChild final : public 
                       ipc::StructuredCloneData&& aData,
                       ErrorResult& aRv) override;
 
  private:
   ~JSWindowActorChild();
 
   bool mCanSend = true;
   RefPtr<WindowGlobalChild> mManager;
+
+  nsCOMPtr<nsIGlobalObject> mGlobal;
 };
 
 }  // namespace dom
 }  // namespace mozilla
 
 #endif  // mozilla_dom_JSWindowActorChild_h
--- a/dom/ipc/JSWindowActorParent.cpp
+++ b/dom/ipc/JSWindowActorParent.cpp
@@ -9,16 +9,20 @@
 #include "mozilla/dom/WindowGlobalParent.h"
 #include "mozilla/dom/MessageManagerBinding.h"
 
 namespace mozilla {
 namespace dom {
 
 JSWindowActorParent::~JSWindowActorParent() { MOZ_ASSERT(!mManager); }
 
+nsIGlobalObject* JSWindowActorParent::GetParentObject() const {
+  return xpc::NativeGlobal(xpc::PrivilegedJunkScope());
+}
+
 JSObject* JSWindowActorParent::WrapObject(JSContext* aCx,
                                           JS::Handle<JSObject*> aGivenProto) {
   return JSWindowActorParent_Binding::Wrap(aCx, this, aGivenProto);
 }
 
 WindowGlobalParent* JSWindowActorParent::GetManager() const { return mManager; }
 
 void JSWindowActorParent::Init(const nsAString& aName,
--- a/dom/ipc/JSWindowActorParent.h
+++ b/dom/ipc/JSWindowActorParent.h
@@ -35,16 +35,18 @@ class JSWindowActorParent final : public
   JSObject* WrapObject(JSContext* aCx,
                        JS::Handle<JSObject*> aGivenProto) override;
 
   static already_AddRefed<JSWindowActorParent> Constructor(
       GlobalObject& aGlobal, ErrorResult& aRv) {
     return MakeAndAddRef<JSWindowActorParent>();
   }
 
+  nsIGlobalObject* GetParentObject() const override;
+
   WindowGlobalParent* GetManager() const;
   void Init(const nsAString& aName, WindowGlobalParent* aManager);
   void StartDestroy();
   void AfterDestroy();
   CanonicalBrowsingContext* GetBrowsingContext(ErrorResult& aRv);
 
  protected:
   void SendRawMessage(const JSWindowActorMessageMeta& aMeta,