Bug 1433958 - Change code that sets nsIURI.ref to use nsIURIMutator r=mayhemer

MozReview-Commit-ID: 4caicZFBkcQ

--HG--
extra : rebase_source : fe32f156392a9e0ce69fa6030278eaca43a69482

browser/extensions/activity-stream/lib/FaviconFeed.jsm

@@ -124,7 +124,9 @@ var FaviconFeed = class FaviconFeed {
     if (domain in sitesByDomain) {
       let iconUri = Services.io.newURI(sitesByDomain[domain].image_url);
       // The #tippytop is to be able to identify them for telemetry.
-      iconUri.ref = "tippytop";
+      iconUri = iconUri.mutate()
+                       .setRef("tippytop")
+                       .finalize();
       PlacesUtils.favicons.setAndFetchFaviconForPage(
         Services.io.newURI(url),
         iconUri,

dom/base/Link.cpp

@@ -578,13 +578,19 @@ Link::SetPort(const nsAString &aPort)
 void
 Link::SetHash(const nsAString &aHash)
 {
-  nsCOMPtr<nsIURI> uri(GetURIToMutate());
+  nsCOMPtr<nsIURI> uri(GetURI());
   if (!uri) {
     // Ignore failures to be compatible with NS4.
     return;
   }
 
-  (void)uri->SetRef(NS_ConvertUTF16toUTF8(aHash));
+  nsresult rv = NS_MutateURI(uri)
+                  .SetRef(NS_ConvertUTF16toUTF8(aHash))
+                  .Finalize(uri);
+  if (NS_FAILED(rv)) {
+    return;
+  }
+
   SetHrefAttribute(uri);
 }
 

dom/url/URLMainThread.cpp

@@ -446,7 +446,9 @@ URLMainThread::GetHash(nsAString& aHash, ErrorResult& aRv) const
 void
 URLMainThread::SetHash(const nsAString& aHash, ErrorResult& aRv)
 {
-  mURI->SetRef(NS_ConvertUTF16toUTF8(aHash));
+  Unused << NS_MutateURI(mURI)
+              .SetRef(NS_ConvertUTF16toUTF8(aHash))
+              .Finalize(mURI);
 }
 
 void

dom/xbl/nsXBLPrototypeBinding.cpp

@@ -11,6 +11,7 @@
 #include "nsIInputStream.h"
 #include "nsNameSpaceManager.h"
 #include "nsIURI.h"
+#include "nsIURIMutator.h"
 #include "nsIURL.h"
 #include "nsIChannel.h"
 #include "nsString.h"
@@ -132,16 +133,22 @@ nsXBLPrototypeBinding::Init(const nsACString& aID,
                             Element* aElement,
                             bool aFirstBinding)
 {
-  nsresult rv = aInfo->DocumentURI()->Clone(getter_AddRefs(mBindingURI));
-  NS_ENSURE_SUCCESS(rv, rv);
+  nsresult rv;
+  nsCOMPtr<nsIURI> bindingURI = aInfo->DocumentURI();
 
   // The binding URI might be an immutable URI (e.g. for about: URIs). In that case,
   // we'll fail in SetRef below, but that doesn't matter much for now.
   if (aFirstBinding) {
-    rv = mBindingURI->Clone(getter_AddRefs(mAlternateBindingURI));
+    rv = bindingURI->Clone(getter_AddRefs(mAlternateBindingURI));
     NS_ENSURE_SUCCESS(rv, rv);
   }
-  mBindingURI->SetRef(aID);
+  rv = NS_MutateURI(bindingURI)
+        .SetRef(aID)
+        .Finalize(mBindingURI);
+  if (NS_FAILED(rv)) {
+    // If SetRef failed, mBindingURI should be a clone.
+    bindingURI->Clone(getter_AddRefs(mBindingURI));
+  }
 
   mXBLDocInfoWeak = aInfo;
 

layout/style/nsCSSValue.cpp

@@ -21,6 +21,7 @@
 #include "imgIRequest.h"
 #include "imgRequestProxy.h"
 #include "nsIDocument.h"
+#include "nsIURIMutator.h"
 #include "nsCSSProps.h"
 #include "nsNetUtil.h"
 #include "nsPresContext.h"
@@ -3013,8 +3014,13 @@ css::URLValueData::ResolveLocalRef(nsIURI* aURI) const
     nsCString ref;
     mURI->GetRef(ref);
 
-    aURI->Clone(getter_AddRefs(result));
-    result->SetRef(ref);
+    nsresult rv = NS_MutateURI(aURI)
+                    .SetRef(ref)
+                    .Finalize(result);
+    if (NS_FAILED(rv)) {
+      // If setting the ref failed, just return a clone.
+      aURI->Clone(getter_AddRefs(result));
+    }
   }
 
   return result.forget();

modules/libjar/nsJARURI.cpp

@@ -699,7 +699,9 @@ nsJARURI::GetRef(nsACString& ref)
 NS_IMETHODIMP
 nsJARURI::SetRef(const nsACString& ref)
 {
-    return mJAREntry->SetRef(ref);
+    return NS_MutateURI(mJAREntry)
+             .SetRef(ref)
+             .Finalize(mJAREntry);
 }
 
 NS_IMETHODIMP

netwerk/protocol/data/nsDataHandler.cpp

@@ -71,10 +71,9 @@ nsDataHandler::NewURI(const nsACString &aSpec,
     if (aBaseURI && !spec.IsEmpty() && spec[0] == '#') {
         // Looks like a reference instead of a fully-specified URI.
         // --> initialize |uri| as a clone of |aBaseURI|, with ref appended.
-        rv = aBaseURI->Clone(getter_AddRefs(uri));
-        if (NS_FAILED(rv))
-            return rv;
-        rv = uri->SetRef(spec);
+        rv = NS_MutateURI(aBaseURI)
+               .SetRef(spec)
+               .Finalize(uri);
     } else {
         // Otherwise, we'll assume |spec| is a fully-specified data URI
         nsAutoCString contentType;

netwerk/protocol/http/HttpBaseChannel.cpp

@@ -3419,7 +3419,7 @@ HttpBaseChannel::IsReferrerSchemeAllowed(nsIURI *aReferrer)
 
 /* static */
 void
-HttpBaseChannel::PropagateReferenceIfNeeded(nsIURI* aURI, nsIURI* aRedirectURI)
+HttpBaseChannel::PropagateReferenceIfNeeded(nsIURI* aURI, nsCOMPtr<nsIURI>& aRedirectURI)
 {
   bool hasRef = false;
   nsresult rv = aRedirectURI->GetHasRef(&hasRef);
@@ -3429,7 +3429,9 @@ HttpBaseChannel::PropagateReferenceIfNeeded(nsIURI* aURI, nsIURI* aRedirectURI)
     if (!ref.IsEmpty()) {
       // NOTE: SetRef will fail if mRedirectURI is immutable
       // (e.g. an about: URI)... Oh well.
-      aRedirectURI->SetRef(ref);
+      Unused << NS_MutateURI(aRedirectURI)
+                  .SetRef(ref)
+                  .Finalize(aRedirectURI);
     }
   }
 }

netwerk/protocol/http/HttpBaseChannel.h

@@ -358,7 +358,7 @@ public: /* Necko internal use only... */
 
     static bool IsReferrerSchemeAllowed(nsIURI *aReferrer);
 
-    static void PropagateReferenceIfNeeded(nsIURI *aURI, nsIURI *aRedirectURI);
+    static void PropagateReferenceIfNeeded(nsIURI *aURI, nsCOMPtr<nsIURI>& aRedirectURI);
 
     // Return whether upon a redirect code of httpStatus for method, the
     // request method should be rewritten to GET.

netwerk/test/unit/test_URIs.js

@@ -457,7 +457,7 @@ function do_test_mutate_ref(aTest, aSuffix) {
   // First: Try setting .ref to our suffix
   do_info("testing that setting .ref on " + aTest.spec +
           " to '" + aSuffix + "' does what we expect");
-  testURI.ref = aSuffix;
+  testURI = testURI.mutate().setRef(aSuffix).finalize();
   do_check_uri_eq(testURI, refURIWithSuffix);
   do_check_uri_eqExceptRef(testURI, refURIWithoutSuffix);
 
@@ -466,7 +466,7 @@ function do_test_mutate_ref(aTest, aSuffix) {
   if (suffixLackingHash) { // (skip this our suffix was *just* a #)
     do_info("testing that setting .ref on " + aTest.spec +
             " to '" + suffixLackingHash + "' does what we expect");
-    testURI.ref = suffixLackingHash;
+    testURI = testURI.mutate().setRef(suffixLackingHash).finalize();
     do_check_uri_eq(testURI, refURIWithSuffix);
     do_check_uri_eqExceptRef(testURI, refURIWithoutSuffix);
   }
@@ -474,7 +474,7 @@ function do_test_mutate_ref(aTest, aSuffix) {
   // Now, clear .ref (should get us back the original spec)
   do_info("testing that clearing .ref on " + testURI.spec +
           " does what we expect");
-  testURI.ref = "";
+  testURI = testURI.mutate().setRef("").finalize();
   do_check_uri_eq(testURI, refURIWithoutSuffix);
   do_check_uri_eqExceptRef(testURI, refURIWithSuffix);
 
@@ -502,8 +502,10 @@ function do_test_mutate_ref(aTest, aSuffix) {
       var pathWithSuffix = aTest.pathQueryRef + aSuffix;
       do_info("testing that setting path to " +
               pathWithSuffix + " and then clearing ref does what we expect");
-      testURI = testURI.mutate().setPathQueryRef(pathWithSuffix).finalize();
-      testURI.ref = "";
+      testURI = testURI.mutate()
+                       .setPathQueryRef(pathWithSuffix)
+                       .setRef("")
+                       .finalize();
       do_check_uri_eq(testURI, refURIWithoutSuffix);
       do_check_uri_eqExceptRef(testURI, refURIWithSuffix);
 
@@ -525,7 +527,7 @@ function do_test_immutable(aTest) {
   var URI = NetUtil.newURI(aTest.spec);
   // All the non-readonly attributes on nsIURI.idl:
   var propertiesToCheck = ["spec", "scheme",
-                           "host", "port", "query", "ref"];
+                           "host", "port", "query"];
 
   propertiesToCheck.forEach(function(aProperty) {
     var threw = false;

netwerk/test/unit/test_URIs2.js

@@ -559,7 +559,7 @@ function do_test_mutate_ref(aTest, aSuffix) {
   // First: Try setting .ref to our suffix
   do_info("testing that setting .ref on " + aTest.spec +
           " to '" + aSuffix + "' does what we expect");
-  testURI.ref = aSuffix;
+  testURI = testURI.mutate().setRef(aSuffix).finalize();
   do_check_uri_eq(testURI, refURIWithSuffix);
   do_check_uri_eqExceptRef(testURI, refURIWithoutSuffix);
 
@@ -568,7 +568,7 @@ function do_test_mutate_ref(aTest, aSuffix) {
   if (suffixLackingHash) { // (skip this our suffix was *just* a #)
     do_info("testing that setting .ref on " + aTest.spec +
             " to '" + suffixLackingHash + "' does what we expect");
-    testURI.ref = suffixLackingHash;
+    testURI = testURI.mutate().setRef(suffixLackingHash).finalize();
     do_check_uri_eq(testURI, refURIWithSuffix);
     do_check_uri_eqExceptRef(testURI, refURIWithoutSuffix);
   }
@@ -576,7 +576,7 @@ function do_test_mutate_ref(aTest, aSuffix) {
   // Now, clear .ref (should get us back the original spec)
   do_info("testing that clearing .ref on " + testURI.spec +
           " does what we expect");
-  testURI.ref = "";
+  testURI = testURI.mutate().setRef("").finalize();
   do_check_uri_eq(testURI, refURIWithoutSuffix);
   do_check_uri_eqExceptRef(testURI, refURIWithSuffix);
 
@@ -603,8 +603,10 @@ function do_test_mutate_ref(aTest, aSuffix) {
       var pathWithSuffix = aTest.pathQueryRef + aSuffix;
       do_info("testing that setting path to " +
               pathWithSuffix + " and then clearing ref does what we expect");
-      testURI = testURI.mutate().setPathQueryRef(pathWithSuffix).finalize();
-      testURI.ref = "";
+      testURI = testURI.mutate()
+                       .setPathQueryRef(pathWithSuffix)
+                       .setRef("")
+                       .finalize();
       do_check_uri_eq(testURI, refURIWithoutSuffix);
       do_check_uri_eqExceptRef(testURI, refURIWithSuffix);
 
@@ -626,7 +628,7 @@ function do_test_immutable(aTest) {
   var URI = NetUtil.newURI(aTest.spec);
   // All the non-readonly attributes on nsIURI.idl:
   var propertiesToCheck = ["scheme",
-                           "host", "port", "query", "ref"];
+                           "host", "port", "query"];
 
   propertiesToCheck.forEach(function(aProperty) {
     var threw = false;

netwerk/test/unit/test_standardurl.js

@@ -141,7 +141,7 @@ add_test(function test_setRef()
   {
     /* Test1: starting with empty ref */
     var a = stringToURL(before);
-    a.ref = ref;
+    a = a.mutate().setRef(ref).finalize().QueryInterface(Ci.nsIURL);
     var b = stringToURL(result);
 
     Assert.equal(a.spec, b.spec);
@@ -149,17 +149,17 @@ add_test(function test_setRef()
     symmetricEquality(true, a, b);
 
     /* Test2: starting with non-empty */
-    a.ref = "yyyy";
+    a = a.mutate().setRef("yyyy").finalize().QueryInterface(Ci.nsIURL);
     var c = stringToURL(before);
-    c.ref = "yyyy";
+    c = c.mutate().setRef("yyyy").finalize().QueryInterface(Ci.nsIURL);
     symmetricEquality(true, a, c);
 
     /* Test3: reset the ref */
-    a.ref = "";
+    a = a.mutate().setRef("").finalize().QueryInterface(Ci.nsIURL);
     symmetricEquality(true, a, stringToURL(before));
 
     /* Test4: verify again after reset */
-    a.ref = ref;
+    a = a.mutate().setRef(ref).finalize().QueryInterface(Ci.nsIURL);
     symmetricEquality(true, a, b);
   }
   run_next_test();
@@ -255,7 +255,7 @@ add_test(function test_escapeQuote()
   var url = stringToURL("http://example.com/#'");
   Assert.equal(url.spec, "http://example.com/#'");
   Assert.equal(url.ref, "'");
-  url.ref = "test'test";
+  url = url.mutate().setRef("test'test").finalize();
   Assert.equal(url.spec, "http://example.com/#test'test");
   Assert.equal(url.ref, "test'test");
   run_next_test();
@@ -302,7 +302,7 @@ add_test(function test_hugeStringThrows()
 
   let hugeString = new Array(maxLen + 1).fill("a").join("");
   let properties = ["scheme",
-                    "host", "ref",
+                    "host",
                     "query"];
   for (let prop of properties) {
     Assert.throws(() => url[prop] = hugeString,
@@ -318,6 +318,7 @@ add_test(function test_hugeStringThrows()
     { method: "setHostPort", qi: Ci.nsIURIMutator },
     { method: "setUserPass", qi: Ci.nsIURIMutator },
     { method: "setPathQueryRef", qi: Ci.nsIURIMutator },
+    { method: "setRef", qi: Ci.nsIURIMutator },
     { method: "setFileName", qi: Ci.nsIURLMutator },
     { method: "setFileExtension", qi: Ci.nsIURLMutator },
     { method: "setFileBaseName", qi: Ci.nsIURLMutator },
@@ -343,7 +344,7 @@ add_test(function test_filterWhitespace()
   Assert.equal(url.spec, "http://test.com/pa%0D%0A%09th?query#hash");
   url.query = "qu\r\n\tery";
   Assert.equal(url.spec, "http://test.com/pa%0D%0A%09th?qu%0D%0A%09ery#hash");
-  url.ref = "ha\r\n\tsh";
+  url = url.mutate().setRef("ha\r\n\tsh").finalize();
   Assert.equal(url.spec, "http://test.com/pa%0D%0A%09th?qu%0D%0A%09ery#ha%0D%0A%09sh");
   url = url.mutate().QueryInterface(Ci.nsIURLMutator).setFileName("fi\r\n\tle.name").finalize();
   Assert.equal(url.spec, "http://test.com/fi%0D%0A%09le.name?qu%0D%0A%09ery#ha%0D%0A%09sh");
@@ -412,7 +413,7 @@ add_test(function test_encode_C0_and_space()
   Assert.equal(url.spec, "http://example.com/pa%00th?query#hash");
   url.query = "qu\0ery";
   Assert.equal(url.spec, "http://example.com/pa%00th?qu%00ery#hash");
-  url.ref = "ha\0sh";
+  url = url.mutate().setRef("ha\0sh").finalize();
   Assert.equal(url.spec, "http://example.com/pa%00th?qu%00ery#ha%00sh");
   url = url.mutate().QueryInterface(Ci.nsIURLMutator).setFileName("fi\0le.name").finalize();
   Assert.equal(url.spec, "http://example.com/fi%00le.name?qu%00ery#ha%00sh");
@@ -556,13 +557,13 @@ add_test(function test_idna_host() {
   equal(url.asciiHostPort, "xn--lt-uia.example.org:8080");
   equal(url.asciiSpec, "http://user:password@xn--lt-uia.example.org:8080/path?query#etc");
 
-  url.ref = ""; // SetRef calls InvalidateCache()
+  url = url.mutate().setRef("").finalize(); // SetRef calls InvalidateCache()
   equal(url.spec, "http://user:password@ält.example.org:8080/path?query");
   equal(url.displaySpec, "http://user:password@ält.example.org:8080/path?query");
   equal(url.asciiSpec, "http://user:password@xn--lt-uia.example.org:8080/path?query");
 
   url = stringToURL("http://user:password@www.ält.com:8080/path?query#etc");
-  url.ref = "";
+  url = url.mutate().setRef("").finalize();
   equal(url.spec, "http://user:password@www.ält.com:8080/path?query");
 
   // We also check that the default behaviour changes once we filp the pref
@@ -584,13 +585,13 @@ add_test(function test_idna_host() {
   equal(url.asciiHostPort, "xn--lt-uia.example.org:8080");
   equal(url.asciiSpec, "http://user:password@xn--lt-uia.example.org:8080/path?query#etc");
 
-  url.ref = ""; // SetRef calls InvalidateCache()
+  url = url.mutate().setRef("").finalize(); // SetRef calls InvalidateCache()
   equal(url.spec, "http://user:password@xn--lt-uia.example.org:8080/path?query");
   equal(url.displaySpec, "http://user:password@ält.example.org:8080/path?query");
   equal(url.asciiSpec, "http://user:password@xn--lt-uia.example.org:8080/path?query");
 
   url = stringToURL("http://user:password@www.ält.com:8080/path?query#etc");
-  url.ref = "";
+  url = url.mutate().setRef("").finalize();
   equal(url.spec, "http://user:password@www.xn--lt-uia.com:8080/path?query");
 
   run_next_test();

uriloader/prefetch/nsOfflineCacheUpdate.cpp

@@ -20,6 +20,7 @@
 #include "nsIDocument.h"
 #include "nsIObserverService.h"
 #include "nsIURL.h"
+#include "nsIURIMutator.h"
 #include "nsIWebProgress.h"
 #include "nsICryptoHash.h"
 #include "nsICacheEntry.h"
@@ -82,11 +83,13 @@ private:
 namespace {
 
 nsresult
-DropReferenceFromURL(nsIURI * aURI)
+DropReferenceFromURL(nsCOMPtr<nsIURI>& aURI)
 {
     // XXXdholbert If this SetRef fails, callers of this method probably
     // want to call aURI->CloneIgnoringRef() and use the result of that.
-    return aURI->SetRef(EmptyCString());
+    return NS_MutateURI(aURI)
+             .SetRef(EmptyCString())
+             .Finalize(aURI);
 }
 
 void