Important: my current implementation of browser side mixed content checks uses static lists that contain all defaults (see the *Schemes() methods in ShemeRegistry.cpp [3]) and the extras registered by Chromium that I could find through codesearch.

I'd appreciate feedback if this is good enough for a first *live* iteration of this work.

[1] https://cs.chromium.org/search/?q=static+URLSchemesSet%5C%26+.*Schemes+file:SchemeRegistry.cpp&sq=package:chromium&type=cs

When/if https://crrev.com/1905033002 lands there will be a few TODOs added there to the newly added mixed_content_navigation_throttle.cc file that are affected by this issue.

We're also going to have to find ways to enable //content's embedder to make changes (like //extensions registering the `chrome-extension` scheme).

Pulling this up out of Blink directly is a good idea, and I think we can make something work, but we'll need to be sure to catch these cases as well.

I looked into this and one solution seems to be putting it in src/url. That is reachable by both blink & content, and it already has lists of standard & referrer schemes. So we could add other sets of schemes to it. Chatted with Brett in hallway and he's fine with it.

I was planning on implementing something like this but didn't see the bug, please keep me in the loop here!

Keep in mind that the SchemeRegistry, and the similar //url version of Standard/Referrer schemes are performance intensive for resource loading. I've put some effort into optimizing both in the past few months.

Adding Blink>Loader as the SchemeRegistry is used all the time in Loader code.

Some more subtle performance things to note:
1. SchemeRegistry *always* operates on canonicalized schemes, whereas the //url code operates on both (but should probably get a fast path for canonicalized schemes).

2. KURL schemes are now AtomicStrings, so their hash is already pre-computed and cached, and the SchemeRegistry is just a HashMap. Note that "httpAtom" and "httpsAtom" are also AtomicStrings, and comparing to them is just a constant pointer comparison. I was thinking of making a change to the SchemeRegistry to do that as a fast path for some of the maps.

3. The code in //url uses a vector of char *s, allocated on the heap. I was planning on moving the code in //url to use a vector of StringPieces backed by a single std::string [1] but I haven't polished up the CL yet. Note that I mentioned in the CL using this in Blink, but it was before the AtomicString change so I'm not sure if this would be more performant.

[1] https://codereview.chromium.org/2492753002/ 

Also +kinuko/esprehn who have been influential around SchemeRegistry/KURL changes here.

Thanks for the comments. To clarify, I'm not planning on moving the logic from SchemeRegistry to /url. Instead, for the few shared lists of schemes needed between the browser process and blink, url/ will hold the initial list.

The current status quo:
-content/common and chrome/common modify url's standard schemes
-content/renderer and chrome/renderer add various types schemes to blink's default lists

Proposed changes:
-(first part is unchanged)
-content/common and chrome/common add their specific types of scheme changes to /url. Blink reads this list at initialization and use it in SchemeRegistry. Similarly, content/browser has access to it for browser-side mixed content check with PlzNavigate.

Ah I understand now. We are duplicating the actual strings for set membership in //url that are currently in SchemeRegistry, not duplicating code/logic.

SGTM, though it would be nice to get to a future where we don't have duplicated logic for this. TBH I'm not sure what a future would look like though.

The goal is to not have any one place in code that duplicates the list of which schemes are considered secure, allow CORS and restrict mixed content. Today, these lists are a mixture of hard coded schemes that blink knows about (i.e. https, data) but then content and chrome in the renderer process add more schemes to them.

The simplest way to get mixed content to work with PlzNavigate is to duplicate these lists of schemes in the browser process. That would be bad for obvious reasons.

So by having chrome and content layers store this data in /url, and then have content/browser reach for it for PlzNavigate, and also Blink reach to it for SchemeRegistry, we avoid the duplication. You're right that this data would be stored twice in memory. But given that this is in total on the order of a dozen small strings, that cost isn't going to matter.

#10 sounds good and I have no objections.

(Charles and I chatted about this, but for others on the bug)

oh and re in the future having one place that has logic as well; agreed that sounds like a good goal. I didn't go down this path at this point for the following reasons:
-I think doing this properly will take a lot of design, i.e. it would be strange if some schemes from SchemeRegistry are there but not the rest. So we'll probably want to move them all
-but then does it make sense that all of them are in /url?
-differences in string types and containers etc...

Since this seemed like it could big a non-trivial project, the goal was to do what's necessary now to ship PlzNavigate without adding duplication.

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

commit e0dcd98dbb8363f64857345c317da57cde0588b3
Author: jam <jam@chromium.org>
Date: Wed Jan 11 03:13:45 2017

Cleanup of static lists of schemes & origins that are created at startup.

This in preparation for using this method to grab other schemes that we need to now share across the browser and renderer for PlzNavigate. This change does:
-combines AddAdditionalSchemes/AddSecureSchemesAndOrigins/AddServiceWorkerSchemes
-standardizes on std::string

BUG= 627502 

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

[modify] https://crrev.com/e0dcd98dbb8363f64857345c317da57cde0588b3/chrome/browser/chrome_content_browser_client.cc
[modify] https://crrev.com/e0dcd98dbb8363f64857345c317da57cde0588b3/chrome/common/BUILD.gn
[modify] https://crrev.com/e0dcd98dbb8363f64857345c317da57cde0588b3/chrome/common/chrome_content_client.cc
[modify] https://crrev.com/e0dcd98dbb8363f64857345c317da57cde0588b3/chrome/common/chrome_content_client.h
[delete] https://crrev.com/cc1cb9fbd275d9b119ffdaf31d7bec56cb5b4d8d/chrome/common/chrome_content_client_ios.mm
[modify] https://crrev.com/e0dcd98dbb8363f64857345c317da57cde0588b3/chrome/common/secure_origin_whitelist.cc
[modify] https://crrev.com/e0dcd98dbb8363f64857345c317da57cde0588b3/chrome/common/secure_origin_whitelist.h
[modify] https://crrev.com/e0dcd98dbb8363f64857345c317da57cde0588b3/chrome/renderer/chrome_content_renderer_client.cc
[modify] https://crrev.com/e0dcd98dbb8363f64857345c317da57cde0588b3/chromecast/common/cast_content_client.cc
[modify] https://crrev.com/e0dcd98dbb8363f64857345c317da57cde0588b3/chromecast/common/cast_content_client.h
[modify] https://crrev.com/e0dcd98dbb8363f64857345c317da57cde0588b3/components/test/components_test_suite.cc
[modify] https://crrev.com/e0dcd98dbb8363f64857345c317da57cde0588b3/content/browser/service_worker/service_worker_provider_host_unittest.cc
[modify] https://crrev.com/e0dcd98dbb8363f64857345c317da57cde0588b3/content/common/BUILD.gn
[modify] https://crrev.com/e0dcd98dbb8363f64857345c317da57cde0588b3/content/common/origin_util.cc
[delete] https://crrev.com/cc1cb9fbd275d9b119ffdaf31d7bec56cb5b4d8d/content/common/savable_url_schemes.cc
[delete] https://crrev.com/cc1cb9fbd275d9b119ffdaf31d7bec56cb5b4d8d/content/common/savable_url_schemes.h
[modify] https://crrev.com/e0dcd98dbb8363f64857345c317da57cde0588b3/content/common/url_schemes.cc
[modify] https://crrev.com/e0dcd98dbb8363f64857345c317da57cde0588b3/content/common/url_schemes.h
[modify] https://crrev.com/e0dcd98dbb8363f64857345c317da57cde0588b3/content/public/common/content_client.cc
[modify] https://crrev.com/e0dcd98dbb8363f64857345c317da57cde0588b3/content/public/common/content_client.h
[modify] https://crrev.com/e0dcd98dbb8363f64857345c317da57cde0588b3/content/public/common/url_utils.cc
[modify] https://crrev.com/e0dcd98dbb8363f64857345c317da57cde0588b3/content/public/common/url_utils.h
[modify] https://crrev.com/e0dcd98dbb8363f64857345c317da57cde0588b3/content/public/test/content_test_suite_base.h
[modify] https://crrev.com/e0dcd98dbb8363f64857345c317da57cde0588b3/content/renderer/render_frame_impl.cc
[modify] https://crrev.com/e0dcd98dbb8363f64857345c317da57cde0588b3/content/renderer/savable_resources.cc
[modify] https://crrev.com/e0dcd98dbb8363f64857345c317da57cde0588b3/content/renderer/savable_resources.h
[modify] https://crrev.com/e0dcd98dbb8363f64857345c317da57cde0588b3/content/renderer/savable_resources_browsertest.cc
[modify] https://crrev.com/e0dcd98dbb8363f64857345c317da57cde0588b3/extensions/shell/common/shell_content_client.cc
[modify] https://crrev.com/e0dcd98dbb8363f64857345c317da57cde0588b3/extensions/shell/common/shell_content_client.h
[modify] https://crrev.com/e0dcd98dbb8363f64857345c317da57cde0588b3/extensions/test/extensions_unittests_main.cc

Mind the comment in measureStricterVersionOfIsMixedContent (MixedContentChecker.cpp) [1]. IIUC, if we ever get to support secure schemes registered with SchemeRegistry::registerURLSchemeAsSecure we might need to also support dynamically registered ones as I understand extensions are capable of doing so.




[1] https://cs.chromium.org/chromium/src/third_party/WebKit/Source/core/loader/MixedContentChecker.cpp?q="measureStricterVersionOfIsMixedContent"&dr=C

@carlosk: not sure what you mean re dynamically registered schemes by extensions? extensions don't get to mark schemes as secure).

all the places that call registerURLSchemeAsSecure are on process startup. I don't see any that run afterwards. Is that what you're referring to?

If nothing can be done by extensions themselves then you can ignore my comment. I understood it wrong. Thanks and sorry for the noise.

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

commit 0901535a93c8bdf5d9a67edc8085beb2786888b3
Author: jam <jam@chromium.org>
Date: Thu Jan 19 01:49:02 2017

Share schemes needed for mixed content checking between the browser and renderer.

At process startup, content/chrome/android_webview add their schemes to the default set in url/. Then Blink and content/browser can grab the list from there for their usage. This enforces that the same scheme list is used for browser side navigation mixed content checks as when they're running in Blink.

BUG= 627502 

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

[modify] https://crrev.com/0901535a93c8bdf5d9a67edc8085beb2786888b3/android_webview/common/aw_content_client.cc
[modify] https://crrev.com/0901535a93c8bdf5d9a67edc8085beb2786888b3/android_webview/common/aw_content_client.h
[modify] https://crrev.com/0901535a93c8bdf5d9a67edc8085beb2786888b3/android_webview/renderer/aw_content_renderer_client.cc
[modify] https://crrev.com/0901535a93c8bdf5d9a67edc8085beb2786888b3/chrome/common/chrome_content_client.cc
[modify] https://crrev.com/0901535a93c8bdf5d9a67edc8085beb2786888b3/chrome/common/secure_origin_whitelist_unittest.cc
[modify] https://crrev.com/0901535a93c8bdf5d9a67edc8085beb2786888b3/chrome/renderer/chrome_content_renderer_client.cc
[modify] https://crrev.com/0901535a93c8bdf5d9a67edc8085beb2786888b3/chrome/renderer/chrome_render_thread_observer.cc
[modify] https://crrev.com/0901535a93c8bdf5d9a67edc8085beb2786888b3/content/browser/service_worker/service_worker_provider_host_unittest.cc
[modify] https://crrev.com/0901535a93c8bdf5d9a67edc8085beb2786888b3/content/common/origin_util.cc
[modify] https://crrev.com/0901535a93c8bdf5d9a67edc8085beb2786888b3/content/common/url_schemes.cc
[modify] https://crrev.com/0901535a93c8bdf5d9a67edc8085beb2786888b3/content/common/url_schemes.h
[modify] https://crrev.com/0901535a93c8bdf5d9a67edc8085beb2786888b3/content/public/common/content_client.h
[modify] https://crrev.com/0901535a93c8bdf5d9a67edc8085beb2786888b3/content/public/common/origin_util.h
[modify] https://crrev.com/0901535a93c8bdf5d9a67edc8085beb2786888b3/content/public/test/test_utils.cc
[modify] https://crrev.com/0901535a93c8bdf5d9a67edc8085beb2786888b3/content/public/test/test_utils.h
[modify] https://crrev.com/0901535a93c8bdf5d9a67edc8085beb2786888b3/content/renderer/render_thread_impl.cc
[modify] https://crrev.com/0901535a93c8bdf5d9a67edc8085beb2786888b3/extensions/renderer/dispatcher.cc
[modify] https://crrev.com/0901535a93c8bdf5d9a67edc8085beb2786888b3/extensions/shell/common/shell_content_client.cc
[modify] https://crrev.com/0901535a93c8bdf5d9a67edc8085beb2786888b3/third_party/WebKit/Source/platform/weborigin/SchemeRegistry.cpp
[modify] https://crrev.com/0901535a93c8bdf5d9a67edc8085beb2786888b3/third_party/WebKit/Source/web/WebSecurityPolicy.cpp
[modify] https://crrev.com/0901535a93c8bdf5d9a67edc8085beb2786888b3/third_party/WebKit/public/web/WebSecurityPolicy.h
[modify] https://crrev.com/0901535a93c8bdf5d9a67edc8085beb2786888b3/url/url_util.cc
[modify] https://crrev.com/0901535a93c8bdf5d9a67edc8085beb2786888b3/url/url_util.h

Apologies, applied the wrong component in bulk.