Now allow reopening BCs even if they were specified in the manifest

BUG=98046
TEST=Create app with BC in manifest. Have app use a different window name (not "background"), window should not be visible.


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

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

5 files changed, 121 insertions, 7 deletions

diff --git a/chrome/browser/extensions/app_background_page_apitest.cc b/chrome/browser/extensions/app_background_page_apitest.cc
index ac1db73..19d1eb5 100644
--- a/chrome/browser/extensions/app_background_page_apitest.cc
+++ b/chrome/browser/extensions/app_background_page_apitest.cc
@@ -17,7 +17,7 @@
 
 class AppBackgroundPageApiTest : public ExtensionApiTest {
  public:
-  void SetUpCommandLine(CommandLine* command_line) {
+  void SetUpCommandLine(CommandLine* command_line) OVERRIDE {
     ExtensionApiTest::SetUpCommandLine(command_line);
     command_line->AppendSwitch(switches::kDisablePopupBlocking);
     command_line->AppendSwitch(switches::kAllowHTTPBackgroundPage);
@@ -163,3 +163,32 @@ IN_PROC_BROWSER_TEST_F(AppBackgroundPageApiTest, OpenTwoBackgroundPages) {
   ASSERT_TRUE(LoadExtension(app_dir));
   ASSERT_TRUE(RunExtensionTest("app_background_page/two_pages")) << message_;
 }
+
+IN_PROC_BROWSER_TEST_F(AppBackgroundPageApiTest, OpenTwoPagesWithManifest) {
+  host_resolver()->AddRule("a.com", "127.0.0.1");
+  ASSERT_TRUE(StartTestServer());
+
+  std::string app_manifest = base::StringPrintf(
+      "{"
+      "  \"name\": \"App\","
+      "  \"version\": \"0.1\","
+      "  \"app\": {"
+      "    \"urls\": ["
+      "      \"http://a.com/\""
+      "    ],"
+      "    \"launch\": {"
+      "      \"web_url\": \"http://a.com:%d/\""
+      "    }"
+      "  },"
+      "  \"background_page\": \"http://a.com:%d/bg.html\","
+      "  \"permissions\": [\"background\"]"
+      "}",
+      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));
+  ASSERT_TRUE(RunExtensionTest("app_background_page/two_with_manifest")) <<
+      message_;
+}
diff --git a/chrome/browser/tab_contents/render_view_host_delegate_helper.cc b/chrome/browser/tab_contents/render_view_host_delegate_helper.cc
index 5163ca8..0e0ae36 100644
--- a/chrome/browser/tab_contents/render_view_host_delegate_helper.cc
+++ b/chrome/browser/tab_contents/render_view_host_delegate_helper.cc
@@ -104,11 +104,6 @@ RenderViewHostDelegateViewHelper::MaybeCreateBackgroundContents(
   if (!extension)
     return NULL;
 
-  // If the extension manifest specifies a background page, then don't allow one
-  // to be created here.
-  if (extension->background_url().is_valid())
-    return NULL;
-
   // No BackgroundContents allowed if BackgroundContentsService doesn't exist.
   BackgroundContentsService* service =
       BackgroundContentsServiceFactory::GetForProfile(profile);
@@ -123,7 +118,7 @@ RenderViewHostDelegateViewHelper::MaybeCreateBackgroundContents(
     return NULL;
 
   // Only allow a single background contents per app. If one already exists,
-  // close it.
+  // close it (even if it was specified in the manifest).
   BackgroundContents* existing =
       service->GetAppBackgroundContents(ASCIIToUTF16(extension->id()));
   if (existing) {
diff --git a/chrome/test/data/extensions/api_test/app_background_page/two_with_manifest/content_script.js b/chrome/test/data/extensions/api_test/app_background_page/two_with_manifest/content_script.js
new file mode 100644
index 0000000..547ce77
--- /dev/null
+++ b/chrome/test/data/extensions/api_test/app_background_page/two_with_manifest/content_script.js
@@ -0,0 +1,18 @@
+// 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.
+
+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/two_with_manifest/manifest.json b/chrome/test/data/extensions/api_test/app_background_page/two_with_manifest/manifest.json
new file mode 100644
index 0000000..a5a9a6a
--- /dev/null
+++ b/chrome/test/data/extensions/api_test/app_background_page/two_with_manifest/manifest.json
@@ -0,0 +1,14 @@
+{
+  "name": "app_background_page/two_pages",
+  "version": "0.1",
+  "description": "Tests that opening a second background page in concert with a manifest-provided page results in the manifest page being closed.",
+  "background_page": "test.html",
+  "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/two_with_manifest/test.html b/chrome/test/data/extensions/api_test/app_background_page/two_with_manifest/test.html
new file mode 100644
index 0000000..b034637
--- /dev/null
+++ b/chrome/test/data/extensions/api_test/app_background_page/two_with_manifest/test.html
@@ -0,0 +1,58 @@
+<script>
+// This test runs through an expected use of a live background page:
+// - A background page is opened via the manifest.
+// - A live (web-extent) web page is loaded (a.html), which opens the background
+//   page using a different name.
+// - We wait for this background page to get loaded. If a visible tab appears
+//   instead, the test fails, otherwise it succeeds as soon as the page is
+//   loaded.
+
+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 create a
+// background page whose name is "bg", and this should 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 a_url =
+        pagePrefix.replace(/PORT/, config.testServer.port) + '/a.html';
+    chrome.tabs.create({ 'url': a_url });
+  });
+}
+
+// Background page opened.
+function onBackgroundPageLoaded() {
+  // If this gets called without failing the test (by opening a visible
+  // background page) we're done.
+  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");
+}
+
+</script>