Allow apps with background pages to request process-per-app-instance.

Requires setting background.allow_js_access to false in manifest.

BUG=113444
TEST=Example hosted app has different processes in different tabs.


Review URL: http://codereview.chromium.org/9508008

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

15 files changed, 354 insertions, 99 deletions

diff --git a/chrome/browser/chrome_content_browser_client.cc b/chrome/browser/chrome_content_browser_client.cc
index d86ad20..bc2bba12 100644
--- a/chrome/browser/chrome_content_browser_client.cc
+++ b/chrome/browser/chrome_content_browser_client.cc
@@ -454,15 +454,19 @@ bool ChromeContentBrowserClient::ShouldUseProcessPerSite(
     return false;
 
   // If the URL is part of a hosted app that does not have the background
-  // permission, we want to give each instance its own process to improve
+  // permission, or that does not allow JavaScript access to the background
+  // page, we want to give each instance its own process to improve
   // responsiveness.
-  if (extension->GetType() == Extension::TYPE_HOSTED_APP &&
-      !extension->HasAPIPermission(ExtensionAPIPermission::kBackground))
-    return false;
+  if (extension->GetType() == Extension::TYPE_HOSTED_APP) {
+    if (!extension->HasAPIPermission(ExtensionAPIPermission::kBackground) ||
+        !extension->allow_background_js_access()) {
+      return false;
+    }
+  }
 
-  // Hosted apps that have the background permission must use process per site,
-  // since all instances can make synchronous calls to the background window.
-  // Other extensions should use process per site as well.
+  // Hosted apps that have script access to their background page must use
+  // process per site, since all instances can make synchronous calls to the
+  // background window.  Other extensions should use process per site as well.
   return true;
 }
 
