Reland "Ensure header modification for a redirect's onBeforeSendHeaders is...

Reland "Ensure header modification for a redirect's onBeforeSendHeaders is applied with network service."

This is a reland of b5e1969b

Original change's description:
> Ensure header modification for a redirect's onBeforeSendHeaders is applied with network service.
>
> This currently worked if 'extraHeaders' was specified, but not without it.
>
> Bug: 942062
> Change-Id: Ic962ad256d89384082e25a5983181bf563021ffb
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1548673
> Commit-Queue: John Abd-El-Malek <jam@chromium.org>
> Reviewed-by: Ken Rockot <rockot@google.com>
> Reviewed-by: Karan Bhatia <karandeepb@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#647160}

TBR=rockot@chromium.org

Bug: 942062, 949478
Change-Id: I11889f6c2973548b28e3ea8029f1516626c950f4
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1559013Reviewed-by: John Abd-El-Malek <jam@chromium.org>
Commit-Queue: John Abd-El-Malek <jam@chromium.org>
Auto-Submit: John Abd-El-Malek <jam@chromium.org>
Cr-Commit-Position: refs/heads/master@{#649768}

chrome/browser/extensions/api/web_request/web_request_api_unittest.cc

@@ -2073,10 +2073,12 @@ TEST(ExtensionWebRequestHelpersTest, TestMergeOnBeforeSendHeadersResponses) {
     deltas.push_back(std::move(d0));
   }
   bool request_headers_modified0;
+  std::set<std::string> ignore1, ignore2;
   net::HttpRequestHeaders headers0;
   headers0.MergeFrom(base_headers);
   MergeOnBeforeSendHeadersResponses(GURL(), deltas, &headers0, &ignored_actions,
-                                    &logger, &request_headers_modified0);
+                                    &logger, &ignore1, &ignore2,
+                                    &request_headers_modified0);
   ASSERT_TRUE(headers0.GetHeader("key1", &header_value));
   EXPECT_EQ("value 1", header_value);
   ASSERT_TRUE(headers0.GetHeader("key2", &header_value));
@@ -2096,11 +2098,14 @@ TEST(ExtensionWebRequestHelpersTest, TestMergeOnBeforeSendHeadersResponses) {
   deltas.sort(&InDecreasingExtensionInstallationTimeOrder);
   ignored_actions.clear();
   logger.clear();
+  ignore1.clear();
+  ignore2.clear();
   bool request_headers_modified1;
   net::HttpRequestHeaders headers1;
   headers1.MergeFrom(base_headers);
   MergeOnBeforeSendHeadersResponses(GURL(), deltas, &headers1, &ignored_actions,
-                                    &logger, &request_headers_modified1);
+                                    &logger, &ignore1, &ignore2,
+                                    &request_headers_modified1);
   EXPECT_FALSE(headers1.HasHeader("key1"));
   ASSERT_TRUE(headers1.GetHeader("key2", &header_value));
   EXPECT_EQ("value 3", header_value);
@@ -2122,11 +2127,14 @@ TEST(ExtensionWebRequestHelpersTest, TestMergeOnBeforeSendHeadersResponses) {
   deltas.sort(&InDecreasingExtensionInstallationTimeOrder);
   ignored_actions.clear();
   logger.clear();
+  ignore1.clear();
+  ignore2.clear();
   bool request_headers_modified2;
   net::HttpRequestHeaders headers2;
   headers2.MergeFrom(base_headers);
   MergeOnBeforeSendHeadersResponses(GURL(), deltas, &headers2, &ignored_actions,
-                                    &logger, &request_headers_modified2);
+                                    &logger, &ignore1, &ignore2,
+                                    &request_headers_modified2);
   EXPECT_FALSE(headers2.HasHeader("key1"));
   ASSERT_TRUE(headers2.GetHeader("key2", &header_value));
   EXPECT_EQ("value 3", header_value);
