New issue
Advanced search Search tips

Issue 911105 link

Starred by 2 users

Issue metadata

Status: Fixed
Owner:
Closed: Jan 7
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Linux , Android , Windows , Chrome , Mac , Fuchsia
Pri: 3
Type: Task
Proj-Servicification

Blocking:
issue 900594



Sign in to add a comment

Complete the implementation of URLLoader::FollowRedirect()

Project Member Reported by arthurso...@chromium.org, Dec 3

Issue description

To be able to add or remove headers on a redirect, one can uses:
URLLoader::FollowRedirect(removed_headers, modified_headers).

This API is not implemented by all URLLoader / ResourceLoader implementations. Some code paths are just using:
~~~
    DCHECK(!modified_request_headers.has_value())
        << "Redirect with modified headers was not supported yet. "
           " crbug.com/845683 ";
~~~
Most code paths aren't forwarding |removed_headers| either.

The goal is to make modifying / removing headers available from the NavigationRequest, which uses URLLoader == NavigationURLLoader.

This CL tracks CLs made to achieve this goal.
 
Components: -Internals>Network Internals>Services>Network
Blocking: 882370
Labels: OS-Android OS-Chrome OS-Fuchsia OS-Linux OS-Mac OS-Windows
I _think_ this applies to all platforms except iOS, but please correct me if that's not the case.
Blocking: -882370 900594
Project Member

Comment 5 by bugdroid1@chromium.org, Jan 4

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/b8465ff7fb2bb3ad71dca8fe5eb1360f8c450010

commit b8465ff7fb2bb3ad71dca8fe5eb1360f8c450010
Author: Arthur Sonzogni <arthursonzogni@chromium.org>
Date: Fri Jan 04 18:44:35 2019

Increase scope of URLLoader::FollowRedirect() API implementation.

URLLoader::FollowRedirect() takes two arguments: |removed_headers| and
|modified_headers|. This is partially implemented. This CL is essentially
plumbing to improve support for this two arguments.

Notable changes:

1) |modified_headers| was exposed from content/public, now
   |removed_headers| is exposed as well. Tests are added.

2) In particular, NavigationURLLoader and ThrottlingURLLoader now
   supports |removed_headers|.

TBR=mef@chromium.org,scottmg@chromium.org

