Bug 1356060 - Just use nsString in URLValueData. r=heycam
authorEric Rahm <erahm@mozilla.com>
Thu, 13 Apr 2017 15:18:25 -0700
changeset 401133 8f790984c97fe85025045bdd60c1b85dbe9c50d7
parent 401132 054f46576d6da24893099b40c0d78881da96d19d
child 401134 6ed02c72a430001dafb67ce14721e07aa0f7d7b6
push id7391
push usermtabara@mozilla.com
push dateMon, 12 Jun 2017 13:08:53 +0000
treeherdermozilla-beta@2191d7f87e2e [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersheycam
bugs1356060
milestone55.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 1356060 - Just use nsString in URLValueData. r=heycam This switches over from using nsStringBuffer to nsString for URLValueData's |mString| member. This avoids various tedious conversions and can provide potential performance improvements by avoiding length calculations. MozReview-Commit-ID: 5eRifUZrAso
dom/base/nsAttrValue.cpp
dom/html/nsGenericHTMLElement.cpp
layout/style/ServoBindings.cpp
layout/style/nsCSSParser.cpp
layout/style/nsCSSValue.cpp
layout/style/nsCSSValue.h
layout/style/nsStyleStruct.cpp
layout/style/nsStyleStruct.h
--- a/dom/base/nsAttrValue.cpp
+++ b/dom/base/nsAttrValue.cpp
@@ -1670,27 +1670,22 @@ nsAttrValue::ParseIntMarginValue(const n
   return true;
 }
 
 void
 nsAttrValue::LoadImage(nsIDocument* aDocument)
 {
   NS_ASSERTION(Type() == eURL, "wrong type");
 
-#ifdef DEBUG
-  {
-    nsString val;
-    ToString(val);
-    NS_ASSERTION(!val.IsEmpty(),
-                 "How did we end up with an empty string for eURL");
-  }
-#endif
-
   MiscContainer* cont = GetMiscContainer();
   mozilla::css::URLValue* url = cont->mValue.mURL;
+
+  NS_ASSERTION(!url->mString.IsEmpty(),
+               "How did we end up with an empty string for eURL");
+
   mozilla::css::ImageValue* image =
     new css::ImageValue(url->GetURI(), url->mString,
                         do_AddRef(url->mExtraData), aDocument);
 
   NS_ADDREF(image);
   cont->mValue.mImage = image;
   NS_RELEASE(url);
   cont->mType = eImage;
--- a/dom/html/nsGenericHTMLElement.cpp
+++ b/dom/html/nsGenericHTMLElement.cpp
@@ -957,24 +957,18 @@ nsGenericHTMLElement::ParseBackgroundAtt
     nsCOMPtr<nsIURI> baseURI = GetBaseURI();
     nsCOMPtr<nsIURI> uri;
     nsresult rv = nsContentUtils::NewURIWithDocumentCharset(
         getter_AddRefs(uri), aValue, doc, baseURI);
     if (NS_FAILED(rv)) {
       return false;
     }
 
-    nsString value(aValue);
-    RefPtr<nsStringBuffer> buffer = nsCSSValue::BufferFromString(value);
-    if (MOZ_UNLIKELY(!buffer)) {
-      return false;
-    }
-
     mozilla::css::URLValue *url =
-      new mozilla::css::URLValue(uri, buffer, baseURI, doc->GetDocumentURI(),
+      new mozilla::css::URLValue(uri, aValue, baseURI, doc->GetDocumentURI(),
                                  NodePrincipal());
     aResult.SetTo(url, &aValue);
     return true;
   }
 
   return false;
 }
 
--- a/layout/style/ServoBindings.cpp
+++ b/layout/style/ServoBindings.cpp
@@ -962,24 +962,21 @@ ServoBundledURI::IntoCssUrl()
 {
   if (!mURLString) {
     return nullptr;
   }
 
   MOZ_ASSERT(mExtraData->GetReferrer());
   MOZ_ASSERT(mExtraData->GetPrincipal());
 
-  nsString url;
-  nsDependentCSubstring urlString(reinterpret_cast<const char*>(mURLString),
-                                  mURLStringLength);
-  AppendUTF8toUTF16(urlString, url);
-  RefPtr<nsStringBuffer> urlBuffer = nsCSSValue::BufferFromString(url);
+  NS_ConvertUTF8toUTF16 url(reinterpret_cast<const char*>(mURLString),
+                            mURLStringLength);
 
   RefPtr<css::URLValue> urlValue =
-    new css::URLValue(urlBuffer, do_AddRef(mExtraData));
+    new css::URLValue(url, do_AddRef(mExtraData));
   return urlValue.forget();
 }
 
 void
 Gecko_SetNullImageValue(nsStyleImage* aImage)
 {
   MOZ_ASSERT(aImage);
   aImage->SetNull();
@@ -995,24 +992,21 @@ Gecko_SetGradientImageValue(nsStyleImage
 static already_AddRefed<nsStyleImageRequest>
 CreateStyleImageRequest(nsStyleImageRequest::Mode aModeFlags,
                         ServoBundledURI aURI)
 {
   MOZ_ASSERT(aURI.mURLString);
   MOZ_ASSERT(aURI.mExtraData->GetReferrer());
   MOZ_ASSERT(aURI.mExtraData->GetPrincipal());
 
-  nsString url;
-  nsDependentCSubstring urlString(reinterpret_cast<const char*>(aURI.mURLString),
-                                  aURI.mURLStringLength);
-  AppendUTF8toUTF16(urlString, url);
-  RefPtr<nsStringBuffer> urlBuffer = nsCSSValue::BufferFromString(url);
+  NS_ConvertUTF8toUTF16 url(reinterpret_cast<const char*>(aURI.mURLString),
+                            aURI.mURLStringLength);
 
   RefPtr<nsStyleImageRequest> req =
-    new nsStyleImageRequest(aModeFlags, urlBuffer, do_AddRef(aURI.mExtraData));
+    new nsStyleImageRequest(aModeFlags, url, do_AddRef(aURI.mExtraData));
   return req.forget();
 }
 
 void
 Gecko_SetUrlImageValue(nsStyleImage* aImage, ServoBundledURI aURI)
 {
   RefPtr<nsStyleImageRequest> req =
     CreateStyleImageRequest(nsStyleImageRequest::Mode::Track, aURI);
--- a/layout/style/nsCSSParser.cpp
+++ b/layout/style/nsCSSParser.cpp
@@ -8187,21 +8187,19 @@ CSSParserImpl::SetValueToURL(nsCSSValue&
       return true;
     }
 
     NS_NOTREACHED("Codepaths that expect to parse URLs MUST pass in an "
                   "origin principal");
     return false;
   }
 
-  RefPtr<nsStringBuffer> buffer(nsCSSValue::BufferFromString(aURL));
-
   // Note: urlVal retains its own reference to |buffer|.
   mozilla::css::URLValue *urlVal =
-    new mozilla::css::URLValue(buffer, mBaseURI, mSheetURI, mSheetPrincipal);
+    new mozilla::css::URLValue(aURL, mBaseURI, mSheetURI, mSheetPrincipal);
   aValue.SetURLValue(urlVal);
   return true;
 }
 
 /**
  * Parse the image-orientation property, which has the grammar:
  * <angle> flip? | flip | from-image
  */
--- a/layout/style/nsCSSValue.cpp
+++ b/layout/style/nsCSSValue.cpp
@@ -25,35 +25,35 @@
 #include "nsDeviceContext.h"
 #include "nsStyleSet.h"
 #include "nsContentUtils.h"
 
 using namespace mozilla;
 using namespace mozilla::css;
 
 static bool
-IsLocalRefURL(nsStringBuffer* aString)
+IsLocalRefURL(const nsString& aString)
 {
   // Find the first non-"C0 controls + space" character.
-  char16_t* current = static_cast<char16_t*>(aString->Data());
+  const char16_t* current = aString.get();
   for (; *current != '\0'; current++) {
     if (*current > 0x20) {
       // if the first non-"C0 controls + space" character is '#', this is a
       // local-ref URL.
       return *current == '#';
     }
   }
 
   return false;
 }
 
 static bool
-MightHaveRef(nsStringBuffer* aString)
+MightHaveRef(const nsString& aString)
 {
-  char16_t* current = static_cast<char16_t*>(aString->Data());
+  const char16_t* current = aString.get();
   for (; *current != '\0'; current++) {
     if (*current == '#') {
       return true;
     }
   }
 
   return false;
 }
@@ -2790,69 +2790,63 @@ nsCSSValue::Array::SizeOfIncludingThis(m
   size_t n = aMallocSizeOf(this);
   for (size_t i = 0; i < mCount; i++) {
     n += mArray[i].SizeOfExcludingThis(aMallocSizeOf);
   }
   return n;
 }
 
 css::URLValueData::URLValueData(already_AddRefed<PtrHolder<nsIURI>> aURI,
-                                nsStringBuffer* aString,
+                                const nsAString& aString,
                                 already_AddRefed<URLExtraData> aExtraData)
   : mURI(Move(aURI))
   , mString(aString)
   , mExtraData(Move(aExtraData))
   , mURIResolved(true)
 {
-  MOZ_ASSERT(mString);
   MOZ_ASSERT(mExtraData);
   MOZ_ASSERT(mExtraData->GetPrincipal());
 }
 
-css::URLValueData::URLValueData(nsStringBuffer* aString,
+css::URLValueData::URLValueData(const nsAString& aString,
                                 already_AddRefed<URLExtraData> aExtraData)
   : mString(aString)
   , mExtraData(Move(aExtraData))
   , mURIResolved(false)
 {
-  MOZ_ASSERT(aString);
   MOZ_ASSERT(mExtraData);
   MOZ_ASSERT(mExtraData->GetPrincipal());
 }
 
 bool
 css::URLValueData::Equals(const URLValueData& aOther) const
 {
   MOZ_ASSERT(NS_IsMainThread());
 
   bool eq;
-  // Cast away const so we can call nsIPrincipal::Equals.
   const URLExtraData* self = mExtraData;
   const URLExtraData* other = aOther.mExtraData;
-  return NS_strcmp(nsCSSValue::GetBufferValue(mString),
-                   nsCSSValue::GetBufferValue(aOther.mString)) == 0 &&
+  return mString == aOther.mString &&
           (GetURI() == aOther.GetURI() || // handles null == null
            (mURI && aOther.mURI &&
             NS_SUCCEEDED(mURI->Equals(aOther.mURI, &eq)) &&
             eq)) &&
           (self->BaseURI() == other->BaseURI() ||
            (NS_SUCCEEDED(self->BaseURI()->Equals(other->BaseURI(), &eq)) &&
             eq)) &&
           (self->GetPrincipal() == other->GetPrincipal() ||
            self->GetPrincipal()->Equals(other->GetPrincipal())) &&
           IsLocalRef() == aOther.IsLocalRef();
 }
 
 bool
 css::URLValueData::DefinitelyEqualURIs(const URLValueData& aOther) const
 {
   return mExtraData->BaseURI() == aOther.mExtraData->BaseURI() &&
-         (mString == aOther.mString ||
-          NS_strcmp(nsCSSValue::GetBufferValue(mString),
-                    nsCSSValue::GetBufferValue(aOther.mString)) == 0);
+         (mString == aOther.mString);
 }
 
 bool
 css::URLValueData::DefinitelyEqualURIsAndPrincipal(
     const URLValueData& aOther) const
 {
   return mExtraData->GetPrincipal() == aOther.mExtraData->GetPrincipal() &&
          DefinitelyEqualURIs(aOther);
@@ -2862,17 +2856,17 @@ nsIURI*
 css::URLValueData::GetURI() const
 {
   MOZ_ASSERT(NS_IsMainThread());
 
   if (!mURIResolved) {
     MOZ_ASSERT(!mURI);
     nsCOMPtr<nsIURI> newURI;
     NS_NewURI(getter_AddRefs(newURI),
-              NS_ConvertUTF16toUTF8(nsCSSValue::GetBufferValue(mString)),
+              NS_ConvertUTF16toUTF8(mString),
               nullptr, mExtraData->BaseURI());
     mURI = new PtrHolder<nsIURI>(newURI.forget());
     mURIResolved = true;
   }
 
   return mURI;
 }
 
@@ -2982,34 +2976,34 @@ css::URLValueData::EqualsExceptRef(nsIUR
   uri->EqualsExceptRef(aURI, &ret);
   return ret;
 }
 
 size_t
 css::URLValueData::SizeOfExcludingThis(mozilla::MallocSizeOf aMallocSizeOf) const
 {
   size_t n = 0;
-  n += mString->SizeOfIncludingThisIfUnshared(aMallocSizeOf);
+  n += mString.SizeOfExcludingThisIfUnshared(aMallocSizeOf);
 
   // Measurement of the following members may be added later if DMD finds it
   // is worthwhile:
   // - mURI
   // - mExtraData
   return n;
 }
 
-URLValue::URLValue(nsStringBuffer* aString, nsIURI* aBaseURI, nsIURI* aReferrer,
+URLValue::URLValue(const nsAString& aString, nsIURI* aBaseURI, nsIURI* aReferrer,
                    nsIPrincipal* aOriginPrincipal)
   : URLValueData(aString, do_AddRef(new URLExtraData(aBaseURI, aReferrer,
                                                      aOriginPrincipal)))
 {
   MOZ_ASSERT(NS_IsMainThread());
 }
 
-URLValue::URLValue(nsIURI* aURI, nsStringBuffer* aString, nsIURI* aBaseURI,
+URLValue::URLValue(nsIURI* aURI, const nsAString& aString, nsIURI* aBaseURI,
                    nsIURI* aReferrer, nsIPrincipal* aOriginPrincipal)
   : URLValueData(do_AddRef(new PtrHolder<nsIURI>(aURI)),
                  aString,
                  do_AddRef(new URLExtraData(aBaseURI, aReferrer,
                                             aOriginPrincipal)))
 {
   MOZ_ASSERT(NS_IsMainThread());
 }
@@ -3021,26 +3015,26 @@ css::URLValue::SizeOfIncludingThis(mozil
   size_t n = 0;
   if (mRefCnt <= 1) {
     n += aMallocSizeOf(this);
     n += URLValueData::SizeOfExcludingThis(aMallocSizeOf);
   }
   return n;
 }
 
-css::ImageValue::ImageValue(nsIURI* aURI, nsStringBuffer* aString,
+css::ImageValue::ImageValue(nsIURI* aURI, const nsAString& aString,
                             already_AddRefed<URLExtraData> aExtraData,
                             nsIDocument* aDocument)
   : URLValueData(do_AddRef(new PtrHolder<nsIURI>(aURI)),
                  aString, Move(aExtraData))
 {
   Initialize(aDocument);
 }
 
-css::ImageValue::ImageValue(nsStringBuffer* aString,
+css::ImageValue::ImageValue(const nsAString& aString,
                             already_AddRefed<URLExtraData> aExtraData)
   : URLValueData(aString, Move(aExtraData))
 {
 }
 
 void
 css::ImageValue::Initialize(nsIDocument* aDocument)
 {
--- a/layout/style/nsCSSValue.h
+++ b/layout/style/nsCSSValue.h
@@ -97,21 +97,21 @@ protected:
   // Methods are not inline because using an nsIPrincipal means requiring
   // caps, which leads to REQUIRES hell, since this header is included all
   // over.
 
   // For both constructors aString must not be null.
   // For both constructors principal of aExtraData must not be null.
   // Construct with a base URI; this will create the actual URI lazily from
   // aString and aExtraData.
-  URLValueData(nsStringBuffer* aString,
+  URLValueData(const nsAString& aString,
                already_AddRefed<URLExtraData> aExtraData);
   // Construct with the actual URI.
   URLValueData(already_AddRefed<PtrHolder<nsIURI>> aURI,
-               nsStringBuffer* aString,
+               const nsAString& aString,
                already_AddRefed<URLExtraData> aExtraData);
 
 public:
   // Returns true iff all fields of the two URLValueData objects are equal.
   //
   // Only safe to call on the main thread, since this will call Equals on the
   // nsIURI and nsIPrincipal objects stored on the URLValueData objects.
   bool Equals(const URLValueData& aOther) const;
@@ -156,17 +156,17 @@ public:
 
   bool EqualsExceptRef(nsIURI* aURI) const;
 
 private:
   // mURI stores the lazily resolved URI.  This may be null if the URI is
   // invalid, even once resolved.
   mutable PtrHandle<nsIURI> mURI;
 public:
-  RefPtr<nsStringBuffer> mString;
+  nsString mString;
   RefPtr<URLExtraData> mExtraData;
 private:
   mutable bool mURIResolved;
   // mIsLocalRef is set when url starts with a U+0023 number sign(#) character.
   mutable Maybe<bool> mIsLocalRef;
   mutable Maybe<bool> mMightHaveRef;
 
 protected:
@@ -177,47 +177,46 @@ protected:
 private:
   URLValueData(const URLValueData& aOther) = delete;
   URLValueData& operator=(const URLValueData& aOther) = delete;
 };
 
 struct URLValue final : public URLValueData
 {
   // These two constructors are safe to call only on the main thread.
-  URLValue(nsStringBuffer* aString, nsIURI* aBaseURI, nsIURI* aReferrer,
+  URLValue(const nsAString& aString, nsIURI* aBaseURI, nsIURI* aReferrer,
            nsIPrincipal* aOriginPrincipal);
-  URLValue(nsIURI* aURI, nsStringBuffer* aString, nsIURI* aBaseURI,
+  URLValue(nsIURI* aURI, const nsAString& aString, nsIURI* aBaseURI,
            nsIURI* aReferrer, nsIPrincipal* aOriginPrincipal);
 
   // This constructor is safe to call from any thread.
-  URLValue(nsStringBuffer* aString,
+  URLValue(const nsAString& aString,
            already_AddRefed<URLExtraData> aExtraData)
     : URLValueData(aString, Move(aExtraData)) {}
 
   URLValue(const URLValue&) = delete;
   URLValue& operator=(const URLValue&) = delete;
 
   size_t SizeOfIncludingThis(mozilla::MallocSizeOf aMallocSizeOf) const;
 };
 
 struct ImageValue final : public URLValueData
 {
   // Not making the constructor and destructor inline because that would
   // force us to include imgIRequest.h, which leads to REQUIRES hell, since
   // this header is included all over.
-  // aString must not be null.
   //
   // This constructor is only safe to call from the main thread.
-  ImageValue(nsIURI* aURI, nsStringBuffer* aString,
+  ImageValue(nsIURI* aURI, const nsAString& aString,
              already_AddRefed<URLExtraData> aExtraData,
              nsIDocument* aDocument);
 
   // This constructor is safe to call from any thread, but Initialize
   // must be called later for the object to be useful.
-  ImageValue(nsStringBuffer* aString,
+  ImageValue(const nsAString& aString,
              already_AddRefed<URLExtraData> aExtraData);
 
   ImageValue(const ImageValue&) = delete;
   ImageValue& operator=(const ImageValue&) = delete;
 
   void Initialize(nsIDocument* aDocument);
 
   // XXXheycam We should have our own SizeOfIncludingThis method.
@@ -851,19 +850,19 @@ public:
                "not a grid-template-areas value");
     return mValue.mGridTemplateAreas;
   }
 
   const char16_t* GetOriginalURLValue() const
   {
     MOZ_ASSERT(mUnit == eCSSUnit_URL || mUnit == eCSSUnit_Image,
                "not a URL value");
-    return GetBufferValue(mUnit == eCSSUnit_URL ?
-                            mValue.mURL->mString :
-                            mValue.mImage->mString);
+    return mUnit == eCSSUnit_URL ?
+             mValue.mURL->mString.get() :
+             mValue.mImage->mString.get();
   }
 
   // Not making this inline because that would force us to include
   // imgIRequest.h, which leads to REQUIRES hell, since this header is included
   // all over.
   imgRequestProxy* GetImageValue(nsIDocument* aDocument) const;
 
   // Like GetImageValue, but additionally will pass the imgRequestProxy
--- a/layout/style/nsStyleStruct.cpp
+++ b/layout/style/nsStyleStruct.cpp
@@ -1986,19 +1986,19 @@ nsStyleImageRequest::nsStyleImageRequest
 
   if (mRequestProxy) {
     MaybeTrackAndLock();
   }
 }
 
 nsStyleImageRequest::nsStyleImageRequest(
     Mode aModeFlags,
-    nsStringBuffer* aURLBuffer,
+    const nsAString& aURL,
     already_AddRefed<URLExtraData> aExtraData)
-  : mImageValue(new css::ImageValue(aURLBuffer, Move(aExtraData)))
+  : mImageValue(new css::ImageValue(aURL, Move(aExtraData)))
   , mModeFlags(aModeFlags)
   , mResolved(false)
 {
 }
 
 nsStyleImageRequest::~nsStyleImageRequest()
 {
   // We may or may not be being destroyed on the main thread.  To clean
--- a/layout/style/nsStyleStruct.h
+++ b/layout/style/nsStyleStruct.h
@@ -335,17 +335,17 @@ public:
                       imgRequestProxy* aRequestProxy,
                       mozilla::css::ImageValue* aImageValue,
                       mozilla::dom::ImageTracker* aImageTracker);
 
   // Can be called from any thread, but Resolve() must be called later
   // on the main thread before get() can be used.
   nsStyleImageRequest(
       Mode aModeFlags,
-      nsStringBuffer* aURLBuffer,
+      const nsAString& aURL,
       already_AddRefed<mozilla::URLExtraData> aExtraData);
 
   bool Resolve(nsPresContext* aPresContext);
   bool IsResolved() const { return mResolved; }
 
   imgRequestProxy* get() {
     MOZ_ASSERT(IsResolved(), "Resolve() must be called first");
     MOZ_ASSERT(NS_IsMainThread());