Bug 1582520: Part 3 - Fix RemoteObjectProxy immutable prototype implementation. r=bzbarsky
authorKris Maglione <maglione.k@gmail.com>
Wed, 25 Sep 2019 17:49:58 +0000
changeset 494944 aa9059b3f9b0550ee66625b8409bc7c3119e3ed1
parent 494943 56d226cfe63c1d66955b70137e54ee6b30c43d66
child 494945 2c2c0d216a2f218c726cb112cbd40c8b11a916e2
push id114131
push userdluca@mozilla.com
push dateThu, 26 Sep 2019 09:47:34 +0000
treeherdermozilla-inbound@1dc1a755079a [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersbzbarsky
bugs1582520
milestone71.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 1582520: Part 3 - Fix RemoteObjectProxy immutable prototype implementation. r=bzbarsky Cross-origin objects are supposed to have null prototypes, and throw when attempting to set the prototype to any value other than null. Ordinary cross-origin objects handle this correctly. RemoteObjectProxy has hooks which are meant to give them the same behavior, but which are never actually triggered, because the proxy objects are missing the required lazy prototype flags. Simply using the built-in prototype slot and setting it to immutable triggers the desired behavior with much less implementation overhead. Differential Revision: https://phabricator.services.mozilla.com/D46735
dom/bindings/RemoteObjectProxy.cpp
dom/bindings/RemoteObjectProxy.h
js/xpconnect/tests/mochitest/mochitest.ini
--- a/dom/bindings/RemoteObjectProxy.cpp
+++ b/dom/bindings/RemoteObjectProxy.cpp
@@ -58,47 +58,16 @@ bool RemoteObjectProxyBase::delete_(JSCo
                                     JS::Handle<jsid> aId,
                                     JS::ObjectOpResult& aResult) const {
   // https://html.spec.whatwg.org/multipage/browsers.html#windowproxy-delete
   // step 3 and
   // https://html.spec.whatwg.org/multipage/browsers.html#location-delete step 2
   return ReportCrossOriginDenial(aCx, aId, NS_LITERAL_CSTRING("delete"));
 }
 
-bool RemoteObjectProxyBase::getPrototype(
-    JSContext* aCx, JS::Handle<JSObject*> aProxy,
-    JS::MutableHandle<JSObject*> aProtop) const {
-  // https://html.spec.whatwg.org/multipage/browsers.html#windowproxy-getprototypeof
-  // step 3 and
-  // https://html.spec.whatwg.org/multipage/browsers.html#location-getprototypeof
-  // step 2
-  aProtop.set(nullptr);
-  return true;
-}
-
-bool RemoteObjectProxyBase::setPrototype(JSContext* aCx,
-                                         JS::Handle<JSObject*> aProxy,
-                                         JS::Handle<JSObject*> aProto,
-                                         JS::ObjectOpResult& aResult) const {
-  // https://html.spec.whatwg.org/multipage/browsers.html#windowproxy-setprototypeof
-  // and
-  // https://html.spec.whatwg.org/multipage/browsers.html#location-setprototypeof
-  // say to call SetImmutablePrototype, which does nothing and just returns
-  // whether the passed-in value equals the current prototype. Our current
-  // prototype is always null, so this just comes down to returning whether null
-  // was passed in.
-  //
-  // In terms of ObjectOpResult that means calling one of the fail*() things on
-  // it if non-null was passed, and it's got one that does just what we want.
-  if (!aProto) {
-    return aResult.succeed();
-  }
-  return aResult.failCantSetProto();
-}
-
 bool RemoteObjectProxyBase::getPrototypeIfOrdinary(
     JSContext* aCx, JS::Handle<JSObject*> aProxy, bool* aIsOrdinary,
     JS::MutableHandle<JSObject*> aProtop) const {
   // WindowProxy's and Location's [[GetPrototypeOf]] traps aren't the ordinary
   // definition:
   //
   //   https://html.spec.whatwg.org/multipage/browsers.html#windowproxy-getprototypeof
   //   https://html.spec.whatwg.org/multipage/browsers.html#location-getprototypeof
@@ -182,16 +151,22 @@ void RemoteObjectProxyBase::GetOrCreateP
   options.setClass(aClasp);
   JS::Rooted<JS::Value> native(aCx, JS::PrivateValue(aNative));
   JS::Rooted<JSObject*> obj(
       aCx, js::NewProxyObject(aCx, this, native, nullptr, options));
   if (!obj) {
     return;
   }
 
+  bool success;
+  if (!JS_SetImmutablePrototype(aCx, obj, &success)) {
+    return;
+  }
+  MOZ_ASSERT(success);
+
   aNewObjectCreated = true;
 
   // If we're transplanting onto an object, we want to make sure that it does
   // not have the same class as aClasp to ensure that the release assert earlier
   // in this function will actually fire if we try to return a proxy object in
   // the middle of a transplant.
   MOZ_ASSERT_IF(aTransplantTo, js::GetObjectClass(aTransplantTo) != aClasp);
 
--- a/dom/bindings/RemoteObjectProxy.h
+++ b/dom/bindings/RemoteObjectProxy.h
@@ -40,21 +40,16 @@ class RemoteObjectProxyBase : public js:
                        JS::MutableHandleVector<jsid> aProps) const override;
   bool defineProperty(JSContext* aCx, JS::Handle<JSObject*> aProxy,
                       JS::Handle<jsid> aId,
                       JS::Handle<JS::PropertyDescriptor> aDesc,
                       JS::ObjectOpResult& result) const final;
   bool delete_(JSContext* aCx, JS::Handle<JSObject*> aProxy,
                JS::Handle<jsid> aId, JS::ObjectOpResult& aResult) const final;
 
-  bool getPrototype(JSContext* aCx, JS::Handle<JSObject*> aProxy,
-                    JS::MutableHandle<JSObject*> aProtop) const final;
-  bool setPrototype(JSContext* aCx, JS::Handle<JSObject*> aProxy,
-                    JS::Handle<JSObject*> aProto,
-                    JS::ObjectOpResult& aResult) const final;
   bool getPrototypeIfOrdinary(JSContext* aCx, JS::Handle<JSObject*> aProxy,
                               bool* aIsOrdinary,
                               JS::MutableHandle<JSObject*> aProtop) const final;
 
   bool preventExtensions(JSContext* aCx, JS::Handle<JSObject*> aProxy,
                          JS::ObjectOpResult& aResult) const final;
   bool isExtensible(JSContext* aCx, JS::Handle<JSObject*> aProxy,
                     bool* aExtensible) const final;
--- a/js/xpconnect/tests/mochitest/mochitest.ini
+++ b/js/xpconnect/tests/mochitest/mochitest.ini
@@ -85,17 +85,16 @@ skip-if = toolkit == "android" && debug 
 [test_bug870423.html]
 [test_bug871887.html]
 [test_bug912322.html]
 [test_bug916945.html]
 [test_bug92773.html]
 [test_bug940783.html]
 fail-if = fission
 [test_bug965082.html]
-fail-if = fission
 skip-if = fission && webrender && debug # Crashes intermittently: @ nsDocShell::SetParentWidget(nsIWidget*)
 [test_bug960820.html]
 [test_bug986542.html]
 [test_bug993423.html]
 [test_bug1005806.html]
 [test_bug1094930.html]
 [test_bug1158558.html]
 [test_bug1448048.html]