I started a CL that does this but ended up dropping it on the floor. Thank you for taking on this work! In fact iterating through the headers can be quite expensive so this will be a perf win.

This is probably best owned by navigation experts, as I wasn't entirely sure which code was only used for navigations.

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

commit 4246472c00afcf4f7e7c55317d6988d158e83e11
Author: lukasza <lukasza@chromium.org>
Date: Tue Oct 18 15:43:42 2016

LoadURLParams::extra_headers should not apply to subresource requests.

Extra http request headers specified in
NavigationController::LoadURLParams should only apply to the *navigation*
request trigerred by NavigationController::LoadURLWithParams method and
should NOT apply to http requests for *subresources*.

BUG=656179
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_site_isolation

Review-Url: https://codereview.chromium.org/2429623002
Cr-Commit-Position: refs/heads/master@{#425972}

[modify] https://crrev.com/4246472c00afcf4f7e7c55317d6988d158e83e11/content/browser/frame_host/navigation_controller_impl_browsertest.cc
[modify] https://crrev.com/4246472c00afcf4f7e7c55317d6988d158e83e11/content/renderer/render_frame_impl.cc

Status update after chatting with creis@:

- Having navigation-specific and subresource-specific helper methods (instead of everything being handled inside a big willStartRequest) sounds good, but seems low priority (because with PlzNavigate the navigational requests won't go through RenderFrameImpl::willStartRequest anyway, right? +clamy@).

- transferred_request_child_id and transferred_request_request_id 1) should be applied only to navigational requests in RenderFrameImpl::willStartRequest and 2) we should double-check on the browser-side - before calling CompleteTransfer in ResourceDispatcherHostImpl::BeginRequest.  Ideally we will want to add some kind of test coverage here, but this might be tricky given that this requires a race.  +nick@ in case this is somehow related to  issue 655114 .

- ui::PageTransition - unclear if this should only apply to navigational requests.  This piece of data ends up being inspected in ResourceRequestPolicy::CanRequestResource (but then this is only called from ChromeExtensionsRendererClient::WillSendRequest if extensions are enabled?).

- allow_download - unclear what this is - might need more investigation.


csharrison@ - would you still have the CL draft you mentioned in #c1 (and/or notes on which things should only apply to navigational requests)?

The CL is here:
https://chromiumcodereview.appspot.com/2356093002

Note: this was a rush job and I didn't end up finishing. Take it with a grain of salt (also, tests fail...).

Note: ui::PageTransition: This is needed for subresource requests, unfortunately. To see callers check  ResourceRequestInfo{Impl}::GetPageTransition. I'm not sure about allow_download (maybe needed for <a download>?).

Doing a bit of refactoring in ResourceRequests and I noticed that the call to addHTTPOriginIfNeeded should only be for main frame navigation requests, as we add the header in ResourceFetcher::addAdditionalRequestHeaders for all requests that are not for the main resource.

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

commit 1f59c2aac5f896e23613af618fbe8444a63911f8
Author: lukasza <lukasza@chromium.org>
Date: Tue Oct 25 22:21:50 2016

Extra browser-side validation of transferred_request_child_id / request_id.

BUG=656179

Review-Url: https://codereview.chromium.org/2442793002
Cr-Commit-Position: refs/heads/master@{#427510}

[modify] https://crrev.com/1f59c2aac5f896e23613af618fbe8444a63911f8/content/browser/bad_message.h
[modify] https://crrev.com/1f59c2aac5f896e23613af618fbe8444a63911f8/content/browser/loader/resource_dispatcher_host_impl.cc
[modify] https://crrev.com/1f59c2aac5f896e23613af618fbe8444a63911f8/content/browser/loader/resource_dispatcher_host_impl.h
[modify] https://crrev.com/1f59c2aac5f896e23613af618fbe8444a63911f8/content/renderer/render_frame_impl.cc

I don't have any more immediate plans for changes related to this (to ensuring that navigational "things" are not used by RenderFrameImpl::willStartRequest for non-navigational (e.g. subresource) requests.  Therefore - marking the bug as available.

This issue has been Available for over a year. If it's no longer important or seems unlikely to be fixed, please consider closing it out. If it is important, please re-triage the issue.

Sorry for the inconvenience if the bug really should have been left as Available. If you change it back, also remove the "Hotlist-Recharge-Cold" label.

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot

This might be a moot point as we remove the old navigation path, so tagging with PlzNavigate to ensure we follow up on this.

Instead of moving the navigation code to another method, we can just delete it :)