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 212969 be135c6277739a4b24cfdf70800b662df0c7333c
parent 212968 3de3f4f8a97087143741557a75a0506d19a441e8
child 212970 a527d47aa97a27840f5fd1e902d1ab29ee295014
push id3857
push userraliiev@mozilla.com
push dateTue, 02 Sep 2014 16:39:23 +0000
treeherdermozilla-beta@5638b907b505 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersbz
bugs1032726
milestone33.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 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.
      */