Bug 1559489, part 4 - Remote-to-local window transplanting. r=tcampbell,bzbarsky

This patch cleans up remote outer window proxies when we navigate back
into the process.

It adds a flag to mDanglingRemoteOuterProxies that is set in between
BrowsingContext::SetDocShell(), where we can tell that the browsing
context is going from being remote to being local, to
nsGlobalWindowOuter::SetNewDocument(), where the local outer window
proxy is actually created. Once the outer window is created, the
remote window proxies can be cleaned up in
CleanUpDanglingRemoteOuterWindowProxies().

The clean up is done by a process that is similar to object
transplanting, except that instead of looking in the cross-compartment
wrapper table for each compartment to find objects to be turned into
CCWs to the new object, it looks in the remote proxy map for each
compartment. SpiderMonkey doesn't know about the proxy maps, so this
has to be done by a new callback object CompartmentTransplantCallback.

Now that this cleanup is being done, it shouldn't be possible to wrap
a remote outer window proxy when the browsing context is local, so
MaybeWrapWindowProxy() can be simplified. I had to drop the assert
here that the browsing context has a window proxy because during clean
up we call wrap on a local outer window proxy before the BC gets the
window proxy set on it. I had the assert because my original plan was
to implicitly fix remote proxies during wrapping, but that is no
longer necessary.

Differential Revision: https://phabricator.services.mozilla.com/D38343

--HG--
extra : moz-landing-system : lando

docshell/base/BrowsingContext.cpp

