Fix a case where non-tabbed browser windows could open tabs. Also includes UI test for this.

http://crbug.com/8472

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

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

4 files changed, 109 insertions, 16 deletions

diff --git a/chrome/browser/browser.cc b/chrome/browser/browser.cc
index d2c3c50..3359003 100644
--- a/chrome/browser/browser.cc
+++ b/chrome/browser/browser.cc
@@ -473,22 +473,26 @@ void Browser::OnWindowClosing() {
 TabContents* Browser::AddTabWithURL(
     const GURL& url, const GURL& referrer, PageTransition::Type transition,
     bool foreground, SiteInstance* instance) {
-  if ((type_ & TYPE_APP) != 0 && tabstrip_model_.count() == 1) {
-    NOTREACHED() << "Cannot add a tab in a mono tab application.";
-    return NULL;
-  }
-
-  GURL url_to_load = url;
-  if (url_to_load.is_empty())
-    url_to_load = GetHomePage();
-  TabContents* contents =
-      CreateTabContentsForURL(url_to_load, referrer, profile_, transition,
-                              false, instance);
-  tabstrip_model_.AddTabContents(contents, -1, transition, foreground);
-  // By default, content believes it is not hidden.  When adding contents
-  // in the background, tell it that it's hidden.
-  if (!foreground)
-    contents->WasHidden();
+  TabContents* contents = NULL;
+  if (type_ == TYPE_NORMAL) {
+    GURL url_to_load = url;
+    if (url_to_load.is_empty())
+      url_to_load = GetHomePage();
+    contents = CreateTabContentsForURL(url_to_load, referrer, profile_,
+                                       transition, false, instance);
+    tabstrip_model_.AddTabContents(contents, -1, transition, foreground);
+    // By default, content believes it is not hidden.  When adding contents
+    // in the background, tell it that it's hidden.
+    if (!foreground)
+      contents->WasHidden();
+  } else {
+    // We're in an app window or a popup window. Find an existing browser to
+    // open this URL in, creating one if none exists.
+    Browser* b = GetOrCreateTabbedBrowser();
+    contents = b->AddTabWithURL(url, referrer, transition, foreground,
+                                instance);
+    b->window()->Show();
+  }
   return contents;
 }
 
diff --git a/chrome/browser/browser.h b/chrome/browser/browser.h
index 24f3e05..2013cf9 100644
--- a/chrome/browser/browser.h
+++ b/chrome/browser/browser.h
@@ -571,6 +571,8 @@ class Browser : public TabStripModelDelegate,
   // as a key to store window location per application URLs.
   static std::wstring ComputeApplicationNameFromURL(const GURL& url);
 
+  FRIEND_TEST(BrowserTest, NoTabsInPopups);
+
   // Create a preference dictionary for the provided application name. This is
   // done only once per application name / per session.
   static void RegisterAppPrefs(const std::wstring& app_name);
diff --git a/chrome/browser/browser_unittest.cc b/chrome/browser/browser_unittest.cc
new file mode 100644
index 0000000..73a49cd
--- /dev/null
+++ b/chrome/browser/browser_unittest.cc
@@ -0,0 +1,83 @@
+// Copyright (c) 2009 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/browser.h"
+#include "chrome/test/in_process_browser_test.h"
+#include "net/base/host_resolver_unittest.h"
+
+class BrowserTest : public InProcessBrowserTest {
+ public:
+  BrowserTest() {
+    host_mapper_ = new net::RuleBasedHostMapper();
+    // Avoid making external DNS lookups. In this test we don't need this
+    // to succeed.
+    host_mapper_->AddSimulatedFailure("*.google.com");
+    scoped_host_mapper_.Init(host_mapper_.get());
+  }
+
+ private:
+  scoped_refptr<net::RuleBasedHostMapper> host_mapper_;
+  net::ScopedHostMapper scoped_host_mapper_;
+};
+
+// This tests that windows without tabstrips can't have new tabs opened in
+// them.
+IN_PROC_BROWSER_TEST_F(BrowserTest, NoTabsInPopups) {
+  Browser::RegisterAppPrefs(L"Test");
+
+  // We start with a normal browser with one tab.
+  EXPECT_EQ(1, browser()->tab_count());
+
+  // Open a popup browser with a single blank foreground tab.
+  Browser* popup_browser = browser()->CreateForPopup(browser()->profile());
+  popup_browser->AddBlankTab(true);
+  EXPECT_EQ(1, popup_browser->tab_count());
+
+  // Now try opening another tab in the popup browser.
+  popup_browser->AddTabWithURL(GURL("about:blank"), GURL(),
+                               PageTransition::TYPED, true, NULL);
+  
+  // The popup should still only have one tab.
+  EXPECT_EQ(1, popup_browser->tab_count());
+
+  // The normal browser should now have two.
+  EXPECT_EQ(2, browser()->tab_count());
+
+  // Open an app frame browser with a single blank foreground tab.
+  Browser* app_browser =
+      browser()->CreateForApp(L"Test", browser()->profile(), false);
+  app_browser->AddBlankTab(true);
+  EXPECT_EQ(1, app_browser->tab_count());
+
+  // Now try opening another tab in the app browser.
+  app_browser->AddTabWithURL(GURL("about:blank"), GURL(),
+                             PageTransition::TYPED, true, NULL);
+  
+  // The popup should still only have one tab.
+  EXPECT_EQ(1, app_browser->tab_count());
+
+  // The normal browser should now have three.
+  EXPECT_EQ(3, browser()->tab_count());
+
+  // Open an app frame popup browser with a single blank foreground tab.
+  Browser* app_popup_browser =
+      browser()->CreateForApp(L"Test", browser()->profile(), false);
+  app_popup_browser->AddBlankTab(true);
+  EXPECT_EQ(1, app_popup_browser->tab_count());
+
+  // Now try opening another tab in the app popup browser.
+  app_popup_browser->AddTabWithURL(GURL("about:blank"), GURL(),
+                                   PageTransition::TYPED, true, NULL);
+  
+  // The popup should still only have one tab.
+  EXPECT_EQ(1, app_popup_browser->tab_count());
+
+  // The normal browser should now have four.
+  EXPECT_EQ(4, browser()->tab_count());
+
+  // Close the additional browsers.
+  popup_browser->CloseAllTabs();
+  app_browser->CloseAllTabs();
+  app_popup_browser->CloseAllTabs();
+}
diff --git a/chrome/test/unit/unittests.vcproj b/chrome/test/unit/unittests.vcproj
index 46f501b..7ca6539 100644
--- a/chrome/test/unit/unittests.vcproj
+++ b/chrome/test/unit/unittests.vcproj
@@ -428,6 +428,10 @@
 				>
 			</File>
 			<File
+				RelativePath="..\..\browser\browser_unittest.cc"
+				>
+			</File>
+			<File
 				RelativePath="..\..\browser\chrome_thread_unittest.cc"
 				>
 			</File>