Bug 1032726 part 5 - Make new DOM bindings work with Latin1 strings. r=bz
authorJan de Mooij <jdemooij@mozilla.com>
Thu, 03 Jul 2014 10:03:56 +0200
changeset 192173 be135c6277739a4b24cfdf70800b662df0c7333c
parent 192172 3de3f4f8a97087143741557a75a0506d19a441e8
child 192174 a527d47aa97a27840f5fd1e902d1ab29ee295014
push id1
push userroot
push dateMon, 20 Oct 2014 17:29:22 +0000
reviewersbz
bugs1032726
milestone33.0a1
Bug 1032726 part 5 - Make new DOM bindings work with Latin1 strings. r=bz
content/html/content/src/nsGenericHTMLElement.cpp
dom/bindings/BindingDeclarations.h
dom/bindings/BindingUtils.cpp
dom/bindings/BindingUtils.h
dom/bindings/Codegen.py
js/src/jsfriendapi.h
js/src/vm/String.h
--- a/content/html/content/src/nsGenericHTMLElement.cpp
+++ b/content/html/content/src/nsGenericHTMLElement.cpp
@@ -3095,17 +3095,17 @@ nsGenericHTMLElement::SetItemValue(JSCon
                                    ErrorResult& aError)
 {
   if (!HasAttr(kNameSpaceID_None, nsGkAtoms::itemprop) ||
       HasAttr(kNameSpaceID_None, nsGkAtoms::itemscope)) {
     aError.Throw(NS_ERROR_DOM_INVALID_ACCESS_ERR);
     return;
   }
 
-  nsDependentString string;
+  nsAutoString string;
   JS::Rooted<JS::Value> value(aCx, aValue);
   if (!ConvertJSValueToString(aCx, value, &value, eStringify, eStringify, string)) {
     aError.Throw(NS_ERROR_UNEXPECTED);
     return;
   }
   SetItemValueText(string);
 }
 
--- a/dom/bindings/BindingDeclarations.h
+++ b/dom/bindings/BindingDeclarations.h
@@ -297,22 +297,22 @@ public:
   // shadowing the one on the superclass.
   OwningNonNull<T>& Value()
   {
     return this->mImpl.ref();
   }
 };
 
 // Specialization for strings.
-// XXXbz we can't pull in FakeDependentString here, because it depends on
-// internal strings.  So we just have to forward-declare it and reimplement its
+// XXXbz we can't pull in FakeString here, because it depends on internal
+// strings.  So we just have to forward-declare it and reimplement its
 // ToAStringPtr.
 
 namespace binding_detail {
-struct FakeDependentString;
+struct FakeString;
 } // namespace binding_detail
 
 template<>
 class Optional<nsAString>
 {
 public:
   Optional() : mPassed(false) {}
 
@@ -324,21 +324,21 @@ public:
   void operator=(const nsAString* str)
   {
     MOZ_ASSERT(str);
     mStr = str;
     mPassed = true;
   }
 
   // If this code ever goes away, remove the comment pointing to it in the
-  // FakeDependentString class in BindingUtils.h.
-  void operator=(const binding_detail::FakeDependentString* str)
+  // FakeString class in BindingUtils.h.
+  void operator=(const binding_detail::FakeString* str)
   {
     MOZ_ASSERT(str);
-    mStr = reinterpret_cast<const nsDependentString*>(str);
+    mStr = reinterpret_cast<const nsString*>(str);
     mPassed = true;
   }
 
   const nsAString& Value() const
   {
     MOZ_ASSERT(WasPassed());
     return *mStr;
   }
--- a/dom/bindings/BindingUtils.cpp
+++ b/dom/bindings/BindingUtils.cpp
@@ -2166,19 +2166,19 @@ ConvertJSValueToByteString(JSContext* cx
       PR_snprintf(badCharArray, sizeof(badCharArray), "%d", badChar);
       ThrowErrorMessage(cx, MSG_INVALID_BYTESTRING, index, badCharArray);
       return false;
     }
   } else {
     length = JS_GetStringLength(s);
   }
 
-  if (length >= UINT32_MAX) {
-    return false;
-  }
+  static_assert(js::MaxStringLength < UINT32_MAX,
+                "length+1 shouldn't overflow");
+
   result.SetCapacity(length+1);
   JS_EncodeStringToBuffer(cx, s, result.BeginWriting(), length);
   result.BeginWriting()[length] = '\0';
   result.SetLength(length);
 
   return true;
 }
 
