Use copy-swap idiom for GURL::operator=

All the work to correctly handle ownership and internal state of GURL
is already done by the copy-constructor, destructor, and GURL::Swap.
Repeating that work for GURL::operator= is just another place where we
might get it wrong.

BUG=309975

Review URL: https://codereview.chromium.org/30693010

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@230829 0039d316-1c4b-4281-b951-d872f2087c98

3 files changed, 45 insertions, 20 deletions

diff --git a/url/gurl.cc b/url/gurl.cc
index 60850e6..15de85a 100644
--- a/url/gurl.cc
+++ b/url/gurl.cc
@@ -167,18 +167,8 @@ void GURL::InitializeFromCanonicalSpec() {
 GURL::~GURL() {
 }
 
-GURL& GURL::operator=(const GURL& other) {
-  if (&other == this)
-    return *this;
-
-  spec_ = other.spec_;
-  is_valid_ = other.is_valid_;
-  parsed_ = other.parsed_;
-  inner_url_.reset(NULL);
-  if (other.inner_url_)
-    inner_url_.reset(new GURL(*other.inner_url_));
-  // Valid filesystem urls should always have an inner_url_.
-  DCHECK(!is_valid_ || !SchemeIsFileSystem() || inner_url_);
+GURL& GURL::operator=(GURL other) {
+  Swap(&other);
   return *this;
 }
 
diff --git a/url/gurl.h b/url/gurl.h
index f750c3a..d4ea10f 100644
--- a/url/gurl.h
+++ b/url/gurl.h
@@ -53,7 +53,7 @@ class URL_EXPORT GURL {
 
   ~GURL();
 
-  GURL& operator=(const GURL& other);
+  GURL& operator=(GURL other);
 
   // Returns true when this object represents a valid parsed URL. When not
   // valid, other functions will still succeed, but you will not get canonical
diff --git a/url/gurl_unittest.cc b/url/gurl_unittest.cc
index ddb55d89..67da8e4 100644
--- a/url/gurl_unittest.cc
+++ b/url/gurl_unittest.cc
@@ -136,6 +136,48 @@ TEST(GURLTest, Copy) {
   EXPECT_EQ("", invalid2.ref());
 }
 
+TEST(GURLTest, Assign) {
+  GURL url(WStringToUTF16(L"http://user:pass@google.com:99/foo;bar?q=a#ref"));
+
+  GURL url2;
+  url2 = url;
+  EXPECT_TRUE(url2.is_valid());
+
+  EXPECT_EQ("http://user:pass@google.com:99/foo;bar?q=a#ref", url2.spec());
+  EXPECT_EQ("http", url2.scheme());
+  EXPECT_EQ("user", url2.username());
+  EXPECT_EQ("pass", url2.password());
+  EXPECT_EQ("google.com", url2.host());
+  EXPECT_EQ("99", url2.port());
+  EXPECT_EQ(99, url2.IntPort());
+  EXPECT_EQ("/foo;bar", url2.path());
+  EXPECT_EQ("q=a", url2.query());
+  EXPECT_EQ("ref", url2.ref());
+
+  // Assignment of invalid URL should be invalid
+  GURL invalid;
+  GURL invalid2;
+  invalid2 = invalid;
+  EXPECT_FALSE(invalid2.is_valid());
+  EXPECT_EQ("", invalid2.spec());
+  EXPECT_EQ("", invalid2.scheme());
+  EXPECT_EQ("", invalid2.username());
+  EXPECT_EQ("", invalid2.password());
+  EXPECT_EQ("", invalid2.host());
+  EXPECT_EQ("", invalid2.port());
+  EXPECT_EQ(url_parse::PORT_UNSPECIFIED, invalid2.IntPort());
+  EXPECT_EQ("", invalid2.path());
+  EXPECT_EQ("", invalid2.query());
+  EXPECT_EQ("", invalid2.ref());
+}
+
+// This is a regression test for http://crbug.com/309975 .
+TEST(GURLTest, SelfAssign) {
+  GURL a("filesystem:http://example.com/temporary/");
+  // This should not crash.
+  a = a;
+}
+
 TEST(GURLTest, CopyFileSystem) {
   GURL url(WStringToUTF16(L"filesystem:https://user:pass@google.com:99/t/foo;bar?q=a#ref"));
 
@@ -487,10 +529,3 @@ TEST(GURLTest, IsStandard) {
   GURL c("foo://bar/baz");
   EXPECT_FALSE(c.IsStandard());
 }
-
-// This is a regression test for http://crbug.com/309975 .
-TEST(GURLTest, SelfAssignment) {
-  GURL a("filesystem:http://example.com/temporary/");
-  // This should not crash.
-  a = a;
-}