@@ -2152,11 +2160,14 @@ TEST(ExtensionWebRequestHelpersTest, TestMergeOnBeforeSendHeadersResponses) {
   deltas.sort(&InDecreasingExtensionInstallationTimeOrder);
   ignored_actions.clear();
   logger.clear();
+  ignore1.clear();
+  ignore2.clear();
   bool request_headers_modified3;
   net::HttpRequestHeaders headers3;
   headers3.MergeFrom(base_headers);
   MergeOnBeforeSendHeadersResponses(GURL(), deltas, &headers3, &ignored_actions,
-                                    &logger, &request_headers_modified3);
+                                    &logger, &ignore1, &ignore2,
+                                    &request_headers_modified3);
   EXPECT_FALSE(headers3.HasHeader("key1"));
   ASSERT_TRUE(headers3.GetHeader("key2", &header_value));
   EXPECT_EQ("value 3", header_value);
@@ -2218,11 +2229,13 @@ TEST(ExtensionWebRequestHelpersTest,
   }
   deltas.sort(&InDecreasingExtensionInstallationTimeOrder);
   bool request_headers_modified1;
+  std::set<std::string> ignore1, ignore2;
   net::HttpRequestHeaders headers1;
   headers1.MergeFrom(base_headers);
   ignored_actions.clear();
   MergeOnBeforeSendHeadersResponses(GURL(), deltas, &headers1, &ignored_actions,
-                                    &logger, &request_headers_modified1);
+                                    &logger, &ignore1, &ignore2,
+                                    &request_headers_modified1);
   EXPECT_TRUE(headers1.HasHeader("Cookie"));
   ASSERT_TRUE(headers1.GetHeader("Cookie", &header_value));
   EXPECT_EQ("name=new value; name2=new value; name4=\"value 4\"", header_value);

chrome/browser/net/chrome_extensions_network_delegate.cc

@@ -197,13 +197,22 @@ int ChromeExtensionsNetworkDelegateImpl::OnBeforeURLRequest(
   return result;
 }
 
