Implement signin header modifications with a URLLoaderThrottle

This change replaces the header modification logic implemented in
ChromeResourceDispatchHostDelegate with a URLLoaderThrottle that is
configured for all navigation requests.

To support modifying headers (in addition to removing them) before
following a redirect a |modified_request_headers| parameter has been
added to URLLoaderThrottle::WillRedirectRequest().

Bug: 789670
Cq-Include-Trybots: luci.chromium.try:linux_mojo
Change-Id: Ibd58b0367bbe2a519dfb1cf85a4e5e6ec7eeeffa
Reviewed-on: https://chromium-review.googlesource.com/1139089
Commit-Queue: Reilly Grant <reillyg@chromium.org>
Reviewed-by: Mihai Sardarescu <msarda@chromium.org>
Reviewed-by: John Abd-El-Malek <jam@chromium.org>
Cr-Commit-Position: refs/heads/master@{#584270}

chrome/browser/BUILD.gn

@@ -1395,6 +1395,10 @@ jumbo_split_static_library("browser") {
     "signin/chrome_signin_client_factory.h",
     "signin/chrome_signin_helper.cc",
     "signin/chrome_signin_helper.h",
+    "signin/chrome_signin_url_loader_throttle.cc",
+    "signin/chrome_signin_url_loader_throttle.h",
+    "signin/chrome_signin_url_loader_throttle_delegate_impl.cc",
+    "signin/chrome_signin_url_loader_throttle_delegate_impl.h",
     "signin/gaia_cookie_manager_service_factory.cc",
     "signin/gaia_cookie_manager_service_factory.h",
     "signin/identity_manager_factory.cc",

chrome/browser/chrome_content_browser_client.cc

@@ -102,6 +102,8 @@
 #include "chrome/browser/safe_browsing/url_checker_delegate_impl.h"
 #include "chrome/browser/search/search.h"
 #include "chrome/browser/sessions/session_tab_helper.h"
+#include "chrome/browser/signin/chrome_signin_url_loader_throttle.h"
+#include "chrome/browser/signin/chrome_signin_url_loader_throttle_delegate_impl.h"
 #include "chrome/browser/signin/signin_manager_factory.h"
 #include "chrome/browser/speech/chrome_speech_recognition_manager_delegate.h"
 #include "chrome/browser/speech/tts_controller.h"
@@ -4309,6 +4311,15 @@ ChromeContentBrowserClient::CreateURLLoaderThrottles(
             resource_context, request.resource_type, frame_tree_node_id));
   }
 #endif
+
+  if (network_service_enabled) {
+    auto delegate = std::make_unique<signin::URLLoaderThrottleDelegateImpl>(
+        resource_context);
+    auto signin_throttle = signin::URLLoaderThrottle::MaybeCreate(
+        std::move(delegate), navigation_ui_data, wc_getter);
+    if (signin_throttle)
+      result.push_back(std::move(signin_throttle));
+  }
   return result;
 }
 

chrome/browser/signin/chrome_signin_url_loader_throttle.cc

