Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(181)

Issue 2053693002: WIP: Move 'Upgrade-Insecure-Requests' to the browser process.

Created:
4 years, 6 months ago by Mike West
Modified:
3 years, 6 months ago
CC:
arthursonzogni, blink-reviews, blink-reviews-api_chromium.org, cbentzel+watch_chromium.org, chromium-reviews, clamy, creis+watch_chromium.org, darin-cc_chromium.org, dglazkov+blink, estark, gavinp+loader_chromium.org, jam, Nate Chapin, loading-reviews_chromium.org, nasko+codewatch_chromium.org, Randy Smith (Not in Mondays), tyoshino+watch_chromium.org, wjmaclean, Yoav Weiss
Base URL:
https://chromium.googlesource.com/chromium/src.git@replicate
Target Ref:
refs/pending/heads/master
Project:
chromium
Visibility:
Public.

Description

WIP: Move 'Upgrade-Insecure-Requests' to the browser process. This is more or less a sanity check to see what mmenke@ thinks of this approach. :) It needs more tests in //content, and some general cleanup of the ductwork between Blink and //net before actual review. BUG=615885

Patch Set 1 #

Total comments: 6

Patch Set 2 : Folding into URLRequestHTTPJob. #

Patch Set 3 : Rebase. #

Patch Set 4 : Punt on blink-side checks. #

Patch Set 5 : Not sure I like this. #

Total comments: 2

Patch Set 6 : Just redirects. #

Total comments: 2

Patch Set 7 : DCHECK. #

Total comments: 15

