Navigation: Move request_headers from NavigationHandleImpl.

This includes moving modified_request_headers_ and
removed_request_headers_.
And SetRequestHeaders() and RemoveRequestHeaders() functions.

Bug: 916537
Change-Id: I71262f8e3667c3fccaa557a60ca069345b603c51
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1719353
Commit-Queue: Mohamed Abdelhalim <zetamoo@chromium.org>
Reviewed-by: Arthur Sonzogni <arthursonzogni@chromium.org>
Reviewed-by: Lowell Manners <lowell@chromium.org>
Cr-Commit-Position: refs/heads/master@{#682707}

content/browser/frame_host/navigation_handle_impl.cc

@@ -32,10 +32,8 @@
 namespace content {
 
 NavigationHandleImpl::NavigationHandleImpl(
-    NavigationRequest* navigation_request,
-    net::HttpRequestHeaders request_headers)
-    : navigation_request_(navigation_request),
-      request_headers_(std::move(request_headers)) {
+    NavigationRequest* navigation_request)
+    : navigation_request_(navigation_request) {
   const GURL& url = navigation_request_->common_params().url;
   TRACE_EVENT_ASYNC_BEGIN2("navigation", "NavigationHandle", this,
                            "frame_tree_node",
@@ -170,23 +168,16 @@ bool NavigationHandleImpl::IsSameDocument() {
 }
 
 const net::HttpRequestHeaders& NavigationHandleImpl::GetRequestHeaders() {
-  return request_headers_;
+  return navigation_request_->request_headers();
 }
 
 void NavigationHandleImpl::RemoveRequestHeader(const std::string& header_name) {
-  DCHECK(state() == NavigationRequest::PROCESSING_WILL_REDIRECT_REQUEST ||
-         state() == NavigationRequest::WILL_REDIRECT_REQUEST);
-  removed_request_headers_.push_back(header_name);
+  navigation_request_->RemoveRequestHeader(header_name);
 }
 
 void NavigationHandleImpl::SetRequestHeader(const std::string& header_name,
                                             const std::string& header_value) {
-  DCHECK(state() == NavigationRequest::INITIAL ||
-         state() == NavigationRequest::PROCESSING_WILL_START_REQUEST ||
-         state() == NavigationRequest::PROCESSING_WILL_REDIRECT_REQUEST ||
-         state() == NavigationRequest::WILL_START_REQUEST ||
-         state() == NavigationRequest::WILL_REDIRECT_REQUEST);
-  modified_request_headers_.SetHeader(header_name, header_value);
+  navigation_request_->SetRequestHeader(header_name, header_value);
 }
 
 const net::HttpResponseHeaders* NavigationHandleImpl::GetResponseHeaders() {

content/browser/frame_host/navigation_handle_impl.h

@@ -185,14 +185,6 @@ class CONTENT_EXPORT NavigationHandleImpl : public NavigationHandle {
     return navigation_request_->common_params().source_location;
   }
 
-  std::vector<std::string> TakeRemovedRequestHeaders() {
-    return std::move(removed_request_headers_);
-  }
-
-  net::HttpRequestHeaders TakeModifiedRequestHeaders() {
-    return std::move(modified_request_headers_);
-  }
-
   NavigationThrottle* GetDeferringThrottleForTesting() const {
     return navigation_request_->GetDeferringThrottleForTesting();
   }
@@ -208,8 +200,7 @@ class CONTENT_EXPORT NavigationHandleImpl : public NavigationHandle {
   // start with |url|. Otherwise |redirect_chain| is used as the starting point.
   // |navigation_start| comes from the CommonNavigationParams associated with
   // this navigation.
-  NavigationHandleImpl(NavigationRequest* navigation_request,
-                       net::HttpRequestHeaders request_headers);
+  NavigationHandleImpl(NavigationRequest* navigation_request);
 
   NavigationRequest::NavigationHandleState state() const {
     return navigation_request_->handle_state();
@@ -218,16 +209,6 @@ class CONTENT_EXPORT NavigationHandleImpl : public NavigationHandle {
   // The NavigationRequest that owns this NavigationHandle.
   NavigationRequest* navigation_request_;
 
-  // The headers used for the request.
-  net::HttpRequestHeaders request_headers_;
-
-  // Used to update the request's headers. When modified during the navigation
-  // start, the headers will be applied to the initial network request. When
-  // modified during a redirect, the headers will be applied to the redirected
-  // request.
-  std::vector<std::string> removed_request_headers_;
-  net::HttpRequestHeaders modified_request_headers_;
-
   // Allows to override response_headers_ in tests.
   // TODO(clamy): Clean this up once the architecture of unit tests is better.
   scoped_refptr<net::HttpResponseHeaders> response_headers_for_testing_;

content/browser/frame_host/navigation_request.cc

@@ -1021,8 +1021,11 @@ void NavigationRequest::CreateNavigationHandle(bool is_for_commit) {
   handle_state_ = NavigationRequest::INITIAL;
   navigation_handle_id_ = CreateUniqueHandleID();
 
+  request_headers_ = std::move(headers);
+  modified_request_headers_.Clear();
+  removed_request_headers_.clear();
   std::unique_ptr<NavigationHandleImpl> navigation_handle =
-      base::WrapUnique(new NavigationHandleImpl(this, std::move(headers)));
+      base::WrapUnique(new NavigationHandleImpl(this));
 
   if (!frame_tree_node->navigation_request() && !is_for_commit) {
     // A callback could have cancelled this request synchronously in which case
@@ -1798,7 +1801,7 @@ void NavigationRequest::OnStartChecksComplete(
   // Merge headers with embedder's headers.
   net::HttpRequestHeaders headers;
   headers.AddHeadersFromString(begin_params_->headers);
-  headers.MergeFrom(navigation_handle_->TakeModifiedRequestHeaders());
+  headers.MergeFrom(TakeModifiedRequestHeaders());
   begin_params_->headers = headers.ToString();
 
   // TODO(clamy): Avoid cloning the navigation params and create the
@@ -1865,10 +1868,8 @@ void NavigationRequest::OnRedirectChecksComplete(
 
   devtools_instrumentation::OnNavigationRequestWillBeSent(*this);
 
-  net::HttpRequestHeaders modified_headers =
-      navigation_handle_->TakeModifiedRequestHeaders();
-  std::vector<std::string> removed_headers =
-      navigation_handle_->TakeRemovedRequestHeaders();
+  net::HttpRequestHeaders modified_headers = TakeModifiedRequestHeaders();
+  std::vector<std::string> removed_headers = TakeRemovedRequestHeaders();
 
   BrowserContext* browser_context =
       frame_tree_node_->navigator()->GetController()->GetBrowserContext();
@@ -3002,4 +3003,20 @@ void NavigationRequest::SetCommitTimeoutForTesting(
     g_commit_timeout = timeout;
 }
 
+void NavigationRequest::RemoveRequestHeader(const std::string& header_name) {
+  DCHECK(handle_state_ == PROCESSING_WILL_REDIRECT_REQUEST ||
+         handle_state_ == WILL_REDIRECT_REQUEST);
+  removed_request_headers_.push_back(header_name);
+}
+
+void NavigationRequest::SetRequestHeader(const std::string& header_name,
+                                         const std::string& header_value) {
+  DCHECK(handle_state_ == INITIAL ||
+         handle_state_ == PROCESSING_WILL_START_REQUEST ||
+         handle_state_ == PROCESSING_WILL_REDIRECT_REQUEST ||
+         handle_state_ == WILL_START_REQUEST ||
+         handle_state_ == WILL_REDIRECT_REQUEST);
+  modified_request_headers_.SetHeader(header_name, header_value);
+}
+
 }  // namespace content

content/browser/frame_host/navigation_request.h

@@ -472,6 +472,19 @@ class CONTENT_EXPORT NavigationRequest : public NavigationURLLoaderDelegate,
   // default value if |timeout| is zero.
   static void SetCommitTimeoutForTesting(const base::TimeDelta& timeout);
 
+  const net::HttpRequestHeaders& request_headers() { return request_headers_; }
+
+  // Remove a request's header. If the header is not present, it has no effect.
+  // Must be called during a redirect.
+  void RemoveRequestHeader(const std::string& header_name);
+
+  // Set a request's header. If the header is already present, its value is
+  // overwritten. When modified during a navigation start, the headers will be
+  // applied to the initial network request. When modified during a redirect,
+  // the headers will be applied to the redirected request.
+  void SetRequestHeader(const std::string& header_name,
+                        const std::string& header_value);
+
  private:
   // TODO(clamy): Transform NavigationHandleImplTest into NavigationRequestTest
   // once NavigationHandleImpl has become a wrapper around NavigationRequest.
@@ -764,6 +777,14 @@ class CONTENT_EXPORT NavigationRequest : public NavigationURLLoaderDelegate,
   void StopCommitTimeout();
   void RestartCommitTimeout();
 
+  std::vector<std::string> TakeRemovedRequestHeaders() {
+    return std::move(removed_request_headers_);
+  }
+
+  net::HttpRequestHeaders TakeModifiedRequestHeaders() {
+    return std::move(modified_request_headers_);
+  }
+
   FrameTreeNode* frame_tree_node_;
 
   // Invariant: At least one of |loader_| or |render_frame_host_| is null.
@@ -992,6 +1013,16 @@ class CONTENT_EXPORT NavigationRequest : public NavigationURLLoaderDelegate,
   std::unique_ptr<base::CallbackList<void(bool)>::Subscription>
       render_process_blocked_state_changed_subscription_;
 
+  // The headers used for the request.
+  net::HttpRequestHeaders request_headers_;
+
+  // Used to update the request's headers. When modified during the navigation
+  // start, the headers will be applied to the initial network request. When
+  // modified during a redirect, the headers will be applied to the redirected
+  // request.
+  std::vector<std::string> removed_request_headers_;
+  net::HttpRequestHeaders modified_request_headers_;
+
   base::WeakPtrFactory<NavigationRequest> weak_factory_{this};
 
   DISALLOW_COPY_AND_ASSIGN(NavigationRequest);