+namespace {
+void OnHeadersReceivedAdapter(net::CompletionOnceCallback callback,
+                              const std::set<std::string>& removed_headers,
+                              const std::set<std::string>& set_headers,
+                              int error_code) {
+  std::move(callback).Run(error_code);
+}
+}  // namespace
+
 int ChromeExtensionsNetworkDelegateImpl::OnBeforeStartTransaction(
     net::URLRequest* request,
     net::CompletionOnceCallback callback,
     net::HttpRequestHeaders* headers) {
   return ExtensionWebRequestEventRouter::GetInstance()->OnBeforeSendHeaders(
       profile_, extension_info_map_.get(), GetWebRequestInfo(request),
-      std::move(callback), headers);
+      base::BindOnce(OnHeadersReceivedAdapter, std::move(callback)), headers);
 }
 
 void ChromeExtensionsNetworkDelegateImpl::OnStartTransaction(

chrome/test/data/extensions/api_test/webrequest/test_extra_headers.js

@@ -8,6 +8,56 @@ function getSetCookieUrl(name, value) {
   return getServerURL('set-cookie?' + name + '=' + value);
 }
 
+function testModifyHeadersOnRedirect(useExtraHeaders) {
+  // Use /echoheader instead of observing headers in onSendHeaders to
+  // ensure we're looking at what the server receives. This avoids bugs in the
+  // webRequest implementation from being masked.
+  var finalURL = getServerURL('echoheader?User-Agent&Accept&X-New-Header');
+  var url = getServerURL('server-redirect?' + finalURL);
+  var listener = callbackPass(function(details) {
+    var headers = details.requestHeaders;
+
+    // Test modification.
+    var accept_value;
+    for (var i = 0; i < headers.length; i++) {
+      if (headers[i].name.toLowerCase() === 'user-agent') {
+        headers[i].value = 'foo';
+      } else if (headers[i].name.toLowerCase() === 'accept') {
+        accept_value = headers[i].value;
+      }
+    }
+
+    // Test removal.
+    chrome.test.assertTrue(accept_value.indexOf('image/webp') >= 0);
+    removeHeader(headers, 'accept');
+
+    // Test addition.
+    headers.push({name: 'X-New-Header', value: 'Baz'});
+
+    return {requestHeaders: headers};
+  });
+
+  var extraInfo = ['requestHeaders', 'blocking'];
+  if (useExtraHeaders)
+    extraInfo.push('extraHeaders');
+  chrome.webRequest.onBeforeSendHeaders.addListener(listener,
+      {urls: [finalURL]}, extraInfo);
+
+  navigateAndWait(url, function(tab) {
+    chrome.webRequest.onBeforeSendHeaders.removeListener(listener);
+    chrome.tabs.executeScript(tab.id, {
+      code: 'document.body.innerText'
+    }, callbackPass(function(results) {
+      chrome.test.assertTrue(results[0].indexOf('foo') >= 0,
+          'User-Agent should be modified.');
+        chrome.test.assertTrue(results[0].indexOf('image/webp') == -1,
+          'Accept should be removed.');
+      chrome.test.assertTrue(results[0].indexOf('Baz') >= 0,
+          'X-New-Header should be added.');
+    }));
+  });
+}
+
 runTests([
   function testSpecialRequestHeadersVisible() {
     // Set a cookie so the cookie request header is set.
@@ -200,6 +250,14 @@ runTests([
     });
   },
 
+  function testModifyHeadersOnRedirectWithoutExtraHeaders() {
+    testModifyHeadersOnRedirect(false);
+  },
+
+  function testModifyHeadersOnRedirectWithExtraHeaders() {
+    testModifyHeadersOnRedirect(true);
+  },
+
   // Successful Set-Cookie modification is tested in test_blocking_cookie.js.
   function testCannotModifySpecialResponseHeadersWithoutExtraHeaders() {
     // Use unique name and value so other tests don't interfere.

extensions/browser/api/web_request/web_request_api.cc

@@ -852,6 +852,20 @@ struct ExtensionWebRequestEventRouter::BlockedRequest {
   // The callback to call when we get a response from all event handlers.
   net::CompletionOnceCallback callback;
 
+  // The callback to invoke for onBeforeSendHeaders. If
+  // |before_send_headers_callback.is_null()| is false, |callback| must be NULL.
+  // Only valid for OnBeforeSendHeaders.
+  BeforeSendHeadersCallback before_send_headers_callback;
+
+  // The callback to invoke for auth. If |auth_callback.is_null()| is false,
+  // |callback| must be NULL.
+  // Only valid for OnAuthRequired.
+  net::NetworkDelegate::AuthCallback auth_callback;
+
+  // If non-empty, this contains the auth credentials that may be filled in.
+  // Only valid for OnAuthRequired.
+  net::AuthCredentials* auth_credentials = nullptr;
+
   // If non-empty, this contains the new URL that the request will redirect to.
   // Only valid for OnBeforeRequest and OnHeadersReceived.
   GURL* new_url = nullptr;
@@ -868,15 +882,6 @@ struct ExtensionWebRequestEventRouter::BlockedRequest {
   // OnHeadersReceived.
   scoped_refptr<net::HttpResponseHeaders>* override_response_headers = nullptr;
 
-  // If non-empty, this contains the auth credentials that may be filled in.
-  // Only valid for OnAuthRequired.
-  net::AuthCredentials* auth_credentials = nullptr;
-
-  // The callback to invoke for auth. If |auth_callback.is_null()| is false,
-  // |callback| must be NULL.
-  // Only valid for OnAuthRequired.
-  net::NetworkDelegate::AuthCallback auth_callback;
-
   // Time the request was paused. Used for logging purposes.
   base::Time blocking_time;
 
@@ -1092,7 +1097,7 @@ int ExtensionWebRequestEventRouter::OnBeforeSendHeaders(
     void* browser_context,
     const InfoMap* extension_info_map,
     const WebRequestInfo* request,
-    net::CompletionOnceCallback callback,
+    BeforeSendHeadersCallback callback,
     net::HttpRequestHeaders* headers) {
   if (ShouldHideEvent(browser_context, extension_info_map, *request))
     return net::OK;
@@ -1130,7 +1135,7 @@ int ExtensionWebRequestEventRouter::OnBeforeSendHeaders(
   blocked_request.event = kOnBeforeSendHeaders;
   blocked_request.is_incognito |= IsIncognitoBrowserContext(browser_context);
   blocked_request.request = request;
-  blocked_request.callback = std::move(callback);
+  blocked_request.before_send_headers_callback = std::move(callback);
   blocked_request.request_headers = headers;
 
   if (blocked_request.num_handlers_blocking == 0) {
@@ -2215,6 +2220,9 @@ int ExtensionWebRequestEventRouter::ExecuteDeltas(void* browser_context,
   bool request_headers_modified = false;
   bool response_headers_modified = false;
   bool credentials_set = false;
+  // The set of request headers which were removed or set to new values.
+  std::set<std::string> request_headers_removed;
+  std::set<std::string> request_headers_set;
 
   deltas.sort(&helpers::InDecreasingExtensionInstallationTimeOrder);
 
@@ -2229,11 +2237,12 @@ int ExtensionWebRequestEventRouter::ExecuteDeltas(void* browser_context,
         request->url, blocked_request.response_deltas, blocked_request.new_url,
         &ignored_actions, request->logger.get());
   } else if (blocked_request.event == kOnBeforeSendHeaders) {
-    CHECK(!blocked_request.callback.is_null());
+    CHECK(!blocked_request.before_send_headers_callback.is_null());
     helpers::MergeOnBeforeSendHeadersResponses(
         request->url, blocked_request.response_deltas,
         blocked_request.request_headers, &ignored_actions,
-        request->logger.get(), &request_headers_modified);
+        request->logger.get(), &request_headers_removed, &request_headers_set,
+        &request_headers_modified);
   } else if (blocked_request.event == kOnHeadersReceived) {
     CHECK(!blocked_request.callback.is_null());
     helpers::MergeOnHeadersReceivedResponses(
@@ -2293,6 +2302,13 @@ int ExtensionWebRequestEventRouter::ExecuteDeltas(void* browser_context,
     blocked_requests_.erase(request->id);
     if (call_callback)
       std::move(callback).Run(rv);
+  } else if (!blocked_request.before_send_headers_callback.is_null()) {
+    auto callback = std::move(blocked_request.before_send_headers_callback);
+    // Ensure that request is removed before callback because the callback
+    // might trigger the next event.
+    blocked_requests_.erase(request->id);
+    if (call_callback)
+      std::move(callback).Run(request_headers_removed, request_headers_set, rv);
   } else if (!blocked_request.auth_callback.is_null()) {
     net::NetworkDelegate::AuthRequiredResponse response;
     if (canceled)

extensions/browser/api/web_request/web_request_api.h

@@ -359,6 +359,11 @@ class ExtensionWebRequestEventRouter {
                       GURL* new_url,
                       bool* should_collapse_initiator);
 
+  using BeforeSendHeadersCallback =
+      base::OnceCallback<void(const std::set<std::string>& removed_headers,
+                              const std::set<std::string>& set_headers,
+                              int error_code)>;
+
   // Dispatches the onBeforeSendHeaders event. This is fired for HTTP(s)
   // requests only, and allows modification of the outgoing request headers.
   // Returns net::ERR_IO_PENDING if an extension is intercepting the request, OK
@@ -366,7 +371,7 @@ class ExtensionWebRequestEventRouter {
   int OnBeforeSendHeaders(void* browser_context,
                           const extensions::InfoMap* extension_info_map,
                           const WebRequestInfo* request,
-                          net::CompletionOnceCallback callback,
+                          BeforeSendHeadersCallback callback,
                           net::HttpRequestHeaders* headers);
 
   // Dispatches the onSendHeaders event. This is fired for HTTP(s) requests

extensions/browser/api/web_request/web_request_api_helpers.cc

@@ -709,15 +709,14 @@ void MergeOnBeforeSendHeadersResponses(
     net::HttpRequestHeaders* request_headers,
     IgnoredActions* ignored_actions,
     extensions::WebRequestInfo::Logger* logger,
+    std::set<std::string>* removed_headers,
+    std::set<std::string>* set_headers,
     bool* request_headers_modified) {
   DCHECK(request_headers_modified);
+  DCHECK(removed_headers->empty());
+  DCHECK(set_headers->empty());
   *request_headers_modified = false;
 
-  // Here we collect which headers we have removed or set to new values
-  // so far due to extensions of higher precedence.
-  std::set<std::string> removed_headers;
-  std::set<std::string> set_headers;
-
   // We assume here that the deltas are sorted in decreasing extension
   // precedence (i.e. decreasing extension installation time).
   for (const auto& delta : deltas) {
@@ -739,14 +738,14 @@ void MergeOnBeforeSendHeadersResponses(
         const std::string& value = modification.value();
 
         // We must not delete anything that has been modified before.
-        if (removed_headers.find(key) != removed_headers.end() &&
+        if (removed_headers->find(key) != removed_headers->end() &&
             !extension_conflicts) {
           extension_conflicts = true;
         }
 
         // We must not modify anything that has been set to a *different*
         // value before.
-        if (set_headers.find(key) != set_headers.end() &&
+        if (set_headers->find(key) != set_headers->end() &&
             !extension_conflicts) {
           std::string current_value;
           if (!request_headers->GetHeader(key, &current_value) ||
@@ -763,7 +762,7 @@ void MergeOnBeforeSendHeadersResponses(
       for (auto key = delta.deleted_request_headers.begin();
            key != delta.deleted_request_headers.end() && !extension_conflicts;
            ++key) {
-        if (set_headers.find(*key) != set_headers.end()) {
+        if (set_headers->find(*key) != set_headers->end()) {
           std::string current_value;
           request_headers->GetHeader(*key, &current_value);
           extension_conflicts = true;
@@ -780,14 +779,14 @@ void MergeOnBeforeSendHeadersResponses(
         net::HttpRequestHeaders::Iterator modification(
             delta.modified_request_headers);
         while (modification.GetNext())
-          set_headers.insert(modification.name());
+          set_headers->insert(modification.name());
       }
 
       // Perform all deletions and record which keys were deleted.
       {
         for (const auto& header : delta.deleted_request_headers) {
           request_headers->RemoveHeader(header);
-          removed_headers.insert(header);
+          removed_headers->insert(header);
         }
       }
       logger->LogEvent(net::NetLogEventType::CHROME_EXTENSION_MODIFIED_HEADERS,
@@ -815,7 +814,7 @@ void MergeOnBeforeSendHeadersResponses(
           {"referer", WebRequestSpecialRequestHeaderModification::kReferer},
       };
   int special_headers_removed = 0;
-  for (const auto& header : removed_headers) {
+  for (const auto& header : *removed_headers) {
     auto it = kHeaderMap.find(base::ToLowerASCII(header));
     if (it != kHeaderMap.end()) {
       special_headers_removed++;
@@ -831,7 +830,7 @@ void MergeOnBeforeSendHeadersResponses(
   }
 
   int special_headers_changed = 0;
-  for (const auto& header : set_headers) {
+  for (const auto& header : *set_headers) {
     auto it = kHeaderMap.find(base::ToLowerASCII(header));
     if (it != kHeaderMap.end()) {
       special_headers_changed++;

extensions/browser/api/web_request/web_request_api_helpers.h

@@ -332,6 +332,8 @@ void MergeOnBeforeSendHeadersResponses(
     net::HttpRequestHeaders* request_headers,
     IgnoredActions* ignored_actions,
     extensions::WebRequestInfo::Logger* logger,
+    std::set<std::string>* removed_headers,
+    std::set<std::string>* set_headers,
     bool* request_headers_modified);
 // Modifies the "Set-Cookie" headers in |override_response_headers| according to
 // |deltas.response_cookie_modifications|. If |override_response_headers| is

extensions/browser/api/web_request/web_request_proxying_url_loader_factory.cc

@@ -26,6 +26,11 @@
 
 namespace extensions {
 
+WebRequestProxyingURLLoaderFactory::InProgressRequest::FollowRedirectParams::
+    FollowRedirectParams() = default;
+WebRequestProxyingURLLoaderFactory::InProgressRequest::FollowRedirectParams::
+    ~FollowRedirectParams() = default;
+
 WebRequestProxyingURLLoaderFactory::InProgressRequest::InProgressRequest(
     WebRequestProxyingURLLoaderFactory* factory,
     uint64_t request_id,
@@ -80,7 +85,12 @@ WebRequestProxyingURLLoaderFactory::InProgressRequest::~InProgressRequest() {
 }
 
 void WebRequestProxyingURLLoaderFactory::InProgressRequest::Restart() {
-  request_completed_ = false;
+  UpdateRequestInfo();
+  RestartInternal();
+}
+
+void WebRequestProxyingURLLoaderFactory::InProgressRequest::
+    UpdateRequestInfo() {
   // Derive a new WebRequestInfo value any time |Restart()| is called, because
   // the details in |request_| may have changed e.g. if we've been redirected.
   // |request_initiator| can be modified on redirects, but we keep the original
@@ -101,6 +111,12 @@ void WebRequestProxyingURLLoaderFactory::InProgressRequest::Restart() {
       ExtensionWebRequestEventRouter::GetInstance()
           ->HasExtraHeadersListenerForRequest(
               factory_->browser_context_, factory_->info_map_, &info_.value());
+}
+
+void WebRequestProxyingURLLoaderFactory::InProgressRequest::RestartInternal() {
+  DCHECK_EQ(info_->url, request_.url)
+      << "UpdateRequestInfo must have been called first";
+  request_completed_ = false;
 
   // If the header client will be used, we start the request immediately, and
   // OnBeforeSendHeaders and OnSendHeaders will be handled there. Otherwise,
@@ -162,11 +178,29 @@ void WebRequestProxyingURLLoaderFactory::InProgressRequest::FollowRedirect(
     request_.headers.RemoveHeader(header);
   request_.headers.MergeFrom(modified_headers);
 
+  // Call this before checking |current_request_uses_header_client_| as it
+  // calculates it.
+  UpdateRequestInfo();
+
   if (target_loader_.is_bound()) {
-    target_loader_->FollowRedirect(removed_headers, modified_headers, new_url);
+    // If header_client_ is used, then we have to call FollowRedirect now as
+    // that's what triggers the network service calling back to
+    // OnBeforeSendHeaders(). Otherwise, don't call FollowRedirect now. Wait for
+    // the onBeforeSendHeaders callback(s) to run as these may modify request
+    // headers and if so we'll pass these modifications to FollowRedirect.
+    if (current_request_uses_header_client_) {
+      target_loader_->FollowRedirect(removed_headers, modified_headers,
+                                     new_url);
+    } else {
+      auto params = std::make_unique<FollowRedirectParams>();
+      params->removed_headers = removed_headers;
+      params->modified_headers = modified_headers;
+      params->new_url = new_url;
+      pending_follow_redirect_params_ = std::move(params);
+    }
   }
 
-  Restart();
+  RestartInternal();
 }
 
 void WebRequestProxyingURLLoaderFactory::InProgressRequest::
@@ -447,7 +481,8 @@ void WebRequestProxyingURLLoaderFactory::InProgressRequest::
     DCHECK_EQ(net::OK, result);
   }
 
-  ContinueToSendHeaders(net::OK);
+  ContinueToSendHeaders(std::set<std::string>(), std::set<std::string>(),
+                        net::OK);
 }
 
 void WebRequestProxyingURLLoaderFactory::InProgressRequest::
@@ -490,7 +525,9 @@ void WebRequestProxyingURLLoaderFactory::InProgressRequest::
 }
 
 void WebRequestProxyingURLLoaderFactory::InProgressRequest::
-    ContinueToSendHeaders(int error_code) {
+    ContinueToSendHeaders(const std::set<std::string>& removed_headers,
+                          const std::set<std::string>& set_headers,
+                          int error_code) {
   if (error_code != net::OK) {
     OnRequestError(network::URLLoaderCompletionStatus(error_code));
     return;
@@ -500,6 +537,29 @@ void WebRequestProxyingURLLoaderFactory::InProgressRequest::
     DCHECK(on_before_send_headers_callback_);
     std::move(on_before_send_headers_callback_)
         .Run(error_code, request_.headers);
+  } else if (pending_follow_redirect_params_) {
+    pending_follow_redirect_params_->removed_headers.insert(
+        pending_follow_redirect_params_->removed_headers.end(),
+        removed_headers.begin(), removed_headers.end());
+
+    for (auto& set_header : set_headers) {
+      std::string header_value;
+      if (request_.headers.GetHeader(set_header, &header_value)) {
+        pending_follow_redirect_params_->modified_headers.SetHeader(
+            set_header, header_value);
+      } else {
+        NOTREACHED();
+      }
+    }
+
+    if (target_loader_.is_bound()) {
+      target_loader_->FollowRedirect(
+          pending_follow_redirect_params_->removed_headers,
+          pending_follow_redirect_params_->modified_headers,
+          pending_follow_redirect_params_->new_url);
+    }
+
+    pending_follow_redirect_params_.reset();
   }
 
   if (proxied_client_binding_.is_bound())

extensions/browser/api/web_request/web_request_proxying_url_loader_factory.h

@@ -103,8 +103,14 @@ class WebRequestProxyingURLLoaderFactory
                            OnHeadersReceivedCallback callback) override;
 
    private:
+    // These two methods combined form the implementation of Restart().
+    void UpdateRequestInfo();
+    void RestartInternal();
+
     void ContinueToBeforeSendHeaders(int error_code);
-    void ContinueToSendHeaders(int error_code);
+    void ContinueToSendHeaders(const std::set<std::string>& removed_headers,
+                               const std::set<std::string>& set_headers,
+                               int error_code);
     void ContinueToStartRequest(int error_code);
     void ContinueToHandleOverrideHeaders(int error_code);
     void ContinueToResponseStarted(int error_code);
@@ -168,6 +174,21 @@ class WebRequestProxyingURLLoaderFactory
     OnHeadersReceivedCallback on_headers_received_callback_;
     mojo::Binding<network::mojom::TrustedHeaderClient> header_client_binding_;
 
+    // If |has_any_extra_headers_listeners_| is set to false and a redirect is
+    // in progress, this stores the parameters to FollowRedirect that came from
+    // the client. That way we can combine it with any other changes that
+    // extensions made to headers in their callbacks.
+    struct FollowRedirectParams {
+      FollowRedirectParams();
+      ~FollowRedirectParams();
+      std::vector<std::string> removed_headers;
+      net::HttpRequestHeaders modified_headers;
+      base::Optional<GURL> new_url;
+
+      DISALLOW_COPY_AND_ASSIGN(FollowRedirectParams);
+    };
+    std::unique_ptr<FollowRedirectParams> pending_follow_redirect_params_;
+
     base::WeakPtrFactory<InProgressRequest> weak_factory_;
 
     DISALLOW_COPY_AND_ASSIGN(InProgressRequest);

extensions/browser/api/web_request/web_request_proxying_websocket.cc

@@ -379,10 +379,14 @@ void WebRequestProxyingWebSocket::OnBeforeRequestComplete(int error_code) {
     return;
 
   DCHECK_EQ(net::OK, result);
-  OnBeforeSendHeadersComplete(net::OK);
+  OnBeforeSendHeadersComplete(std::set<std::string>(), std::set<std::string>(),
+                              net::OK);
 }
 
-void WebRequestProxyingWebSocket::OnBeforeSendHeadersComplete(int error_code) {
+void WebRequestProxyingWebSocket::OnBeforeSendHeadersComplete(
+    const std::set<std::string>& removed_headers,
+    const std::set<std::string>& set_headers,
+    int error_code) {
   DCHECK(binding_as_header_client_ || !binding_as_client_.is_bound());
   if (error_code != net::OK) {
     OnError(error_code);

extensions/browser/api/web_request/web_request_proxying_websocket.h

@@ -112,7 +112,9 @@ class WebRequestProxyingWebSocket
 
  private:
   void OnBeforeRequestComplete(int error_code);
-  void OnBeforeSendHeadersComplete(int error_code);
+  void OnBeforeSendHeadersComplete(const std::set<std::string>& removed_headers,
+                                   const std::set<std::string>& set_headers,
+                                   int error_code);
   void ContinueToStartRequest(int error_code);
   void OnHeadersReceivedComplete(int error_code);
   void ContinueToHeadersReceived();

extensions/shell/browser/shell_network_delegate.cc

@@ -70,13 +70,22 @@ int ShellNetworkDelegate::OnBeforeURLRequest(
   return result;
 }
 
+namespace {
+void OnHeadersReceivedAdapter(net::CompletionOnceCallback callback,
+                              const std::set<std::string>& removed_headers,
+                              const std::set<std::string>& set_headers,
+                              int error_code) {
+  std::move(callback).Run(error_code);
+}
+}  // namespace
+
 int ShellNetworkDelegate::OnBeforeStartTransaction(
     net::URLRequest* request,
     net::CompletionOnceCallback callback,
     net::HttpRequestHeaders* headers) {
   return ExtensionWebRequestEventRouter::GetInstance()->OnBeforeSendHeaders(
       browser_context_, extension_info_map_.get(), GetWebRequestInfo(request),
-      std::move(callback), headers);
+      base::BindOnce(OnHeadersReceivedAdapter, std::move(callback)), headers);
 }
 
 void ShellNetworkDelegate::OnStartTransaction(