@@ -197,7 +197,8 @@ BrowsingContext::BrowsingContext(BrowsingContext* aParent,
       mGroup(aGroup),
       mParent(aParent),
       mIsInProcess(false),
-      mIsDiscarded(false) {
+      mIsDiscarded(false),
+      mDanglingRemoteOuterProxies(false) {
   MOZ_RELEASE_ASSERT(!mParent || mParent->Group() == mGroup);
   MOZ_RELEASE_ASSERT(mBrowsingContextId != 0);
   MOZ_RELEASE_ASSERT(mGroup);
@@ -208,9 +209,54 @@ void BrowsingContext::SetDocShell(nsIDocShell* aDocShell) {
   // process to the parent & do other validation here.
   MOZ_RELEASE_ASSERT(aDocShell->GetBrowsingContext() == this);
   mDocShell = aDocShell;
+  mDanglingRemoteOuterProxies = !mIsInProcess;
   mIsInProcess = true;
 }
 
+// This class implements a callback that will return the remote window proxy for
+// mBrowsingContext in that compartment, if it has one. It also removes the
+// proxy from the map, because the object will be transplanted into another kind
+// of object.
+class MOZ_STACK_CLASS CompartmentRemoteProxyTransplantCallback
+    : public js::CompartmentTransplantCallback {
+ public:
+  explicit CompartmentRemoteProxyTransplantCallback(
+      BrowsingContext* aBrowsingContext)
+      : mBrowsingContext(aBrowsingContext) {}
+
+  virtual JSObject* getObjectToTransplant(
+      JS::Compartment* compartment) override {
+    auto* priv = xpc::CompartmentPrivate::Get(compartment);
+    if (!priv) {
+      return nullptr;
+    }
+
+    auto& map = priv->GetRemoteProxyMap();
+    auto result = map.lookup(mBrowsingContext);
+    if (!result) {
+      return nullptr;
+    }
+    JSObject* resultObject = result->value();
+    map.remove(result);
+
+    return resultObject;
+  }
+
+ private:
+  BrowsingContext* mBrowsingContext;
+};
+
+void BrowsingContext::CleanUpDanglingRemoteOuterWindowProxies(
+    JSContext* aCx, JS::MutableHandle<JSObject*> aOuter) {
+  if (!mDanglingRemoteOuterProxies) {
+    return;
+  }
+  mDanglingRemoteOuterProxies = false;
+
+  CompartmentRemoteProxyTransplantCallback cb(this);
+  js::RemapRemoteWindowProxies(aCx, &cb, aOuter);
+}
+
 void BrowsingContext::SetEmbedderElement(Element* aEmbedder) {
   // Notify the parent process of the embedding status. We don't need to do
   // this when clearing our embedder, as we're being destroyed either way.

docshell/base/BrowsingContext.h

@@ -139,6 +139,15 @@ class BrowsingContext : public nsWrapperCache, public BrowsingContextBase {
   void SetDocShell(nsIDocShell* aDocShell);
   void ClearDocShell() { mDocShell = nullptr; }
 
+  // This cleans up remote outer window proxies that might have been left behind
+  // when the browsing context went from being remote to local. It does this by
+  // turning them into cross-compartment wrappers to aOuter. If there is already
+  // a remote proxy in the compartment of aOuter, then aOuter will get swapped
+  // to it and the value of aOuter will be set to the object that used to be the
+  // remote proxy and is now an OuterWindowProxy.
+  void CleanUpDanglingRemoteOuterWindowProxies(
+      JSContext* aCx, JS::MutableHandle<JSObject*> aOuter);
+
   // Get the embedder element for this BrowsingContext if the embedder is
   // in-process, or null if it's not.
   Element* GetEmbedderElement() const { return mEmbedderElement; }
@@ -491,6 +500,10 @@ class BrowsingContext : public nsWrapperCache, public BrowsingContextBase {
   // Has this browsing context been discarded? BrowsingContexts should
   // only be discarded once.
   bool mIsDiscarded : 1;
+
+  // This is true if the BrowsingContext was out of process, but is now in
+  // process, and might have remote window proxies that need to be cleaned up.
+  bool mDanglingRemoteOuterProxies : 1;
 };
 
 /**

dom/base/nsGlobalWindowOuter.cpp

@@ -2070,6 +2070,9 @@ nsresult nsGlobalWindowOuter::SetNewDocument(Document* aDocument,
           cx, NewOuterWindowProxy(cx, newInnerGlobal, thisChrome));
       NS_ENSURE_TRUE(outer, NS_ERROR_FAILURE);
 
+      mBrowsingContext->CleanUpDanglingRemoteOuterWindowProxies(cx, &outer);
+      MOZ_ASSERT(js::IsWindowProxy(outer));
+
       js::SetProxyReservedSlot(outer, OUTER_WINDOW_SLOT,
                                js::PrivateValue(ToSupports(this)));
 

dom/file/tests/mochitest.ini

@@ -44,9 +44,7 @@ skip-if = (toolkit == 'android') # Android: Bug 775227
 [test_fileapi_slice_image.html]
 skip-if = (toolkit == 'android') # Android: Bug 775227
 [test_mozfiledataurl.html]
-skip-if =
-  toolkit == 'android' || #TIMED_OUT
-  fission # Crashes: @ mozilla::dom::RemoteObjectProxyBase::GetOrCreateProxyObject(JSContext*, void*, js::Class const*, JS::Handle<JSObject*>, JS::MutableHandle<JSObject*>, bool&) const
+skip-if = toolkit == 'android' #TIMED_OUT
 [test_bug1507893.html]
 support-files = worker_bug1507893.js
 [test_blob_reading.html]

dom/security/test/general/mochitest.ini

@@ -39,13 +39,11 @@ skip-if = toolkit == 'android'
 skip-if = toolkit == 'android'
 [test_block_subresource_redir_to_data.html]
 [test_same_site_cookies_subrequest.html]
-skip-if = fission && debug # Crashes: @ mozilla::dom::RemoteObjectProxyBase::GetOrCreateProxyObject(JSContext*, void*, js::Class const*, JS::Handle<JSObject*>, JS::MutableHandle<JSObject*>, bool&) const
 [test_same_site_cookies_toplevel_nav.html]
 skip-if = fission # Crashes: @ mozilla::dom::ContentParent::CommonCreateWindow(mozilla::dom::PBrowserParent*, bool, unsigned int const&, bool const&, bool const&, bool const&, nsIURI*, nsTString<char> const&, float const&, unsigned long, nsTString<char16_t> const&, nsresult&, nsCOMPtr<nsIRemoteTab>&, bool*, int&, nsIPrincipal*, nsIReferrerInfo*, bool, nsIContentSecurityPolicy*)
 [test_same_site_cookies_cross_origin_context.html]
 [test_same_site_cookies_from_script.html]
 [test_same_site_cookies_redirect.html]
-skip-if = fission # Crashes: @ mozilla::dom::RemoteObjectProxyBase::GetOrCreateProxyObject(JSContext*, void*, js::Class const*, JS::Handle<JSObject*>, JS::MutableHandle<JSObject*>, bool&) const
 [test_same_site_cookies_toplevel_set_cookie.html]
 skip-if = fission
 [test_same_site_cookies_iframe.html]

dom/tests/browser/browser_windowProxy_transplant.js

@@ -90,6 +90,11 @@ add_task(async function() {
 
         content.win1 = iframe.contentWindow;
         let chromeWin1 = iframe.contentWindow;
+        let chromeWin1x = Cu.waiveXrays(iframe.contentWindow);
+        content.win1x = Cu.waiveXrays(iframe.contentWindow);
+
+        ok(chromeWin1 != chromeWin1x, "waiving xrays creates a new thing?");
+
         content.bc1 = iframe.browsingContext;
 
         is(
@@ -148,6 +153,14 @@ add_task(async function() {
           !Cu.isDeadWrapper(chromeWin1),
           "chromeWin1 shouldn't be a dead wrapper after navigation"
         );
+        ok(
+          Cu.isDeadWrapper(chromeWin1x),
+          "chromeWin1x should be a dead wrapper after navigation"
+        );
+        ok(
+          Cu.isDeadWrapper(content.win1x),
+          "content.win1x should be a dead wrapper after navigation"
+        );
 
         is(
           content.bc1,
@@ -193,8 +206,9 @@ add_task(async function() {
           content.bc4,
           "cross to same-origin navigation BrowsingContext match"
         );
-        todo(
-          content.win3 == content.win4,
+        is(
+          content.win3,
+          content.win4,
           "cross to same-origin navigation WindowProxy match"
         );
       }

js/src/jsapi.cpp

@@ -741,6 +741,71 @@ JS_PUBLIC_API JSObject* JS_TransplantObject(JSContext* cx, HandleObject origobj,
   return newIdentity;
 }
 
+JS_FRIEND_API void js::RemapRemoteWindowProxies(
+    JSContext* cx, CompartmentTransplantCallback* callback,
+    MutableHandleObject target) {
+  AssertHeapIsIdle();
+  CheckTransplantObject(target);
+  ReleaseAssertObjectHasNoWrappers(cx, target);
+
+  // |target| can't be a remote proxy, because we expect it to get a CCW when
+  // wrapped across compartments.
+  MOZ_ASSERT(!js::IsDOMRemoteProxyObject(target));
+
+  // Don't allow a compacting GC to observe any intermediate state.
+  AutoDisableCompactingGC nocgc(cx);
+
+  AutoDisableProxyCheck adpc;
+
+  AutoEnterOOMUnsafeRegion oomUnsafe;
+  if (!CheckSystemRecursionLimit(cx)) {
+    oomUnsafe.crash("js::RemapRemoteWindowProxies");
+  }
+
+  RootedObject targetCompartmentProxy(cx);
+  JS::RootedVector<JSObject*> otherProxies(cx);
+
+  // Use the callback to find remote proxies in all compartments that match
+  // whatever criteria callback uses.
+  for (CompartmentsIter c(cx->runtime()); !c.done(); c.next()) {
+    RootedObject remoteProxy(cx, callback->getObjectToTransplant(c));
+    if (!remoteProxy) {
+      continue;
+    }
+    // The object the callback returns should be a DOM remote proxy object in
+    // the compartment c. We rely on it being a DOM remote proxy because that
+    // means that it won't have any cross-compartment wrappers.
+    MOZ_ASSERT(js::IsDOMRemoteProxyObject(remoteProxy));
+    MOZ_ASSERT(remoteProxy->compartment() == c);
+    CheckTransplantObject(remoteProxy);
+
+    // Immediately turn the DOM remote proxy object into a dead proxy object
+    // so we don't have to worry about anything weird going on with it.
+    js::NukeNonCCWProxy(cx, remoteProxy);
+
+    if (remoteProxy->compartment() == target->compartment()) {
+      targetCompartmentProxy = remoteProxy;
+    } else if (!otherProxies.append(remoteProxy)) {
+      oomUnsafe.crash("js::RemapRemoteWindowProxies");
+    }
+  }
+
+  // If there was a remote proxy in |target|'s compartment, we need to use it
+  // instead of |target|, in case it had any references, so swap it. Do this
+  // before any other compartment so that the target object will be set up
+  // correctly before we start wrapping it into other compartments.
+  if (targetCompartmentProxy) {
+    AutoRealm ar(cx, targetCompartmentProxy);
+    JSObject::swap(cx, targetCompartmentProxy, target);
+    target.set(targetCompartmentProxy);
+  }
+
+  for (JSObject*& obj : otherProxies) {
+    RootedObject deadWrapper(cx, obj);
+    js::RemapDeadWrapper(cx, deadWrapper, target);
+  }
+}
+
 /*
  * Recompute all cross-compartment wrappers for an object, resetting state.
  * Gecko uses this to clear Xray wrappers when doing a navigation that reuses

js/src/jsfriendapi.h

@@ -2684,6 +2684,22 @@ extern JS_FRIEND_API uint64_t GetGCHeapUsageForObjectZone(JSObject* obj);
  */
 extern JS_FRIEND_API bool GlobalHasInstrumentation(JSObject* global);
 
+class JS_FRIEND_API CompartmentTransplantCallback {
+ public:
+  virtual JSObject* getObjectToTransplant(JS::Compartment* compartment) = 0;
+};
+
+// Gather a set of remote window proxies by calling the callback on every
+// compartment, then transform them into cross-compartment wrappers to newTarget
+// via brain transplants. If there's a proxy in newTarget's compartment, it will
+// get swapped with newTarget, and the value of newTarget will be updated. If
+// the callback returns null for a compartment, no cross-compartment wrapper
+// will be created for that compartment. Any non-null values it returns must be
+// DOM remote proxies from the compartment that was passed in.
+extern JS_FRIEND_API void RemapRemoteWindowProxies(
+    JSContext* cx, CompartmentTransplantCallback* callback,
+    JS::MutableHandleObject newTarget);
+
 } /* namespace js */
 
 #endif /* jsfriendapi_h */

js/xpconnect/wrappers/WrapperFactory.cpp

@@ -180,19 +180,12 @@ static bool MaybeWrapWindowProxy(JSContext* cx, HandleObject origObj,
   }
 
   if (bc->IsInProcess()) {
-    // If the bc is in process, we should have a local window proxy for it
-    // already.
-    MOZ_ASSERT(bc->GetWindowProxy());
+    // Any remote window proxies for bc should have been cleaned up by a call to
+    // CleanUpDanglingRemoteOuterWindowProxies() before now, so obj must be a
+    // local outer window proxy.
+    MOZ_RELEASE_ASSERT(isWindowProxy);
 
-    if (isWindowProxy) {
-      retObj.set(obj);
-    } else {
-      // XXX Turning remote window proxies into local window proxies will be
-      // implemented as part of bug 1559489, so don't allow it for now. To
-      // implement bug 1559489, we can return bc->GetWindowProxy() for the
-      // remote proxy case.
-      retObj.set(JS_NewDeadWrapper(cx));
-    }
+    retObj.set(obj);
   } else {
     // If bc is not in process, then use a remote window proxy, whether or not
     // obj is one already.