--- a/dom/bindings/BindingUtils.h
+++ b/dom/bindings/BindingUtils.h
@@ -1716,95 +1716,164 @@ HasPropertyOnPrototype(JSContext* cx, JS
 // then the "proxy" argument is ignored.
 bool
 AppendNamedPropertyIds(JSContext* cx, JS::Handle<JSObject*> proxy,
                        nsTArray<nsString>& names,
                        bool shadowPrototypeProperties, JS::AutoIdVector& props);
 
 namespace binding_detail {
 
-// A struct that has the same layout as an nsDependentString but much
-// faster constructor and destructor behavior
-struct FakeDependentString {
-  FakeDependentString() :
-    mFlags(nsDependentString::F_TERMINATED)
+// A struct that has the same layout as an nsString but much faster
+// constructor and destructor behavior. FakeString uses inline storage
+// for small strings and a nsStringBuffer for longer strings.
+struct FakeString {
+  FakeString() :
+    mFlags(nsString::F_TERMINATED)
   {
   }
 
-  void Rebind(const nsDependentString::char_type* aData,
-              nsDependentString::size_type aLength) {
-    MOZ_ASSERT(mFlags == nsDependentString::F_TERMINATED);
-    mData = aData;
+  ~FakeString() {
+    if (mFlags & nsString::F_SHARED) {
+      nsStringBuffer::FromData(mData)->Release();
+    }
+  }
+
+  void Rebind(const nsString::char_type* aData, nsString::size_type aLength) {
+    MOZ_ASSERT(mFlags == nsString::F_TERMINATED);
+    mData = const_cast<nsString::char_type*>(aData);
     mLength = aLength;
   }
 
   void Truncate() {
-    mData = nsDependentString::char_traits::sEmptyBuffer;
+    MOZ_ASSERT(mFlags == nsString::F_TERMINATED);
+    mData = nsString::char_traits::sEmptyBuffer;
     mLength = 0;
   }
 
   void SetIsVoid(bool aValue) {
     MOZ_ASSERT(aValue,
-               "We don't support SetIsVoid(false) on FakeDependentString!");
+               "We don't support SetIsVoid(false) on FakeString!");
     Truncate();
-    mFlags |= nsDependentString::F_VOIDED;
+    mFlags |= nsString::F_VOIDED;
   }
 
-  const nsDependentString::char_type* Data() const
+  const nsString::char_type* Data() const
+  {
+    return mData;
+  }
+
+  nsString::char_type* BeginWriting()
   {
     return mData;
   }
 
-  nsDependentString::size_type Length() const
+  nsString::size_type Length() const
   {
     return mLength;
   }
 
+  // Reserve space to write aCapacity chars, including null-terminator.
+  // Caller is responsible for calling SetLength and writing the null-
+  // terminator.
+  bool SetCapacity(nsString::size_type aCapacity, mozilla::fallible_t const&) {
+    MOZ_ASSERT(aCapacity > 0, "Capacity must include null-terminator");
+
+    // Use mInlineStorage for small strings.
+    if (aCapacity <= sInlineCapacity) {
+      SetData(mInlineStorage);
+      return true;
+    }
+
+    nsRefPtr<nsStringBuffer> buf = nsStringBuffer::Alloc(aCapacity * sizeof(nsString::char_type));
+    if (MOZ_UNLIKELY(!buf)) {
+      return false;
+    }
+
+    SetData(static_cast<nsString::char_type*>(buf.forget().take()->Data()));
+    mFlags = nsString::F_SHARED | nsString::F_TERMINATED;
+    return true;
+  }
+
+  void SetLength(nsString::size_type aLength) {
+    mLength = aLength;
+  }
+
   // If this ever changes, change the corresponding code in the
   // Optional<nsAString> specialization as well.
   const nsAString* ToAStringPtr() const {
-    return reinterpret_cast<const nsDependentString*>(this);
+    return reinterpret_cast<const nsString*>(this);
   }
 
   nsAString* ToAStringPtr() {
-    return reinterpret_cast<nsDependentString*>(this);
+    return reinterpret_cast<nsString*>(this);
   }
 
   operator const nsAString& () const {
-    return *reinterpret_cast<const nsDependentString*>(this);
+    return *reinterpret_cast<const nsString*>(this);
   }
 
 private:
-  const nsDependentString::char_type* mData;
-  nsDependentString::size_type mLength;
+  nsString::char_type* mData;
+  nsString::size_type mLength;
   uint32_t mFlags;
 
+  static const size_t sInlineCapacity = 64;
+  nsString::char_type mInlineStorage[sInlineCapacity];
+
+  FakeString(const FakeString& other) MOZ_DELETE;
+  void operator=(const FakeString& other) MOZ_DELETE;
+
+  void SetData(nsString::char_type* aData) {
+    MOZ_ASSERT(mFlags == nsString::F_TERMINATED);
+    mData = const_cast<nsString::char_type*>(aData);
+  }
+
   // A class to use for our static asserts to ensure our object layout
-  // matches that of nsDependentString.
-  class DependentStringAsserter;
-  friend class DependentStringAsserter;
-
-  class DepedentStringAsserter : public nsDependentString {
+  // matches that of nsString.
+  class StringAsserter;
+  friend class StringAsserter;
+
+  class StringAsserter : public nsString {
   public:
     static void StaticAsserts() {
-      static_assert(sizeof(FakeDependentString) == sizeof(nsDependentString),
-                    "Must have right object size");
-      static_assert(offsetof(FakeDependentString, mData) ==
-                      offsetof(DepedentStringAsserter, mData),
+      static_assert(offsetof(FakeString, mInlineStorage) ==
+                      sizeof(nsString),
+                    "FakeString should include all nsString members");
+      static_assert(offsetof(FakeString, mData) ==
+                      offsetof(StringAsserter, mData),
                     "Offset of mData should match");
-      static_assert(offsetof(FakeDependentString, mLength) ==
-                      offsetof(DepedentStringAsserter, mLength),
+      static_assert(offsetof(FakeString, mLength) ==
+                      offsetof(StringAsserter, mLength),
                     "Offset of mLength should match");
-      static_assert(offsetof(FakeDependentString, mFlags) ==
-                      offsetof(DepedentStringAsserter, mFlags),
+      static_assert(offsetof(FakeString, mFlags) ==
+                      offsetof(StringAsserter, mFlags),
                     "Offset of mFlags should match");
     }
   };
 };
 
+template<typename T>
+inline bool
+AssignJSString(JSContext *cx, T &dest, JSString *s)
+{
+  size_t len = js::GetStringLength(s);
+  static_assert(js::MaxStringLength < (1 << 28),
+                "Shouldn't overflow here or in FakeString::SetCapacity");
+  if (MOZ_UNLIKELY(!dest.SetCapacity(len + 1, mozilla::fallible_t()))) {
+    JS_ReportOutOfMemory(cx);
+    return false;
+  }
+  if (MOZ_UNLIKELY(!js::CopyStringChars(cx, dest.BeginWriting(), s, len))) {
+    return false;
+  }
+  dest.BeginWriting()[len] = '\0';
+  dest.SetLength(len);
+  return true;
+}
+
 } // namespace binding_detail
 
 enum StringificationBehavior {
   eStringify,
   eEmpty,
   eNull
 };
 
@@ -1841,24 +1910,17 @@ ConvertJSValueToString(JSContext* cx, JS
 
     s = JS::ToString(cx, v);
     if (!s) {
       return false;
     }
     pval.set(JS::StringValue(s));  // Root the new string.
   }
 
-  size_t len;
-  const jschar *chars = JS_GetStringCharsZAndLength(cx, s, &len);
-  if (!chars) {
-    return false;
-  }
-
-  result.Rebind(chars, len);
-  return true;
+  return binding_detail::AssignJSString(cx, result, s);
 }
 
 bool
 ConvertJSValueToByteString(JSContext* cx, JS::Handle<JS::Value> v,
                            JS::MutableHandle<JS::Value> pval, bool nullable,
                            nsACString& result);
 
 template<typename T>
@@ -2401,44 +2463,44 @@ T& NonNullHelper(OwningNonNull<T>& aArg)
 
 template<typename T>
 const T& NonNullHelper(const OwningNonNull<T>& aArg)
 {
   return aArg;
 }
 
 inline
-void NonNullHelper(NonNull<binding_detail::FakeDependentString>& aArg)
+void NonNullHelper(NonNull<binding_detail::FakeString>& aArg)
 {
   // This overload is here to make sure that we never end up applying
-  // NonNullHelper to a NonNull<binding_detail::FakeDependentString>. If we
+  // NonNullHelper to a NonNull<binding_detail::FakeString>. If we
   // try to, it should fail to compile, since presumably the caller will try to
   // use our nonexistent return value.
 }
 
 inline
-void NonNullHelper(const NonNull<binding_detail::FakeDependentString>& aArg)
+void NonNullHelper(const NonNull<binding_detail::FakeString>& aArg)
 {
   // This overload is here to make sure that we never end up applying
-  // NonNullHelper to a NonNull<binding_detail::FakeDependentString>. If we
+  // NonNullHelper to a NonNull<binding_detail::FakeString>. If we
   // try to, it should fail to compile, since presumably the caller will try to
   // use our nonexistent return value.
 }
 
 inline
-void NonNullHelper(binding_detail::FakeDependentString& aArg)
+void NonNullHelper(binding_detail::FakeString& aArg)
 {
   // This overload is here to make sure that we never end up applying
-  // NonNullHelper to a FakeDependentString before we've constified it.  If we
+  // NonNullHelper to a FakeString before we've constified it.  If we
   // try to, it should fail to compile, since presumably the caller will try to
   // use our nonexistent return value.
 }
 
 MOZ_ALWAYS_INLINE
-const nsAString& NonNullHelper(const binding_detail::FakeDependentString& aArg)
+const nsAString& NonNullHelper(const binding_detail::FakeString& aArg)
 {
   return aArg;
 }
 
 // Reparent the wrapper of aObj to whatever its native now thinks its
 // parent should be.
 nsresult
 ReparentWrapper(JSContext* aCx, JS::Handle<JSObject*> aObj);
--- a/dom/bindings/Codegen.py
+++ b/dom/bindings/Codegen.py
@@ -3840,17 +3840,17 @@ def getJSToNativeConversionInfo(type, de
             JS::Rooted<JS::Value> propNameValue(cx);
             JS::Rooted<JS::Value> temp(cx);
             JS::Rooted<jsid> curId(cx);
             for (size_t i = 0; i < ids.length(); ++i) {
               // Make sure we get the value before converting the name, since
               // getting the value can trigger GC but our name is a dependent
               // string.
               curId = ids[i];
-              binding_detail::FakeDependentString propName;
+              binding_detail::FakeString propName;
               if (!JS_GetPropertyById(cx, mozMapObj, curId, &temp) ||
                   !JS_IdToValue(cx, curId, &propNameValue) ||
                   !ConvertJSValueToString(cx, propNameValue, &propNameValue,
                                           eStringify, eStringify, propName)) {
                 $*{exceptionCode}
               }
 
               ${valueType}* slotPtr = mozMap.AddEntry(propName);
@@ -4437,47 +4437,31 @@ def getJSToNativeConversionInfo(type, de
                 assert(type.nullable())
                 defaultCode = "%s.SetIsVoid(true)" % varName
             else:
                 defaultCode = handleDefaultStringValue(defaultValue,
                                                        "%s.Rebind" % varName)
             return handleDefault(conversionCode, defaultCode + ";\n")
 
         if isMember:
-            # We have to make a copy, except in the variadic case, because our
-            # jsval may well not live as long as our string needs to.
+            # Convert directly into the nsString member we have.
             declType = CGGeneric("nsString")
-            if isMember == "Variadic":
-                # The string is kept alive by the argument, so we can just
-                # depend on it.
-                assignString = "${declName}.Rebind(str.Data(), str.Length());\n"
-            else:
-                assignString = "${declName} = str;\n"
             return JSToNativeConversionInfo(
-                fill(
-                    """
-                    {
-                      binding_detail::FakeDependentString str;
-                      $*{convert}
-                      $*{assign}
-                    }
-                    """,
-                    convert=getConversionCode("str"),
-                    assign=assignString),
+                getConversionCode("${declName}"),
                 declType=declType,
                 dealWithOptional=isOptional)
 
         if isOptional:
             declType = "Optional<nsAString>"
-            holderType = CGGeneric("binding_detail::FakeDependentString")
+            holderType = CGGeneric("binding_detail::FakeString")
             conversionCode = ("%s"
                               "${declName} = &${holderName};\n" %
                               getConversionCode("${holderName}"))
         else:
-            declType = "binding_detail::FakeDependentString"
+            declType = "binding_detail::FakeString"
             holderType = None
             conversionCode = getConversionCode("${declName}")
 
         # No need to deal with optional here; we handled it already
         return JSToNativeConversionInfo(
             conversionCode,
             declType=CGGeneric(declType),
             holderType=holderType)
@@ -9369,20 +9353,20 @@ class CGProxyNamedOperation(CGProxySpeci
             }
             """,
             argName=argName)
         if self.value is None:
             # We're just using 'id', and if it's an atom we can take a
             # fast path here.
             unwrapString = fill(
                 """
-                if (MOZ_LIKELY(JSID_IS_ATOM(${idName}))) {
-                  JS::AutoCheckCannotGC nogc;
-                  JSAtom* atom = JSID_TO_ATOM(${idName});
-                  ${argName}.Rebind(js::GetTwoByteAtomChars(nogc, atom), js::GetAtomLength(atom));
+                if (MOZ_LIKELY(JSID_IS_STRING(${idName}))) {
+                  if (!AssignJSString(cx, ${argName}, JSID_TO_STRING(${idName}))) {
+                    return false;
+                  }
                 } else {
                   nameVal = js::IdToValue(${idName});
                   $*{unwrapString}
                 }
                 """,
                 idName=idName,
                 argName=argName,
                 unwrapString=unwrapString)
@@ -9392,17 +9376,17 @@ class CGProxyNamedOperation(CGProxySpeci
         # Sadly, we have to set up nameVal even if we have an atom id,
         # because we don't know for sure, and we can end up needing it
         # so it needs to be higher up the stack.  Using a Maybe here
         # seems like probable overkill.
         return fill(
             """
             JS::Rooted<JS::Value> nameVal(cx);
             $*{idDecl}
-            binding_detail::FakeDependentString ${argName};
+            binding_detail::FakeString ${argName};
             $*{unwrapString}
 
             ${nativeType}* self = UnwrapProxy(proxy);
             $*{op}
             """,
             idDecl=idDecl,
             argName=argName,
             unwrapString=unwrapString,
@@ -13538,17 +13522,17 @@ class GlobalGenRoots():
         (includes, implincludes,
          declarations, unions) = UnionTypes(config.getDescriptors(),
                                             config.getDictionaries(),
                                             config.getCallbacks(),
                                             config)
         includes.add("mozilla/dom/OwningNonNull.h")
         includes.add("mozilla/dom/UnionMember.h")
         includes.add("mozilla/dom/BindingDeclarations.h")
-        # Need BindingUtils.h for FakeDependentString
+        # Need BindingUtils.h for FakeString
         includes.add("mozilla/dom/BindingUtils.h")
         implincludes.add("mozilla/dom/PrimitiveConversions.h")
 
         # Wrap all of that in our namespaces.
         curr = CGNamespace.build(['mozilla', 'dom'], unions)
 
         curr = CGWrapper(curr, post='\n')
 
--- a/js/src/jsfriendapi.h
+++ b/js/src/jsfriendapi.h
@@ -765,16 +765,18 @@ GetObjectSlot(JSObject *obj, size_t slot
 }
 
 MOZ_ALWAYS_INLINE size_t
 GetAtomLength(JSAtom *atom)
 {
     return reinterpret_cast<shadow::String*>(atom)->length;
 }
 
+static const uint32_t MaxStringLength = (1 << 28) - 1;
+
 MOZ_ALWAYS_INLINE size_t
 GetStringLength(JSString *s)
 {
     return reinterpret_cast<shadow::String*>(s)->length;
 }
 
 MOZ_ALWAYS_INLINE bool
 LinearStringHasLatin1Chars(JSLinearString *s)
--- a/js/src/vm/String.h
+++ b/js/src/vm/String.h
@@ -251,17 +251,17 @@ class JSString : public js::gc::Barriere
     /* Initial flags for inline and fat inline strings. */
     static const uint32_t INIT_INLINE_FLAGS     = FLAT_BIT | INLINE_CHARS_BIT;
     static const uint32_t INIT_FAT_INLINE_FLAGS = FLAT_BIT | FAT_INLINE_MASK;
 
     static const uint32_t TYPE_FLAGS_MASK       = JS_BIT(6) - 1;
 
     static const uint32_t LATIN1_CHARS_BIT      = JS_BIT(6);
 
-    static const uint32_t MAX_LENGTH            = JS_BIT(28) - 1;
+    static const uint32_t MAX_LENGTH            = js::MaxStringLength;
 
     static const JS::Latin1Char MAX_LATIN1_CHAR = 0xff;
 
     /*
      * Helper function to validate that a string of a given length is
      * representable by a JSString. An allocation overflow is reported if false
      * is returned.
      */