This makes sense to me.  Matt, you see any problems?

Alex, you willing to take this on?  The change looks pretty simple.

Works for me.  Note that as of https://chromium-review.googlesource.com/c/chromium/src/+/654143, NavigationURLLoaderNetworkService needs to be updated correspondingly as well.  :(

Yes, we don't navigate with PUTs, but I have a nagging suspicion we'll end up needing a single utility class/method to of handling redirects that need us to switch URLLoaders

(Alex, were you planning on putting together a change here?)

Sorry for the radio silence, two different sheriffing rotations have aligned this week so I've been pretty distracted.  Yes I'm planning on writing a patch.  

Thanks!

I've been digging a bit into the history of the internal test and the RFCs and I'm beginning to think changing this might be a mistake :)

Specifically https://www.w3.org/Protocols/rfc2616/rfc2616-sec10.html sugegsts
notes that "some existing HTTP/1.0 user agents will erroneously change it into a GET request".  It doesn't mention PUT.

The other spec https://tools.ietf.org/html/rfc7231#section-6.4.2 mentions

      Note: For historical reasons, a user agent MAY change the request
      method from POST to GET for the subsequent request.  If this
      behavior is undesired, the 307 (Temporary Redirect) status code
      can be used instead. 

It doesn't mention PUT either.

The internal patch circa 2016 claimed it was making a system follow Chrome's behaviour in converting POST -> GET and PUT also.  Chrome turns a POST into a GET but doing a little code spelunking* I don't think it's ever turned a PUT into a get.  I suspect the patch author was confused.

I think this is probably a WontFix but I'll leave it over the weekend in case somebody knows better. 

alexclarke:  303s unconditionally convert non-HEAD requests to GETs and remove the body.  301s and 302s only change the method for POSTs, but that is not the case for 303s.

See https://cs.chromium.org/chromium/src/net/url_request/url_request_job.cc?type=cs&q=file:src/net++303+redirect&sq=package:chromium&l=51

Interesting. I'll double check the internal test (it's testing just that) but failing for some reason.

Looks like I managed to completely confuse myself in #7

Anyway I have a patch in progress which I'll send assuming it passes the presubmit.

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

commit 1e08882b3b35589c01534dcca07cef1d605dca85
Author: Alex Clarke <alexclarke@chromium.org>
Date: Fri Oct 06 14:22:40 2017

When any redirect changes HTTP method we remove the Origin header

The Origin header is sent on anything that is not a GET or HEAD, which
suggests all redirects that change methods (since they always change to
GET) should drop the Origin header.

Bug:  762954 
Change-Id: I1d8c91a391b455315e278cdfd5c8c5fef681c2b2
Reviewed-on: https://chromium-review.googlesource.com/678505
Commit-Queue: Alex Clarke <alexclarke@chromium.org>
Reviewed-by: Matt Menke <mmenke@chromium.org>
Reviewed-by: Kinuko Yasuda <kinuko@chromium.org>
Cr-Commit-Position: refs/heads/master@{#507057}
[modify] https://crrev.com/1e08882b3b35589c01534dcca07cef1d605dca85/content/browser/loader/navigation_url_loader.cc
[modify] https://crrev.com/1e08882b3b35589c01534dcca07cef1d605dca85/content/browser/loader/navigation_url_loader_network_service.cc
[modify] https://crrev.com/1e08882b3b35589c01534dcca07cef1d605dca85/content/browser/loader/navigation_url_loader_network_service.h
[add] https://crrev.com/1e08882b3b35589c01534dcca07cef1d605dca85/content/browser/loader/navigation_url_loader_network_service_unittest.cc
[modify] https://crrev.com/1e08882b3b35589c01534dcca07cef1d605dca85/content/test/BUILD.gn
[add] https://crrev.com/1e08882b3b35589c01534dcca07cef1d605dca85/content/test/data/redirect301-to-echo
[add] https://crrev.com/1e08882b3b35589c01534dcca07cef1d605dca85/content/test/data/redirect301-to-echo.mock-http-headers
[add] https://crrev.com/1e08882b3b35589c01534dcca07cef1d605dca85/content/test/data/redirect301-to-https
[add] https://crrev.com/1e08882b3b35589c01534dcca07cef1d605dca85/content/test/data/redirect301-to-https.mock-http-headers
[add] https://crrev.com/1e08882b3b35589c01534dcca07cef1d605dca85/content/test/data/redirect302-to-echo
[add] https://crrev.com/1e08882b3b35589c01534dcca07cef1d605dca85/content/test/data/redirect302-to-echo.mock-http-headers
[add] https://crrev.com/1e08882b3b35589c01534dcca07cef1d605dca85/content/test/data/redirect302-to-https
[add] https://crrev.com/1e08882b3b35589c01534dcca07cef1d605dca85/content/test/data/redirect302-to-https.mock-http-headers
[add] https://crrev.com/1e08882b3b35589c01534dcca07cef1d605dca85/content/test/data/redirect303-to-echo
[add] https://crrev.com/1e08882b3b35589c01534dcca07cef1d605dca85/content/test/data/redirect303-to-echo.mock-http-headers
[add] https://crrev.com/1e08882b3b35589c01534dcca07cef1d605dca85/content/test/data/redirect303-to-https
[add] https://crrev.com/1e08882b3b35589c01534dcca07cef1d605dca85/content/test/data/redirect303-to-https.mock-http-headers
[add] https://crrev.com/1e08882b3b35589c01534dcca07cef1d605dca85/content/test/data/redirect307-to-echo
[add] https://crrev.com/1e08882b3b35589c01534dcca07cef1d605dca85/content/test/data/redirect307-to-echo.mock-http-headers
[add] https://crrev.com/1e08882b3b35589c01534dcca07cef1d605dca85/content/test/data/redirect307-to-https
[add] https://crrev.com/1e08882b3b35589c01534dcca07cef1d605dca85/content/test/data/redirect307-to-https.mock-http-headers
[add] https://crrev.com/1e08882b3b35589c01534dcca07cef1d605dca85/content/test/data/redirect308-to-echo
[add] https://crrev.com/1e08882b3b35589c01534dcca07cef1d605dca85/content/test/data/redirect308-to-echo.mock-http-headers
[add] https://crrev.com/1e08882b3b35589c01534dcca07cef1d605dca85/content/test/data/redirect308-to-https
[add] https://crrev.com/1e08882b3b35589c01534dcca07cef1d605dca85/content/test/data/redirect308-to-https.mock-http-headers
[modify] https://crrev.com/1e08882b3b35589c01534dcca07cef1d605dca85/net/url_request/url_request.cc
[modify] https://crrev.com/1e08882b3b35589c01534dcca07cef1d605dca85/net/url_request/url_request_unittest.cc