chrome-search: should not be display-isolated

chrome/renderer/chrome_content_renderer_client.cc

Index: chrome/renderer/chrome_content_renderer_client.cc
diff --git a/chrome/renderer/chrome_content_renderer_client.cc b/chrome/renderer/chrome_content_renderer_client.cc
index b7814592f2b28dab52ffb6676546c2535c18f6f7..efadf3f5bd1ef748b9d0b327cec93e8174a71244 100644
--- a/chrome/renderer/chrome_content_renderer_client.cc
+++ b/chrome/renderer/chrome_content_renderer_client.cc
@@ -248,7 +248,8 @@ void ChromeContentRendererClient::RenderThreadStarted() {
   WebSecurityPolicy::registerURLSchemeAsDisplayIsolated(chrome_ui_scheme);
 
   WebString chrome_search_scheme(ASCIIToUTF16(chrome::kChromeSearchScheme));
-  WebSecurityPolicy::registerURLSchemeAsDisplayIsolated(chrome_search_scheme);
+  if (!command_line->HasSwitch(switches::kInstantProcess))

[Charlie Reis, 2013/03/21 00:08:17]

Why would we register this (or the one above) at all in a non-instant process?

[dhollowa, 2013/03/21 01:30:19]

The one above, chrome:, is linked from external places I suppose, like help
pages.  But has nothing to do with Instant.

The registering of the chrome-search: protocol in general is a somewhat global
affair, ProfileIOData::IsHandledProtocol for example, so is cumbersome at best
to avoid.

A cleaner approach is to use this display-isolated setting here to disallow
access except from chrome-search: and Instant pages.

To reiterate, a non-Instant process, when trying to access
chrome-search://something, will bump up against the following:

  1. Renderer: the display-isolated check in WebKit, so will be denied,
  2. Browser: (Assuming they got by #1), the scheme checks in
URLDataSource::ShouldServiceRequest,
  3. Browser: for specific sub-classes of URLDataSource, like ThemeSource there
are additional Insant-PID checks that make sure the request is coming from a
blessed Instant process.

So with all this, I believe we're good.

[Charlie Reis, 2013/03/21 18:06:42]

My mistake; I was misreading that.
Thanks for the explanation.  I don't think it's obvious from reading this code,
so could you add a comment here explaining that the Instant process can *only*
display the content (not read it) and other processes can't display it or read
it.

I think it would also be worthwhile to add some of this context to the
kChromeSearchScheme declaration in url_constants.h, to avoid confusion about how
we expect this scheme to be handled.

+    WebSecurityPolicy::registerURLSchemeAsDisplayIsolated(chrome_search_scheme);
 
   WebString dev_tools_scheme(ASCIIToUTF16(chrome::kChromeDevToolsScheme));
   WebSecurityPolicy::registerURLSchemeAsDisplayIsolated(dev_tools_scheme);