Fix for bug 814821 (Dromaeo dom-traverse regression from bug 812333) - part 2: make WrapNewBindingObject a little faster. r=bz.
authorPeter Van der Beken <peterv@propagandism.org>
Tue, 27 Nov 2012 10:20:04 +0100
changeset 114342 2997ef8891c41006b1ce5dc19762fd109c4ecda3
parent 114341 3db4085a10b2fcb585cbc19f15012e69bf218e2d
child 114343 ae93793daecd682d55996f9fe6140daf74d181c7
push id18708
push userpvanderbeken@mozilla.com
push dateWed, 28 Nov 2012 09:02:49 +0000
treeherdermozilla-inbound@7d678d658159 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersbz
bugs814821, 812333
milestone20.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
Fix for bug 814821 (Dromaeo dom-traverse regression from bug 812333) - part 2: make WrapNewBindingObject a little faster. r=bz.
content/base/src/nsINode.cpp
dom/bindings/BindingUtils.h
--- a/content/base/src/nsINode.cpp
+++ b/content/base/src/nsINode.cpp
@@ -2306,21 +2306,17 @@ nsINode::QuerySelectorAll(const nsAStrin
   aResult = FindMatchingElements<false>(this, aSelector, *contentList);
 
   return contentList.forget();
 }
 
 JSObject*
 nsINode::WrapObject(JSContext *aCx, JSObject *aScope, bool *aTriedToWrap)
 {
-  // Not all nodes have been converted
-  if (!IsDOMBinding()) {
-    *aTriedToWrap = false;
-    return nullptr;
-  }
+  MOZ_ASSERT(IsDOMBinding());
 
   // Make sure one of these is true
   // (1) our owner document has a script handling object,
   // (2) Our owner document has had a script handling object, or has been marked
   //     to have had one,
   // (3) we are running a privileged script.
   // Event handling is possible only if (1). If (2) event handling is
   // prevented.
--- a/dom/bindings/BindingUtils.h
+++ b/dom/bindings/BindingUtils.h
@@ -492,33 +492,50 @@ CheckWrapperCacheCast<T, true>
 {
   static bool Check()
   {
     return true;
   }
 };
 #endif
 
-// Create a JSObject wrapping "value", for cases when "value" is a
-// non-wrapper-cached object using WebIDL bindings.  "value" must implement a
-// WrapObject() method taking a JSContext and a scope.
+MOZ_ALWAYS_INLINE bool
+CouldBeDOMBinding(void*)
+{
+  return true;
+}
+
+MOZ_ALWAYS_INLINE bool
+CouldBeDOMBinding(nsWrapperCache* aCache)
+{
+  return aCache->IsDOMBinding();
+}
+
+// Create a JSObject wrapping "value", if there isn't one already, and store it
+// in *vp.  "value" must be a concrete class that implements a
+// GetWrapperPreserveColor() which can return its existing wrapper, if any, and
+// a WrapObject() which will try to create a wrapper. Typically, this is done by
+// having "value" inherit from nsWrapperCache.
 template <class T>
 MOZ_ALWAYS_INLINE bool
 WrapNewBindingObject(JSContext* cx, JSObject* scope, T* value, JS::Value* vp)
 {
   JSObject* obj = value->GetWrapperPreserveColor();
   if (obj) {
     xpc_UnmarkNonNullGrayObject(obj);
     if (js::GetObjectCompartment(obj) == js::GetObjectCompartment(scope)) {
       *vp = JS::ObjectValue(*obj);
       return true;
     }
-  }
+  } else {
+    // Inline this here while we have non-dom objects in wrapper caches.
+    if (!CouldBeDOMBinding(value)) {
+      return false;
+    }
 
-  if (!obj) {
     bool triedToWrap;
     obj = value->WrapObject(cx, scope, &triedToWrap);
     if (!obj) {
       // At this point, obj is null, so just return false.  We could
       // try to communicate triedToWrap to the caller, but in practice
       // callers seem to be testing JS_IsExceptionPending(cx) to
       // figure out whether WrapObject() threw instead.
       return false;
@@ -548,21 +565,19 @@ WrapNewBindingObject(JSContext* cx, JSOb
   // whatever the content compartment is.  So at this point we need to
   // make sure it's correctly wrapped for the compartment of |scope|.
   // cx should already be in the compartment of |scope| here.
   MOZ_ASSERT(js::IsObjectInContextCompartment(scope, cx));
   *vp = JS::ObjectValue(*obj);
   return JS_WrapValue(cx, vp);
 }
 
-// Create a JSObject wrapping "value", if there isn't one already, and store it
-// in *vp.  "value" must be a concrete class that implements a GetWrapper()
-// which can return its existing wrapper, if any, and a WrapObject() which will
-// try to create a wrapper.  Typically, this is done by having "value" inherit
-// from nsWrapperCache.
+// Create a JSObject wrapping "value", for cases when "value" is a
+// non-wrapper-cached object using WebIDL bindings.  "value" must implement a
+// WrapObject() method taking a JSContext and a scope.
 template <class T>
 inline bool
 WrapNewBindingNonWrapperCachedObject(JSContext* cx, JSObject* scope, T* value,
                                      JS::Value* vp)
 {
   // We try to wrap in the compartment of the underlying object of "scope"
   JSObject* obj;
   {
@@ -943,17 +958,23 @@ struct WrapNativeParentHelper
     MOZ_ASSERT(cache);
 
     JSObject* obj;
     if ((obj = cache->GetWrapper())) {
       return obj;
     }
 
     bool triedToWrap;
-    obj = parent->WrapObject(cx, scope, &triedToWrap);
+    // Inline this here while we have non-dom objects in wrapper caches.
+    if (!CouldBeDOMBinding(parent)) {
+      triedToWrap = false;
+    } else {
+      obj = parent->WrapObject(cx, scope, &triedToWrap);
+    }
+
     if (!triedToWrap) {
       obj = WrapNativeParentFallback<T>::Wrap(cx, scope, parent, cache);
     }
     return obj;
   }
 };
 
 // Wrapping of our native parent, for cases when it's not a WebIDL object.  In