Reland r55750. Broke AppApiTest.*.
[email protected]
BUG=49234
git-svn-id: svn://svn.chromium.org/chrome/trunk/src@55909 0039d316-1c4b-4281-b951-d872f2087c98
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 @@
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 @@
// 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 d88e0456..f1b09eb 100644
--- a/chrome/browser/extensions/extension_protocols.cc
+++ b/chrome/browser/extensions/extension_protocols.cc
@@ -65,6 +65,50 @@
int resource_id_;
};
+// 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).
+ 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.
+ if (context->is_off_the_record() &&
+ 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://
@@ -74,35 +118,8 @@
ChromeURLRequestContext* context =
static_cast<ChromeURLRequestContext*>(request->context());
- const ResourceDispatcherHostRequestInfo* info =
- ResourceDispatcherHost::InfoForRequest(request);
-
- // 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);
- }
-
- // 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)
+ 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 @@
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..06c2b8003 100644
--- a/chrome/browser/net/chrome_url_request_context.h
+++ b/chrome/browser/net/chrome_url_request_context.h
@@ -79,6 +79,10 @@
// 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 @@
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..d69527e5 100644
--- a/chrome/common/extensions/extension.cc
+++ b/chrome/common/extensions/extension.cc
@@ -71,6 +71,28 @@
const int kValidWebExtentSchemes =
URLPattern::SCHEME_HTTP | URLPattern::SCHEME_HTTPS;
+// 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::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[] =
@@ -80,19 +102,6 @@
const FilePath::CharType Extension::kMessagesFilename[] =
FILE_PATH_LITERAL("messages.json");
-// A list of all the keys allowed by themes.
-static const char* kValidThemeKeys[] = {
- keys::kCurrentLocale,
- keys::kDefaultLocale,
- keys::kDescription,
- keys::kName,
- keys::kPublicKey,
- keys::kSignature,
- keys::kTheme,
- keys::kVersion,
- keys::kUpdateURL
-};
-
#if defined(OS_WIN)
const char* Extension::kExtensionRegistryPath =
"Software\\Google\\Chrome\\Extensions";
@@ -512,22 +521,9 @@
}
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 @@
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 @@
!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 @@
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 8c306c78..0f4f5dd1 100644
--- a/chrome/common/extensions/extension_constants.cc
+++ b/chrome/common/extensions/extension_constants.cc
@@ -97,6 +97,8 @@
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 2b04abee..3258449 100644
--- a/chrome/common/extensions/extension_constants.h
+++ b/chrome/common/extensions/extension_constants.h
@@ -89,6 +89,7 @@
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 36c4b890..df2dad0d 100644
--- a/chrome/common/extensions/extension_manifests_unittest.cc
+++ b/chrome/common/extensions/extension_manifests_unittest.cc
@@ -263,3 +263,10 @@
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..8cca1129
--- /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"
+}
