Bug 1426501 - Change nsIURIMutator to call set spec on cloned URI if available draft
authorValentin Gosu <valentin.gosu@gmail.com>
Mon, 08 Jan 2018 11:23:59 +0100
changeset 717193 369c881aee3fa22121fd6a7333357731a54fd56d
parent 717192 413d67e6d0ef872b22279a46bfae62b15c39bb91
child 717194 cd4fc2912cc2b1b3aa809955db3115556b0725f3
child 717301 3d8e37a3b1b34e8226a8096a953c6f93da8d00c0
push id94593
push uservalentin.gosu@gmail.com
push dateMon, 08 Jan 2018 11:44:46 +0000
bugs1426501
milestone59.0a1
Bug 1426501 - Change nsIURIMutator to call set spec on cloned URI if available Calling SetSpec on an nsIURI object doesn't reinitialize the object, meaning it's not equivalent with creating a new URI with the same spec. While this might be counter-intuitive we want to preserve existing behaviour for the moment. This change makes BaseURIMutator::InitFromSpec call SetSpec on the existing cloned URI if available. Otherwise it creates a new one. MozReview-Commit-ID: LuHVRhBItiP
netwerk/base/nsIURIMutator.idl
netwerk/test/unit/test_URIs.js
netwerk/test/unit/test_URIs2.js
--- a/netwerk/base/nsIURIMutator.idl
+++ b/netwerk/base/nsIURIMutator.idl
@@ -61,18 +61,26 @@ protected:
       return NS_ERROR_FAILURE;
     }
     mURI = uri;
     return NS_OK;
   }
 
   MOZ_MUST_USE nsresult InitFromSpec(const nsACString& aSpec)
   {
-    RefPtr<T> uri = new T();
-    nsresult rv = uri->SetSpec(aSpec);
+    nsresult rv = NS_OK;
+    RefPtr<T> uri;
+    if (mURI) {
+      // This only works because all other Init methods create a new object
+      mURI.swap(uri);
+    } else {
+      uri = new T();
+    }
+
+    rv = uri->SetSpec(aSpec);
     if (NS_FAILED(rv)) {
       return rv;
     }
     mURI = uri;
     return NS_OK;
   }
 
   RefPtr<T> mURI;
--- a/netwerk/test/unit/test_URIs.js
+++ b/netwerk/test/unit/test_URIs.js
@@ -480,18 +480,21 @@ function do_test_mutate_ref(aTest, aSuff
 
   if (!aTest.relativeURI) {
     // TODO: These tests don't work as-is for relative URIs.
 
     // Now try setting .spec directly (including suffix) and then clearing .ref
     var specWithSuffix = aTest.spec + aSuffix;
     do_info("testing that setting spec to " +
             specWithSuffix + " and then clearing ref does what we expect");
-    testURI.spec = specWithSuffix;
-    testURI.ref = "";
+
+    testURI = testURI.mutate()
+                     .setSpec(specWithSuffix)
+                     .setRef("")
+                     .finalize();
     do_check_uri_eq(testURI, refURIWithoutSuffix);
     do_check_uri_eqExceptRef(testURI, refURIWithSuffix);
 
     // XXX nsIJARURI throws an exception in SetPath(), so skip it for next part.
     if (!(testURI instanceof Ci.nsIJARURI)) {
       // Now try setting .pathQueryRef directly (including suffix) and then clearing .ref
       // (same as above, but with now with .pathQueryRef instead of .spec)
       testURI = NetUtil.newURI(aTest.spec);
--- a/netwerk/test/unit/test_URIs2.js
+++ b/netwerk/test/unit/test_URIs2.js
@@ -582,18 +582,20 @@ function do_test_mutate_ref(aTest, aSuff
 
   if (!aTest.relativeURI) {
     // TODO: These tests don't work as-is for relative URIs.
 
     // Now try setting .spec directly (including suffix) and then clearing .ref
     var specWithSuffix = aTest.spec + aSuffix;
     do_info("testing that setting spec to " +
             specWithSuffix + " and then clearing ref does what we expect");
-    testURI.spec = specWithSuffix;
-    testURI.ref = "";
+    testURI = testURI.mutate()
+                     .setSpec(specWithSuffix)
+                     .setRef("")
+                     .finalize();
     do_check_uri_eq(testURI, refURIWithoutSuffix);
     do_check_uri_eqExceptRef(testURI, refURIWithSuffix);
 
     // XXX nsIJARURI throws an exception in SetPath(), so skip it for next part.
     if (!(testURI instanceof Ci.nsIJARURI)) {
       // Now try setting .pathQueryRef directly (including suffix) and then clearing .ref
       // (same as above, but with now with .pathQueryRef instead of .spec)
       testURI = NetUtil.newURI(aTest.spec);