Bug 994453 part 1. Factor out the to-JS conversions from Promise.h into mozilla::dom::ToJSValue methods. r=bholley

Apart from moving the methods, I made the following changes:

1)  Renamed them to ToJSValue.
2)  Inlined the boolean overload.
3)  Added overloads that take integer types.
4)  Changed the order of the aCx and aArgument arguments so aCx comes first.
5)  Renamed "abv" to "obj" in the typed array overload.

dom/bindings/ToJSValue.cpp

@@ -0,0 +1,53 @@
+/* -*- Mode: C++; tab-width: 2; indent-tabs-mode: nil; c-basic-offset: 2 -*-*/
+/* vim: set ts=2 sw=2 et tw=79: */
+/* This Source Code Form is subject to the terms of the Mozilla Public
+ * License, v. 2.0. If a copy of the MPL was not distributed with this file,
+ * You can obtain one at http://mozilla.org/MPL/2.0/. */
+
+#include "mozilla/dom/ToJSValue.h"
+#include "nsAString.h"
+#include "nsContentUtils.h"
+#include "nsStringBuffer.h"
+#include "xpcpublic.h"
+
+namespace mozilla {
+namespace dom {
+
+bool
+ToJSValue(JSContext* aCx, const nsAString& aArgument,
+          JS::MutableHandle<JS::Value> aValue)
+{
+  // Make sure we're called in a compartment
+  MOZ_ASSERT(JS::CurrentGlobalOrNull(aCx));
+
+// XXXkhuey I'd love to use xpc::NonVoidStringToJsval here, but it requires
+  // a non-const nsAString for silly reasons.
+  nsStringBuffer* sharedBuffer;
+  if (!XPCStringConvert::ReadableToJSVal(aCx, aArgument, &sharedBuffer,
+                                         aValue)) {
+    return false;
+  }
+
+  if (sharedBuffer) {
+    NS_ADDREF(sharedBuffer);
+  }
+
+  return true;
+}
+
+
+namespace tojsvalue_detail {
+
+bool
+ISupportsToJSValue(JSContext* aCx,
+                   nsISupports* aArgument,
+                   JS::MutableHandle<JS::Value> aValue)
+{
+  nsresult rv = nsContentUtils::WrapNative(aCx, aArgument, aValue);
+  return NS_SUCCEEDED(rv);
+}
+
+} // namespace tojsvalue_detail
+
+} // namespace dom
+} // namespace mozilla

dom/bindings/ToJSValue.h

