[go: nahoru, domu]

Give up trying to prevent copying HttpRequestHeaders

net::HttpRequestHeaders had a comment to try to make the type
uncopyable. In practice, copying HttpRequestHeaders objects is widely
done and frequently necessary. Remove the comment.

Remove the CopyFrom() method in favour of using the assignment operator
or copy constructor. In some places this is more efficient.

Change-Id: I381a80fb47b23347a785cda0ad6012861ded8b5c
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4904007
Reviewed-by: Kenichi Ishibashi <bashi@chromium.org>
Reviewed-by: Nathan Parker <nparker@chromium.org>
Reviewed-by: Gang Wu <gangwu@chromium.org>
Commit-Queue: Adam Rice <ricea@chromium.org>
Reviewed-by: David Roger <droger@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1205710}
diff --git a/chrome/browser/signin/chrome_signin_proxying_url_loader_factory.cc b/chrome/browser/signin/chrome_signin_proxying_url_loader_factory.cc
index da647d3..3201dfbb 100644
--- a/chrome/browser/signin/chrome_signin_proxying_url_loader_factory.cc
+++ b/chrome/browser/signin/chrome_signin_proxying_url_loader_factory.cc
@@ -352,8 +352,8 @@
 
     // We need to keep a full copy of the request headers in case there is a
     // redirect and the request headers need to be modified again.
-    headers_.CopyFrom(request.headers);
-    cors_exempt_headers_.CopyFrom(request.cors_exempt_headers);
+    headers_ = request.headers;
+    cors_exempt_headers_ = request.cors_exempt_headers;
   } else {
     network::ResourceRequest request_copy = request;
     request_copy.headers.MergeFrom(modified_headers);
diff --git a/chrome/browser/signin/chrome_signin_url_loader_throttle.cc b/chrome/browser/signin/chrome_signin_url_loader_throttle.cc
index 47f880f..16e8ea4 100644
--- a/chrome/browser/signin/chrome_signin_url_loader_throttle.cc
+++ b/chrome/browser/signin/chrome_signin_url_loader_throttle.cc
@@ -148,8 +148,8 @@
   // We need to keep a full copy of the request headers for later calls to
   // FixAccountConsistencyRequestHeader. Perhaps this could be replaced with
   // more specific per-request state.
-  request_headers_.CopyFrom(request->headers);
-  request_cors_exempt_headers_.CopyFrom(request->cors_exempt_headers);
+  request_headers_ = request->headers;
+  request_cors_exempt_headers_ = request->cors_exempt_headers;
 }
 
 void URLLoaderThrottle::WillRedirectRequest(
diff --git a/components/contextual_search/core/browser/contextual_search_delegate_impl.cc b/components/contextual_search/core/browser/contextual_search_delegate_impl.cc
index 12599b3..8542e9ea 100644
--- a/components/contextual_search/core/browser/contextual_search_delegate_impl.cc
+++ b/components/contextual_search/core/browser/contextual_search_delegate_impl.cc
@@ -181,7 +181,7 @@
 
   // Populates the discourse context and adds it to the HTTP header of the
   // search term resolution request.
-  resource_request->headers.CopyFrom(GetDiscourseContext(*context));
+  resource_request->headers = GetDiscourseContext(*context);
 
   // Disable cookies for this request.
   resource_request->credentials_mode = network::mojom::CredentialsMode::kOmit;
diff --git a/components/grpc_support/bidirectional_stream.cc b/components/grpc_support/bidirectional_stream.cc
index eb33b8c..3d030748 100644
--- a/components/grpc_support/bidirectional_stream.cc
+++ b/components/grpc_support/bidirectional_stream.cc
@@ -97,7 +97,7 @@
   request_info->method = method;
   if (!net::HttpUtil::IsValidHeaderName(request_info->method))
     return -1;
-  request_info->extra_headers.CopyFrom(headers);
+  request_info->extra_headers = headers;
   request_info->end_stream_on_headers = end_of_stream;
   write_end_of_stream_ = end_of_stream;
   PostToNetworkThread(FROM_HERE,
diff --git a/components/safe_browsing/content/renderer/renderer_url_loader_throttle.cc b/components/safe_browsing/content/renderer/renderer_url_loader_throttle.cc
index f56ac66..037e4c1 100644
--- a/components/safe_browsing/content/renderer/renderer_url_loader_throttle.cc
+++ b/components/safe_browsing/content/renderer/renderer_url_loader_throttle.cc
@@ -146,12 +146,11 @@
   is_start_request_called_ = true;
   // Use a weak pointer to self because |safe_browsing_| may not be owned by
   // this object.
-  net::HttpRequestHeaders headers;
-  headers.CopyFrom(request->headers);
   safe_browsing_->CreateCheckerAndCheck(
       render_frame_id_, url_checker_.BindNewPipeAndPassReceiver(), request->url,
-      request->method, headers, request->load_flags, request->destination,
-      request->has_user_gesture, request->originated_from_service_worker,
+      request->method, request->headers, request->load_flags,
+      request->destination, request->has_user_gesture,
+      request->originated_from_service_worker,
       base::BindOnce(&RendererURLLoaderThrottle::OnCheckUrlResult,
                      weak_factory_.GetWeakPtr()));
   safe_browsing_ = nullptr;
diff --git a/net/http/http_request_headers.h b/net/http/http_request_headers.h
index 668e525..fe6e754 100644
--- a/net/http/http_request_headers.h
+++ b/net/http/http_request_headers.h
@@ -178,9 +178,6 @@
   // Calls SetHeader() on each header from |other|, maintaining order.
   void MergeFrom(const HttpRequestHeaders& other);
 
-  // Copies from |other| to |this|.
-  void CopyFrom(const HttpRequestHeaders& other) { *this = other; }
-
   void Swap(HttpRequestHeaders* other) { headers_.swap(other->headers_); }
 
   // Serializes HttpRequestHeaders to a string representation.  Joins all the
@@ -211,12 +208,6 @@
   void SetHeaderInternal(base::StringPiece key, std::string&& value);
 
   HeaderVector headers_;
-
-  // Allow the copy construction and operator= to facilitate copying in
-  // HttpRequestHeaders.
-  // TODO(willchan): Investigate to see if we can remove the need to copy
-  // HttpRequestHeaders.
-  // DISALLOW_COPY_AND_ASSIGN(HttpRequestHeaders);
 };
 
 }  // namespace net
