Options for fixes include:

 1. Having GetEffectiveURLForInstant() clear the port when remapping the scheme.
 2. registering chrome-search:// as SCHEME_WITH_PORT 
 3. Having GURL clear the ports for schemes that are SCHEME_WITHOUT_PORT.

If #1 works, it seems preferable, but I don't yet know if losing the port will break tests.

Adding Instant folks who might be able to provide background on the chrome-search:// scheme and its portlessness.

I don't recall any particular reason it was set to SCHEME_WITHOUT_PORT. My guess would be that we didn't expect to ever use ports (which is the case outside tests).

Given that, option 2 seems OK to me. I have no idea though if there are any security implications of this, so it may be worth running it by creis@ or someone.

#1 seems to be working in practice. Since ports in the chrome-search:// scheme is a test-only situation, and the tests seem to be happy, I say let's do that. https://codereview.chromium.org/1911573002/

samarth: Stripping out the port means we have a problem if we ever need to transform the chrome-search:// URL back to a meaningful https:// URL. Can you confirm that this doesn't happen (if it does, we apparently don't have test coverage)?

I'm not aware of any problems with using SCHEME_WITH_PORT instead.  It seems like a better fit in this case (where there's an actual network host inside the URL) compared to kExtensionScheme or kChromeNativeScheme.  I'd recommend #2.

Not sure about #1-- does the NTP logic ever pull the host out of the chrome-search:// URL?  If so, we shouldn't do that.

#3 seems like it could be good for consistency (to avoid cases where using Origin leads to different behavior than using GURL).  That seems like a separate issue, since it could uncover other bugs like this.

nick: maybe I'm confused but I was proposing #2 where we register chrome-search:// with SCHEME_WITH_PORT, which shouldn't require any munging of the URL.  That seems like the cleanest solution.

#3: When I introduced SchemeWithType to address some Origin issue, I tried to give right value for each of existing registered schemes, but some might be wrong.

https://chromium.googlesource.com/chromium/src/+/11a7c9fe93850341043844ab48bf379c485d05b1%5E%21/chrome/common/chrome_content_client.cc

So, please feel free to fix it.

So it seems that switching to SCHEME_WITH_PORT causes equally bad problems, and doesn't work. What happens is, we start failing origin extraction for chrome-search:// URLs like "chrome-search://local-ntp/" that lack ports -- GURL::EffectiveIntPort returns UNSPECIFIED, which is treated as an error by SchemeHostPort when SCHEME_WITH_PORT is expected.

GURL doesn't have a mechanism to specify a default port for a custom scheme like chrome-search:// . If it did, I guess we would set it to 443, under the assumption that chrome-search can be reverse-mapped to https.

I think it's a much better soluion to simply clear the port. Even if we preserved the port when switching the scheme to chrome-search, we've lost the original scheme (http vs https) so a reverse mapping isn't possible without knowing the original search provider URL, to determine whether to use http or https.

Interesting. SGTM.

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

commit 1dd47922979b128ce7ee8debf138b83c65661024
Author: nick <nick@chromium.org>
Date: Fri Apr 29 16:44:48 2016

Teach SiteInstance::GetSiteForURL() about blob and filesystem URLs.

Use url::Origin to do the heavy lifting.

This fixes FrameTreeBrowsertest.NavigateGrandchildToBlob under --site-per-process, and fixes process transfers for blob URLs in general.

url::Origin extraction exposed a bug where tests encountered chrome-search:// URLs with ports. This is fixed by clearing the port.

BUG= 602818 , 564316 ,490074, 605720 

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

[modify] https://crrev.com/1dd47922979b128ce7ee8debf138b83c65661024/chrome/browser/search/search.cc
[modify] https://crrev.com/1dd47922979b128ce7ee8debf138b83c65661024/chrome/browser/search/search_unittest.cc
[modify] https://crrev.com/1dd47922979b128ce7ee8debf138b83c65661024/content/browser/site_instance_impl.cc
[modify] https://crrev.com/1dd47922979b128ce7ee8debf138b83c65661024/content/browser/site_instance_impl_unittest.cc
[modify] https://crrev.com/1dd47922979b128ce7ee8debf138b83c65661024/testing/buildbot/filters/site-per-process.content_browsertests.filter