Use copy-swap idiom for GURL::operator=

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
Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=230829

[cjhopman, 2013-10-22 17:11:36]

darin: *

jyasskin: didn't see your comment on the other change until after uploading
this, so I hope this isn't duplicating work

[cjhopman, 2013-10-22 17:14:01]

https://codereview.chromium.org/30693010/diff/1/url/gurl.cc
File url/gurl.cc (right):

https://codereview.chromium.org/30693010/diff/1/url/gurl.cc#newcode172
url/gurl.cc:172: GURL copy(other);
This is less subtle than taking the thing by value. I don't really have a
preference either way.

[akalin, 2013-10-22 17:56:40]

lgtm

https://codereview.chromium.org/30693010/diff/1/url/gurl.cc
File url/gurl.cc (right):

https://codereview.chromium.org/30693010/diff/1/url/gurl.cc#newcode172
url/gurl.cc:172: GURL copy(other);
On 2013/10/22 17:14:01, cjhopman wrote:
> This is less subtle than taking the thing by value. I don't really have a
> preference either way.

http://gotw.ca/gotw/059.htm uses your way so I'm inclined to go with it. :)

https://codereview.chromium.org/30693010/diff/1/url/gurl_unittest.cc
File url/gurl_unittest.cc (right):

https://codereview.chromium.org/30693010/diff/1/url/gurl_unittest.cc#newcode527
url/gurl_unittest.cc:527: TEST(GURLTest, SelfAssignment) {
Perhaps move this to be right next to the Assign test?

Perhaps rename to SelfAssign (or a more specific name) to be consistent with
Assign test?

[Jeffrey Yasskin, 2013-10-22 18:02:54]

https://codereview.chromium.org/30693010/diff/1/url/gurl.cc
File url/gurl.cc (right):

https://codereview.chromium.org/30693010/diff/1/url/gurl.cc#newcode172
url/gurl.cc:172: GURL copy(other);
On 2013/10/22 17:14:01, cjhopman wrote:
> This is less subtle than taking the thing by value. I don't really have a
> preference either way.

Taking GURL by value saves a copy when the argument is a temporary. The details
are at
http://webcache.googleusercontent.com/search?q=cache:GLdM1_YvqhEJ:cpp-next.co...

[cjhopman, 2013-10-22 19:03:44]

https://codereview.chromium.org/30693010/diff/1/url/gurl.cc
File url/gurl.cc (right):

https://codereview.chromium.org/30693010/diff/1/url/gurl.cc#newcode172
url/gurl.cc:172: GURL copy(other);
On 2013/10/22 18:02:54, Jeffrey Yasskin wrote:
> On 2013/10/22 17:14:01, cjhopman wrote:
> > This is less subtle than taking the thing by value. I don't really have a
> > preference either way.
> 
> Taking GURL by value saves a copy when the argument is a temporary. The
details
> are at
>
http://webcache.googleusercontent.com/search?q=cache:GLdM1_YvqhEJ:cpp-next.co...

Done.

https://codereview.chromium.org/30693010/diff/1/url/gurl_unittest.cc
File url/gurl_unittest.cc (right):

https://codereview.chromium.org/30693010/diff/1/url/gurl_unittest.cc#newcode527
url/gurl_unittest.cc:527: TEST(GURLTest, SelfAssignment) {
On 2013/10/22 17:56:41, akalin wrote:
> Perhaps move this to be right next to the Assign test?
> 
> Perhaps rename to SelfAssign (or a more specific name) to be consistent with
> Assign test?

Done.

[akalin, 2013-10-22 21:07:08]

lgtm

[akalin, 2013-10-23 20:01:09]

On 2013/10/22 21:07:08, akalin wrote:
> lgtm

darin, jeffrey, ping?

[Jeffrey Yasskin, 2013-10-24 00:27:13]

Oops, I was taking akalin's lgtm as enough. lgtm from me too.

[darin (slow to review), 2013-10-24 02:15:40]

lgtm

[akalin, 2013-10-24 06:58:21]

On 2013/10/24 02:15:40, darin wrote:
> lgtm

Probably needs to be rebased before checking in

[commit-bot: I haz the power, 2013-10-24 16:50:54]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/cjhopman@chromium.org/30693010/250001

[commit-bot: I haz the power, 2013-10-24 21:56:36]

Change committed as 230829