Reland r55750. Broke AppApiTest.*.

TBR=mpcomplete@chromium.org
BUG=49234

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@55909 0039d316-1c4b-4281-b951-d872f2087c98

13 files changed, 171 insertions, 81 deletions

diff --git a/chrome/browser/extensions/app_process_apitest.cc b/chrome/browser/extensions/app_process_apitest.cc
index 17a370cc..aa3fad1 100644
--- a/chrome/browser/extensions/app_process_apitest.cc
+++ b/chrome/browser/extensions/app_process_apitest.cc
@@ -58,50 +58,54 @@ IN_PROC_BROWSER_TEST_F(AppApiTest, AppProcess) {
   host_resolver()->AddRule("*", "127.0.0.1");
   ASSERT_TRUE(StartHTTPServer());
 
-  ASSERT_TRUE(RunExtensionTest("app_process")) << message_;
+  ASSERT_TRUE(LoadExtension(test_data_dir_.AppendASCII("app_process")));
 
-  Extension* extension = GetSingleLoadedExtension();
-  ExtensionHost* host = browser()->profile()->GetExtensionProcessManager()->
-      GetBackgroundHostForExtension(extension);
-  ASSERT_TRUE(host);
+  // Open two tabs in the app, one outside it.
+  GURL base_url("http://localhost:1337/files/extensions/api_test/app_process/");
+  browser()->NewTab();
+  ui_test_utils::NavigateToURL(browser(), base_url.Resolve("path1/empty.html"));
+  browser()->NewTab();
+  ui_test_utils::NavigateToURL(browser(), base_url.Resolve("path2/empty.html"));
+  browser()->NewTab();
+  ui_test_utils::NavigateToURL(browser(), base_url.Resolve("path3/empty.html"));
 
   // The extension should have opened 3 new tabs. Including the original blank
   // tab, we now have 4 tabs. Two should be part of the extension app, and
   // grouped in the extension process.
   ASSERT_EQ(4, browser()->tab_count());
-  EXPECT_EQ(host->render_process_host(),
-            browser()->GetTabContentsAt(1)->render_view_host()->process());
-  EXPECT_EQ(host->render_process_host(),
+  RenderViewHost* host = browser()->GetTabContentsAt(1)->render_view_host();
+  EXPECT_TRUE(host->is_extension_process());
+
+  EXPECT_EQ(host->process(),
             browser()->GetTabContentsAt(2)->render_view_host()->process());
-  EXPECT_NE(host->render_process_host(),
+  EXPECT_NE(host->process(),
             browser()->GetTabContentsAt(3)->render_view_host()->process());
 
   // Now let's do the same using window.open. The same should happen.
-  GURL base_url("http://localhost:1337/files/extensions/api_test/app_process/");
-  WindowOpenHelper(browser(), host->render_view_host(),
+  WindowOpenHelper(browser(), host,
                    base_url.Resolve("path1/empty.html"));
-  WindowOpenHelper(browser(), host->render_view_host(),
+  WindowOpenHelper(browser(), host,
                    base_url.Resolve("path2/empty.html"));
-  WindowOpenHelper(browser(), host->render_view_host(),
+  WindowOpenHelper(browser(), host,
                    base_url.Resolve("path3/empty.html"));
 
   ASSERT_EQ(7, browser()->tab_count());
-  EXPECT_EQ(host->render_process_host(),
+  EXPECT_EQ(host->process(),
             browser()->GetTabContentsAt(4)->render_view_host()->process());
-  EXPECT_EQ(host->render_process_host(),
+  EXPECT_EQ(host->process(),
             browser()->GetTabContentsAt(5)->render_view_host()->process());
-  EXPECT_NE(host->render_process_host(),
+  EXPECT_NE(host->process(),
             browser()->GetTabContentsAt(6)->render_view_host()->process());
 
   // Now let's have these pages navigate, into or out of the extension web
   // extent. They should switch processes.
   const GURL& app_url(base_url.Resolve("path1/empty.html"));
   const GURL& non_app_url(base_url.Resolve("path3/empty.html"));
-  NavigateTabHelper(browser()->GetTabContentsAt(1), non_app_url);
+  NavigateTabHelper(browser()->GetTabContentsAt(2), non_app_url);
   NavigateTabHelper(browser()->GetTabContentsAt(3), app_url);
-  EXPECT_NE(host->render_process_host(),
-            browser()->GetTabContentsAt(1)->render_view_host()->process());
-  EXPECT_EQ(host->render_process_host(),
+  EXPECT_NE(host->process(),
+            browser()->GetTabContentsAt(2)->render_view_host()->process());
+  EXPECT_EQ(host->process(),
             browser()->GetTabContentsAt(3)->render_view_host()->process());
 
   // Navigate the non-app tab into the browse extent. It should not enter the
@@ -109,10 +113,10 @@ IN_PROC_BROWSER_TEST_F(AppApiTest, AppProcess) {
   // Navigate the app tab into the browse extent. It should stay in the app
   // process.
   const GURL& browse_url(base_url.Resolve("path4/empty.html"));
-  NavigateTabHelper(browser()->GetTabContentsAt(1), browse_url);
+  NavigateTabHelper(browser()->GetTabContentsAt(2), browse_url);
   NavigateTabHelper(browser()->GetTabContentsAt(3), browse_url);
-  EXPECT_NE(host->render_process_host(),
-            browser()->GetTabContentsAt(1)->render_view_host()->process());
-  EXPECT_EQ(host->render_process_host(),
+  EXPECT_NE(host->process(),
+            browser()->GetTabContentsAt(2)->render_view_host()->process());
+  EXPECT_EQ(host->process(),
             browser()->GetTabContentsAt(3)->render_view_host()->process());
 }
diff --git a/chrome/browser/extensions/extension_protocols.cc b/chrome/browser/extensions/extension_protocols.cc
index d88e045..f1b09eb 100644
--- a/chrome/browser/extensions/extension_protocols.cc
+++ b/chrome/browser/extensions/extension_protocols.cc
@@ -65,44 +65,61 @@ class URLRequestResourceBundleJob : public URLRequestSimpleJob {
   int resource_id_;
 };
 
-}  // namespace
-
-// Factory registered with URLRequest to create URLRequestJobs for extension://
-// URLs.
-static URLRequestJob* CreateExtensionURLRequestJob(URLRequest* request,
-                                                   const std::string& scheme) {
-  ChromeURLRequestContext* context =
-      static_cast<ChromeURLRequestContext*>(request->context());
-
+// Returns true if an chrome-extension:// resource should be allowed to load.
+bool AllowExtensionResourceLoad(URLRequest* request,
+                                ChromeURLRequestContext* context,
+                                const std::string& scheme) {
   const ResourceDispatcherHostRequestInfo* info =
       ResourceDispatcherHost::InfoForRequest(request);
 
+  GURL origin_url(info->frame_origin());
+
+  // chrome:// URLs are always allowed to load chrome-extension:// resources.
+  // The app launcher in the NTP uses this feature, as does dev tools.
+  if (origin_url.SchemeIs(chrome::kChromeUIScheme))
+    return true;
+
+  // Disallow loading of packaged resources for hosted apps. We don't allow
+  // hybrid hosted/packaged apps.
+  if (context->ExtensionHasWebExtent(request->url().host()))
+    return false;
+
+  // chrome-extension:// pages can load resources from extensions and packaged
+  // apps. This is allowed for legacy reasons.
+  if (origin_url.SchemeIs(chrome::kExtensionScheme))
+    return true;
+
   // Extension resources should only be loadable from web pages which the
   // extension has host permissions to (and therefore could be running script
   // in, which might need access to the extension resources).
-  //
-  // chrome:// pages are exempt. We allow them to load any extension resource.
-  // This is used for, eg, the app launcher in the NTP.
-  //
-  // chrome-extension:// pages are also exempt, mostly for legacy reasons. Some
-  // extensions did this to integrate with each other before we added this code.
-  GURL origin_url(info->frame_origin());
-  if (!origin_url.is_empty() &&
-      !origin_url.SchemeIs(chrome::kChromeUIScheme) &&
-      !origin_url.SchemeIs(chrome::kExtensionScheme)) {
-    ExtensionExtent host_permissions =
-        context->GetEffectiveHostPermissionsForExtension(
-            request->url().host());
-    if (!host_permissions.ContainsURL(GURL(info->frame_origin())))
-      return new URLRequestErrorJob(request, net::ERR_ADDRESS_UNREACHABLE);
-  }
+  ExtensionExtent host_permissions =
+      context->GetEffectiveHostPermissionsForExtension(request->url().host());
+  if (!origin_url.is_empty() && !host_permissions.ContainsURL(origin_url))
+    return false;
 
   // Don't allow toplevel navigations to extension resources in incognito mode.
   // This is because an extension must run in a single process, and an
   // incognito tab prevents that.
-  // TODO(mpcomplete): better error code.
   if (context->is_off_the_record() &&
-      info && info->resource_type() == ResourceType::MAIN_FRAME)
+      info->resource_type() == ResourceType::MAIN_FRAME) {
+    return false;
+  }
+
+  // Otherwise, the resource load is allowed.
+  return true;
+}
+
+}  // namespace
+
+// Factory registered with URLRequest to create URLRequestJobs for extension://
+// URLs.
+static URLRequestJob* CreateExtensionURLRequestJob(URLRequest* request,
+                                                   const std::string& scheme) {
+  ChromeURLRequestContext* context =
+      static_cast<ChromeURLRequestContext*>(request->context());
+
+  // TODO(mpcomplete): better error code.
+  if (!AllowExtensionResourceLoad(request, context, scheme))
     return new URLRequestErrorJob(request, net::ERR_ADDRESS_UNREACHABLE);
 
   // chrome-extension://extension-id/resource/path.js
diff --git a/chrome/browser/net/chrome_url_request_context.cc b/chrome/browser/net/chrome_url_request_context.cc
index 82ffd3dc..59f85e3 100644
--- a/chrome/browser/net/chrome_url_request_context.cc
+++ b/chrome/browser/net/chrome_url_request_context.cc
@@ -783,6 +783,11 @@ FilePath ChromeURLRequestContext::GetPathForExtension(const std::string& id) {
     return FilePath();
 }
 
+bool ChromeURLRequestContext::ExtensionHasWebExtent(const std::string& id) {
+  ExtensionInfoMap::iterator iter = extension_info_.find(id);
+  return iter != extension_info_.end() && !iter->second->extent.is_empty();
+}
+
 std::string ChromeURLRequestContext::GetDefaultLocaleForExtension(
     const std::string& id) {
   ExtensionInfoMap::iterator iter = extension_info_.find(id);
diff --git a/chrome/browser/net/chrome_url_request_context.h b/chrome/browser/net/chrome_url_request_context.h
index dff0e6f..06c2b80 100644
--- a/chrome/browser/net/chrome_url_request_context.h
+++ b/chrome/browser/net/chrome_url_request_context.h
@@ -79,6 +79,10 @@ class ChromeURLRequestContext : public URLRequestContext {
   // Gets the path to the directory for the specified extension.
   FilePath GetPathForExtension(const std::string& id);
 
+  // Returns true if the specified extension exists and has a non-empty web
+  // extent.
+  bool ExtensionHasWebExtent(const std::string& id);
+
   // Returns an empty string if the extension with |id| doesn't have a default
   // locale.
   std::string GetDefaultLocaleForExtension(const std::string& id);
diff --git a/chrome/browser/renderer_host/render_view_host.h b/chrome/browser/renderer_host/render_view_host.h
index b47bb57..6d770cd 100644
--- a/chrome/browser/renderer_host/render_view_host.h
+++ b/chrome/browser/renderer_host/render_view_host.h
@@ -323,6 +323,7 @@ class RenderViewHost : public RenderWidgetHost {
   int enabled_bindings() { return enabled_bindings_; }
 
   // See variable comment.
+  bool is_extension_process() { return is_extension_process_; }
   void set_is_extension_process(bool is_extension_process) {
     is_extension_process_ = is_extension_process;
   }
diff --git a/chrome/common/extensions/extension.cc b/chrome/common/extensions/extension.cc
index f494c67..d69527e 100644
--- a/chrome/common/extensions/extension.cc
+++ b/chrome/common/extensions/extension.cc
@@ -71,28 +71,37 @@ static void ConvertHexadecimalToIDAlphabet(std::string* id) {
 const int kValidWebExtentSchemes =
     URLPattern::SCHEME_HTTP | URLPattern::SCHEME_HTTPS;
 
-}  // namespace
-
-const FilePath::CharType Extension::kManifestFilename[] =
-    FILE_PATH_LITERAL("manifest.json");
-const FilePath::CharType Extension::kLocaleFolder[] =
-    FILE_PATH_LITERAL("_locales");
-const FilePath::CharType Extension::kMessagesFilename[] =
-    FILE_PATH_LITERAL("messages.json");
-
-// A list of all the keys allowed by themes.
-static const char* kValidThemeKeys[] = {
+// These keys are allowed by all crx files (apps, extensions, themes, etc).
+static const char* kBaseCrxKeys[] = {
   keys::kCurrentLocale,
   keys::kDefaultLocale,
   keys::kDescription,
+  keys::kIcons,
   keys::kName,
   keys::kPublicKey,
   keys::kSignature,
-  keys::kTheme,
   keys::kVersion,
   keys::kUpdateURL
 };
 
+bool IsBaseCrxKey(const std::string& key) {
+  for (size_t i = 0; i < arraysize(kBaseCrxKeys); ++i) {
+    if (key == kBaseCrxKeys[i])
+      return true;
+  }
+
+  return false;
+}
+
+}  // namespace
+
+const FilePath::CharType Extension::kManifestFilename[] =
+    FILE_PATH_LITERAL("manifest.json");
+const FilePath::CharType Extension::kLocaleFolder[] =
+    FILE_PATH_LITERAL("_locales");
+const FilePath::CharType Extension::kMessagesFilename[] =
+    FILE_PATH_LITERAL("messages.json");
+
 #if defined(OS_WIN)
 const char* Extension::kExtensionRegistryPath =
     "Software\\Google\\Chrome\\Extensions";
@@ -512,22 +521,9 @@ ExtensionAction* Extension::LoadExtensionActionHelper(
 }
 
 bool Extension::ContainsNonThemeKeys(const DictionaryValue& source) {
-  // Generate a map of allowable keys
-  static std::map<std::string, bool> theme_keys;
-  static bool theme_key_mapped = false;
-  if (!theme_key_mapped) {
-    for (size_t i = 0; i < arraysize(kValidThemeKeys); ++i) {
-      theme_keys[kValidThemeKeys[i]] = true;
-    }
-    theme_key_mapped = true;
-  }
-
-  // Go through all the root level keys and verify that they're in the map
-  // of keys allowable by themes. If they're not, then make a not of it for
-  // later.
-  for (DictionaryValue::key_iterator iter = source.begin_keys();
-       iter != source.end_keys(); ++iter) {
-    if (theme_keys.find(*iter) == theme_keys.end())
+  for (DictionaryValue::key_iterator key = source.begin_keys();
+       key != source.end_keys(); ++key) {
+    if (!IsBaseCrxKey(*key) && *key != keys::kTheme)
       return true;
   }
   return false;
@@ -722,6 +718,23 @@ bool Extension::LoadLaunchFullscreen(const DictionaryValue* manifest,
   return true;
 }
 
+bool Extension::EnsureNotHybridApp(const DictionaryValue* manifest,
+                                   std::string* error) {
+  if (web_extent().is_empty())
+    return true;
+
+  for (DictionaryValue::key_iterator key = manifest->begin_keys();
+       key != manifest->end_keys(); ++key) {
+    if (!IsBaseCrxKey(*key) && *key != keys::kApp &&
+        *key != keys::kPermissions) {
+      *error = errors::kHostedAppsCannotIncludeExtensionFeatures;
+      return false;
+    }
+  }
+
+  return true;
+}
+
 Extension::Extension(const FilePath& path)
     : converted_from_user_script_(false),
       is_theme_(false),
@@ -1503,6 +1516,7 @@ bool Extension::InitFromValue(const DictionaryValue& source, bool require_key,
       !LoadExtent(manifest_value_.get(), keys::kBrowseURLs, &browse_extent_,
                   errors::kInvalidBrowseURLs, errors::kInvalidBrowseURL,
                   error) ||
+      !EnsureNotHybridApp(manifest_value_.get(), error) ||
       !LoadLaunchURL(manifest_value_.get(), error) ||
       !LoadLaunchContainer(manifest_value_.get(), error) ||
       !LoadLaunchFullscreen(manifest_value_.get(), error)) {
diff --git a/chrome/common/extensions/extension.h b/chrome/common/extensions/extension.h
index d4ffbdb..9bcd7b1 100644
--- a/chrome/common/extensions/extension.h
+++ b/chrome/common/extensions/extension.h
@@ -424,6 +424,7 @@ class Extension {
   bool LoadLaunchFullscreen(const DictionaryValue* manifest,
                             std::string* error);
   bool LoadLaunchURL(const DictionaryValue* manifest, std::string* error);
+  bool EnsureNotHybridApp(const DictionaryValue* manifest, std::string* error);
 
   // Helper method to load an ExtensionAction from the page_action or
   // browser_action entries in the manifest.
diff --git a/chrome/common/extensions/extension_constants.cc b/chrome/common/extensions/extension_constants.cc
index 8c306c7..0f4f5dd 100644
--- a/chrome/common/extensions/extension_constants.cc
+++ b/chrome/common/extensions/extension_constants.cc
@@ -97,6 +97,8 @@ const char* kDisabledByPolicy =
 const char* kDevToolsExperimental =
     "You must request the 'experimental' permission in order to use the"
     " DevTools API.";
+const char* kHostedAppsCannotIncludeExtensionFeatures =
+    "Hosted apps cannot use extension features.";
 const char* kInvalidAllFrames =
     "Invalid value for 'content_scripts[*].all_frames'.";
 const char* kInvalidBackground =
diff --git a/chrome/common/extensions/extension_constants.h b/chrome/common/extensions/extension_constants.h
index 2b04abe..3258449 100644
--- a/chrome/common/extensions/extension_constants.h
+++ b/chrome/common/extensions/extension_constants.h
@@ -89,6 +89,7 @@ namespace extension_manifest_errors {
   extern const char* kCannotScriptGallery;
   extern const char* kChromeVersionTooLow;
   extern const char* kDevToolsExperimental;
+  extern const char* kHostedAppsCannotIncludeExtensionFeatures;
   extern const char* kInvalidAllFrames;
   extern const char* kInvalidBackground;
   extern const char* kInvalidBrowserAction;
diff --git a/chrome/common/extensions/extension_manifests_unittest.cc b/chrome/common/extensions/extension_manifests_unittest.cc
index 36c4b89..df2dad0 100644
--- a/chrome/common/extensions/extension_manifests_unittest.cc
+++ b/chrome/common/extensions/extension_manifests_unittest.cc
@@ -263,3 +263,10 @@ TEST_F(ManifestTest, DevToolsExtensions) {
             extension->devtools_url().spec());
   *CommandLine::ForCurrentProcess() = old_command_line;
 }
+
+TEST_F(ManifestTest, DisallowHybridApps) {
+  LoadAndExpectError("disallow_hybrid_1.json",
+                     errors::kHostedAppsCannotIncludeExtensionFeatures);
+  LoadAndExpectError("disallow_hybrid_2.json",
+                     errors::kHostedAppsCannotIncludeExtensionFeatures);
+}
diff --git a/chrome/test/data/extensions/api_test/app_process/manifest.json b/chrome/test/data/extensions/api_test/app_process/manifest.json
index 67b54e0..2e35457 100644
--- a/chrome/test/data/extensions/api_test/app_process/manifest.json
+++ b/chrome/test/data/extensions/api_test/app_process/manifest.json
@@ -2,8 +2,6 @@
   "name": "app_process",
   "version": "0.1",
   "description": "Tests that app URLs are grouped into the right process",
-  "background_page": "test.html",
-  "permissions": ["tabs"],
   "app": {
     "urls": [
       "http://localhost/files/extensions/api_test/app_process/path1",
diff --git a/chrome/test/data/extensions/manifest_tests/disallow_hybrid_1.json b/chrome/test/data/extensions/manifest_tests/disallow_hybrid_1.json
new file mode 100644
index 0000000..8cca112
--- /dev/null
+++ b/chrome/test/data/extensions/manifest_tests/disallow_hybrid_1.json
@@ -0,0 +1,18 @@
+{
+  "name": "test",
+  "version": "1",
+  "app": {
+    "urls": [
+      "http://www.google.com/mail/",
+      "http://www.google.com/foobar/"
+    ],
+    "launch": {
+      "container": "window",
+      "web_url": "http://www.google.com/mail/"
+    }
+  },
+  "permissions": [
+    "notifications"
+  ],
+  "browser_action": {}
+}
diff --git a/chrome/test/data/extensions/manifest_tests/disallow_hybrid_2.json b/chrome/test/data/extensions/manifest_tests/disallow_hybrid_2.json
new file mode 100644
index 0000000..bad2e6c
--- /dev/null
+++ b/chrome/test/data/extensions/manifest_tests/disallow_hybrid_2.json
@@ -0,0 +1,18 @@
+{
+  "name": "test",
+  "version": "1",
+  "app": {
+    "urls": [
+      "http://www.google.com/mail/",
+      "http://www.google.com/foobar/"
+    ],
+    "launch": {
+      "container": "window",
+      "web_url": "http://www.google.com/mail/"
+    }
+  },
+  "permissions": [
+    "notifications"
+  ],
+  "background_page": "foo.html"
+}