Bug 1297963 - Part 1: Preserve base URI on URLValueData objects. r=emilio
authorCameron McCormack <cam@mcc.id.au>
Sat, 03 Sep 2016 00:22:47 +1000
changeset 312441 d91354854339f34daf412da475bf3293cc75ef76
parent 312440 e89e213ef1facaf2d3c9cf4b0920f52ba269a3e3
child 312442 0549f87f0c292b5bc27f2d62cf62cb7ec1ec4cff
push id20447
push userkwierso@gmail.com
push dateFri, 02 Sep 2016 20:36:44 +0000
treeherderfx-team@969397f22187 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersemilio
bugs1297963
milestone51.0a1
Bug 1297963 - Part 1: Preserve base URI on URLValueData objects. r=emilio MozReview-Commit-ID: Amjz1AcdxwN
dom/base/nsAttrValue.cpp
dom/html/nsGenericHTMLElement.cpp
layout/style/StyleAnimationValue.cpp
layout/style/nsCSSValue.cpp
layout/style/nsCSSValue.h
--- a/dom/base/nsAttrValue.cpp
+++ b/dom/base/nsAttrValue.cpp
@@ -1729,19 +1729,19 @@ nsAttrValue::LoadImage(nsIDocument* aDoc
     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;
-  mozilla::css::ImageValue* image = 
-    new css::ImageValue(url->GetURI(), url->mString, url->mReferrer,
-                        url->mOriginPrincipal, aDocument);
+  mozilla::css::ImageValue* image =
+    new css::ImageValue(url->GetURI(), url->mString, url->mBaseURI,
+                        url->mReferrer, url->mOriginPrincipal, aDocument);
 
   NS_ADDREF(image);
   cont->mValue.mImage = image;
   NS_RELEASE(url);
   cont->mType = eImage;
 }
 
 bool
--- a/dom/html/nsGenericHTMLElement.cpp
+++ b/dom/html/nsGenericHTMLElement.cpp
@@ -976,17 +976,17 @@ nsGenericHTMLElement::ParseBackgroundAtt
 
     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, doc->GetDocumentURI(),
+      new mozilla::css::URLValue(uri, buffer, baseURI, doc->GetDocumentURI(),
                                  NodePrincipal());
     aResult.SetTo(url, &aValue);
     return true;
   }
 
   return false;
 }
 
--- a/layout/style/StyleAnimationValue.cpp
+++ b/layout/style/StyleAnimationValue.cpp
@@ -286,19 +286,23 @@ AppendCSSShadowValue(const nsCSSShadowIt
 }
 
 static already_AddRefed<mozilla::css::URLValue>
 FragmentOrURLToURLValue(FragmentOrURL* aUrl, nsIDocument* aDoc)
 {
   nsString path;
   aUrl->GetSourceString(path);
   RefPtr<nsStringBuffer> uriStringBuffer = nsCSSValue::BufferFromString(path);
+  // XXXheycam We should store URLValue objects inside FragmentOrURLs so that
+  // we can extract the original base URI, referrer and principal to pass in
+  // here.
   RefPtr<mozilla::css::URLValue> result =
     new mozilla::css::URLValue(aUrl->GetSourceURL(), uriStringBuffer,
-                               aDoc->GetDocumentURI(), aDoc->NodePrincipal());
+                               aUrl->GetSourceURL(), aDoc->GetDocumentURI(),
+                               aDoc->NodePrincipal());
 
   return result.forget();
 }
 
 // Like nsStyleCoord::CalcValue, but with length in float pixels instead
 // of nscoord.
 struct PixelCalcValue
 {
--- a/layout/style/nsCSSValue.cpp
+++ b/layout/style/nsCSSValue.cpp
@@ -756,16 +756,17 @@ void nsCSSValue::SetDummyInheritValue()
 }
 
 void nsCSSValue::StartImageLoad(nsIDocument* aDocument) const
 {
   MOZ_ASSERT(eCSSUnit_URL == mUnit, "Not a URL value!");
   mozilla::css::ImageValue* image =
     new mozilla::css::ImageValue(mValue.mURL->GetURI(),
                                  mValue.mURL->mString,
+                                 mValue.mURL->mBaseURI,
                                  mValue.mURL->mReferrer,
                                  mValue.mURL->mOriginPrincipal,
                                  aDocument);
 
   nsCSSValue* writable = const_cast<nsCSSValue*>(this);
   writable->SetImageValue(image);
 }
 