+// Copyright 2018 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include "chrome/browser/signin/chrome_signin_url_loader_throttle.h"
+
+#include "chrome/browser/signin/chrome_signin_helper.h"
+#include "components/signin/core/browser/signin_header_helper.h"
+
+namespace signin {
+
+class URLLoaderThrottle::ThrottleRequestAdapter : public ChromeRequestAdapter {
+ public:
+  ThrottleRequestAdapter(URLLoaderThrottle* throttle,
+                         const net::HttpRequestHeaders& original_headers,
+                         net::HttpRequestHeaders* modified_headers,
+                         std::vector<std::string>* headers_to_remove)
+      : ChromeRequestAdapter(nullptr),
+        throttle_(throttle),
+        original_headers_(original_headers),
+        modified_headers_(modified_headers),
+        headers_to_remove_(headers_to_remove) {}
+
+  ~ThrottleRequestAdapter() override = default;
+
+  // ChromeRequestAdapter
+  bool IsMainRequestContext(ProfileIOData* io_data) override {
+    // The <webview> check in URLLoaderThrottle::MaybeCreate means this is
+    // always true.
+    return true;
+  }
+
+  content::ResourceRequestInfo::WebContentsGetter GetWebContentsGetter()
+      const override {
+    return throttle_->web_contents_getter_;
+  }
+
+  content::ResourceType GetResourceType() const override {
+    return throttle_->request_resource_type_;
+  }
+
+  GURL GetReferrerOrigin() const override {
+    return throttle_->request_referrer_.GetOrigin();
+  }
+
+  void SetDestructionCallback(base::OnceClosure closure) override {
+    if (!throttle_->destruction_callback_)
+      throttle_->destruction_callback_ = std::move(closure);
+  }
+
+  // RequestAdapter
+  const GURL& GetUrl() override { return throttle_->request_url_; }
+
+  bool HasHeader(const std::string& name) override {
+    return (original_headers_.HasHeader(name) ||
+            modified_headers_->HasHeader(name)) &&
+           !base::ContainsValue(*headers_to_remove_, name);
+  }
+
+  void RemoveRequestHeaderByName(const std::string& name) override {
+    if (!base::ContainsValue(*headers_to_remove_, name))
+      headers_to_remove_->push_back(name);
+  }
+
+  void SetExtraHeaderByName(const std::string& name,
+                            const std::string& value) override {
+    modified_headers_->SetHeader(name, value);
+
+    auto it =
+        std::find(headers_to_remove_->begin(), headers_to_remove_->end(), name);
+    if (it != headers_to_remove_->end())
+      headers_to_remove_->erase(it);
+  }
+
+ private:
+  URLLoaderThrottle* const throttle_;
+  const net::HttpRequestHeaders& original_headers_;
+  net::HttpRequestHeaders* const modified_headers_;
+  std::vector<std::string>* const headers_to_remove_;
+
+  DISALLOW_COPY_AND_ASSIGN(ThrottleRequestAdapter);
+};
+
+class URLLoaderThrottle::ThrottleResponseAdapter : public ResponseAdapter {
+ public:
+  ThrottleResponseAdapter(URLLoaderThrottle* throttle,
+                          net::HttpResponseHeaders* headers)
+      : ResponseAdapter(nullptr), throttle_(throttle), headers_(headers) {}
+
+  ~ThrottleResponseAdapter() override = default;
+
+  // ResponseAdapter
+  content::ResourceRequestInfo::WebContentsGetter GetWebContentsGetter()
+      const override {
+    return throttle_->web_contents_getter_;
+  }
+
+  bool IsMainFrame() const override { return throttle_->is_main_frame_; }
+
+  GURL GetOrigin() const override {
+    return throttle_->request_url_.GetOrigin();
+  }
+
+  const net::HttpResponseHeaders* GetHeaders() const override {
+    return headers_;
+  }
+
+  void RemoveHeader(const std::string& name) override {
+    headers_->RemoveHeader(name);
+  }
+
+ private:
+  URLLoaderThrottle* const throttle_;
+  net::HttpResponseHeaders* headers_;
+
+  DISALLOW_COPY_AND_ASSIGN(ThrottleResponseAdapter);
+};
+
+URLLoaderThrottle::Delegate::Delegate() = default;
+
+URLLoaderThrottle::Delegate::~Delegate() = default;
+
+// static
+std::unique_ptr<URLLoaderThrottle> URLLoaderThrottle::MaybeCreate(
+    std::unique_ptr<Delegate> delegate,
+    content::NavigationUIData* navigation_ui_data,
+    content::ResourceRequestInfo::WebContentsGetter web_contents_getter) {
+  if (!delegate->ShouldIntercept(navigation_ui_data))
+    return nullptr;
+
+  return base::WrapUnique(new URLLoaderThrottle(
+      std::move(delegate), std::move(web_contents_getter)));
+}
+
+URLLoaderThrottle::~URLLoaderThrottle() {
+  if (destruction_callback_)
+    std::move(destruction_callback_).Run();
+}
+
+void URLLoaderThrottle::WillStartRequest(network::ResourceRequest* request,
+                                         bool* defer) {
+  request_url_ = request->url;
+  request_referrer_ = request->referrer;
+  request_resource_type_ =
+      static_cast<content::ResourceType>(request->resource_type);
+  is_main_frame_ = request->is_main_frame;
+
+  net::HttpRequestHeaders modified_request_headers;
+  std::vector<std::string> to_be_removed_request_headers;
+
+  ThrottleRequestAdapter adapter(this, request->headers,
+                                 &modified_request_headers,
+                                 &to_be_removed_request_headers);
+  delegate_->ProcessRequest(&adapter, GURL() /* redirect_url */);
+
+  request->headers.MergeFrom(modified_request_headers);
+  for (const std::string& name : to_be_removed_request_headers)
+    request->headers.RemoveHeader(name);
+
+  // 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);
+}
+
+void URLLoaderThrottle::WillRedirectRequest(
+    const net::RedirectInfo& redirect_info,
+    const network::ResourceResponseHead& response_head,
+    bool* /* defer */,
+    std::vector<std::string>* to_be_removed_request_headers,
+    net::HttpRequestHeaders* modified_request_headers) {
+  ThrottleRequestAdapter request_adapter(this, request_headers_,
+                                         modified_request_headers,
+                                         to_be_removed_request_headers);
+  delegate_->ProcessRequest(&request_adapter, redirect_info.new_url);
+
+  request_headers_.MergeFrom(*modified_request_headers);
+  for (const std::string& name : *to_be_removed_request_headers)
+    request_headers_.RemoveHeader(name);
+
+  // Modifications to |response_head.headers| will be passed to the
+  // URLLoaderClient even though |response_head| is const.
+  ThrottleResponseAdapter response_adapter(this, response_head.headers.get());
+  delegate_->ProcessResponse(&response_adapter, redirect_info.new_url);
+
+  request_url_ = redirect_info.new_url;
+  request_referrer_ = GURL(redirect_info.new_referrer);
+}
+
+void URLLoaderThrottle::WillProcessResponse(
+    const GURL& response_url,
+    network::ResourceResponseHead* response_head,
+    bool* defer) {
+  ThrottleResponseAdapter adapter(this, response_head->headers.get());
+  delegate_->ProcessResponse(&adapter, GURL() /* redirect_url */);
+}
+
+URLLoaderThrottle::URLLoaderThrottle(
+    std::unique_ptr<Delegate> delegate,
+    content::ResourceRequestInfo::WebContentsGetter web_contents_getter)
+    : delegate_(std::move(delegate)),
+      web_contents_getter_(std::move(web_contents_getter)) {}
+
+}  // namespace signin