@@ -0,0 +1,192 @@
+/* -*- Mode: C++; tab-width: 2; indent-tabs-mode: nil; c-basic-offset: 2 -*-*/
+/* vim: set ts=2 sw=2 et tw=79: */
+/* This Source Code Form is subject to the terms of the Mozilla Public
+ * License, v. 2.0. If a copy of the MPL was not distributed with this file,
+ * You can obtain one at http://mozilla.org/MPL/2.0/. */
+
+#ifndef mozilla_dom_ToJSValue_h
+#define mozilla_dom_ToJSValue_h
+
+#include "mozilla/TypeTraits.h"
+#include "mozilla/Assertions.h"
+#include "mozilla/dom/BindingUtils.h"
+#include "mozilla/dom/TypedArray.h"
+#include "jsapi.h"
+#include "nsISupports.h"
+#include "nsTArray.h"
+#include "nsWrapperCache.h"
+
+namespace mozilla {
+namespace dom {
+
+// If ToJSValue returns false, it must set an exception on the
+// JSContext.
+
+// Accept strings.
+bool
+ToJSValue(JSContext* aCx,
+          const nsAString& aArgument,
+          JS::MutableHandle<JS::Value> aValue);
+
+// Accept booleans.
+inline bool
+ToJSValue(JSContext* aCx,
+          bool aArgument,
+          JS::MutableHandle<JS::Value> aValue)
+{
+  // Make sure we're called in a compartment
+  MOZ_ASSERT(JS::CurrentGlobalOrNull(aCx));
+
+  aValue.setBoolean(aArgument);
+  return true;
+}
+
+// Accept integer types
+inline bool
+ToJSValue(JSContext* aCx,
+          int32_t aArgument,
+          JS::MutableHandle<JS::Value> aValue)
+{
+  // Make sure we're called in a compartment
+  MOZ_ASSERT(JS::CurrentGlobalOrNull(aCx));
+
+  aValue.setInt32(aArgument);
+  return true;
+}
+
+inline bool
+ToJSValue(JSContext* aCx,
+          uint32_t aArgument,
+          JS::MutableHandle<JS::Value> aValue)
+{
+  // Make sure we're called in a compartment
+  MOZ_ASSERT(JS::CurrentGlobalOrNull(aCx));
+
+  aValue.setNumber(aArgument);
+  return true;
+}
+
+inline bool
+ToJSValue(JSContext* aCx,
+          int64_t aArgument,
+          JS::MutableHandle<JS::Value> aValue)
+{
+  // Make sure we're called in a compartment
+  MOZ_ASSERT(JS::CurrentGlobalOrNull(aCx));
+
+  aValue.setNumber(double(aArgument));
+  return true;
+}
+
+inline bool
+ToJSValue(JSContext* aCx,
+          uint64_t aArgument,
+          JS::MutableHandle<JS::Value> aValue)
+{
+  // Make sure we're called in a compartment
+  MOZ_ASSERT(JS::CurrentGlobalOrNull(aCx));
+
+  aValue.setNumber(double(aArgument));
+  return true;
+}
+
+// Accept objects that inherit from nsWrapperCache and nsISupports (e.g. most
+// DOM objects).
+template <class T>
+typename EnableIf<IsBaseOf<nsWrapperCache, T>::value &&
+                  IsBaseOf<nsISupports, T>::value, bool>::Type
+ToJSValue(JSContext* aCx,
+          T& aArgument,
+          JS::MutableHandle<JS::Value> aValue)
+{
+  // Make sure we're called in a compartment
+  MOZ_ASSERT(JS::CurrentGlobalOrNull(aCx));
+  // Make sure non-webidl objects don't sneak in here
+  MOZ_ASSERT(aArgument.IsDOMBinding());
+
+  return WrapNewBindingObject(aCx, aArgument, aValue);
+}
+
+// Accept typed arrays built from appropriate nsTArray values
+template<typename T>
+typename EnableIf<IsBaseOf<AllTypedArraysBase, T>::value, bool>::Type
+ToJSValue(JSContext* aCx,
+          const TypedArrayCreator<T>& aArgument,
+          JS::MutableHandle<JS::Value> aValue)
+{
+  // Make sure we're called in a compartment
+  MOZ_ASSERT(JS::CurrentGlobalOrNull(aCx));
+
+  JSObject* obj = aArgument.Create(aCx);
+  if (!obj) {
+    return false;
+  }
+  aValue.setObject(*obj);
+  return true;
+}
+
+// We don't want to include nsContentUtils here, so use a helper
+// function for the nsISupports case.
+namespace tojsvalue_detail {
+bool
+ISupportsToJSValue(JSContext* aCx,
+                   nsISupports* aArgument,
+                   JS::MutableHandle<JS::Value> aValue);
+} // namespace tojsvalue_detail
+
+// Accept objects that inherit from nsISupports but not nsWrapperCache (e.g.
+// nsIDOMFile).
+template <class T>
+typename EnableIf<!IsBaseOf<nsWrapperCache, T>::value &&
+                  IsBaseOf<nsISupports, T>::value, bool>::Type
+ToJSValue(JSContext* aCx,
+          T& aArgument,
+          JS::MutableHandle<JS::Value> aValue)
+{
+  // Make sure we're called in a compartment
+  MOZ_ASSERT(JS::CurrentGlobalOrNull(aCx));
+
+  return tojsvalue_detail::ISupportsToJSValue(aCx, &aArgument, aValue);
+}
+
+// Accept nsRefPtr/nsCOMPtr
+template <template <typename> class SmartPtr, typename T>
+bool
+ToJSValue(JSContext* aCx,
+          const SmartPtr<T>& aArgument,
+          JS::MutableHandle<JS::Value> aValue)
+{
+  return ToJSValue(aCx, *aArgument.get(), aValue);
+}
+
+// Accept arrays of other things we accept
+template <typename T>
+bool
+ToJSValue(JSContext* aCx,
+          const nsTArray<T>& aArgument,
+          JS::MutableHandle<JS::Value> aValue)
+{
+  // Make sure we're called in a compartment
+  MOZ_ASSERT(JS::CurrentGlobalOrNull(aCx));
+
+  JS::AutoValueVector v(aCx);
+  if (!v.resize(aArgument.Length())) {
+    return false;
+  }
+  for (uint32_t i = 0; i < aArgument.Length(); ++i) {
+    if (!ToJSValue(aCx, aArgument[i], v.handleAt(i))) {
+      return false;
+    }
+  }
+  JSObject* arrayObj = JS_NewArrayObject(aCx, v);
+  if (!arrayObj) {
+    return false;
+  }
+  aValue.setObject(*arrayObj);
+  return true;
+}
+
+} // namespace dom
+} // namespace mozilla
+
+#endif /* mozilla_dom_ToJSValue_h */