diff --git a/net/http/http_request_headers_unittest.cc b/net/http/http_request_headers_unittest.cc
index 18d9bb5..3334a2a 100644
--- a/net/http/http_request_headers_unittest.cc
+++ b/net/http/http_request_headers_unittest.cc
@@ -4,10 +4,6 @@
 
 #include "net/http/http_request_headers.h"
 
-#include <memory>
-
-#include "base/values.h"
-#include "net/log/net_log_capture_mode.h"
 #include "testing/gtest/include/gtest/gtest.h"
 
 namespace net {
@@ -155,7 +151,7 @@
   EXPECT_EQ("A: A\r\nB: b\r\nC: c\r\n\r\n", headers.ToString());
 }
 
-TEST(HttpRequestHeaders, CopyFrom) {
+TEST(HttpRequestHeaders, Assign) {
   HttpRequestHeaders headers;
   headers.SetHeader("A", "A");
   headers.SetHeader("B", "B");
@@ -163,10 +159,19 @@
   HttpRequestHeaders headers2;
   headers2.SetHeader("B", "b");
   headers2.SetHeader("C", "c");
-  headers.CopyFrom(headers2);
+  headers = headers2;
   EXPECT_EQ("B: b\r\nC: c\r\n\r\n", headers.ToString());
 }
 
+TEST(HttpRequestHeaders, Copy) {
+  HttpRequestHeaders headers;
+  headers.SetHeader("A", "A");
+  headers.SetHeader("B", "B");
+
+  HttpRequestHeaders headers2 = headers;
+  EXPECT_EQ(headers.ToString(), headers2.ToString());
+}
+
 }  // namespace
 
 }  // namespace net
diff --git a/net/http/partial_data.cc b/net/http/partial_data.cc
index 157a69f..caafba71 100644
--- a/net/http/partial_data.cc
+++ b/net/http/partial_data.cc
@@ -62,7 +62,7 @@
 
 void PartialData::SetHeaders(const HttpRequestHeaders& headers) {
   DCHECK(extra_headers_.IsEmpty());
-  extra_headers_.CopyFrom(headers);
+  extra_headers_ = headers;
 }
 
 void PartialData::RestoreHeaders(HttpRequestHeaders* headers) const {
@@ -71,7 +71,7 @@
                     ? byte_range_.suffix_length()
                     : byte_range_.last_byte_position();
 
-  headers->CopyFrom(extra_headers_);
+  *headers = extra_headers_;
   if (truncated_ || !byte_range_.IsValid())
     return;
 
@@ -143,7 +143,7 @@
   }
   range_present_ = false;
 
