Bug 597291. Create nsIURI objects lazily for nsCSSValue::URL, so that we don't pay the cost of creating the ones we don't actually need. r=dbaron
authorBoris Zbarsky <bzbarsky@mit.edu>
Wed, 11 May 2011 11:28:53 -0400
changeset 69586 d5728312d7564f298556ea73d63bf3f4043f2b40
parent 69585 5a456fd32d3d8c0d19167cc373cfc67ba1e7aeba
child 69587 916987d881346e5bc8ba17cc843fc42ce5566ea4
push id76
push userbzbarsky@mozilla.com
push dateTue, 05 Jul 2011 17:00:57 +0000
treeherdermozilla-beta@d3a2732c35f1 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersdbaron
bugs597291
milestone6.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 597291. Create nsIURI objects lazily for nsCSSValue::URL, so that we don't pay the cost of creating the ones we don't actually need. r=dbaron In the new setup, the mURL member of nsCSSValue::URL stores either the actual URI pointed to or the base URI; a boolean flag keeps track of which is stored. Consumers use GetURI() to get the URI instead of raw access to mURI, and GetURI calls NS_NewURI as needed.
dom/base/nsDOMClassInfo.cpp
layout/base/nsCSSFrameConstructor.cpp
layout/style/nsCSSParser.cpp
layout/style/nsCSSValue.cpp
layout/style/nsCSSValue.h
layout/style/nsComputedDOMStyle.cpp
layout/style/nsRuleNode.cpp
layout/style/nsStyleStruct.h
--- a/dom/base/nsDOMClassInfo.cpp
+++ b/dom/base/nsDOMClassInfo.cpp
@@ -8018,17 +8018,17 @@ nsElementSH::PostCreate(nsIXPConnectWrap
   PRBool ok = GetBindingURL(element, doc, &bindingURL);
   NS_ENSURE_TRUE(ok, NS_ERROR_FAILURE);
 
   if (!bindingURL) {
     // No binding, nothing left to do here.
     return NS_OK;
   }
 
-  nsCOMPtr<nsIURI> uri = bindingURL->mURI;
+  nsCOMPtr<nsIURI> uri = bindingURL->GetURI();
   nsCOMPtr<nsIPrincipal> principal = bindingURL->mOriginPrincipal;
 
   // We have a binding that must be installed.
   PRBool dummy;
 
   nsCOMPtr<nsIXBLService> xblService(do_GetService("@mozilla.org/xbl;1"));
   NS_ENSURE_TRUE(xblService, NS_ERROR_NOT_AVAILABLE);
 
--- a/layout/base/nsCSSFrameConstructor.cpp
+++ b/layout/base/nsCSSFrameConstructor.cpp
@@ -2341,17 +2341,17 @@ nsCSSFrameConstructor::ConstructDocEleme
     nsresult rv;
     PRBool resolveStyle;
     
     nsIXBLService * xblService = GetXBLService();
     if (!xblService)
       return NS_ERROR_FAILURE;
 
     nsRefPtr<nsXBLBinding> binding;
-    rv = xblService->LoadBindings(aDocElement, display->mBinding->mURI,
+    rv = xblService->LoadBindings(aDocElement, display->mBinding->GetURI(),
                                   display->mBinding->mOriginPrincipal,
                                   PR_FALSE, getter_AddRefs(binding),
                                   &resolveStyle);
     if (NS_FAILED(rv) && rv != NS_ERROR_XBL_BLOCKED)
       return NS_OK; // Binding will load asynchronously.
 
     if (binding) {
       // For backwards compat, keep firing the root's constructor
@@ -5088,17 +5088,17 @@ nsCSSFrameConstructor::AddFrameConstruct
       return;
 
     PRBool resolveStyle;
 
     nsAutoPtr<PendingBinding> newPendingBinding(new PendingBinding());
     if (!newPendingBinding) {
       return;
     }
-    nsresult rv = xblService->LoadBindings(aContent, display->mBinding->mURI,
+    nsresult rv = xblService->LoadBindings(aContent, display->mBinding->GetURI(),
                                            display->mBinding->mOriginPrincipal,
                                            PR_FALSE,
                                            getter_AddRefs(newPendingBinding->mBinding),
                                            &resolveStyle);
     if (NS_FAILED(rv) && rv != NS_ERROR_XBL_BLOCKED)
       return;
 
     if (newPendingBinding->mBinding) {
--- a/layout/style/nsCSSParser.cpp
+++ b/layout/style/nsCSSParser.cpp
@@ -4869,30 +4869,25 @@ PRBool
 CSSParserImpl::SetValueToURL(nsCSSValue& aValue, const nsString& aURL)
 {
   if (!mSheetPrincipal) {
     NS_NOTREACHED("Codepaths that expect to parse URLs MUST pass in an "
                   "origin principal");
     return PR_FALSE;
   }
 
-  // Translate url into an absolute url if the url is relative to the
-  // style sheet.
-  nsCOMPtr<nsIURI> uri;
-  NS_NewURI(getter_AddRefs(uri), aURL, nsnull, mBaseURI);
-
   nsRefPtr<nsStringBuffer> buffer(nsCSSValue::BufferFromString(aURL));
   if (NS_UNLIKELY(!buffer)) {
     mScanner.SetLowLevelError(NS_ERROR_OUT_OF_MEMORY);
     return PR_FALSE;
   }
 
   // Note: urlVal retains its own reference to |buffer|.
   nsCSSValue::URL *urlVal =
-    new nsCSSValue::URL(uri, buffer, mSheetURI, mSheetPrincipal);
+    new nsCSSValue::URL(buffer, mBaseURI, mSheetURI, mSheetPrincipal);
 
   if (NS_UNLIKELY(!urlVal)) {
     mScanner.SetLowLevelError(NS_ERROR_OUT_OF_MEMORY);
     return PR_FALSE;
   }
   aValue.SetURLValue(urlVal);
   return PR_TRUE;
 }
--- a/layout/style/nsCSSValue.cpp
+++ b/layout/style/nsCSSValue.cpp
@@ -40,16 +40,17 @@
 #include "nsCSSValue.h"
 
 #include "imgIRequest.h"
 #include "nsIPrincipal.h"
 #include "nsCSSProps.h"
 #include "nsContentUtils.h"
 #include "nsStyleUtil.h"
 #include "CSSCalc.h"
+#include "nsNetUtil.h"
 
 namespace css = mozilla::css;
 
 nsCSSValue::nsCSSValue(PRInt32 aValue, nsCSSUnit aUnit)
   : mUnit(aUnit)
 {
   NS_ABORT_IF_FALSE(aUnit == eCSSUnit_Integer || aUnit == eCSSUnit_Enumerated ||
                     aUnit == eCSSUnit_EnumColor, "not an int value");
@@ -537,17 +538,17 @@ void nsCSSValue::SetDummyInheritValue()
   Reset();
   mUnit = eCSSUnit_DummyInherit;
 }
 
 void nsCSSValue::StartImageLoad(nsIDocument* aDocument) const
 {
   NS_ABORT_IF_FALSE(eCSSUnit_URL == mUnit, "Not a URL value!");
   nsCSSValue::Image* image =
-    new nsCSSValue::Image(mValue.mURL->mURI,
+    new nsCSSValue::Image(mValue.mURL->GetURI(),
                           mValue.mURL->mString,
                           mValue.mURL->mReferrer,
                           mValue.mURL->mOriginPrincipal,
                           aDocument);
   if (image) {
     nsCSSValue* writable = const_cast<nsCSSValue*>(this);
     writable->SetImageValue(image);
   }
@@ -1225,74 +1226,104 @@ nsCSSValuePairList::operator==(const nsC
   for ( ; p1 && p2; p1 = p1->mNext, p2 = p2->mNext) {
     if (p1->mXValue != p2->mXValue ||
         p1->mYValue != p2->mYValue)
       return false;
   }
   return !p1 && !p2; // true if same length, false otherwise
 }
 
-nsCSSValue::URL::URL(nsIURI* aURI, nsStringBuffer* aString, nsIURI* aReferrer,
-                     nsIPrincipal* aOriginPrincipal)
+nsCSSValue::URL::URL(nsIURI* aURI, nsStringBuffer* aString,
+                     nsIURI* aReferrer, nsIPrincipal* aOriginPrincipal)
   : mURI(aURI),
     mString(aString),
     mReferrer(aReferrer),
-    mOriginPrincipal(aOriginPrincipal)
+    mOriginPrincipal(aOriginPrincipal),
+    mURIResolved(PR_TRUE)
+{
+  NS_ABORT_IF_FALSE(aOriginPrincipal, "Must have an origin principal");
+  mString->AddRef();
+}
+
+nsCSSValue::URL::URL(nsStringBuffer* aString, nsIURI* aBaseURI,
+                     nsIURI* aReferrer, nsIPrincipal* aOriginPrincipal)
+  : mURI(aBaseURI),
+    mString(aString),
+    mReferrer(aReferrer),
+    mOriginPrincipal(aOriginPrincipal),
+    mURIResolved(PR_FALSE)
 {
   NS_ABORT_IF_FALSE(aOriginPrincipal, "Must have an origin principal");
   mString->AddRef();
 }
 
 nsCSSValue::URL::~URL()
 {
   mString->Release();
 }
 
 PRBool
 nsCSSValue::URL::operator==(const URL& aOther) const
 {
   PRBool eq;
   return NS_strcmp(GetBufferValue(mString),
                    GetBufferValue(aOther.mString)) == 0 &&
-          (mURI == aOther.mURI || // handles null == null
+          (GetURI() == aOther.GetURI() || // handles null == null
            (mURI && aOther.mURI &&
             NS_SUCCEEDED(mURI->Equals(aOther.mURI, &eq)) &&
             eq)) &&
           (mOriginPrincipal == aOther.mOriginPrincipal ||
            (NS_SUCCEEDED(mOriginPrincipal->Equals(aOther.mOriginPrincipal,
                                                   &eq)) && eq));
 }
 
 PRBool
 nsCSSValue::URL::URIEquals(const URL& aOther) const
 {
+  NS_ABORT_IF_FALSE(mURIResolved && aOther.mURIResolved,
+                    "How do you know the URIs aren't null?");
   PRBool eq;
-  // Worth comparing mURI to aOther.mURI and mOriginPrincipal to
+  // Worth comparing GetURI() to aOther.GetURI() and mOriginPrincipal to
   // aOther.mOriginPrincipal, because in the (probably common) case when this
   // value was one of the ones that in fact did not change this will be our
   // fast path to equality
   return (mURI == aOther.mURI ||
           (NS_SUCCEEDED(mURI->Equals(aOther.mURI, &eq)) && eq)) &&
          (mOriginPrincipal == aOther.mOriginPrincipal ||
           (NS_SUCCEEDED(mOriginPrincipal->Equals(aOther.mOriginPrincipal,
                                                  &eq)) && eq));
 }
 
+nsIURI*
+nsCSSValue::URL::GetURI() const
+{
+  if (!mURIResolved) {
+    mURIResolved = PR_TRUE;
+    // Be careful to not null out mURI before we've passed it as the base URI
+    nsCOMPtr<nsIURI> newURI;
+    NS_NewURI(getter_AddRefs(newURI),
+              NS_ConvertUTF16toUTF8(GetBufferValue(mString)), nsnull, mURI);
+    newURI.swap(mURI);
+  }
+
+  return mURI;
+}
+
 nsCSSValue::Image::Image(nsIURI* aURI, nsStringBuffer* aString,
                          nsIURI* aReferrer, nsIPrincipal* aOriginPrincipal,
                          nsIDocument* aDocument)
   : URL(aURI, aString, aReferrer, aOriginPrincipal)
 {
   if (aDocument->GetOriginalDocument()) {
     aDocument = aDocument->GetOriginalDocument();
   }
-  if (mURI &&
-      nsContentUtils::CanLoadImage(mURI, aDocument, aDocument,
+  if (aURI &&
+      nsContentUtils::CanLoadImage(aURI, aDocument, aDocument,
                                    aOriginPrincipal)) {
-    nsContentUtils::LoadImage(mURI, aDocument, aOriginPrincipal, aReferrer,
+    nsContentUtils::LoadImage(aURI, aDocument, aOriginPrincipal, aReferrer,
                               nsnull, nsIRequest::LOAD_NORMAL,
                               getter_AddRefs(mRequest));
   }
 }
 
 nsCSSValue::Image::~Image()
 {
 }
--- a/layout/style/nsCSSValue.h
+++ b/layout/style/nsCSSValue.h
@@ -343,17 +343,17 @@ public:
     return mValue.mArray;
   }
 
   nsIURI* GetURLValue() const
   {
     NS_ABORT_IF_FALSE(mUnit == eCSSUnit_URL || mUnit == eCSSUnit_Image,
                  "not a URL value");
     return mUnit == eCSSUnit_URL ?
-      mValue.mURL->mURI : mValue.mImage->mURI;
+      mValue.mURL->GetURI() : mValue.mImage->GetURI();
   }
 
   nsCSSValueGradient* GetGradientValue() const
   {
     NS_ABORT_IF_FALSE(mUnit == eCSSUnit_Gradient, "not a gradient value");
     return mValue.mGradient;
   }
 
@@ -445,40 +445,53 @@ public:
   static already_AddRefed<nsStringBuffer>
     BufferFromString(const nsString& aValue);
 
   struct URL {
     // Methods are not inline because using an nsIPrincipal means requiring
     // caps, which leads to REQUIRES hell, since this header is included all
     // over.
 
-    // aString must not be null.
-    // aOriginPrincipal must not be null.
+    // For both constructors aString must not be null.
+    // For both constructors aOriginPrincipal must not be null.
+    // Construct with a base URI; this will create the actual URI lazily from
+    // aString and aBaseURI.
+    URL(nsStringBuffer* aString, nsIURI* aBaseURI, nsIURI* aReferrer,
+        nsIPrincipal* aOriginPrincipal);
+    // Construct with the actual URI.
     URL(nsIURI* aURI, nsStringBuffer* aString, nsIURI* aReferrer,
         nsIPrincipal* aOriginPrincipal);
 
     ~URL();
 
     PRBool operator==(const URL& 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
     // unless you're sure this is the case.
     PRBool URIEquals(const URL& aOther) const;
 
-    nsCOMPtr<nsIURI> mURI; // null == invalid URL
+    nsIURI* GetURI() const;
+
+  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.
+    mutable nsCOMPtr<nsIURI> mURI;
+  public:
     nsStringBuffer* mString; // Could use nsRefPtr, but it'd add useless
                              // null-checks; this is never null.
     nsCOMPtr<nsIURI> mReferrer;
     nsCOMPtr<nsIPrincipal> mOriginPrincipal;
 
     NS_INLINE_DECL_REFCOUNTING(nsCSSValue::URL)
 
-  protected:
+  private:
+    mutable PRBool mURIResolved;
 
     // not to be implemented
     URL(const URL& aOther);
     URL& operator=(const URL& aOther);
   };
 
   struct Image : public URL {
     // Not making the constructor and destructor inline because that would
--- a/layout/style/nsComputedDOMStyle.cpp
+++ b/layout/style/nsComputedDOMStyle.cpp
@@ -608,17 +608,17 @@ nsComputedDOMStyle::Item(PRUint32 aIndex
 nsIDOMCSSValue*
 nsComputedDOMStyle::DoGetBinding()
 {
   nsROCSSPrimitiveValue *val = GetROCSSPrimitiveValue();
 
   const nsStyleDisplay* display = GetStyleDisplay();
 
   if (display->mBinding) {
-    val->SetURI(display->mBinding->mURI);
+    val->SetURI(display->mBinding->GetURI());
   } else {
     val->SetIdent(eCSSKeyword_none);
   }
 
   return val;
 }
 
 nsIDOMCSSValue*
--- a/layout/style/nsRuleNode.cpp
+++ b/layout/style/nsRuleNode.cpp
@@ -4196,17 +4196,17 @@ nsRuleNode::ComputeDisplayData(void* aSt
               NS_THEME_NONE, 0, 0, 0, 0);
 
   // binding: url, none, inherit
   const nsCSSValue* bindingValue = aRuleData->ValueForBinding();
   if (eCSSUnit_URL == bindingValue->GetUnit()) {
     nsCSSValue::URL* url = bindingValue->GetURLStructValue();
     NS_ASSERTION(url, "What's going on here?");
 
-    if (NS_LIKELY(url->mURI)) {
+    if (NS_LIKELY(url->GetURI())) {
       display->mBinding = url;
     } else {
       display->mBinding = nsnull;
     }
   }
   else if (eCSSUnit_None == bindingValue->GetUnit() ||
            eCSSUnit_Initial == bindingValue->GetUnit()) {
     display->mBinding = nsnull;
--- a/layout/style/nsStyleStruct.h
+++ b/layout/style/nsStyleStruct.h
@@ -1492,17 +1492,17 @@ struct nsStyleDisplay {
   }
 
   nsChangeHint CalcDifference(const nsStyleDisplay& aOther) const;
 #ifdef DEBUG
   static nsChangeHint MaxDifference();
 #endif
   static PRBool ForceCompare() { return PR_TRUE; }
 
-  // We guarantee that if mBinding is non-null, so are mBinding->mURI and
+  // We guarantee that if mBinding is non-null, so are mBinding->GetURI() and
   // mBinding->mOriginPrincipal.
   nsRefPtr<nsCSSValue::URL> mBinding;    // [reset]
   nsRect    mClip;              // [reset] offsets from upper-left border edge
   float   mOpacity;             // [reset]
   PRUint8 mDisplay;             // [reset] see nsStyleConsts.h NS_STYLE_DISPLAY_*
   PRUint8 mOriginalDisplay;     // [reset] saved mDisplay for position:absolute/fixed
   PRUint8 mAppearance;          // [reset]
   PRUint8 mPosition;            // [reset] see nsStyleConsts.h