chrome/browser/signin/chrome_signin_url_loader_throttle.h

+// Copyright 2018 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#ifndef CHROME_BROWSER_SIGNIN_CHROME_SIGNIN_URL_LOADER_THROTTLE_H_
+#define CHROME_BROWSER_SIGNIN_CHROME_SIGNIN_URL_LOADER_THROTTLE_H_
+
+#include "base/macros.h"
+#include "content/public/browser/resource_request_info.h"
+#include "content/public/common/url_loader_throttle.h"
+
+namespace content {
+class NavigationUIData;
+}  // namespace content
+
+namespace signin {
+
+class ChromeRequestAdapter;
+class ResponseAdapter;
+
+// This class is used to modify the main frame request made when loading the
+// GAIA signin realm.
+class URLLoaderThrottle : public content::URLLoaderThrottle {
+ public:
+  class Delegate {
+   public:
+    Delegate();
+    virtual ~Delegate();
+
+    virtual bool ShouldIntercept(
+        content::NavigationUIData* navigation_ui_data) = 0;
+    virtual void ProcessRequest(ChromeRequestAdapter* request_adapter,
+                                const GURL& redirect_url) = 0;
+    virtual void ProcessResponse(ResponseAdapter* response_adapter,
+                                 const GURL& redirect_url) = 0;
+  };
+
+  // Creates a new throttle if |delegate| says that this request should be
+  // intercepted.
+  static std::unique_ptr<URLLoaderThrottle> MaybeCreate(
+      std::unique_ptr<Delegate> delegate,
+      content::NavigationUIData* navigation_ui_data,
+      content::ResourceRequestInfo::WebContentsGetter web_contents_getter);
+
+  ~URLLoaderThrottle() override;
+
+  // content::URLLoaderThrottle
+  void WillStartRequest(network::ResourceRequest* request,
+                        bool* defer) override;
+  void WillRedirectRequest(const net::RedirectInfo& redirect_info,
+                           const network::ResourceResponseHead& response_head,
+                           bool* defer,
+                           std::vector<std::string>* headers_to_remove,
+                           net::HttpRequestHeaders* modified_headers) override;
+  void WillProcessResponse(const GURL& response_url,
+                           network::ResourceResponseHead* response_head,
+                           bool* defer) override;
+
+ private:
+  class ThrottleRequestAdapter;
+  class ThrottleResponseAdapter;
+
+  URLLoaderThrottle(
+      std::unique_ptr<Delegate> delegate,
+      content::ResourceRequestInfo::WebContentsGetter web_contents_getter);
+
+  const std::unique_ptr<Delegate> delegate_;
+  const content::ResourceRequestInfo::WebContentsGetter web_contents_getter_;
+
+  // Information about the current request.
+  GURL request_url_;
+  GURL request_referrer_;
+  net::HttpRequestHeaders request_headers_;
+  content::ResourceType request_resource_type_;
+  bool is_main_frame_ = false;
+
+  base::OnceClosure destruction_callback_;
+
+  DISALLOW_COPY_AND_ASSIGN(URLLoaderThrottle);
+};
+
+}  // namespace signin
+
+#endif  // CHROME_BROWSER_SIGNIN_CHROME_SIGNIN_URL_LOADER_THROTTLE_H_

chrome/browser/signin/chrome_signin_url_loader_throttle_delegate_impl.cc

