Bug 859957 - Simplify [[DefaultValue]] security wrapping setup. r=mrbkap,gabor,ejpbruel

js/src/jswrapper.cpp

@@ -126,34 +126,6 @@ Wrapper::~Wrapper()
 {
 }
 
-/*
- * Ordinarily, the convert trap would require unwrapping. However, the default
- * implementation of convert, JS_ConvertStub, obtains a default value by calling
- * the toString/valueOf method on the wrapper, if any. Throwing if we can't unwrap
- * in this case would be overly conservative. To make matters worse, XPConnect sometimes
- * installs a custom convert trap that obtains a default value by calling the
- * toString method on the wrapper. Doing a puncture in this case would be overly
- * conservative as well. We deal with these anomalies by falling back to the DefaultValue
- * algorithm whenever unwrapping is forbidden.
- */
-bool
-Wrapper::defaultValue(JSContext *cx, HandleObject wrapper, JSType hint, MutableHandleValue vp)
-{
-    if (!wrapperHandler(wrapper)->isSafeToUnwrap())
-        return DefaultValue(cx, wrapper, hint, vp);
-
-    /*
-     * We enter the compartment of the wrappee here, even if we're not a cross
-     * compartment wrapper. Moreover, cross compartment wrappers do not enter
-     * the compartment of the wrappee before calling this function. This is
-     * necessary because the DefaultValue algorithm above operates on the
-     * wrapper, not the wrappee, so we want to delay the decision to switch
-     * compartments until this point.
-     */
-    AutoCompartment call(cx, wrappedObject(wrapper));
-    return DirectProxyHandler::defaultValue(cx, wrapper, hint, vp);
-}
-
 Wrapper Wrapper::singleton((unsigned)0);
 Wrapper Wrapper::singletonWithPrototype((unsigned)0, true);
 
@@ -608,9 +580,10 @@ bool
 CrossCompartmentWrapper::defaultValue(JSContext *cx, HandleObject wrapper, JSType hint,
                                       MutableHandleValue vp)
 {
-    if (!Wrapper::defaultValue(cx, wrapper, hint, vp))
-        return false;
-    return cx->compartment->wrap(cx, vp);
+    PIERCE(cx, wrapper,
+           NOTHING,
+           Wrapper::defaultValue(cx, wrapper, hint, vp),
+           cx->compartment->wrap(cx, vp));
 }
 
 bool
@@ -684,6 +657,17 @@ SecurityWrapper<Base>::nativeCall(JSContext *cx, IsAcceptableThis test, NativeIm
     return false;
 }
 
+// For security wrappers, we run the DefaultValue algorithm on the wrapper
+// itself, which means that the existing security policy on operations like
+// toString() will take effect and do the right thing here.
+template <class Base>
+bool
+SecurityWrapper<Base>::defaultValue(JSContext *cx, HandleObject wrapper,
+                                    JSType hint, MutableHandleValue vp)
+{
+    return DefaultValue(cx, wrapper, hint, vp);
+}
+
 template <class Base>
 bool
 SecurityWrapper<Base>::objectClassIs(HandleObject obj, ESClassValue classValue, JSContext *cx)

js/src/jswrapper.h

@@ -65,10 +65,6 @@ class JS_FRIEND_API(Wrapper) : public DirectProxyHandler
 
     virtual ~Wrapper();
 
-    /* ES5 Harmony fundamental wrapper traps. */
-    virtual bool defaultValue(JSContext *cx, HandleObject wrapper, JSType hint,
-                              MutableHandleValue vp) MOZ_OVERRIDE;
-
     static Wrapper singleton;
     static Wrapper singletonWithPrototype;
 
@@ -150,6 +146,8 @@ class JS_FRIEND_API(SecurityWrapper) : public Base
                        bool *bp) MOZ_OVERRIDE;
     virtual bool nativeCall(JSContext *cx, IsAcceptableThis test, NativeImpl impl,
                             CallArgs args) MOZ_OVERRIDE;
+    virtual bool defaultValue(JSContext *cx, HandleObject wrapper, JSType hint,
+                              MutableHandleValue vp) MOZ_OVERRIDE;
     virtual bool objectClassIs(HandleObject obj, ESClassValue classValue,
                                JSContext *cx) MOZ_OVERRIDE;
     virtual bool regexp_toShared(JSContext *cx, HandleObject proxy, RegExpGuard *g) MOZ_OVERRIDE;