Fix app process initialization when opening an app in a new tab.

chrome/browser/ui/webui/chrome_web_ui_factory.cc

 // Copyright (c) 2011 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "chrome/browser/ui/webui/chrome_web_ui_factory.h"
 
 #include "base/command_line.h"
 #include "chrome/browser/about_flags.h"
 #include "chrome/browser/extensions/extension_service.h"
 #include "chrome/browser/extensions/extension_web_ui.h"
@@ skipping 13 matching lines @@
 #include "chrome/browser/ui/webui/net_internals_ui.h"
 #include "chrome/browser/ui/webui/ntp/new_tab_ui.h"
 #include "chrome/browser/ui/webui/options/options_ui.h"
 #include "chrome/browser/ui/webui/plugins_ui.h"
 #include "chrome/browser/ui/webui/print_preview_ui.h"
 #include "chrome/browser/ui/webui/sync_internals_ui.h"
 #include "chrome/browser/ui/webui/textfields_ui.h"
 #include "chrome/common/chrome_switches.h"
 #include "chrome/common/extensions/extension_constants.h"
 #include "chrome/common/url_constants.h"
+#include "content/browser/content_browser_client.h"
 #include "content/browser/tab_contents/tab_contents.h"
 #include "content/browser/webui/web_ui.h"
+#include "content/common/content_client.h"
 #include "googleurl/src/gurl.h"
 
 #if defined(OS_CHROMEOS)
 #include "chrome/browser/ui/webui/chromeos/choose_mobile_network_ui.h"
 #include "chrome/browser/ui/webui/chromeos/enterprise_enrollment_ui.h"
 #include "chrome/browser/ui/webui/chromeos/imageburner_ui.h"
 #include "chrome/browser/ui/webui/chromeos/keyboard_overlay_ui.h"
 #include "chrome/browser/ui/webui/chromeos/mobile_setup_ui.h"
 #include "chrome/browser/ui/webui/chromeos/proxy_settings_ui.h"
 #include "chrome/browser/ui/webui/chromeos/register_page_ui.h"
@@ skipping 174 matching lines @@
 }  // namespace
 
 WebUI::TypeID ChromeWebUIFactory::GetWebUIType(Profile* profile,
                                                const GURL& url) const {
   WebUIFactoryFunction function = GetWebUIFactoryFunction(profile, url);
   return function ? reinterpret_cast<WebUI::TypeID>(function) : WebUI::kNoWebUI;
 }
 
 bool ChromeWebUIFactory::UseWebUIForURL(Profile* profile,
                                         const GURL& url) const {
-  return GetWebUIType(profile, url) != WebUI::kNoWebUI;
+  // Convert installed app URLs to extension URLs to classify them properly.

[Matt Perry, 2011/05/19 22:15:11]

Shouldn't this be done in GetWebUIType so that we use the right WebUI as well?

Actually now that I think about it, do we want to use WebUI for hosted apps at
all? I think hosted apps are unprivileged. Unfortunately I'm not very familiar
with how hosted apps work.

It might be safer just to keep the check local to BrowserNavigator's
GetSiteInstance (which also seems weird to be basing its decision off of WebUI).

[Charlie Reis, 2011/05/19 23:02:15]

Ah, yes, good point.  I think it actually belongs in GetUIFactoryFunction.
We're already treating hosted apps as extension URLs in some places, which
affects the process model we use for them (process-per-app, rather than
process-per-app-instance) even if they're not privileged.  It's also important
that hosted apps get the proper type of RenderProcessHost, especially for
isolated apps that have their own cookie store.
It looks like the other call sites of UseWebUIForURL in RenderViewHostManager
face the same risk of getting this wrong.  I think we want to be consistent
about treating hosted apps as WebUI, but if you can think of a case where that's
not true, let me know.

(If we decide there's a reason to avoid this, we should add a test, because this
version is passing the try bots, apart from the FavIconChange flake.)

[Matt Perry, 2011/05/19 23:04:43]

Could you verify that we are using the same WebUI for hosted app URLs before and
after this change? (i.e. if we weren't using a WebUI before, we still
shouldn't.)

[Charlie Reis, 2011/05/19 23:53:07]

Wow, the WebUI code is kind of hard to follow.  I focused on what happened when
CreateWebUIForURL was called.

Before my change, we were not creating a WebUI for hosted apps, and
RVHM::pending_web_ui_ was null.  My change (once I fixed an inconsistency in
which URL we were using) did cause it to create a WebUI.

Sounds like my approach is wrong, then, but can you explain what's wrong about
it?  What are the implications for having a WebUI or not?  Seems strange to me
that we consider hosted app URLs to be extension URLs (and thus worthy of WebUI)
in some places but not when we actually create the WebUI.

I can move my change to BrowserNavigator, but I'd like to add a check (and
explanation) in the AppProcess test if we expect a hosted app not to have WebUI.

[Matt Perry, 2011/05/20 00:03:47]

WebUI is a way of exposing extra bindings to a tab. Extensions pages loaded in a
tab are given an ExtensionWebUI, which gives that tab extension bindings. We
want to do this for chrome-extension URLs, but not for hosted app URLs, because
the latter should be unprivileged.

For hosted app URLs, we want them to be grouped in a single process, so the app
has script access to all its components (including backround windows), but we
don't want to given them extension bindings.

[Charlie Reis, 2011/05/20 15:32:47]

Thanks-- that clarifies things.  I've added a check and explanation to the
browser test, and I changed BrowserNavigator to explicitly check for both WebUI
and extensions (as the comment already claimed it did).

+  GURL effective_url =
+      content::GetContentClient()->browser()->GetEffectiveURL(profile, url);
+  return GetWebUIType(profile, effective_url) != WebUI::kNoWebUI;
 }
 
 bool ChromeWebUIFactory::HasWebUIScheme(const GURL& url) const {
   return url.SchemeIs(chrome::kChromeDevToolsScheme) ||
          url.SchemeIs(chrome::kChromeInternalScheme) ||
          url.SchemeIs(chrome::kChromeUIScheme) ||
          url.SchemeIs(chrome::kExtensionScheme);
 }
 
 bool ChromeWebUIFactory::IsURLAcceptableForWebUI(
@@ skipping 94 matching lines @@
     return FlagsUI::GetFaviconResourceBytes();
 
   if (page_url.host() == chrome::kChromeUISettingsHost)
     return OptionsUI::GetFaviconResourceBytes();
 
   if (page_url.host() == chrome::kChromeUIPluginsHost)
     return PluginsUI::GetFaviconResourceBytes();
 
   return NULL;
 }