+// Copyright 2018 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include "chrome/browser/signin/chrome_signin_url_loader_throttle_delegate_impl.h"
+
+#include "chrome/browser/profiles/profile_io_data.h"
+#include "chrome/browser/renderer_host/chrome_navigation_ui_data.h"
+#include "chrome/browser/signin/chrome_signin_helper.h"
+#include "extensions/browser/extension_navigation_ui_data.h"
+
+namespace signin {
+
+URLLoaderThrottleDelegateImpl::URLLoaderThrottleDelegateImpl(
+    content::ResourceContext* resource_context)
+    : io_data_(ProfileIOData::FromResourceContext(resource_context)) {}
+
+URLLoaderThrottleDelegateImpl::~URLLoaderThrottleDelegateImpl() = default;
+
+bool URLLoaderThrottleDelegateImpl::ShouldIntercept(
+    content::NavigationUIData* navigation_ui_data) {
+  if (io_data_->IsOffTheRecord())
+    return false;
+
+#if BUILDFLAG(ENABLE_EXTENSIONS)
+  // Note: InlineLoginUI uses an isolated request context and thus should
+  // bypass the account consistency flow. See http://crbug.com/428396
+  ChromeNavigationUIData* chrome_navigation_ui_data =
+      static_cast<ChromeNavigationUIData*>(navigation_ui_data);
+  if (chrome_navigation_ui_data) {
+    extensions::ExtensionNavigationUIData* extension_navigation_ui_data =
+        chrome_navigation_ui_data->GetExtensionNavigationUIData();
+    if (extension_navigation_ui_data &&
+        extension_navigation_ui_data->is_web_view()) {
+      return false;
+    }
+  }
+#endif
+
+  return true;
+}
+
+void URLLoaderThrottleDelegateImpl::ProcessRequest(
+    ChromeRequestAdapter* request_adapter,
+    const GURL& redirect_url) {
+  FixAccountConsistencyRequestHeader(request_adapter, redirect_url, io_data_);
+}
+
+void URLLoaderThrottleDelegateImpl::ProcessResponse(
+    ResponseAdapter* response_adapter,
+    const GURL& redirect_url) {
+  ProcessAccountConsistencyResponseHeaders(response_adapter, redirect_url,
+                                           io_data_->IsOffTheRecord());
+}
+
+}  // namespace signin

chrome/browser/signin/chrome_signin_url_loader_throttle_delegate_impl.h

+// Copyright 2018 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#ifndef CHROME_BROWSER_SIGNIN_CHROME_SIGNIN_URL_LOADER_THROTTLE_DELEGATE_IMPL_H_
+#define CHROME_BROWSER_SIGNIN_CHROME_SIGNIN_URL_LOADER_THROTTLE_DELEGATE_IMPL_H_
+
+#include "chrome/browser/signin/chrome_signin_url_loader_throttle.h"
+
+class ProfileIOData;
+
+namespace signin {
+
+class URLLoaderThrottleDelegateImpl : public URLLoaderThrottle::Delegate {
+ public:
+  explicit URLLoaderThrottleDelegateImpl(
+      content::ResourceContext* resource_context);
+  ~URLLoaderThrottleDelegateImpl() override;
+
+  // URLLoaderThrottle::Delegate
+  bool ShouldIntercept(content::NavigationUIData* navigation_ui_data) override;
+  void ProcessRequest(ChromeRequestAdapter* request_adapter,
+                      const GURL& redirect_url) override;
+  void ProcessResponse(ResponseAdapter* response_adapter,
+                       const GURL& redirect_url) override;
+
+ private:
+  ProfileIOData* const io_data_;
+
+  DISALLOW_COPY_AND_ASSIGN(URLLoaderThrottleDelegateImpl);
+};
+
+}  // namespace signin
+
+#endif  // CHROME_BROWSER_SIGNIN_CHROME_SIGNIN_URL_LOADER_THROTTLE_DELEGATE_IMPL_H_

chrome/common/google_url_loader_throttle.cc

