Bug 771908 - Get rid of Wrapper::leave; r=bholley, sr=mrbkap
authorEddy Bruel <ejpbruel@mozilla.com>
Wed, 11 Jul 2012 14:01:10 +0200
changeset 98949 ba89d552b9dc7da024b6756488512906870c7a43
parent 98948 9bfc3490136e127e743bbe8af59ea4f02e09faa5
child 98950 12c1f43db3d159619ec62d7216d290a41ab489d6
push id11738
push userejpbruel@mozilla.com
push dateWed, 11 Jul 2012 12:01:07 +0000
treeherdermozilla-inbound@ba89d552b9dc [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersbholley, mrbkap
bugs771908
milestone16.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 771908 - Get rid of Wrapper::leave; r=bholley, sr=mrbkap
js/src/jswrapper.cpp
js/src/jswrapper.h
js/xpconnect/wrappers/XrayWrapper.cpp
--- a/js/src/jswrapper.cpp
+++ b/js/src/jswrapper.cpp
@@ -75,21 +75,16 @@ Wrapper::Wrapper(unsigned flags) : mFlag
 
 bool
 Wrapper::enter(JSContext *cx, JSObject *wrapper, jsid id, Action act, bool *bp)
 {
     *bp = true;
     return true;
 }
 
-void
-Wrapper::leave(JSContext *cx, JSObject *wrapper)
-{
-}
-
 JS_FRIEND_API(JSObject *)
 js::UnwrapObject(JSObject *wrapped, bool stopAtOuter, unsigned *flagsp)
 {
     unsigned flags = 0;
     while (wrapped->isWrapper() &&
            !JS_UNLIKELY(stopAtOuter && wrapped->getClass()->ext.innerObject)) {
         flags |= Wrapper::wrapperHandler(wrapped)->flags();
         wrapped = GetProxyPrivate(wrapped).toObjectOrNull();
@@ -107,17 +102,16 @@ js::UnwrapObjectChecked(JSContext *cx, J
         JSObject *wrapper = obj;
         Wrapper *handler = Wrapper::wrapperHandler(obj);
         bool rvOnFailure;
         if (!handler->enter(cx, wrapper, JSID_VOID,
                             Wrapper::PUNCTURE, &rvOnFailure))
             return rvOnFailure ? obj : NULL;
         obj = Wrapper::wrappedObject(obj);
         JS_ASSERT(obj);
-        handler->leave(cx, wrapper);
     }
     return obj;
 }
 
 bool
 js::IsCrossCompartmentWrapper(const JSObject *wrapper)
 {
     return wrapper->isWrapper() &&
@@ -129,19 +123,17 @@ IndirectWrapper::IndirectWrapper(unsigne
 {
 }
 
 #define CHECKED(op, act)                                                     \
     JS_BEGIN_MACRO                                                           \
         bool status;                                                         \
         if (!enter(cx, wrapper, id, act, &status))                           \
             return status;                                                   \
-        bool ok = (op);                                                      \
-        leave(cx, wrapper);                                                  \
-        return ok;                                                           \
+        return (op);                                                         \
     JS_END_MACRO
 
 #define SET(action) CHECKED(action, SET)
 #define GET(action) CHECKED(action, GET)
 
 bool
 IndirectWrapper::getPropertyDescriptor(JSContext *cx, JSObject *wrapper,
                                        jsid id, bool set,
@@ -333,17 +325,16 @@ DirectWrapper::obj_toString(JSContext *c
     if (!enter(cx, wrapper, JSID_VOID, GET, &status)) {
         if (status) {
             // Perform some default behavior that doesn't leak any information.
             return JS_NewStringCopyZ(cx, "[object Object]");
         }
         return NULL;
     }
     JSString *str = IndirectProxyHandler::obj_toString(cx, wrapper);
-    leave(cx, wrapper);
     return str;
 }
 
 JSString *
 DirectWrapper::fun_toString(JSContext *cx, JSObject *wrapper, unsigned indent)
 {
     bool status;
     if (!enter(cx, wrapper, JSID_VOID, GET, &status)) {
@@ -352,17 +343,16 @@ DirectWrapper::fun_toString(JSContext *c
             if (wrapper->isCallable())
                 return JS_NewStringCopyZ(cx, "function () {\n    [native code]\n}");
             ReportIsNotFunction(cx, ObjectValue(*wrapper));
             return NULL;
         }
         return NULL;
     }
     JSString *str = IndirectProxyHandler::fun_toString(cx, wrapper, indent);
-    leave(cx, wrapper);
     return str;
 }
 
 DirectWrapper DirectWrapper::singleton((unsigned)0);
 
 /* Compartments. */
 
 namespace js {
--- a/js/src/jswrapper.h
+++ b/js/src/jswrapper.h
@@ -89,20 +89,16 @@ class JS_FRIEND_API(Wrapper)
     virtual BaseProxyHandler *toBaseProxyHandler() = 0;
 
     /* Policy enforcement traps.
      *
      * enter() allows the policy to specify whether the caller may perform |act|
      * on the underlying object's |id| property. In the case when |act| is CALL,
      * |id| is generally JSID_VOID.
      *
-     * leave() allows the policy to undo various scoped state changes taken in
-     * enter(). If enter() succeeds, leave() must be called upon completion of
-     * the approved action.
-     *
      * The |act| parameter to enter() specifies the action being performed. GET,
      * SET, and CALL are self-explanatory, but PUNCTURE requires more
      * explanation:
      *
      * GET and SET allow for a very fine-grained security membrane, through
      * which access can be granted or denied on a per-property, per-object, and
      * per-action basis. Sometimes though, we just want to asks if we can access
      * _everything_ behind the wrapper barrier. For example, when the structured
@@ -114,18 +110,16 @@ class JS_FRIEND_API(Wrapper)
      * be lifted - that is to say, whether the caller is allowed to access
      * anything that the wrapped object could access. This is a very powerful
      * permission, and thus should generally be denied for security wrappers
      * except under very special circumstances. When |act| is PUNCTURE, |id|
      * should be JSID_VOID.
      */
     virtual bool enter(JSContext *cx, JSObject *wrapper, jsid id, Action act,
                        bool *bp);
-
-    virtual void leave(JSContext *cx, JSObject *wrapper);
 };
 
 /*
  * IndirectWrapper forwards its traps by forwarding them to
  * IndirectProxyHandler. In effect, IndirectWrapper behaves the same as
  * IndirectProxyHandler, except that it adds policy enforcement checks to each
  * fundamental trap.
  */
--- a/js/xpconnect/wrappers/XrayWrapper.cpp
+++ b/js/xpconnect/wrappers/XrayWrapper.cpp
@@ -771,34 +771,16 @@ ContentScriptHasUniversalXPConnect()
 
         bool privileged;
         if (NS_SUCCEEDED(ssm->IsCapabilityEnabled("UniversalXPConnect", &privileged)) && privileged)
             return true;
     }
     return false;
 }
 
-class AutoLeaveHelper
-{
-  public:
-    AutoLeaveHelper(js::Wrapper &xray, JSContext *cx, JSObject *wrapper)
-      : xray(xray), cx(cx), wrapper(wrapper)
-    {
-    }
-    ~AutoLeaveHelper()
-    {
-        xray.leave(cx, wrapper);
-    }
-
-  private:
-    js::Wrapper &xray;
-    JSContext *cx;
-    JSObject *wrapper;
-};
-
 bool
 XPCWrappedNativeXrayTraits::resolveOwnProperty(JSContext *cx, js::Wrapper &jsWrapper,
                                                JSObject *wrapper, JSObject *holder, jsid id,
                                                bool set, PropertyDescriptor *desc)
 {
     // Xray wrappers don't use the regular wrapper hierarchy, so we should be
     // in the wrapper's compartment here, not the wrappee.
     MOZ_ASSERT(js::IsObjectInContextCompartment(wrapper, cx));
@@ -811,18 +793,16 @@ XPCWrappedNativeXrayTraits::resolveOwnPr
           Is<nsIDocument>(wrapper))) &&
         (AccessCheck::callerIsChrome() || ContentScriptHasUniversalXPConnect())) {
         bool status;
         Wrapper::Action action = set ? Wrapper::SET : Wrapper::GET;
         desc->obj = NULL; // default value
         if (!jsWrapper.enter(cx, wrapper, id, action, &status))
             return status;
 
-        AutoLeaveHelper helper(jsWrapper, cx, wrapper);
-
         desc->obj = wrapper;
         desc->attrs = JSPROP_ENUMERATE|JSPROP_SHARED;
         if (id == rt->GetStringID(XPCJSRuntime::IDX_BASEURIOBJECT))
             desc->getter = baseURIObject_getter;
         else if (id == rt->GetStringID(XPCJSRuntime::IDX_DOCUMENTURIOBJECT))
             desc->getter = documentURIObject_getter;
         else
             desc->getter = nodePrincipal_getter;
@@ -1258,18 +1238,16 @@ XrayWrapper<Base, Traits>::getPropertyDe
     }
 
     bool status;
     Wrapper::Action action = set ? Wrapper::SET : Wrapper::GET;
     desc->obj = NULL; // default value
     if (!this->enter(cx, wrapper, id, action, &status))
         return status;
 
-    AutoLeaveHelper helper(*this, cx, wrapper);
-
     typename Traits::ResolvingId resolving(wrapper, id);
 
     // Redirect access straight to the wrapper if we should be transparent.
     if (XrayUtils::IsTransparent(cx, wrapper)) {
         JSObject *obj = Traits::getInnerObject(wrapper);
         {
             JSAutoEnterCompartment ac;
             if (!ac.enter(cx, obj))
@@ -1296,18 +1274,16 @@ XrayWrapper<Base, Traits>::getPropertyDe
     if (!WrapperFactory::IsPartiallyTransparent(wrapper) &&
         id == rt->GetStringID(XPCJSRuntime::IDX_WRAPPED_JSOBJECT)) {
         bool status;
         Wrapper::Action action = set ? Wrapper::SET : Wrapper::GET;
         desc->obj = NULL; // default value
         if (!this->enter(cx, wrapper, id, action, &status))
             return status;
 
-        AutoLeaveHelper helper(*this, cx, wrapper);
-
         desc->obj = wrapper;
         desc->attrs = JSPROP_ENUMERATE|JSPROP_SHARED;
         desc->getter = wrappedJSObject_getter;
         desc->setter = NULL;
         desc->shortid = 0;
         desc->value = JSVAL_VOID;
         return true;
     }
@@ -1364,18 +1340,16 @@ XrayWrapper<Base, Traits>::getOwnPropert
     }
 
     bool status;
     Wrapper::Action action = set ? Wrapper::SET : Wrapper::GET;
     desc->obj = NULL; // default value
     if (!this->enter(cx, wrapper, id, action, &status))
         return status;
 
-    AutoLeaveHelper helper(*this, cx, wrapper);
-
     typename Traits::ResolvingId resolving(wrapper, id);
 
     // NB: Nothing we do here acts on the wrapped native itself, so we don't
     // enter our policy.
     // Redirect access straight to the wrapper if we should be transparent.
     if (XrayUtils::IsTransparent(cx, wrapper)) {
         JSObject *obj = Traits::getInnerObject(wrapper);
         {