dom/bindings/moz.build

@@ -29,6 +29,7 @@ EXPORTS.mozilla.dom += [
     'OwningNonNull.h',
     'PrimitiveConversions.h',
     'RootedDictionary.h',
+    'ToJSValue.h',
     'TypedArray.h',
     'UnionMember.h',
 ]
@@ -76,6 +77,7 @@ UNIFIED_SOURCES += [
     'Date.cpp',
     'DOMJSProxyHandler.cpp',
     'Exceptions.cpp',
+    'ToJSValue.cpp',
 ]
 
 include('/ipc/chromium/chromium-config.mozbuild')

dom/promise/Promise.cpp

@@ -1129,34 +1129,5 @@ PromiseReportRejectFeature::Notify(JSContext* aCx, workers::Status aStatus)
   return true;
 }
 
-bool
-Promise::ArgumentToJSValue(const nsAString& aArgument,
-                           JSContext* aCx,
-                           JS::MutableHandle<JS::Value> aValue)
-{
-  // XXXkhuey I'd love to use xpc::NonVoidStringToJsval here, but it requires
-  // a non-const nsAString for silly reasons.
-  nsStringBuffer* sharedBuffer;
-  if (!XPCStringConvert::ReadableToJSVal(aCx, aArgument, &sharedBuffer,
-                                         aValue)) {
-    return false;
-  }
-
-  if (sharedBuffer) {
-    NS_ADDREF(sharedBuffer);
-  }
-
-  return true;
-}
-
-bool
-Promise::ArgumentToJSValue(bool aArgument,
-                           JSContext* aCx,
-                           JS::MutableHandle<JS::Value> aValue)
-{
-  aValue.setBoolean(aArgument);
-  return true;
-}
-
 } // namespace dom
 } // namespace mozilla

dom/promise/Promise.h

@@ -13,11 +13,10 @@
 #include "mozilla/dom/BindingDeclarations.h"
 #include "nsCycleCollectionParticipant.h"
 #include "mozilla/dom/PromiseBinding.h"
-#include "mozilla/dom/TypedArray.h"
+#include "mozilla/dom/ToJSValue.h"
 #include "nsWrapperCache.h"
 #include "nsAutoPtr.h"
 #include "js/TypeDecls.h"
-#include "jsapi.h"
 
 #include "mozilla/dom/workers/bindings/WorkerFeature.h"
 
@@ -80,7 +79,7 @@ public:
 
   // Helpers for using Promise from C++.
   // Most DOM objects are handled already.  To add a new type T, such as ints,