@@ -1239,7 +1243,21 @@ bool ChromeContentBrowserClient::CanCreateWindow(
   // the appropriate permission, fail the attempt.
   if (container_type == WINDOW_CONTAINER_TYPE_BACKGROUND) {
     ProfileIOData* io_data = ProfileIOData::FromResourceContext(context);
-    return io_data->GetExtensionInfoMap()->SecurityOriginHasAPIPermission(
+    ExtensionInfoMap* map = io_data->GetExtensionInfoMap();
+
+    // If the opener is not allowed to script its background window, then return
+    // false so that the window.open call returns null.  In this case, only
+    // the manifest is permitted to create a background window.
+    // Note: this use of GetExtensionOrAppByURL is safe but imperfect.  It may
+    // return a recently installed Extension even if this CanCreateWindow call
+    // was made by an old copy of the page in a normal web process.  That's ok,
+    // because the permission check below will still fail.
+    const Extension* extension = map->extensions().GetExtensionOrAppByURL(
+        ExtensionURLInfo(source_origin));
+    if (extension && !extension->allow_background_js_access())
+      return false;
+
+    return map->SecurityOriginHasAPIPermission(
         source_origin, render_process_id, ExtensionAPIPermission::kBackground);
   }
   return true;
diff --git a/chrome/browser/extensions/app_background_page_apitest.cc b/chrome/browser/extensions/app_background_page_apitest.cc
index 9c9585b..32af291 100644
--- a/chrome/browser/extensions/app_background_page_apitest.cc
+++ b/chrome/browser/extensions/app_background_page_apitest.cc
@@ -143,6 +143,44 @@ IN_PROC_BROWSER_TEST_F(AppBackgroundPageApiTest, ManifestBackgroundPage) {
           GetAppBackgroundContents(ASCIIToUTF16(extension->id())));
 }
 
+IN_PROC_BROWSER_TEST_F(AppBackgroundPageApiTest, NoJsManifestBackgroundPage) {
+  host_resolver()->AddRule("a.com", "127.0.0.1");
+  ASSERT_TRUE(StartTestServer());
+
+  std::string app_manifest = base::StringPrintf(
+      "{"
+      "  \"name\": \"App\","
+      "  \"version\": \"0.1\","
+      "  \"manifest_version\": 2,"
+      "  \"app\": {"
+      "    \"urls\": ["
+      "      \"http://a.com/\""
+      "    ],"
+      "    \"launch\": {"
+      "      \"web_url\": \"http://a.com:%d/\""
+      "    }"
+      "  },"
+      "  \"permissions\": [\"background\"],"
+      "  \"background\": {"
+      "    \"page\": \"http://a.com:%d/bg.html\","
+      "    \"allow_js_access\": false"
+      "  }"
+      "}",
+      test_server()->host_port_pair().port(),
+      test_server()->host_port_pair().port());
+
+  FilePath app_dir;
+  ASSERT_TRUE(CreateApp(app_manifest, &app_dir));
+  ASSERT_TRUE(LoadExtension(app_dir));
+
+  // The background page should load, but window.open should return null.
+  const Extension* extension = GetSingleLoadedExtension();
+  ASSERT_TRUE(
+      BackgroundContentsServiceFactory::GetForProfile(browser()->profile())->
+          GetAppBackgroundContents(ASCIIToUTF16(extension->id())));
+  ASSERT_TRUE(RunExtensionTest("app_background_page/no_js")) << message_;
+}
+
 IN_PROC_BROWSER_TEST_F(AppBackgroundPageApiTest, OpenTwoBackgroundPages) {
   host_resolver()->AddRule("a.com", "127.0.0.1");
   ASSERT_TRUE(StartTestServer());
diff --git a/chrome/browser/extensions/app_process_apitest.cc b/chrome/browser/extensions/app_process_apitest.cc
index 7f67045..7192af2 100644
--- a/chrome/browser/extensions/app_process_apitest.cc
+++ b/chrome/browser/extensions/app_process_apitest.cc
@@ -28,21 +28,6 @@
 using content::NavigationController;
 using content::WebContents;
 
-class AppApiTest : public ExtensionApiTest {
- protected:
-  // Gets the base URL for files for a specific test, making sure that it uses
-  // "localhost" as the hostname, since that is what the extent is declared
-  // as in the test apps manifests.
-  GURL GetTestBaseURL(std::string test_directory) {
-    GURL::Replacements replace_host;
-    std::string host_str("localhost");  // must stay in scope with replace_host
-    replace_host.SetHostStr(host_str);
-    GURL base_url = test_server()->GetURL(
-        "files/extensions/api_test/" + test_directory + "/");
-    return base_url.ReplaceComponents(replace_host);
-  }
-};
-
 // Simulates a page calling window.open on an URL, and waits for the navigation.
 static void WindowOpenHelper(Browser* browser,
                              RenderViewHost* opener_host,
@@ -87,12 +72,97 @@ static void NavigateTabHelper(WebContents* contents, const GURL& url) {
   EXPECT_EQ(url, contents->GetController().GetLastCommittedEntry()->GetURL());
 }
 
+class AppApiTest : public ExtensionApiTest {
+ protected:
+  // Gets the base URL for files for a specific test, making sure that it uses
+  // "localhost" as the hostname, since that is what the extent is declared
+  // as in the test apps manifests.
+  GURL GetTestBaseURL(std::string test_directory) {
+    GURL::Replacements replace_host;
+    std::string host_str("localhost");  // must stay in scope with replace_host
+    replace_host.SetHostStr(host_str);
+    GURL base_url = test_server()->GetURL(
+        "files/extensions/api_test/" + test_directory + "/");
+    return base_url.ReplaceComponents(replace_host);
+  }
+
+  // Pass flags to make testing apps easier.
+  void SetUpCommandLine(CommandLine* command_line) {
+    ExtensionApiTest::SetUpCommandLine(command_line);
+    CommandLine::ForCurrentProcess()->AppendSwitch(
+        switches::kDisablePopupBlocking);
+    CommandLine::ForCurrentProcess()->AppendSwitch(
+        switches::kAllowHTTPBackgroundPage);
+  }
+
+  // Helper function to test that independent tabs of the named app are loaded
+  // into separate processes.
+  void TestAppInstancesHelper(std::string app_name) {
+    LOG(INFO) << "Start of test.";
+
+    extensions::ProcessMap* process_map =
+        browser()->profile()->GetExtensionService()->process_map();
+
+    host_resolver()->AddRule("*", "127.0.0.1");
+    ASSERT_TRUE(test_server()->Start());
+
+    ASSERT_TRUE(LoadExtension(
+        test_data_dir_.AppendASCII(app_name)));
+
+    // Open two tabs in the app, one outside it.
+    GURL base_url = GetTestBaseURL(app_name);
+
+    // Test both opening a URL in a new tab, and opening a tab and then
+    // navigating it.  Either way, app tabs should be considered extension
+    // processes, but they have no elevated privileges and thus should not
+    // have WebUI bindings.
+    ui_test_utils::NavigateToURLWithDisposition(
+        browser(), base_url.Resolve("path1/empty.html"), NEW_FOREGROUND_TAB,
+        ui_test_utils::BROWSER_TEST_WAIT_FOR_NAVIGATION);
+    LOG(INFO) << "Nav 1.";
+    EXPECT_TRUE(process_map->Contains(
+        browser()->GetWebContentsAt(1)->GetRenderProcessHost()->GetID()));
+    EXPECT_FALSE(browser()->GetWebContentsAt(1)->GetWebUI());
+
+    ui_test_utils::WindowedNotificationObserver tab_added_observer(
+        content::NOTIFICATION_TAB_ADDED,
+        content::NotificationService::AllSources());
+    browser()->NewTab();
+    tab_added_observer.Wait();
+    LOG(INFO) << "New tab.";
+    ui_test_utils::NavigateToURL(browser(),
+                                 base_url.Resolve("path2/empty.html"));
+    LOG(INFO) << "Nav 2.";
+    EXPECT_TRUE(process_map->Contains(
+        browser()->GetWebContentsAt(2)->GetRenderProcessHost()->GetID()));
+    EXPECT_FALSE(browser()->GetWebContentsAt(2)->GetWebUI());
+
+    // We should have opened 2 new extension tabs. Including the original blank
+    // tab, we now have 3 tabs. The two app tabs should not be in the same
+    // process, since they do not have the background permission.  (Thus, we
+    // want to separate them to improve responsiveness.)
+    ASSERT_EQ(3, browser()->tab_count());
+    RenderViewHost* host1 = browser()->GetWebContentsAt(1)->GetRenderViewHost();
+    RenderViewHost* host2 = browser()->GetWebContentsAt(2)->GetRenderViewHost();
+    EXPECT_NE(host1->process(), host2->process());
+
+    // Opening tabs with window.open should keep the page in the opener's
+    // process.
+    ASSERT_EQ(1u, BrowserList::GetBrowserCount(browser()->profile()));
+    WindowOpenHelper(browser(), host1,
+                    base_url.Resolve("path1/empty.html"), true);
+    LOG(INFO) << "WindowOpenHelper 1.";
+    WindowOpenHelper(browser(), host2,
+                    base_url.Resolve("path2/empty.html"), true);
+    LOG(INFO) << "End of test.";
+  }
+};
+
+// Tests that hosted apps with the background permission get a process-per-app
+// model, since all pages need to be able to script the background page.
 IN_PROC_BROWSER_TEST_F(AppApiTest, AppProcess) {
   LOG(INFO) << "Start of test.";
 
-  CommandLine::ForCurrentProcess()->AppendSwitch(
-      switches::kDisablePopupBlocking);
-
   extensions::ProcessMap* process_map =
       browser()->profile()->GetExtensionService()->process_map();
 
@@ -200,71 +270,19 @@ IN_PROC_BROWSER_TEST_F(AppApiTest, AppProcess) {
 // Test that hosted apps without the background permission use a process per app
 // instance model, such that separate instances are in separate processes.
 IN_PROC_BROWSER_TEST_F(AppApiTest, AppProcessInstances) {
-  LOG(INFO) << "Start of test.";
-
-  CommandLine::ForCurrentProcess()->AppendSwitch(
-      switches::kDisablePopupBlocking);
-
-  extensions::ProcessMap* process_map =
-      browser()->profile()->GetExtensionService()->process_map();
-
-  host_resolver()->AddRule("*", "127.0.0.1");
-  ASSERT_TRUE(test_server()->Start());
-
-  ASSERT_TRUE(LoadExtension(
-      test_data_dir_.AppendASCII("app_process_instances")));
-
-  // Open two tabs in the app, one outside it.
-  GURL base_url = GetTestBaseURL("app_process_instances");
-
-  // Test both opening a URL in a new tab, and opening a tab and then navigating
-  // it.  Either way, app tabs should be considered extension processes, but
-  // they have no elevated privileges and thus should not have WebUI bindings.
-  ui_test_utils::NavigateToURLWithDisposition(
-      browser(), base_url.Resolve("path1/empty.html"), NEW_FOREGROUND_TAB,
-      ui_test_utils::BROWSER_TEST_WAIT_FOR_NAVIGATION);
-  LOG(INFO) << "Nav 1.";
-  EXPECT_TRUE(process_map->Contains(
-      browser()->GetWebContentsAt(1)->GetRenderProcessHost()->GetID()));
-  EXPECT_FALSE(browser()->GetWebContentsAt(1)->GetWebUI());
-
-  ui_test_utils::WindowedNotificationObserver tab_added_observer(
-      content::NOTIFICATION_TAB_ADDED,
-      content::NotificationService::AllSources());
-  browser()->NewTab();
-  tab_added_observer.Wait();
-  LOG(INFO) << "New tab.";
-  ui_test_utils::NavigateToURL(browser(), base_url.Resolve("path2/empty.html"));
-  LOG(INFO) << "Nav 2.";
-  EXPECT_TRUE(process_map->Contains(
-      browser()->GetWebContentsAt(2)->GetRenderProcessHost()->GetID()));
-  EXPECT_FALSE(browser()->GetWebContentsAt(2)->GetWebUI());
-
-  // We should have opened 2 new extension tabs. Including the original blank
-  // tab, we now have 3 tabs. The two app tabs should not be in the same
-  // process, since they do not have the background permission.  (Thus, we want
-  // to separate them to improve responsiveness.)
-  ASSERT_EQ(3, browser()->tab_count());
-  RenderViewHost* host1 = browser()->GetWebContentsAt(1)->GetRenderViewHost();
-  RenderViewHost* host2 = browser()->GetWebContentsAt(2)->GetRenderViewHost();
-  EXPECT_NE(host1->process(), host2->process());
+  TestAppInstancesHelper("app_process_instances");
+}
 
-  // Opening tabs with window.open should keep the page in the opener's process.
-  ASSERT_EQ(1u, BrowserList::GetBrowserCount(browser()->profile()));
-  WindowOpenHelper(browser(), host1,
-                   base_url.Resolve("path1/empty.html"), true);
-  LOG(INFO) << "WindowOpenHelper 1.";
-  WindowOpenHelper(browser(), host2,
-                   base_url.Resolve("path2/empty.html"), true);
-  LOG(INFO) << "End of test.";
+// Test that hosted apps with the background permission but that set
+// allow_js_access to false also use a process per app instance model.
+// Separate instances should be in separate processes.
+IN_PROC_BROWSER_TEST_F(AppApiTest, AppProcessBackgroundInstances) {
+  TestAppInstancesHelper("app_process_background_instances");
 }
 
 // Tests that bookmark apps do not use the app process model and are treated
 // like normal web pages instead.  http://crbug.com/104636.
 IN_PROC_BROWSER_TEST_F(AppApiTest, BookmarkAppGetsNormalProcess) {
-  CommandLine::ForCurrentProcess()->AppendSwitch(
-      switches::kDisablePopupBlocking);
-
   ExtensionService* service = browser()->profile()->GetExtensionService();
   extensions::ProcessMap* process_map = service->process_map();
 
@@ -345,9 +363,6 @@ IN_PROC_BROWSER_TEST_F(AppApiTest, BookmarkAppGetsNormalProcess) {
 #define MAYBE_AppProcessRedirectBack AppProcessRedirectBack
 #endif
 IN_PROC_BROWSER_TEST_F(AppApiTest, MAYBE_AppProcessRedirectBack) {
-  CommandLine::ForCurrentProcess()->AppendSwitch(
-      switches::kDisablePopupBlocking);
-
   host_resolver()->AddRule("*", "127.0.0.1");
   ASSERT_TRUE(test_server()->Start());
 
@@ -388,9 +403,6 @@ IN_PROC_BROWSER_TEST_F(AppApiTest, MAYBE_AppProcessRedirectBack) {
 // Ensure that reloading a URL after installing or uninstalling it as an app
 // correctly swaps the process.  (http://crbug.com/80621)
 IN_PROC_BROWSER_TEST_F(AppApiTest, ReloadIntoAppProcess) {
-  CommandLine::ForCurrentProcess()->AppendSwitch(
-      switches::kDisablePopupBlocking);
-
   extensions::ProcessMap* process_map =
       browser()->profile()->GetExtensionService()->process_map();
 
@@ -478,9 +490,6 @@ IN_PROC_BROWSER_TEST_F(AppApiTest, ReloadIntoAppProcess) {
 // empty.html) results in the new window being in an app process. See
 // http://crbug.com/89272 for more details.
 IN_PROC_BROWSER_TEST_F(AppApiTest, OpenAppFromIframe) {
-  CommandLine::ForCurrentProcess()->AppendSwitch(
-      switches::kDisablePopupBlocking);
-
   extensions::ProcessMap* process_map =
       browser()->profile()->GetExtensionService()->process_map();
 
@@ -576,9 +585,6 @@ IN_PROC_BROWSER_TEST_F(AppApiTest, OpenAppFromExtension) {
 // missing special permissions and should be scriptable from the iframe.
 // See http://crbug.com/92669 for more details.
 IN_PROC_BROWSER_TEST_F(AppApiTest, OpenWebPopupFromWebIframe) {
-  CommandLine::ForCurrentProcess()->AppendSwitch(
-      switches::kDisablePopupBlocking);
-
   extensions::ProcessMap* process_map =
       browser()->profile()->GetExtensionService()->process_map();
 
diff --git a/chrome/common/extensions/extension.cc b/chrome/common/extensions/extension.cc
index dd9fb92..2c1cfc5 100644
--- a/chrome/common/extensions/extension.cc
+++ b/chrome/common/extensions/extension.cc
@@ -1515,6 +1515,27 @@ bool Extension::LoadBackgroundPersistent(
   return true;
 }
 
+bool Extension::LoadBackgroundAllowJsAccess(
+    const ExtensionAPIPermissionSet& api_permissions,
+    string16* error) {
+  Value* allow_js_access = NULL;
+  if (!manifest_->Get(keys::kBackgroundAllowJsAccess, &allow_js_access))
+    return true;
+
+  if (!allow_js_access->IsType(Value::TYPE_BOOLEAN) ||
+      !allow_js_access->GetAsBoolean(&allow_background_js_access_)) {
+    *error = ASCIIToUTF16(errors::kInvalidBackgroundAllowJsAccess);
+    return false;
+  }
+
+  if (!has_background_page()) {
+    *error = ASCIIToUTF16(errors::kInvalidBackgroundAllowJsAccessNoPage);
+    return false;
+  }
+
+  return true;
+}
+
 // static
 bool Extension::IsTrustedId(const std::string& id) {
   // See http://b/4946060 for more details.
@@ -1528,6 +1549,7 @@ Extension::Extension(const FilePath& path,
       offline_enabled_(false),
       converted_from_user_script_(false),
       background_page_persists_(true),
+      allow_background_js_access_(true),
       manifest_(manifest.release()),
       is_storage_isolated_(false),
       launch_container_(extension_misc::LAUNCH_TAB),
@@ -2325,6 +2347,9 @@ bool Extension::InitFromValue(int flags, string16* error) {
   if (!LoadBackgroundPersistent(api_permissions, error))
     return false;
 
+  if (!LoadBackgroundAllowJsAccess(api_permissions, error))
+    return false;
+
   if (manifest_->HasKey(keys::kDefaultLocale)) {
     if (!manifest_->GetString(keys::kDefaultLocale, &default_locale_) ||
         !l10n_util::IsValidLocaleSyntax(default_locale_)) {
diff --git a/chrome/common/extensions/extension.h b/chrome/common/extensions/extension.h
index 0f789eb..9558d21 100644
--- a/chrome/common/extensions/extension.h
+++ b/chrome/common/extensions/extension.h
@@ -557,6 +557,9 @@ class Extension : public base::RefCountedThreadSafe<Extension> {
   bool has_background_page() const {
     return background_url_.is_valid() || !background_scripts_.empty();
   }
+  bool allow_background_js_access() const {
+    return allow_background_js_access_;
+  }
   const std::vector<std::string>& background_scripts() const {
     return background_scripts_;
   }
@@ -704,6 +707,9 @@ class Extension : public base::RefCountedThreadSafe<Extension> {
   bool LoadBackgroundPersistent(
       const ExtensionAPIPermissionSet& api_permissions,
       string16* error);
+  bool LoadBackgroundAllowJsAccess(
+      const ExtensionAPIPermissionSet& api_permissions,
+      string16* error);
 
   // Helper method that loads a UserScript object from a
   // dictionary in the content_script list of the manifest.
@@ -857,6 +863,12 @@ class Extension : public base::RefCountedThreadSafe<Extension> {
   // load on-demand (when it needs to handle an event). Defaults to true.
   bool background_page_persists_;
 
+  // True if the background page can be scripted by pages of the app or
+  // extension, in which case all such pages must run in the same process.
+  // False if such pages are not permitted to script the background page,
+  // allowing them to run in different processes.
+  bool allow_background_js_access_;
+
   // Optional URL to a page for setting options/preferences.
   GURL options_url_;
 
diff --git a/chrome/common/extensions/extension_constants.cc b/chrome/common/extensions/extension_constants.cc
index c50c2e5..868dc3c 100644
--- a/chrome/common/extensions/extension_constants.cc
+++ b/chrome/common/extensions/extension_constants.cc
@@ -16,10 +16,11 @@ const char kAllFrames[] = "all_frames";
 const char kAltKey[] = "altKey";
 const char kApp[] = "app";
 const char kBackground[] = "background";
+const char kBackgroundAllowJsAccess[] = "background.allow_js_access";
 const char kBackgroundPage[] = "background.page";
 const char kBackgroundPageLegacy[] = "background_page";
-const char kBackgroundScripts[] = "background.scripts";
 const char kBackgroundPersistent[] = "background.persistent";
+const char kBackgroundScripts[] = "background.scripts";
 const char kBrowserAction[] = "browser_action";
 const char kChromeURLOverrides[] = "chrome_url_overrides";
 const char kCommands[] = "commands";
@@ -179,6 +180,11 @@ const char kInvalidAllFrames[] =
     "Invalid value for 'content_scripts[*].all_frames'.";
 const char kInvalidBackground[] =
     "Invalid value for 'background_page'.";
+const char kInvalidBackgroundAllowJsAccess[] =
+    "Invalid value for 'background.allow_js_access'.";
+const char kInvalidBackgroundAllowJsAccessNoPage[] =
+    "Must specify one of background.page or background.scripts to use"
+    " background.allow_js_access.";
 const char kInvalidBackgroundCombination[] =
     "The background.page and background.scripts properties cannot be used at "
     "the same time.";
diff --git a/chrome/common/extensions/extension_constants.h b/chrome/common/extensions/extension_constants.h
index ca87caa..4ceb45e 100644
--- a/chrome/common/extensions/extension_constants.h
+++ b/chrome/common/extensions/extension_constants.h
@@ -17,10 +17,11 @@ namespace extension_manifest_keys {
   extern const char kAltKey[];
   extern const char kApp[];
   extern const char kBackground[];
+  extern const char kBackgroundAllowJsAccess[];
   extern const char kBackgroundPage[];
   extern const char kBackgroundPageLegacy[];
-  extern const char kBackgroundScripts[];
   extern const char kBackgroundPersistent[];
+  extern const char kBackgroundScripts[];
   extern const char kBrowserAction[];
   extern const char kBrowseURLs[];
   extern const char kChromeURLOverrides[];
@@ -162,6 +163,8 @@ namespace extension_manifest_errors {
   extern const char kFeatureNotAllowed[];
   extern const char kInvalidAllFrames[];
   extern const char kInvalidBackground[];
+  extern const char kInvalidBackgroundAllowJsAccess[];
+  extern const char kInvalidBackgroundAllowJsAccessNoPage[];
   extern const char kInvalidBackgroundCombination[];
   extern const char kInvalidBackgroundScript[];
   extern const char kInvalidBackgroundScripts[];
diff --git a/chrome/common/extensions/extension_manifests_unittest.cc b/chrome/common/extensions/extension_manifests_unittest.cc
index e1d108f..23e3847 100644
--- a/chrome/common/extensions/extension_manifests_unittest.cc
+++ b/chrome/common/extensions/extension_manifests_unittest.cc
@@ -1037,6 +1037,7 @@ TEST_F(ExtensionManifestTest, BackgroundPage) {
       LoadAndExpectSuccess("background_page.json"));
   ASSERT_TRUE(extension);
   EXPECT_EQ("/foo.html", extension->GetBackgroundURL().path());
+  EXPECT_TRUE(extension->allow_background_js_access());
 
   std::string error;
   scoped_ptr<DictionaryValue> manifest(
@@ -1077,6 +1078,13 @@ TEST_F(ExtensionManifestTest, BackgroundScripts) {
                      errors::kInvalidBackgroundCombination);
 }
 
+TEST_F(ExtensionManifestTest, BackgroundAllowNoJsAccess) {
+  scoped_refptr<Extension> extension;
+  extension = LoadAndExpectSuccess("background_allow_no_js_access.json");
+  ASSERT_TRUE(extension);
+  EXPECT_FALSE(extension->allow_background_js_access());
+}
+
 TEST_F(ExtensionManifestTest, PageActionManifestVersion2) {
   scoped_refptr<Extension> extension(
       LoadAndExpectSuccess("page_action_manifest_version_2.json"));
diff --git a/chrome/test/data/extensions/api_test/app_background_page/no_js/content_script.js b/chrome/test/data/extensions/api_test/app_background_page/no_js/content_script.js
new file mode 100644
index 0000000..ba714c6
--- /dev/null
+++ b/chrome/test/data/extensions/api_test/app_background_page/no_js/content_script.js
@@ -0,0 +1,18 @@
+// Copyright (c) 2012 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.
+
+var scriptMessageEvent = document.createEvent("Event");
+scriptMessageEvent.initEvent('scriptMessage', true, true);
+
+var pageToScriptTunnel = document.getElementById("pageToScriptTunnel");
+pageToScriptTunnel.addEventListener("scriptMessage", function() {
+  var data = JSON.parse(pageToScriptTunnel.innerText);
+  chrome.extension.sendRequest(data);
+});
+
+chrome.extension.onRequest.addListener(function(request) {
+  var scriptToPageTunnel = document.getElementById("scriptToPageTunnel");
+  scriptToPageTunnel.innerText = JSON.stringify(request);
+  scriptToPageTunnel.dispatchEvent(scriptMessageEvent);
+});
diff --git a/chrome/test/data/extensions/api_test/app_background_page/no_js/manifest.json b/chrome/test/data/extensions/api_test/app_background_page/no_js/manifest.json
new file mode 100644
index 0000000..25fa056
--- /dev/null
+++ b/chrome/test/data/extensions/api_test/app_background_page/no_js/manifest.json
@@ -0,0 +1,17 @@
+{
+  "name": "app_background_page/no_js",
+  "version": "0.1",
+  "manifest_version": 2,
+  "description": "Tests that window.open returns null without JS access to background page.",
+  "background": {
+    "scripts": ["test.js"]
+  },
+  "permissions": ["tabs", "http://a.com/*"],
+  "content_scripts": [
+    {
+      "matches": ["http://a.com/*"],
+      "js": ["content_script.js"],
+      "run_at": "document_end"
+    }
+  ]
+}
diff --git a/chrome/test/data/extensions/api_test/app_background_page/no_js/test.js b/chrome/test/data/extensions/api_test/app_background_page/no_js/test.js
new file mode 100644
index 0000000..ef9332b
--- /dev/null
+++ b/chrome/test/data/extensions/api_test/app_background_page/no_js/test.js
@@ -0,0 +1,61 @@
+// Copyright (c) 2012 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.
+
+// This test checks that setting allow_js_access to false is effective:
+// - A background page is opened via the manifest (which is verified by the
+//   AppBackgroundPageApiTest.NoJsManifestBackgroundPage code).
+// - A live (web-extent) web page is loaded (a.html), which tries to opens a
+//   background page.  This fails because allow_js_access is false.
+
+var pagePrefix =
+    'http://a.com:PORT/files/extensions/api_test/app_background_page/common';
+
+// Dispatch "tunneled" functions from the live web pages to this testing page.
+chrome.extension.onRequest.addListener(function(request) {
+  window[request.name](request.args);
+});
+
+// At no point should a window be created that contains the background page
+// (bg.html).
+chrome.tabs.onUpdated.addListener(function(tabId, changeInfo, tab) {
+  if (tab.url.match("bg\.html$")) {
+    chrome.test.notifyFail("popup opened instead of background page");
+  }
+});
+
+// Start the test by opening the first page in the app. This will try to create
+// a background page whose name is "bg", but it should not replace the
+// background page created by the manifest (named "background").
+window.onload = function() {
+  // We wait for window.onload before getting the test config.  If the
+  // config is requested before onload, then sometimes onload has already
+  // fired by the time chrome.test.getConfig()'s callback runs.
+  chrome.test.getConfig(function(config) {
+    var aUrl =
+        pagePrefix.replace(/PORT/, config.testServer.port) + '/a.html';
+    chrome.tabs.create({ 'url': aUrl });
+  });
+}
+
+// Background page opened.
+function onBackgroundPageLoaded() {
+  // The window.open call in a.html should not succeed.
+  chrome.test.notifyFail("Background page unexpectedly loaded.");
+}
+
+function onBackgroundPagePermissionDenied() {
+  // a.html will call this if it receives null from window.open, as we expect.
+  chrome.test.notifyPass();
+}
+
+// A second background page opened.
+function onBackgroundPageResponded() {
+  chrome.test.notifyFail("onBackgroundPageResponded called unexpectedly");
+}
+
+// The background counter check found an unexpected value (most likely caused
+// by an unwanted navigation).
+function onCounterError() {
+  chrome.test.notifyFail("checkCounter found an unexpected value");
+}
diff --git a/chrome/test/data/extensions/api_test/app_process_background_instances/manifest.json b/chrome/test/data/extensions/api_test/app_process_background_instances/manifest.json
new file mode 100644
index 0000000..9ee5d40
--- /dev/null
+++ b/chrome/test/data/extensions/api_test/app_process_background_instances/manifest.json
@@ -0,0 +1,22 @@
+{
+  "name": "app_process_background_instances",
+  "version": "0.1",
+  "manifest_version": 2,
+  "description": "Tests that apps with no background JS are not consolidated.",
+  "app": {
+    "urls": [
+      "http://localhost/files/extensions/api_test/app_process_background_instances/path1",
+      "http://localhost/files/extensions/api_test/app_process_background_instances/path2"
+    ],
+    "launch": {
+      "web_url": "http://localhost/files/extensions/api_test/app_process_background_instances/path1/empty.html"
+    }
+  },
+  "permissions": [
+    "background"
+  ],
+  "background": {
+    "page": "http://localhost/files/extensions/api_test/app_process_background_instances/path1/empty.html",
+    "allow_js_access": false
+  }
+}
diff --git a/chrome/test/data/extensions/api_test/app_process_background_instances/path1/empty.html b/chrome/test/data/extensions/api_test/app_process_background_instances/path1/empty.html
new file mode 100644
index 0000000..d3cdf0a
--- /dev/null
+++ b/chrome/test/data/extensions/api_test/app_process_background_instances/path1/empty.html
@@ -0,0 +1 @@
+<title>Unmodified</title>
diff --git a/chrome/test/data/extensions/api_test/app_process_background_instances/path2/empty.html b/chrome/test/data/extensions/api_test/app_process_background_instances/path2/empty.html
new file mode 100644
index 0000000..d3cdf0a
--- /dev/null
+++ b/chrome/test/data/extensions/api_test/app_process_background_instances/path2/empty.html
@@ -0,0 +1 @@
+<title>Unmodified</title>
diff --git a/chrome/test/data/extensions/manifest_tests/background_allow_no_js_access.json b/chrome/test/data/extensions/manifest_tests/background_allow_no_js_access.json
new file mode 100644
index 0000000..340b090
--- /dev/null
+++ b/chrome/test/data/extensions/manifest_tests/background_allow_no_js_access.json
@@ -0,0 +1,19 @@
+{
+  "name": "test",
+  "version": "1",
+  "app": {
+    "urls": [
+      "https://www.google.com/mail/"
+    ],
+    "launch": {
+      "web_url": "https://www.google.com/mail/"
+    }
+  },
+  "permissions": [
+    "background"
+  ],
+  "background": {
+    "page": "https://www.google.com/mail/foo.html",
+    "allow_js_access": false
+  }
+}