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.
2011-05-11 11:28:53 -04:00 · 2011-05-11 11:28:53 -04:00 · d034eefa80
--- a/dom/base/nsDOMClassInfo.cpp
+++ b/dom/base/nsDOMClassInfo.cpp
@ -8023,7 +8023,7 @@ nsElementSH::PostCreate(nsIXPConnectWrappedNative *wrapper, JSContext *cx,
    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.
--- a/layout/base/nsCSSFrameConstructor.cpp
+++ b/layout/base/nsCSSFrameConstructor.cpp
@ -2346,7 +2346,7 @@ nsCSSFrameConstructor::ConstructDocElementFrame(Element*                 aDocEle
      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);
@ -5093,7 +5093,7 @@ nsCSSFrameConstructor::AddFrameConstructionItemsInternal(nsFrameConstructorState
    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),
--- a/layout/style/nsCSSParser.cpp
+++ b/layout/style/nsCSSParser.cpp
@ -4874,11 +4874,6 @@ CSSParserImpl::SetValueToURL(nsCSSValue& aValue, const nsString& aURL)
    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);
@ -4887,7 +4882,7 @@ CSSParserImpl::SetValueToURL(nsCSSValue& aValue, const nsString& aURL)

  // 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);
--- a/layout/style/nsCSSValue.cpp
+++ b/layout/style/nsCSSValue.cpp
@ -45,6 +45,7 @@
 #include "nsContentUtils.h"
 #include "nsStyleUtil.h"
 #include "CSSCalc.h"
+#include "nsNetUtil.h"

 namespace css = mozilla::css;

@ -542,7 +543,7 @@ 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,
@ -1230,12 +1231,25 @@ nsCSSValuePairList::operator==(const nsCSSValuePairList& aOther) const
  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();
@ -1252,7 +1266,7 @@ 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)) &&
@ -1264,8 +1278,10 @@ nsCSSValue::URL::operator==(const URL& aOther) const
 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
@ -1276,6 +1292,21 @@ nsCSSValue::URL::URIEquals(const URL& aOther) const
                                                 &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)
@ -1284,10 +1315,10 @@ nsCSSValue::Image::Image(nsIURI* aURI, nsStringBuffer* aString,
  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));
  }
--- a/layout/style/nsCSSValue.h
+++ b/layout/style/nsCSSValue.h
@ -348,7 +348,7 @@ public:
    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
@ -450,8 +450,13 @@ public:
    // 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);

@ -465,7 +470,14 @@ public:
    // 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;
@ -473,7 +485,8 @@ public:

    NS_INLINE_DECL_REFCOUNTING(nsCSSValue::URL)

-  protected:
+  private:
+    mutable PRBool mURIResolved;

    // not to be implemented
    URL(const URL& aOther);
--- a/layout/style/nsComputedDOMStyle.cpp
+++ b/layout/style/nsComputedDOMStyle.cpp
@ -613,7 +613,7 @@ nsComputedDOMStyle::DoGetBinding()
  const nsStyleDisplay* display = GetStyleDisplay();

  if (display->mBinding) {
-    val->SetURI(display->mBinding->mURI);
+    val->SetURI(display->mBinding->GetURI());
  } else {
    val->SetIdent(eCSSKeyword_none);
  }
--- a/layout/style/nsRuleNode.cpp
+++ b/layout/style/nsRuleNode.cpp
@ -4201,7 +4201,7 @@ nsRuleNode::ComputeDisplayData(void* aStartStruct,
    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;
--- a/layout/style/nsStyleStruct.h
+++ b/layout/style/nsStyleStruct.h
@ -1497,7 +1497,7 @@ struct nsStyleDisplay {
 #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