-  // or dictionaries, add an ArgumentToJSValue overload below.
+  // or dictionaries, add a ToJSValue overload in ToJSValue.h.
   template <typename T>
   void MaybeResolve(T& aArg) {
     MaybeSomething(aArg, &Promise::MaybeResolve);
@@ -204,94 +203,6 @@ private:
   // Helper methods for using Promises from C++
   JSObject* GetOrCreateWrapper(JSContext* aCx);
 
-  // If ArgumentToJSValue returns false, it must set an exception on the
-  // JSContext.
-
-  // Accept strings.
-  bool
-  ArgumentToJSValue(const nsAString& aArgument,
-                    JSContext* aCx,
-                    JS::MutableHandle<JS::Value> aValue);
-
-  // Accept booleans.
-  bool
-  ArgumentToJSValue(bool aArgument,
-                    JSContext* aCx,
-                    JS::MutableHandle<JS::Value> aValue);
-
-  // Accept objects that inherit from nsWrapperCache and nsISupports (e.g. most
-  // DOM objects).
-  template <class T>
-  typename EnableIf<IsBaseOf<nsWrapperCache, T>::value &&
-                    IsBaseOf<nsISupports, T>::value, bool>::Type
-  ArgumentToJSValue(T& aArgument,
-                    JSContext* aCx,
-                    JS::MutableHandle<JS::Value> aValue)
-  {
-    return WrapNewBindingObject(aCx, aArgument, aValue);
-  }
-
-  // Accept typed arrays built from appropriate nsTArray values
-  template<typename T>
-  typename EnableIf<IsBaseOf<AllTypedArraysBase, T>::value, bool>::Type
-  ArgumentToJSValue(const TypedArrayCreator<T>& aArgument,
-                    JSContext* aCx,
-                    JS::MutableHandle<JS::Value> aValue)
-  {
-    JSObject* abv = aArgument.Create(aCx);
-    if (!abv) {
-      return false;
-    }
-    aValue.setObject(*abv);
-    return true;
-  }
-
-  // Accept objects that inherit from nsISupports but not nsWrapperCache (e.g.
-  // nsIDOMFile).
-  template <class T>
-  typename EnableIf<!IsBaseOf<nsWrapperCache, T>::value &&
-                    IsBaseOf<nsISupports, T>::value, bool>::Type
-  ArgumentToJSValue(T& aArgument,
-                    JSContext* aCx,
-                    JS::MutableHandle<JS::Value> aValue)
-  {
-    nsresult rv = nsContentUtils::WrapNative(aCx, &aArgument, aValue);
-    return NS_SUCCEEDED(rv);
-  }
-
-  template <template <typename> class SmartPtr, typename T>
-  bool
-  ArgumentToJSValue(const SmartPtr<T>& aArgument,
-                    JSContext* aCx,
-                    JS::MutableHandle<JS::Value> aValue)
-  {
-    return ArgumentToJSValue(*aArgument.get(), aCx, aValue);
-  }
-
-  // Accept arrays of other things we accept
-  template <typename T>
-  bool
-  ArgumentToJSValue(const nsTArray<T>& aArgument,
-                    JSContext* aCx,
-                    JS::MutableHandle<JS::Value> aValue)
-  {
-    JS::AutoValueVector v(aCx);
-    if (!v.resize(aArgument.Length())) {
-      return false;
-    }
-    for (uint32_t i = 0; i < aArgument.Length(); ++i) {
-      if (!ArgumentToJSValue(aArgument[i], aCx, v.handleAt(i))) {
-        return false;
-      }
-    }
-    JSObject* arrayObj = JS_NewArrayObject(aCx, v);
-    if (!arrayObj) {
-      return false;
-    }
-    aValue.setObject(*arrayObj);
-    return true;
-  }
-
   template <typename T>
   void MaybeSomething(T& aArgument, MaybeFunc aFunc) {
     ThreadsafeAutoJSContext cx;
@@ -304,7 +215,7 @@ private:
 
     JSAutoCompartment ac(cx, wrapper);
     JS::Rooted<JS::Value> val(cx);
-    if (!ArgumentToJSValue(aArgument, cx, &val)) {
+    if (!ToJSValue(cx, aArgument, &val)) {
       HandleException(cx);
       return;
     }