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
This commit is contained in:
14
url/gurl.cc
14
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;
|
||||
}
|
||||
|
||||
|
@ -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
|
||||
|
@ -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;
|
||||
}
|
||||
|
Reference in New Issue
Block a user