Cleanup: Pass std::string as const reference if possible

url/gurl.h

Index: url/gurl.h
diff --git a/url/gurl.h b/url/gurl.h
index cdff0fd146bb814105602ea4f4dd16439c842129..dae87ff630ab243fe8f37d8246074836a850b96c 100644
--- a/url/gurl.h
+++ b/url/gurl.h
@@ -46,13 +46,17 @@ class URL_EXPORT GURL {
        size_t canonical_spec_len,
        const url::Parsed& parsed,
        bool is_valid);
-  // Notice that we take the canonical_spec by value so that we can convert
-  // from WebURL without copying the string. When we call this constructor
-  // we pass in a temporary std::string, which lets the compiler skip the
-  // copy and just move the std::string into the function argument. In the
-  // implementation, we use swap to move the data into the GURL itself,
-  // which means we end up with zero copies.
-  GURL(std::string canonical_spec, const url::Parsed& parsed, bool is_valid);
+
+  // Notice that we can take the canonical_spec by rvalue reference so that we
+  // can convert from WebURL without copying the string. When we call this

[Peter Kasting, 2015/09/20 21:58:20]

This sort of change is likely wrong.  The old code is very explicit about why
it's passing by value, and changing that in the new code probably does the
opposite of what you want, by forcing the compiler to generate an object whose
address can be taken.

Unless whoever wrote the old comment was flat-out wrong, which I very much
doubt, places like this should not be changed.

This was only the second file in this CL I looked at; it worries me that it took
2 files to find a potential red flag.  Reviewers are likely to rubber-stamp this
sort of change, so you need to be very careful it's really right.

[ki.stfu, 2015/09/21 05:26:06]

I'll copy and answer on it in https://codereview.chromium.org/1360533002.

+  // constructor we pass in a temporary std::string, which lets the compiler
+  // skip the copy and just move the std::string into the function argument.
+  // The const reference version is used in unittests.
+  GURL(const std::string& canonical_spec,
+       const url::Parsed& parsed,
+       bool is_valid);
+  GURL(std::string&& canonical_spec, const url::Parsed& parsed,
+       bool is_valid) noexcept;
 
   ~GURL();