Patch Set 8 : Rebase. :( #

Unified diffs Side-by-side diffs Delta from patch set Stats (+338 lines, -139 lines) Patch
M content/browser/loader/resource_dispatcher_host_impl.cc View 1 2 3 4 5 6 7 2 chunks +18 lines, -0 lines 0 comments Download
M content/child/web_url_loader_impl.cc View 1 2 3 4 5 6 7 1 chunk +2 lines, -0 lines 0 comments Download
M content/common/resource_messages.h View 1 2 3 4 5 6 7 1 chunk +1 line, -0 lines 0 comments Download
M content/common/resource_request.h View 1 2 3 4 5 6 7 2 chunks +6 lines, -0 lines 0 comments Download
M net/log/net_log_event_type_list.h View 1 2 3 4 5 6 7 1 chunk +7 lines, -0 lines 0 comments Download
M net/url_request/url_request.h View 1 2 3 4 5 4 chunks +31 lines, -0 lines 0 comments Download
M net/url_request/url_request.cc View 1 2 3 4 5 6 7 2 chunks +11 lines, -0 lines 0 comments Download
M net/url_request/url_request_http_job.h View 1 2 3 4 5 1 chunk +2 lines, -0 lines 0 comments Download
M net/url_request/url_request_http_job.cc View 1 2 3 4 5 6 7 1 chunk +35 lines, -0 lines 0 comments Download
M net/url_request/url_request_job.h View 1 2 3 4 5 2 chunks +5 lines, -4 lines 0 comments Download
M net/url_request/url_request_unittest.cc View 1 2 3 4 5 6 7 1 chunk +157 lines, -0 lines 0 comments Download
M third_party/WebKit/LayoutTests/NeverFixTests View 1 2 3 4 5 6 7 1 chunk +0 lines, -1 line 0 comments Download
M third_party/WebKit/LayoutTests/W3CImportExpectations View 1 2 3 4 5 6 7 1 chunk +2 lines, -0 lines 0 comments Download
A third_party/WebKit/LayoutTests/external/wpt/upgrade-insecure-requests/websocket-upgrade.https-expected.txt View 1 2 3 4 5 6 7 1 chunk +11 lines, -0 lines 0 comments Download
D third_party/WebKit/LayoutTests/http/tests/security/upgrade-insecure-requests/basic-upgrade.https.html View 1 2 1 chunk +0 lines, -56 lines 0 comments Download
M third_party/WebKit/LayoutTests/http/tests/security/upgrade-insecure-requests/iframe-upgrade.https.html View 1 2 3 4 5 6 7 1 chunk +0 lines, -75 lines 0 comments Download
M third_party/WebKit/Source/core/loader/FrameFetchContext.h View 1 2 3 4 5 6 7 1 chunk +2 lines, -0 lines 0 comments Download
M third_party/WebKit/Source/core/loader/FrameFetchContext.cpp View 1 2 3 4 5 6 7 1 chunk +7 lines, -0 lines 0 comments Download
M third_party/WebKit/Source/core/loader/FrameFetchContextTest.cpp View 1 2 3 4 5 6 7 1 chunk +2 lines, -0 lines 0 comments Download
M third_party/WebKit/Source/core/loader/FrameLoader.cpp View 1 2 3 4 5 6 7 1 chunk +2 lines, -0 lines 0 comments Download
M third_party/WebKit/Source/platform/exported/WebURLRequest.cpp View 1 2 3 4 5 6 7 1 chunk +4 lines, -0 lines 0 comments Download
M third_party/WebKit/Source/platform/loader/fetch/FetchContext.h View 1 2 3 4 5 6 7 2 chunks +5 lines, -0 lines 0 comments Download
M third_party/WebKit/Source/platform/network/ResourceRequest.h View 1 2 3 4 5 6 7 5 chunks +13 lines, -1 line 0 comments Download
M third_party/WebKit/Source/platform/network/ResourceRequest.cpp View 1 2 3 4 5 6 7 3 chunks +3 lines, -0 lines 0 comments Download
M third_party/WebKit/Source/platform/network/ResourceRequestTest.cpp View 1 2 3 4 5 6 7 4 chunks +5 lines, -1 line 0 comments Download
M third_party/WebKit/public/platform/WebURLRequest.h View 1 2 3 4 5 6 7 2 chunks +7 lines, -1 line 0 comments Download

Messages

Total messages: 105 (57 generated)
Mike West
mmenke@: Is this anything like what you had in mind? I've put together a tiny ...
4 years, 6 months ago (2016-06-09 13:57:34 UTC) #3
mmenke
https://codereview.chromium.org/2053693002/diff/1/content/browser/loader/resource_dispatcher_host_impl.cc File content/browser/loader/resource_dispatcher_host_impl.cc (right): https://codereview.chromium.org/2053693002/diff/1/content/browser/loader/resource_dispatcher_host_impl.cc#newcode1476 content/browser/loader/resource_dispatcher_host_impl.cc:1476: net::URLRequest::UPGRADE_ALL_INSECURE_REQUESTS); Does this have to be a member of ...
4 years, 6 months ago (2016-06-09 18:56:56 UTC) #4
mmenke
Let me think about this a bit - unfortunately, I haven't had the time to ...
4 years, 6 months ago (2016-06-09 18:59:04 UTC) #5
mmenke
On 2016/06/09 18:59:04, mmenke wrote: > Let me think about this a bit - unfortunately, ...
4 years, 6 months ago (2016-06-09 18:59:49 UTC) #6
Mike West
On 2016/06/09 at 18:59:04, mmenke wrote: > Let me think about this a bit - ...
4 years, 6 months ago (2016-06-10 09:55:09 UTC) #7
Mike West
https://codereview.chromium.org/2053693002/diff/1/content/browser/loader/resource_dispatcher_host_impl.cc File content/browser/loader/resource_dispatcher_host_impl.cc (right): https://codereview.chromium.org/2053693002/diff/1/content/browser/loader/resource_dispatcher_host_impl.cc#newcode1476 content/browser/loader/resource_dispatcher_host_impl.cc:1476: net::URLRequest::UPGRADE_ALL_INSECURE_REQUESTS); On 2016/06/09 at 18:56:55, mmenke wrote: > 1) ...
4 years, 6 months ago (2016-06-10 09:55:16 UTC) #8
mmenke
On 2016/06/10 09:55:09, Mike West wrote: > On 2016/06/09 at 18:59:04, mmenke wrote: > > ...
4 years, 6 months ago (2016-06-10 15:14:50 UTC) #9
mmenke
On 2016/06/10 15:14:50, mmenke wrote: > On 2016/06/10 09:55:09, Mike West wrote: > > On ...
4 years, 6 months ago (2016-06-10 16:33:06 UTC) #10
mmenke
On 2016/06/10 16:33:06, mmenke wrote: > On 2016/06/10 15:14:50, mmenke wrote: > > On 2016/06/10 ...
4 years, 6 months ago (2016-06-10 16:44:41 UTC) #11
mmenke
On 2016/06/10 16:44:41, mmenke wrote: > On 2016/06/10 16:33:06, mmenke wrote: > > On 2016/06/10 ...
4 years, 6 months ago (2016-06-10 16:48:23 UTC) #12
Mike West
On 2016/06/10 at 16:33:06, mmenke wrote: > On 2016/06/10 15:14:50, mmenke wrote: > > On ...
4 years, 6 months ago (2016-06-10 17:35:22 UTC) #13
mmenke
On 2016/06/10 17:35:22, Mike West wrote: > On 2016/06/10 at 16:33:06, mmenke wrote: > > ...
4 years, 6 months ago (2016-06-10 18:18:21 UTC) #14
mmenke
On 2016/06/10 18:18:21, mmenke wrote: > On 2016/06/10 17:35:22, Mike West wrote: > > On ...
4 years, 6 months ago (2016-06-10 18:21:14 UTC) #15
mmenke
On 2016/06/10 18:21:14, mmenke wrote: > On 2016/06/10 18:18:21, mmenke wrote: > > On 2016/06/10 ...
4 years, 6 months ago (2016-06-10 18:24:30 UTC) #16
mmenke
Sorry, forgot about this one. In my queue for tomorrow, after I do my simpler ...
4 years, 6 months ago (2016-06-13 21:05:51 UTC) #17
mmenke
Not a huge fan of the idea, but yea, I think the way to go ...
4 years, 6 months ago (2016-06-14 18:49:31 UTC) #18
mmenke
On 2016/06/14 18:49:31, mmenke wrote: > Not a huge fan of the idea, but yea, ...
4 years, 6 months ago (2016-06-14 18:50:27 UTC) #19
Mike West
On 2016/06/14 at 18:50:27, mmenke wrote: > On 2016/06/14 18:49:31, mmenke wrote: > Sorry, forgot ...
4 years, 6 months ago (2016-06-16 11:59:53 UTC) #20
mmenke
On 2016/06/16 11:59:53, Mike West (OOO - BlinkOn) wrote: > On 2016/06/14 at 18:50:27, mmenke ...
4 years, 6 months ago (2016-06-16 15:31:52 UTC) #22
Mike West
Hello, friendly mmenke@. :) I'm resurrecting this patch. Skimming through the comments, you suggested that ...
4 years ago (2016-12-06 14:57:34 UTC) #28
arthursonzogni
+CC myself (arthursonzogni) I write almost the same code 1 month ago without knowing you ...
4 years ago (2016-12-06 15:55:39 UTC) #30
mmenke
On 2016/12/06 14:57:34, Mike West (slow) wrote: > Hello, friendly mmenke@. :) Friendly? I think ...
4 years ago (2016-12-06 16:03:28 UTC) #31
Mike West
On 2016/12/06 at 16:03:28, mmenke wrote: > On 2016/12/06 14:57:34, Mike West (slow) wrote: > ...
4 years ago (2016-12-06 16:31:15 UTC) #32
mmenke
On 2016/12/06 16:31:15, Mike West (slow) wrote: > On 2016/12/06 at 16:03:28, mmenke wrote: > ...
4 years ago (2016-12-06 16:39:38 UTC) #33
mmenke
On 2016/12/06 16:39:38, mmenke wrote: > On 2016/12/06 16:31:15, Mike West (slow) wrote: > > ...
4 years ago (2016-12-06 16:40:38 UTC) #34
Mike West
On 2016/12/06 at 15:55:39, arthursonzogni wrote: > +CC myself (arthursonzogni) > > I write almost ...
4 years ago (2016-12-06 16:44:53 UTC) #35
Mike West
On 2016/12/06 at 16:39:38, mmenke wrote: > On 2016/12/06 16:31:15, Mike West (slow) wrote: > ...
4 years ago (2016-12-06 17:01:10 UTC) #36
mmenke
On 2016/12/06 17:01:10, Mike West (slow) wrote: > On 2016/12/06 at 16:39:38, mmenke wrote: > ...
4 years ago (2016-12-06 17:07:47 UTC) #37
Mike West
On 2016/12/06 at 17:07:47, mmenke wrote: > Ok, so this is all to avoid the ...
4 years ago (2016-12-06 17:39:10 UTC) #38
mmenke
On 2016/12/06 17:39:10, Mike West (slow) wrote: > On 2016/12/06 at 17:07:47, mmenke wrote: > ...
4 years ago (2016-12-06 17:46:31 UTC) #39
Mike West
On 2016/12/06 at 17:46:31, mmenke wrote: > This is behavior that was added for appcache, ...
4 years ago (2016-12-06 18:04:02 UTC) #40
Mike West
On 2016/12/06 at 18:04:02, Mike West (slow) wrote: > On 2016/12/06 at 17:46:31, mmenke wrote: ...
4 years ago (2016-12-08 05:42:19 UTC) #41
mmenke
On 2016/12/08 05:42:19, Mike West (slow) wrote: > On 2016/12/06 at 18:04:02, Mike West (slow) ...
4 years ago (2016-12-08 17:45:35 UTC) #42
Mike West
On 2016/12/08 at 17:45:35, mmenke wrote: > Layers like to do weird things. Extensions do ...
4 years ago (2016-12-08 20:56:42 UTC) #43
mmenke
On 2016/12/08 20:56:42, Mike West (slow) wrote: > On 2016/12/08 at 17:45:35, mmenke wrote: > ...
4 years ago (2016-12-08 21:15:22 UTC) #44
Mike West
The latest patchset moves the upgrade back to URLHttpRequestJob, lumping UIR and HSTS together. As ...
4 years ago (2016-12-09 15:12:22 UTC) #49
mmenke
On 2016/12/09 15:12:22, Mike West (slow) wrote: > The latest patchset moves the upgrade back ...
4 years ago (2016-12-09 15:20:19 UTC) #54
mmenke
On 2016/12/09 15:20:19, mmenke wrote: > On 2016/12/09 15:12:22, Mike West (slow) wrote: > > ...
4 years ago (2016-12-09 15:20:59 UTC) #55
Mike West
I took a stab at rewriting things in URLRequestHttpJob instead of in Blink. I'm not ...
4 years ago (2016-12-13 15:18:06 UTC) #64
mmenke
Hrm...yea, I think that the RewriteURL is pretty ugly. I prefer it to the Interceptor ...
4 years ago (2016-12-13 19:00:24 UTC) #65
Mike West
On 2016/12/13 at 19:00:24, mmenke wrote: > Hrm...yea, I think that the RewriteURL is pretty ...
4 years ago (2016-12-14 06:48:32 UTC) #66
Mike West
WDYT of the latest patchset, mmenke@?
4 years ago (2016-12-14 12:18:21 UTC) #71
mmenke
From a net/ standpoint, this seems reasonable to me. This seems pretty similar to what ...
4 years ago (2016-12-14 19:41:05 UTC) #72
Mike West
On 2016/12/14 at 19:41:05, mmenke wrote: > From a net/ standpoint, this seems reasonable to ...
4 years ago (2016-12-15 07:41:18 UTC) #78
jochen (gone - plz use gerrit)
content/ lgtm
4 years ago (2016-12-15 14:21:54 UTC) #83
mmenke
Bunch of mostly minor comments https://codereview.chromium.org/2053693002/diff/120001/content/browser/loader/resource_dispatcher_host_impl.cc File content/browser/loader/resource_dispatcher_host_impl.cc (right): https://codereview.chromium.org/2053693002/diff/120001/content/browser/loader/resource_dispatcher_host_impl.cc#newcode1450 content/browser/loader/resource_dispatcher_host_impl.cc:1450: // If the initiating ...
4 years ago (2016-12-15 19:24:23 UTC) #84
estark
https://codereview.chromium.org/2053693002/diff/120001/net/url_request/url_request_http_job.cc File net/url_request/url_request_http_job.cc (right): https://codereview.chromium.org/2053693002/diff/120001/net/url_request/url_request_http_job.cc#newcode1130 net/url_request/url_request_http_job.cc:1130: RedirectInfo URLRequestHttpJob::ComputeRedirectInfo(const GURL& location, Why is this logic in ...
4 years ago (2016-12-15 20:49:54 UTC) #85
mmenke
3 years, 6 months ago (2017-06-12 15:07:52 UTC) #103
Removing myself from this CL.  Feel free to add me back if you start working on
it again, I have like 20 dead CLs on my pending review list.  :(

Powered by Google App Engine
This is Rietveld 408576698