@@ -2587,57 +2588,66 @@ nsCSSValue::Array::SizeOfIncludingThis(m
   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,
+                                already_AddRefed<PtrHolder<nsIURI>> aBaseURI,
                                 already_AddRefed<PtrHolder<nsIURI>> aReferrer,
                                 already_AddRefed<PtrHolder<nsIPrincipal>>
                                   aOriginPrincipal)
   : mURI(Move(aURI))
+  , mBaseURI(Move(aBaseURI))
   , mString(aString)
   , mReferrer(Move(aReferrer))
   , mOriginPrincipal(Move(aOriginPrincipal))
   , mURIResolved(true)
   , mLocalURLFlag(IsLocalRefURL(aString))
 {
-  MOZ_ASSERT(mOriginPrincipal, "Must have an origin principal");
+  MOZ_ASSERT(mString);
+  MOZ_ASSERT(mBaseURI);
+  MOZ_ASSERT(mOriginPrincipal);
 }
 
 css::URLValueData::URLValueData(nsStringBuffer* aString,
                                 already_AddRefed<PtrHolder<nsIURI>> aBaseURI,
                                 already_AddRefed<PtrHolder<nsIURI>> aReferrer,
                                 already_AddRefed<PtrHolder<nsIPrincipal>>
                                   aOriginPrincipal)