-  headers->CopyFrom(extra_headers_);
+  *headers = extra_headers_;
 
   if (!cached_min_len_) {
     // We don't have anything else stored.
diff --git a/net/url_request/url_request_http_job.cc b/net/url_request/url_request_http_job.cc
index db9ca10..1fcbb5e 100644
--- a/net/url_request/url_request_http_job.cc
+++ b/net/url_request/url_request_http_job.cc
@@ -1222,7 +1222,7 @@
     const HttpRequestHeaders& headers) {
   DCHECK(!transaction_.get() && !override_response_info_)
       << "cannot change once started";
-  request_info_.extra_headers.CopyFrom(headers);
+  request_info_.extra_headers = headers;
 }
 
 LoadState URLRequestHttpJob::GetLoadState() const {
diff --git a/net/websockets/websocket_basic_handshake_stream.cc b/net/websockets/websocket_basic_handshake_stream.cc
index 9bc4788..ad014d8f 100644
--- a/net/websockets/websocket_basic_handshake_stream.cc
+++ b/net/websockets/websocket_basic_handshake_stream.cc
@@ -250,9 +250,8 @@
   http_response_info_ = response;
 
   // Create a copy of the headers object, so that we can add the
-  // Sec-WebSockey-Key header.
-  HttpRequestHeaders enriched_headers;
-  enriched_headers.CopyFrom(headers);
+  // Sec-WebSocket-Key header.
+  HttpRequestHeaders enriched_headers = headers;
   std::string handshake_challenge;
   if (handshake_challenge_for_testing_.has_value()) {
     handshake_challenge = handshake_challenge_for_testing_.value();
@@ -275,7 +274,7 @@
   DCHECK(connect_delegate_);
   auto request =
       std::make_unique<WebSocketHandshakeRequestInfo>(url_, base::Time::Now());
-  request->headers.CopyFrom(enriched_headers);
+  request->headers = enriched_headers;
   connect_delegate_->OnStartOpeningHandshake(std::move(request));
 
   return parser()->SendRequest(
diff --git a/net/websockets/websocket_http2_handshake_stream.cc b/net/websockets/websocket_http2_handshake_stream.cc
index e7ae62b..f9e0206 100644
--- a/net/websockets/websocket_http2_handshake_stream.cc
+++ b/net/websockets/websocket_http2_handshake_stream.cc
@@ -112,7 +112,7 @@
 
   auto request = std::make_unique<WebSocketHandshakeRequestInfo>(
       request_info_->url, base::Time::Now());
-  request->headers.CopyFrom(headers);
+  request->headers = headers;
 
   AddVectorHeaderIfNonEmpty(websockets::kSecWebSocketExtensions,
                             requested_extensions_, &request->headers);
diff --git a/net/websockets/websocket_http3_handshake_stream.cc b/net/websockets/websocket_http3_handshake_stream.cc
index 45a583b..112f572 100644
--- a/net/websockets/websocket_http3_handshake_stream.cc
+++ b/net/websockets/websocket_http3_handshake_stream.cc
@@ -109,7 +109,7 @@
 
   auto request = std::make_unique<WebSocketHandshakeRequestInfo>(
       request_info_->url, base::Time::Now());
-  request->headers.CopyFrom(request_headers);
+  request->headers = request_headers;
 
   AddVectorHeaderIfNonEmpty(websockets::kSecWebSocketExtensions,
                             requested_extensions_, &request->headers);
diff --git a/services/network/url_loader.cc b/services/network/url_loader.cc
index 544842e..e5347d7 100644
--- a/services/network/url_loader.cc
+++ b/services/network/url_loader.cc
@@ -1108,8 +1108,7 @@
   deferred_redirect_url_.reset();
   new_redirect_url_ = new_url;
 
-  net::HttpRequestHeaders merged_modified_headers;
-  merged_modified_headers.CopyFrom(modified_headers);
+  net::HttpRequestHeaders merged_modified_headers = modified_headers;
   merged_modified_headers.MergeFrom(modified_cors_exempt_headers);
   url_request_->FollowDeferredRedirect(removed_headers,
                                        merged_modified_headers);