Bug:  911105 
Change-Id: I46916d98a3900c331b25bd75eb51c21aef3e1533
Reviewed-on: https://chromium-review.googlesource.com/c/1369856
Commit-Queue: Arthur Sonzogni <arthursonzogni@chromium.org>
Reviewed-by: Misha Efimov <mef@chromium.org>
Reviewed-by: Camille Lamy <clamy@chromium.org>
Reviewed-by: Matt Menke <mmenke@chromium.org>
Cr-Commit-Position: refs/heads/master@{#620011}
[modify] https://crrev.com/b8465ff7fb2bb3ad71dca8fe5eb1360f8c450010/chrome/browser/chrome_content_browser_client.cc
[modify] https://crrev.com/b8465ff7fb2bb3ad71dca8fe5eb1360f8c450010/chrome/browser/chrome_content_browser_client.h
[modify] https://crrev.com/b8465ff7fb2bb3ad71dca8fe5eb1360f8c450010/components/cronet/cronet_url_request.cc
[modify] https://crrev.com/b8465ff7fb2bb3ad71dca8fe5eb1360f8c450010/content/browser/appcache/appcache_url_loader_request.cc
[modify] https://crrev.com/b8465ff7fb2bb3ad71dca8fe5eb1360f8c450010/content/browser/devtools/devtools_url_loader_interceptor.cc
[modify] https://crrev.com/b8465ff7fb2bb3ad71dca8fe5eb1360f8c450010/content/browser/loader/cross_site_document_resource_handler.cc
[modify] https://crrev.com/b8465ff7fb2bb3ad71dca8fe5eb1360f8c450010/content/browser/loader/detachable_resource_handler.cc
[modify] https://crrev.com/b8465ff7fb2bb3ad71dca8fe5eb1360f8c450010/content/browser/loader/intercepting_resource_handler.cc
[modify] https://crrev.com/b8465ff7fb2bb3ad71dca8fe5eb1360f8c450010/content/browser/loader/mime_sniffing_resource_handler.cc
[modify] https://crrev.com/b8465ff7fb2bb3ad71dca8fe5eb1360f8c450010/content/browser/loader/mock_resource_loader.cc
[modify] https://crrev.com/b8465ff7fb2bb3ad71dca8fe5eb1360f8c450010/content/browser/loader/mojo_async_resource_handler.cc
[modify] https://crrev.com/b8465ff7fb2bb3ad71dca8fe5eb1360f8c450010/content/browser/loader/mojo_async_resource_handler.h
[modify] https://crrev.com/b8465ff7fb2bb3ad71dca8fe5eb1360f8c450010/content/browser/loader/navigation_url_loader_impl.cc
[modify] https://crrev.com/b8465ff7fb2bb3ad71dca8fe5eb1360f8c450010/content/browser/loader/null_resource_controller.cc
[modify] https://crrev.com/b8465ff7fb2bb3ad71dca8fe5eb1360f8c450010/content/browser/loader/null_resource_controller.h
[modify] https://crrev.com/b8465ff7fb2bb3ad71dca8fe5eb1360f8c450010/content/browser/loader/resource_controller.h
[modify] https://crrev.com/b8465ff7fb2bb3ad71dca8fe5eb1360f8c450010/content/browser/loader/resource_handler.cc
[modify] https://crrev.com/b8465ff7fb2bb3ad71dca8fe5eb1360f8c450010/content/browser/loader/resource_handler.h
[modify] https://crrev.com/b8465ff7fb2bb3ad71dca8fe5eb1360f8c450010/content/browser/loader/resource_loader.cc
[modify] https://crrev.com/b8465ff7fb2bb3ad71dca8fe5eb1360f8c450010/content/browser/loader/resource_loader.h
[modify] https://crrev.com/b8465ff7fb2bb3ad71dca8fe5eb1360f8c450010/content/browser/worker_host/worker_script_fetcher.cc
[modify] https://crrev.com/b8465ff7fb2bb3ad71dca8fe5eb1360f8c450010/content/browser/worker_host/worker_script_loader.cc
[modify] https://crrev.com/b8465ff7fb2bb3ad71dca8fe5eb1360f8c450010/content/common/throttling_url_loader.cc
[modify] https://crrev.com/b8465ff7fb2bb3ad71dca8fe5eb1360f8c450010/content/common/throttling_url_loader.h
[modify] https://crrev.com/b8465ff7fb2bb3ad71dca8fe5eb1360f8c450010/content/common/throttling_url_loader_unittest.cc
[modify] https://crrev.com/b8465ff7fb2bb3ad71dca8fe5eb1360f8c450010/content/public/browser/content_browser_client.h
[modify] https://crrev.com/b8465ff7fb2bb3ad71dca8fe5eb1360f8c450010/content/renderer/loader/resource_dispatcher.cc
[modify] https://crrev.com/b8465ff7fb2bb3ad71dca8fe5eb1360f8c450010/content/renderer/service_worker/service_worker_subresource_loader.cc
[modify] https://crrev.com/b8465ff7fb2bb3ad71dca8fe5eb1360f8c450010/content/renderer/service_worker/service_worker_subresource_loader.h
[modify] https://crrev.com/b8465ff7fb2bb3ad71dca8fe5eb1360f8c450010/net/spdy/spdy_network_transaction_unittest.cc
[modify] https://crrev.com/b8465ff7fb2bb3ad71dca8fe5eb1360f8c450010/net/url_request/redirect_util.cc
[modify] https://crrev.com/b8465ff7fb2bb3ad71dca8fe5eb1360f8c450010/net/url_request/redirect_util.h
[modify] https://crrev.com/b8465ff7fb2bb3ad71dca8fe5eb1360f8c450010/net/url_request/redirect_util_unittest.cc
[modify] https://crrev.com/b8465ff7fb2bb3ad71dca8fe5eb1360f8c450010/net/url_request/url_request.cc
[modify] https://crrev.com/b8465ff7fb2bb3ad71dca8fe5eb1360f8c450010/net/url_request/url_request.h
[modify] https://crrev.com/b8465ff7fb2bb3ad71dca8fe5eb1360f8c450010/net/url_request/url_request_job.cc
[modify] https://crrev.com/b8465ff7fb2bb3ad71dca8fe5eb1360f8c450010/net/url_request/url_request_job.h
[modify] https://crrev.com/b8465ff7fb2bb3ad71dca8fe5eb1360f8c450010/net/url_request/url_request_unittest.cc
[modify] https://crrev.com/b8465ff7fb2bb3ad71dca8fe5eb1360f8c450010/services/network/url_loader.cc
[modify] https://crrev.com/b8465ff7fb2bb3ad71dca8fe5eb1360f8c450010/webrunner/net_http/url_loader_impl.cc

Status: Fixed (was: Assigned)

Sign in to add a comment