@@ -62,9 +62,10 @@ void GoogleURLLoaderThrottle::WillStartRequest(
 
 void GoogleURLLoaderThrottle::WillRedirectRequest(
     const net::RedirectInfo& redirect_info,
-    const network::ResourceResponseHead& response_head,
-    bool* defer,
-    std::vector<std::string>* to_be_removed_headers) {
+    const network::ResourceResponseHead& /* response_head */,
+    bool* /* defer */,
+    std::vector<std::string>* to_be_removed_headers,
+    net::HttpRequestHeaders* /* modified_headers */) {
   if (!variations::ShouldAppendVariationHeaders(redirect_info.new_url))
     to_be_removed_headers->push_back(variations::kClientDataHeader);
 }

chrome/common/google_url_loader_throttle.h

@@ -27,11 +27,11 @@ class GoogleURLLoaderThrottle
   void DetachFromCurrentSequence() override;
   void WillStartRequest(network::ResourceRequest* request,
                         bool* defer) override;
-  void WillRedirectRequest(
-      const net::RedirectInfo& redirect_info,
-      const network::ResourceResponseHead& response_head,
-      bool* defer,
-      std::vector<std::string>* to_be_removed_headers) override;
+  void WillRedirectRequest(const net::RedirectInfo& redirect_info,
+                           const network::ResourceResponseHead& response_head,
+                           bool* defer,
+                           std::vector<std::string>* to_be_removed_headers,
+                           net::HttpRequestHeaders* modified_headers) override;
 #if BUILDFLAG(ENABLE_EXTENSIONS)
   void WillProcessResponse(const GURL& response_url,
                            network::ResourceResponseHead* response_head,

chrome/common/prerender_url_loader_throttle.cc

@@ -143,7 +143,8 @@ void PrerenderURLLoaderThrottle::WillRedirectRequest(
     const net::RedirectInfo& redirect_info,
     const network::ResourceResponseHead& response_head,
     bool* defer,
-    std::vector<std::string>* to_be_removed_headers) {
+    std::vector<std::string>* /* to_be_removed_headers */,
+    net::HttpRequestHeaders* /* modified_headers */) {
   redirect_count_++;
   if (mode_ == PREFETCH_ONLY) {
     RecordPrefetchResponseReceived(

chrome/common/prerender_url_loader_throttle.h

@@ -46,11 +46,11 @@ class PrerenderURLLoaderThrottle
   void DetachFromCurrentSequence() override;
   void WillStartRequest(network::ResourceRequest* request,
                         bool* defer) override;
-  void WillRedirectRequest(
-      const net::RedirectInfo& redirect_info,
-      const network::ResourceResponseHead& response_head,
-      bool* defer,
-      std::vector<std::string>* to_be_removed_headers) override;
+  void WillRedirectRequest(const net::RedirectInfo& redirect_info,
+                           const network::ResourceResponseHead& response_head,
+                           bool* defer,
+                           std::vector<std::string>* to_be_removed_headers,
+                           net::HttpRequestHeaders* modified_headers) override;
   void WillProcessResponse(const GURL& response_url,
                            network::ResourceResponseHead* response_head,
                            bool* defer) override;

chrome/test/BUILD.gn

@@ -2655,6 +2655,7 @@ test("unit_tests") {
     "../browser/signin/chrome_signin_client_unittest.cc",
     "../browser/signin/chrome_signin_helper_unittest.cc",
     "../browser/signin/chrome_signin_status_metrics_provider_delegate_unittest.cc",
+    "../browser/signin/chrome_signin_url_loader_throttle_unittest.cc",
     "../browser/signin/local_auth_unittest.cc",
     "../browser/signin/signin_status_metrics_provider_chromeos_unittest.cc",
     "../browser/signin/signin_tracker_unittest.cc",

components/safe_browsing/browser/base_parallel_resource_throttle.cc

@@ -140,12 +140,15 @@ void BaseParallelResourceThrottle::WillRedirectRequest(
     return;
   }
 
-  // The safe browsing URLLoaderThrottle doesn't use ResourceResponse, so pass
+  // The safe browsing URLLoaderThrottle doesn't use the |resource_head|,
+  // |to_be_modified_headers| or |modified_headers| parameters, so pass
   // in an empty struct to avoid changing ResourceThrottle signature.
-  network::ResourceResponseHead resource_response;
+  network::ResourceResponseHead resource_head;
   std::vector<std::string> to_be_removed_headers;
+  net::HttpRequestHeaders modified_headers;
   url_loader_throttle_holder_->throttle()->WillRedirectRequest(
-      redirect_info, resource_response, defer, &to_be_removed_headers);
+      redirect_info, resource_head, defer, &to_be_removed_headers,
+      &modified_headers);
   DCHECK(!*defer);
   throttle_in_band_ = false;
 }

components/safe_browsing/browser/browser_url_loader_throttle.cc

@@ -68,9 +68,10 @@ void BrowserURLLoaderThrottle::WillStartRequest(
 
 void BrowserURLLoaderThrottle::WillRedirectRequest(
     const net::RedirectInfo& redirect_info,
-    const network::ResourceResponseHead& response_head,
+    const network::ResourceResponseHead& /* response_head */,
     bool* defer,
-    std::vector<std::string>* to_be_removed_headers) {
+    std::vector<std::string>* /* to_be_removed_headers */,
+    net::HttpRequestHeaders* /* modified_headers */) {
   if (blocked_) {
     // OnCheckUrlResult() has set |blocked_| to true and called
     // |delegate_->CancelWithError|, but this method is called before the

components/safe_browsing/browser/browser_url_loader_throttle.h

@@ -43,11 +43,11 @@ class BrowserURLLoaderThrottle : public content::URLLoaderThrottle {
   // content::URLLoaderThrottle implementation.
   void WillStartRequest(network::ResourceRequest* request,
                         bool* defer) override;
-  void WillRedirectRequest(
-      const net::RedirectInfo& redirect_info,
-      const network::ResourceResponseHead& response_head,
-      bool* defer,
-      std::vector<std::string>* to_be_removed_headers) override;
+  void WillRedirectRequest(const net::RedirectInfo& redirect_info,
+                           const network::ResourceResponseHead& response_head,
+                           bool* defer,
+                           std::vector<std::string>* to_be_removed_headers,
+                           net::HttpRequestHeaders* modified_headers) override;
   void WillProcessResponse(const GURL& response_url,
                            network::ResourceResponseHead* response_head,
                            bool* defer) override;

components/safe_browsing/renderer/renderer_url_loader_throttle.cc

@@ -71,9 +71,10 @@ void RendererURLLoaderThrottle::WillStartRequest(
 
 void RendererURLLoaderThrottle::WillRedirectRequest(
     const net::RedirectInfo& redirect_info,
-    const network::ResourceResponseHead& response_head,
-    bool* defer,
-    std::vector<std::string>* to_be_removed_headers) {
+    const network::ResourceResponseHead& /* response_head */,
+    bool* /* defer */,
+    std::vector<std::string>* /* to_be_removed_headers */,
+    net::HttpRequestHeaders* /* modified_headers */) {
   // If |blocked_| is true, the resource load has been canceled and there
   // shouldn't be such a notification.
   DCHECK(!blocked_);

components/safe_browsing/renderer/renderer_url_loader_throttle.h

@@ -34,11 +34,11 @@ class RendererURLLoaderThrottle : public content::URLLoaderThrottle,
   void DetachFromCurrentSequence() override;
   void WillStartRequest(network::ResourceRequest* request,
                         bool* defer) override;
-  void WillRedirectRequest(
-      const net::RedirectInfo& redirect_info,
-      const network::ResourceResponseHead& response_head,
-      bool* defer,
-      std::vector<std::string>* to_be_removed_headers) override;
+  void WillRedirectRequest(const net::RedirectInfo& redirect_info,
+                           const network::ResourceResponseHead& response_head,
+                           bool* defer,
+                           std::vector<std::string>* to_be_removed_headers,
+                           net::HttpRequestHeaders* modified_headers) override;
   void WillProcessResponse(const GURL& response_url,
                            network::ResourceResponseHead* response_head,
                            bool* defer) override;

components/subresource_filter/content/common/ad_delay_throttle.cc

@@ -155,9 +155,10 @@ void AdDelayThrottle::WillStartRequest(network::ResourceRequest* request,
 
 void AdDelayThrottle::WillRedirectRequest(
     const net::RedirectInfo& redirect_info,
-    const network::ResourceResponseHead& response_head,
+    const network::ResourceResponseHead& /* response_head */,
     bool* defer,
-    std::vector<std::string>* to_be_removed_headers) {
+    std::vector<std::string>* /* to_be_removed_headers */,
+    net::HttpRequestHeaders* /* modified_headers */) {
   // Note: some MetadataProviders may not be able to distinguish requests that
   // are only tagged as ads after a redirect.
   *defer = MaybeDefer(redirect_info.new_url);

components/subresource_filter/content/common/ad_delay_throttle.h

@@ -138,11 +138,11 @@ class AdDelayThrottle : public content::URLLoaderThrottle {
   void DetachFromCurrentSequence() override;
   void WillStartRequest(network::ResourceRequest* request,
                         bool* defer) override;
-  void WillRedirectRequest(
-      const net::RedirectInfo& redirect_info,
-      const network::ResourceResponseHead& response_head,
-      bool* defer,
-      std::vector<std::string>* to_be_removed_headers) override;
+  void WillRedirectRequest(const net::RedirectInfo& redirect_info,
+                           const network::ResourceResponseHead& response_head,
+                           bool* defer,
+                           std::vector<std::string>* to_be_removed_headers,
+                           net::HttpRequestHeaders* modified_headers) override;
 
   // Returns whether the request to |url| should be deferred.
   bool MaybeDefer(const GURL& url);

content/browser/web_package/signed_exchange_cert_fetcher_unittest.cc

@@ -40,10 +40,11 @@ class DeferringURLLoaderThrottle final : public URLLoaderThrottle {
   }
 
   void WillRedirectRequest(
-      const net::RedirectInfo& redirect_info,
-      const network::ResourceResponseHead& response_head,
+      const net::RedirectInfo& /* redirect_info */,
+      const network::ResourceResponseHead& /* response_head */,
       bool* defer,
-      std::vector<std::string>* to_be_removed_headers) override {
+      std::vector<std::string>* /* to_be_removed_headers */,
+      net::HttpRequestHeaders* /* modified_headers */) override {
     will_redirect_request_called_ = true;
     *defer = true;
   }

content/common/throttling_url_loader.cc

@@ -189,35 +189,48 @@ ThrottlingURLLoader::~ThrottlingURLLoader() {
 }
 
 void ThrottlingURLLoader::FollowRedirect(
-    const base::Optional<net::HttpRequestHeaders>& modified_request_headers) {
+    const base::Optional<net::HttpRequestHeaders>& modified_headers) {
+  const base::Optional<net::HttpRequestHeaders>* modified_headers_to_send =
+      &modified_headers;
+  if (modified_request_headers_) {
+    if (modified_headers)
+      modified_request_headers_->MergeFrom(*modified_headers);
+    modified_headers_to_send = &modified_request_headers_;
+  }
+
   if (!throttle_redirect_url_.is_empty()) {
     throttle_redirect_url_ = GURL();
     // This is a synthesized redirect, so no need to tell the URLLoader.
-    DCHECK(!modified_request_headers.has_value())
-        << "ThrottlingURLLoader doesn't support modified_request_headers for "
+    DCHECK(!modified_headers_to_send->has_value())
+        << "ThrottlingURLLoader doesn't support modifying headers for "
            "synthesized requests.";
     StartNow();
     return;
   }
 
   if (url_loader_) {
-    if (to_be_removed_request_headers_.empty()) {
-      url_loader_->FollowRedirect(base::nullopt, modified_request_headers);
-    } else {
-      url_loader_->FollowRedirect(to_be_removed_request_headers_,
-                                  modified_request_headers);
-    }
-    to_be_removed_request_headers_.clear();
+    url_loader_->FollowRedirect(to_be_removed_request_headers_,
+                                *modified_headers_to_send);
   }
+
+  to_be_removed_request_headers_.reset();
+  modified_request_headers_.reset();
 }
 
 void ThrottlingURLLoader::FollowRedirectForcingRestart() {
   url_loader_.reset();
   client_binding_.Close();
 
-  for (const std::string& key : to_be_removed_request_headers_)
-    start_info_->url_request.headers.RemoveHeader(key);
-  to_be_removed_request_headers_.clear();
+  if (to_be_removed_request_headers_) {
+    for (const std::string& key : *to_be_removed_request_headers_)
+      start_info_->url_request.headers.RemoveHeader(key);
+    to_be_removed_request_headers_.reset();
+  }
+
+  if (modified_request_headers_) {
+    start_info_->url_request.headers.MergeFrom(*modified_request_headers_);
+    modified_request_headers_.reset();
+  }
 
   StartNow();
 }
@@ -417,16 +430,33 @@ void ThrottlingURLLoader::OnReceiveRedirect(
       auto* throttle = entry.throttle.get();
       bool throttle_deferred = false;
       auto weak_ptr = weak_factory_.GetWeakPtr();
-      std::vector<std::string> headers;
+      std::vector<std::string> to_be_removed_headers;
+      net::HttpRequestHeaders modified_headers;
       throttle->WillRedirectRequest(redirect_info, response_head,
-                                    &throttle_deferred, &headers);
+                                    &throttle_deferred, &to_be_removed_headers,
+                                    &modified_headers);
       if (!weak_ptr)
         return;
       if (!HandleThrottleResult(throttle, throttle_deferred, &deferred))
         return;
 
-      to_be_removed_request_headers_.insert(
-          to_be_removed_request_headers_.end(), headers.begin(), headers.end());
+      if (!to_be_removed_headers.empty()) {
+        if (to_be_removed_request_headers_) {
+          for (auto& header : to_be_removed_headers) {
+            if (!base::ContainsValue(*to_be_removed_request_headers_, header))
+              to_be_removed_request_headers_->push_back(std::move(header));
+          }
+        } else {
+          to_be_removed_request_headers_ = std::move(to_be_removed_headers);
+        }
+      }
+
+      if (!modified_headers.IsEmpty()) {
+        if (modified_request_headers_)
+          modified_request_headers_->MergeFrom(modified_headers);
+        else
+          modified_request_headers_ = std::move(modified_headers);
+      }
     }
 
     if (deferred) {

content/common/throttling_url_loader.h

@@ -235,7 +235,8 @@ class CONTENT_EXPORT ThrottlingURLLoader
 
   bool response_intercepted_ = false;
 
-  std::vector<std::string> to_be_removed_request_headers_;
+  base::Optional<std::vector<std::string>> to_be_removed_request_headers_;
+  base::Optional<net::HttpRequestHeaders> modified_request_headers_;
 
   base::WeakPtrFactory<ThrottlingURLLoader> weak_factory_;
 

content/public/common/url_loader_throttle.cc

@@ -37,7 +37,8 @@ void URLLoaderThrottle::WillRedirectRequest(
     const net::RedirectInfo& redirect_info,
     const network::ResourceResponseHead& response_head,
     bool* defer,
-    std::vector<std::string>* to_be_removed_request_headers) {}
+    std::vector<std::string>* to_be_removed_request_headers,
+    net::HttpRequestHeaders* modified_request_headers) {}
 
 void URLLoaderThrottle::WillProcessResponse(
     const GURL& response_url,

content/public/common/url_loader_throttle.h

@@ -101,11 +101,17 @@ class CONTENT_EXPORT URLLoaderThrottle {
   // redirect responses's HTTP status code and some information about the new
   // request that will be sent if the redirect is followed, including the new
   // URL and new method.
+  //
+  // Request headers added to |to_be_removed_request_headers| will be removed
+  // before the redirect is followed. Headers added to
+  // |modified_request_headers| will be merged into the existing request headers
+  // before the redirect is followed.
   virtual void WillRedirectRequest(
       const net::RedirectInfo& redirect_info,
       const network::ResourceResponseHead& response_head,
       bool* defer,
-      std::vector<std::string>* to_be_removed_request_headers);
+      std::vector<std::string>* to_be_removed_request_headers,
+      net::HttpRequestHeaders* modified_request_headers);
 
   // Called when the response headers and meta data are available.
   // TODO(776312): Migrate this URL to ResourceResponseHead.

extensions/renderer/extension_url_loader_throttle.cc

@@ -33,12 +33,14 @@ void ExtensionURLLoaderThrottle::WillStartRequest(
 
 void ExtensionURLLoaderThrottle::WillRedirectRequest(
     const net::RedirectInfo& redirect_info,
-    const network::ResourceResponseHead& response_head,
-    bool* defer,
-    std::vector<std::string>* to_be_removed_request_headers) {
+    const network::ResourceResponseHead& /* response_head */,
+    bool* /* defer */,
+    std::vector<std::string>* /* to_be_removed_request_headers */,
+    net::HttpRequestHeaders* /* modified_request_headers */) {
   if (manager_->ShouldRejectRedirect(start_request_url_, request_load_flags_,
-                                     redirect_info))
+                                     redirect_info)) {
     delegate_->CancelWithError(net::ERR_TEMPORARILY_THROTTLED, kCancelReason);
+  }
 }
 
 void ExtensionURLLoaderThrottle::WillProcessResponse(

extensions/renderer/extension_url_loader_throttle.h

@@ -32,7 +32,8 @@ class ExtensionURLLoaderThrottle : public content::URLLoaderThrottle {
       const net::RedirectInfo& redirect_info,
       const network::ResourceResponseHead& response_head,
       bool* defer,
-      std::vector<std::string>* to_be_removed_request_headers) override;
+      std::vector<std::string>* to_be_removed_request_headers,
+      net::HttpRequestHeaders* modified_request_headers) override;
   void WillProcessResponse(const GURL& response_url,
                            network::ResourceResponseHead* response_head,
                            bool* defer) override;

testing/buildbot/filters/mojo.fyi.network_browser_tests.filter

@@ -88,18 +88,8 @@
 -DnsProbeBrowserTest.CorrectionsLoadStopped
 -DnsProbeBrowserTest.NoInternetProbeResultWithSlowBrokenCorrections
 
-# https://crbug.com/789670
--DiceBrowserTest.Reauth
--DiceBrowserTest.Signin
--DiceBrowserTest.SignoutAllAccounts
--DiceBrowserTest.SignoutMainAccount
--DiceBrowserTest.SignoutSecondaryAccount
--DiceFixAuthErrorsBrowserTest.ReauthFixAuthError
--DiceFixAuthErrorsBrowserTest.SigninAccountMismatch
--DicePrepareMigrationBrowserTest.EnableSyncAfterToken
--DicePrepareMigrationBrowserTest.EnableSyncBeforeToken
--DicePrepareMigrationBrowserTest.Signout
--ChromeResourceDispatcherHostDelegateMirrorBrowserTest.MirrorRequestHeader
+# crbug.com/859594 Migrate storage::FileWriterDelegate to use SimpleURLLoader
+-SBNavigationObserverBrowserTest.DownloadViaHTML5FileApi
 
 # https://crbug.com/778793
 # Started failing in r514649.