-  : mURI(Move(aBaseURI))
+  : mBaseURI(Move(aBaseURI))
   , mString(aString)
   , mReferrer(Move(aReferrer))
   , mOriginPrincipal(Move(aOriginPrincipal))
   , mURIResolved(false)
   , mLocalURLFlag(IsLocalRefURL(aString))
 {
-  MOZ_ASSERT(mOriginPrincipal, "Must have an origin principal");
+  MOZ_ASSERT(aString);
+  MOZ_ASSERT(mBaseURI);
+  MOZ_ASSERT(mOriginPrincipal);
 }
 
 bool
 css::URLValueData::operator==(const URLValueData& aOther) const
 {
   bool eq;
   // Cast away const so we can call nsIPrincipal::Equals.
   auto& self = *const_cast<URLValueData*>(this);
   auto& other = const_cast<URLValueData&>(aOther);
   return NS_strcmp(nsCSSValue::GetBufferValue(mString),
                    nsCSSValue::GetBufferValue(aOther.mString)) == 0 &&
           (GetURI() == aOther.GetURI() || // handles null == null
            (mURI && aOther.mURI &&
             NS_SUCCEEDED(mURI->Equals(aOther.mURI, &eq)) &&
             eq)) &&
+          (mBaseURI == aOther.mBaseURI ||
+           (NS_SUCCEEDED(self.mBaseURI.get()->Equals(other.mBaseURI.get(), &eq)) &&
+            eq)) &&
           (mOriginPrincipal == aOther.mOriginPrincipal ||
            self.mOriginPrincipal.get()->Equals(other.mOriginPrincipal.get())) &&
           mLocalURLFlag == aOther.mLocalURLFlag;
 }
 
 bool
 css::URLValueData::MaybeUnresolvedURIEquals(const URLValueData& aOther) const
 {
@@ -2666,24 +2676,26 @@ css::URLValueData::URIEquals(const URLVa
          (mOriginPrincipal == aOther.mOriginPrincipal ||
           self.mOriginPrincipal.get()->Equals(other.mOriginPrincipal.get())) &&
          mLocalURLFlag == aOther.mLocalURLFlag;
 }
 
 nsIURI*
 css::URLValueData::GetURI() const
 {
+  MOZ_ASSERT(NS_IsMainThread());
+
   if (!mURIResolved) {
-    mURIResolved = true;
-    // Be careful to not null out mURI before we've passed it as the base URI
+    MOZ_ASSERT(!mURI);
     nsCOMPtr<nsIURI> newURI;
     NS_NewURI(getter_AddRefs(newURI),
               NS_ConvertUTF16toUTF8(nsCSSValue::GetBufferValue(mString)),
-              nullptr, mURI);
+              nullptr, const_cast<nsIURI*>(mBaseURI.get()));
     mURI = new PtrHolder<nsIURI>(newURI.forget());
+    mURIResolved = true;
   }
 
   return mURI;
 }
 
 size_t
 css::URLValueData::SizeOfExcludingThis(mozilla::MallocSizeOf aMallocSizeOf) const
 {
@@ -2703,20 +2715,21 @@ URLValue::URLValue(nsStringBuffer* aStri
   : URLValueData(aString,
                  do_AddRef(new PtrHolder<nsIURI>(aBaseURI)),
                  do_AddRef(new PtrHolder<nsIURI>(aReferrer)),
                  do_AddRef(new PtrHolder<nsIPrincipal>(aOriginPrincipal)))
 {
   MOZ_ASSERT(NS_IsMainThread());
 }
 
-URLValue::URLValue(nsIURI* aURI, nsStringBuffer* aString, nsIURI* aReferrer,
-                   nsIPrincipal* aOriginPrincipal)
+URLValue::URLValue(nsIURI* aURI, nsStringBuffer* aString, nsIURI* aBaseURI,
+                   nsIURI* aReferrer, nsIPrincipal* aOriginPrincipal)
   : URLValueData(do_AddRef(new PtrHolder<nsIURI>(aURI)),
                  aString,
+                 do_AddRef(new PtrHolder<nsIURI>(aBaseURI)),
                  do_AddRef(new PtrHolder<nsIURI>(aReferrer)),
                  do_AddRef(new PtrHolder<nsIPrincipal>(aOriginPrincipal)))
 {
   MOZ_ASSERT(NS_IsMainThread());
 }
 
 size_t
 css::URLValue::SizeOfIncludingThis(mozilla::MallocSizeOf aMallocSizeOf) const
@@ -2726,20 +2739,22 @@ css::URLValue::SizeOfIncludingThis(mozil
   if (mRefCnt <= 1) {
     n += aMallocSizeOf(this);
     n += URLValueData::SizeOfExcludingThis(aMallocSizeOf);
   }
   return n;
 }
 
 css::ImageValue::ImageValue(nsIURI* aURI, nsStringBuffer* aString,
-                            nsIURI* aReferrer, nsIPrincipal* aOriginPrincipal,
+                            nsIURI* aBaseURI, nsIURI* aReferrer,
+                            nsIPrincipal* aOriginPrincipal,
                             nsIDocument* aDocument)
   : URLValueData(do_AddRef(new PtrHolder<nsIURI>(aURI)),
                  aString,
+                 do_AddRef(new PtrHolder<nsIURI>(aBaseURI, false)),
                  do_AddRef(new PtrHolder<nsIURI>(aReferrer)),
                  do_AddRef(new PtrHolder<nsIPrincipal>(aOriginPrincipal)))
 {
   MOZ_ASSERT(NS_IsMainThread());
 
   // NB: If aDocument is not the original document, we may not be able to load
   // images from aDocument.  Instead we do the image load from the original doc
   // and clone it to aDocument.
--- a/layout/style/nsCSSValue.h
+++ b/layout/style/nsCSSValue.h
@@ -101,16 +101,17 @@ struct URLValueData
   // aString and aBaseURI.
   URLValueData(nsStringBuffer* aString,
                already_AddRefed<PtrHolder<nsIURI>> aBaseURI,
                already_AddRefed<PtrHolder<nsIURI>> aReferrer,
                already_AddRefed<PtrHolder<nsIPrincipal>> aOriginPricinpal);
   // Construct with the actual URI.
   URLValueData(already_AddRefed<PtrHolder<nsIURI>> aURI,
                nsStringBuffer* aString,
+               already_AddRefed<PtrHolder<nsIURI>> aBaseURI,
                already_AddRefed<PtrHolder<nsIURI>> aReferrer,
                already_AddRefed<PtrHolder<nsIPrincipal>> aOriginPrincipal);
 
   bool operator==(const URLValueData& aOther) const;
 
   // URIEquals only compares URIs and principals (unlike operator==, which
   // also compares the original strings).  URIEquals also assumes that the
   // mURI member of both URL objects is non-null.  Do NOT call this method
@@ -123,21 +124,21 @@ struct URLValueData
 
   nsIURI* GetURI() const;
 
   size_t SizeOfExcludingThis(mozilla::MallocSizeOf aMallocSizeOf) const;
 
   bool GetLocalURLFlag() const { return mLocalURLFlag; }
 
 private:
-  // If mURIResolved is false, mURI stores the base URI.
-  // If mURIResolved is true, mURI stores the URI we resolve to; this may be
-  // null if the URI is invalid.
+  // mURI stores the lazily resolved URI.  This may be null if the URI is
+  // invalid, even once resolved.
   mutable PtrHandle<nsIURI> mURI;
 public:
+  PtrHandle<nsIURI> mBaseURI;
   RefPtr<nsStringBuffer> mString;
   PtrHandle<nsIURI> mReferrer;
   PtrHandle<nsIPrincipal> mOriginPrincipal;
 private:
   mutable bool mURIResolved;
   // mLocalURLFlag is set when url starts with a U+0023 number
   // sign(#) character.
   bool mLocalURLFlag;
@@ -146,18 +147,18 @@ private:
   URLValueData& operator=(const URLValueData& aOther) = delete;
 };
 
 struct URLValue : public URLValueData
 {
   // These two constructors are safe to call only on the main thread.
   URLValue(nsStringBuffer* aString, nsIURI* aBaseURI, nsIURI* aReferrer,
            nsIPrincipal* aOriginPrincipal);
-  URLValue(nsIURI* aURI, nsStringBuffer* aString, nsIURI* aReferrer,
-           nsIPrincipal* aOriginPrincipal);
+  URLValue(nsIURI* aURI, nsStringBuffer* aString, nsIURI* aBaseURI,
+           nsIURI* aReferrer, nsIPrincipal* aOriginPrincipal);
 
   // This constructor is safe to call from any thread.
   URLValue(nsStringBuffer* aString,
            already_AddRefed<PtrHolder<nsIURI>> aBaseURI,
            already_AddRefed<PtrHolder<nsIURI>> aReferrer,
            already_AddRefed<PtrHolder<nsIPrincipal>> aOriginPrincipal)
     : URLValueData(aString, Move(aBaseURI), Move(aReferrer),
                    Move(aOriginPrincipal)) {}
@@ -177,18 +178,19 @@ public:
 struct ImageValue : 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, nsIURI* aReferrer,
-             nsIPrincipal* aOriginPrincipal, nsIDocument* aDocument);
+  ImageValue(nsIURI* aURI, nsStringBuffer* aString, nsIURI* aBaseURI,
+             nsIURI* aReferrer, nsIPrincipal* aOriginPrincipal,
+             nsIDocument* aDocument);
 
   ImageValue(const ImageValue&) = delete;
   ImageValue& operator=(const ImageValue&) = delete;
 
   // XXXheycam We should have our own SizeOfIncludingThis method.
 
 private:
   ~ImageValue();