Bug 1521906 part 4 - Remove CheckedUnwrap and rename UnwrapOneChecked to UnwrapOneCheckedStatic. r=bzbarsky
authorJan de Mooij <jdemooij@mozilla.com>
Fri, 01 Mar 2019 09:21:11 +0000
changeset 519816 4325cfacfc496ac188a4307ad1e373d1736d3aa9
parent 519815 860d0c16a0fcc81f659000e6eb20f05794adb8a2
child 519817 10ba5f802d6666a39ce50f4bc597c68e2d130672
push id10862
push userffxbld-merge
push dateMon, 11 Mar 2019 13:01:11 +0000
treeherdermozilla-beta@a2e7f5c935da [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersbzbarsky
bugs1521906
milestone67.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 1521906 part 4 - Remove CheckedUnwrap and rename UnwrapOneChecked to UnwrapOneCheckedStatic. r=bzbarsky The CacheIR code only sees transparent CCWs so it's fine to do a static unwrap. DebuggerObject::unwrap is more complicated. We're in the debugger's compartment there; I went with UnwrapOneCheckedStatic as it seems safest and simplest for now. Differential Revision: https://phabricator.services.mozilla.com/D21354
js/public/Wrapper.h
js/rust/src/glue.rs
js/rust/src/jsglue.cpp
js/src/jit/CacheIR.cpp
js/src/proxy/Wrapper.cpp
js/src/vm/Debugger.cpp
--- a/js/public/Wrapper.h
+++ b/js/public/Wrapper.h
@@ -412,27 +412,19 @@ JS_FRIEND_API JSObject* UncheckedUnwrap(
 // example, if you want to test whether your target object is a specific class
 // that's not WindowProxy or Location, you can use this.
 //
 // ExposeToActiveJS is called on wrapper targets to allow gray marking
 // assertions to work while an incremental GC is in progress, but this means
 // that this cannot be called from the GC or off the main thread.
 JS_FRIEND_API JSObject* CheckedUnwrapStatic(JSObject* obj);
 
-// Old CheckedUnwrap API that we would like to remove once we convert all
-// callers to CheckedUnwrapStatic or CheckedUnwrapDynamic.  If stopAtWindowProxy
-// is true, then this returns the WindowProxy if a WindowProxy is encountered;
-// otherwise it will unwrap the WindowProxy and return a Window.
-JS_FRIEND_API JSObject* CheckedUnwrap(JSObject* obj,
-                                      bool stopAtWindowProxy = true);
-
 // Unwrap only the outermost security wrapper, with the same semantics as
 // above. This is the checked version of Wrapper::wrappedObject.
-JS_FRIEND_API JSObject* UnwrapOneChecked(JSObject* obj,
-                                         bool stopAtWindowProxy = true);
+JS_FRIEND_API JSObject* UnwrapOneCheckedStatic(JSObject* obj);
 
 // Given a JSObject, returns that object stripped of wrappers. At each stage,
 // the security wrapper has the opportunity to veto the unwrap. If
 // stopAtWindowProxy is true, then this returns the WindowProxy if it was
 // previously wrapped.  Null is returned if there are security wrappers that
 // can't be unwrapped.
 //
 // ExposeToActiveJS is called on wrapper targets to allow gray marking
--- a/js/rust/src/glue.rs
+++ b/js/rust/src/glue.rs
@@ -250,17 +250,17 @@ extern "C" {
     pub fn RUST_js_GetErrorMessage(userRef: *mut ::libc::c_void,
                                    errorNumber: u32)
                                    -> *const JSErrorFormatString;
     pub fn IsProxyHandlerFamily(obj: *mut JSObject) -> u8;
     pub fn GetProxyHandlerExtra(obj: *mut JSObject) -> *const ::libc::c_void;
     pub fn GetProxyHandler(obj: *mut JSObject) -> *const ::libc::c_void;
     pub fn ReportError(aCx: *mut JSContext, aError: *const i8);
     pub fn IsWrapper(obj: *mut JSObject) -> bool;
-    pub fn UnwrapObject(obj: *mut JSObject, stopAtOuter: u8) -> *mut JSObject;
+    pub fn UnwrapObjectStatic(obj: *mut JSObject) -> *mut JSObject;
     pub fn UncheckedUnwrapObject(obj: *mut JSObject, stopAtOuter: u8) -> *mut JSObject;
     pub fn CreateAutoIdVector(cx: *mut JSContext) -> *mut JS::AutoIdVector;
     pub fn AppendToAutoIdVector(v: *mut JS::AutoIdVector, id: jsid) -> bool;
     pub fn SliceAutoIdVector(v: *const JS::AutoIdVector, length: *mut usize) -> *const jsid;
     pub fn DestroyAutoIdVector(v: *mut JS::AutoIdVector);
     pub fn CreateAutoObjectVector(aCx: *mut JSContext) -> *mut JS::AutoObjectVector;
     pub fn AppendToAutoObjectVector(v: *mut JS::AutoObjectVector, obj: *mut JSObject) -> bool;
     pub fn DeleteAutoObjectVector(v: *mut JS::AutoObjectVector);
--- a/js/rust/src/jsglue.cpp
+++ b/js/rust/src/jsglue.cpp
@@ -525,18 +525,18 @@ void ReportError(JSContext* aCx, const c
     assert(*p != '%');
   }
 #endif
   JS_ReportErrorASCII(aCx, "%s", aError);
 }
 
 bool IsWrapper(JSObject* obj) { return js::IsWrapper(obj); }
 
-JSObject* UnwrapObject(JSObject* obj, bool stopAtOuter) {
-  return js::CheckedUnwrap(obj, stopAtOuter);
+JSObject* UnwrapObjectStatic(JSObject* obj) {
+  return js::CheckedUnwrapStatic(obj);
 }
 
 JSObject* UncheckedUnwrapObject(JSObject* obj, bool stopAtOuter) {
   return js::UncheckedUnwrap(obj, stopAtOuter);
 }
 
 JS::AutoIdVector* CreateAutoIdVector(JSContext* cx) {
   return new JS::AutoIdVector(cx);
--- a/js/src/jit/CacheIR.cpp
+++ b/js/src/jit/CacheIR.cpp
@@ -1274,17 +1274,17 @@ bool GetPropIRGenerator::tryAttachCrossC
 
   // If we're megamorphic prefer a generic proxy stub that handles a lot more
   // cases.
   if (mode_ == ICState::Mode::Megamorphic) {
     return false;
   }
 
   RootedObject unwrapped(cx_, Wrapper::wrappedObject(obj));
-  MOZ_ASSERT(unwrapped == UnwrapOneChecked(obj));
+  MOZ_ASSERT(unwrapped == UnwrapOneCheckedStatic(obj));
   MOZ_ASSERT(!IsCrossCompartmentWrapper(unwrapped),
              "CCWs must not wrap other CCWs");
 
   // If we allowed different zones we would have to wrap strings.
   if (unwrapped->compartment()->zone() != cx_->compartment()->zone()) {
     return false;
   }
 
--- a/js/src/proxy/Wrapper.cpp
+++ b/js/src/proxy/Wrapper.cpp
@@ -351,39 +351,34 @@ JS_FRIEND_API JSObject* js::UncheckedUnw
   }
   if (flagsp) {
     *flagsp = flags;
   }
   return wrapped;
 }
 
 JS_FRIEND_API JSObject* js::CheckedUnwrapStatic(JSObject* obj) {
-  // For now, just forward to the old API.  Once we remove it, we can
-  // inline it here, without the stopAtWindowProxy bits.
-  return CheckedUnwrap(obj);
-}
-
-JS_FRIEND_API JSObject* js::CheckedUnwrap(JSObject* obj,
-                                          bool stopAtWindowProxy) {
   while (true) {
     JSObject* wrapper = obj;
-    obj = UnwrapOneChecked(obj, stopAtWindowProxy);
+    obj = UnwrapOneCheckedStatic(obj);
     if (!obj || obj == wrapper) {
       return obj;
     }
   }
 }
 
-JS_FRIEND_API JSObject* js::UnwrapOneChecked(JSObject* obj,
-                                             bool stopAtWindowProxy) {
+JS_FRIEND_API JSObject* js::UnwrapOneCheckedStatic(JSObject* obj) {
   MOZ_ASSERT(!JS::RuntimeHeapIsCollecting());
   MOZ_ASSERT(CurrentThreadCanAccessRuntime(obj->runtimeFromAnyThread()));
 
-  if (!obj->is<WrapperObject>() ||
-      MOZ_UNLIKELY(stopAtWindowProxy && IsWindowProxy(obj))) {
+  // Note: callers that care about WindowProxy unwrapping should use
+  // CheckedUnwrapDynamic or UnwrapOneCheckedDynamic instead of this. We don't
+  // unwrap WindowProxy here to preserve legacy behavior and for consistency
+  // with CheckedUnwrapDynamic's default stopAtWindowProxy = true.
+  if (!obj->is<WrapperObject>() || MOZ_UNLIKELY(IsWindowProxy(obj))) {
     return obj;
   }
 
   const Wrapper* handler = Wrapper::wrapperHandler(obj);
   return handler->hasSecurityPolicy() ? nullptr : Wrapper::wrappedObject(obj);
 }
 
 JS_FRIEND_API JSObject* js::CheckedUnwrapDynamic(JSObject* obj, JSContext* cx,
--- a/js/src/vm/Debugger.cpp
+++ b/js/src/vm/Debugger.cpp
@@ -12073,17 +12073,17 @@ double DebuggerObject::promiseTimeToReso
 }
 
 /* static */ bool DebuggerObject::unwrap(JSContext* cx,
                                          HandleDebuggerObject object,
                                          MutableHandleDebuggerObject result) {
   RootedObject referent(cx, object->referent());
   Debugger* dbg = object->owner();
 
-  RootedObject unwrapped(cx, UnwrapOneChecked(referent));
+  RootedObject unwrapped(cx, UnwrapOneCheckedStatic(referent));
   if (!unwrapped) {
     result.set(nullptr);
     return true;
   }
 
   // Don't allow unwrapping to create a D.O whose referent is in an
   // invisible-to-Debugger compartment. (If our referent is a *wrapper* to such,
   // and the wrapper is in a visible compartment, that's fine.)