Revert 30194 - Makes session restore on Chrome OS restore popups.

BUG=18862
TEST=none

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

TBR=sky@chromium.org
Review URL: http://codereview.chromium.org/335047

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

10 files changed, 31 insertions, 164 deletions

diff --git a/chrome/browser/automation/automation_provider.cc b/chrome/browser/automation/automation_provider.cc
index c47a326..d791afb 100644
--- a/chrome/browser/automation/automation_provider.cc
+++ b/chrome/browser/automation/automation_provider.cc
@@ -309,7 +309,6 @@ void AutomationProvider::OnMessageReceived(const IPC::Message& message) {
                                     WindowSimulateDrag)
 #endif  // defined(OS_WIN) || defined(OS_LINUX)
     IPC_MESSAGE_HANDLER(AutomationMsg_TabCount, GetTabCount)
-    IPC_MESSAGE_HANDLER(AutomationMsg_Type, GetType)
     IPC_MESSAGE_HANDLER(AutomationMsg_Tab, GetTab)
 #if defined(OS_WIN)
     IPC_MESSAGE_HANDLER(AutomationMsg_TabHWND, GetTabHWND)
@@ -882,15 +881,6 @@ void AutomationProvider::GetTabCount(int handle, int* tab_count) {
   }
 }
 
-void AutomationProvider::GetType(int handle, int* type_as_int) {
-  *type_as_int = -1;  // -1 is the error code
-
-  if (browser_tracker_->ContainsHandle(handle)) {
-    Browser* browser = browser_tracker_->GetResource(handle);
-    *type_as_int = static_cast<int>(browser->type());
-  }
-}
-
 void AutomationProvider::GetTab(int win_handle, int tab_index,
                                 int* tab_handle) {
   *tab_handle = 0;
@@ -1287,14 +1277,12 @@ void AutomationProvider::GetDownloadDirectory(
   }
 }
 
-void AutomationProvider::OpenNewBrowserWindow(int type,
-                                              bool show,
+void AutomationProvider::OpenNewBrowserWindow(bool show,
                                               IPC::Message* reply_message) {
   new BrowserOpenedNotificationObserver(this, reply_message);
   // We may have no current browser windows open so don't rely on
   // asking an existing browser to execute the IDC_NEWWINDOW command
-  Browser* browser = new Browser(static_cast<Browser::Type>(type), profile_);
-  browser->CreateBrowserWindow();
+  Browser* browser = Browser::Create(profile_);
   browser->AddBlankTab(true);
   if (show)
     browser->window()->Show();
diff --git a/chrome/browser/automation/automation_provider.h b/chrome/browser/automation/automation_provider.h
index 471f00f..44ee790 100644
--- a/chrome/browser/automation/automation_provider.h
+++ b/chrome/browser/automation/automation_provider.h
@@ -185,7 +185,6 @@ class AutomationProvider : public base::RefCounted<AutomationProvider>,
   void IsWindowMaximized(int handle, bool* is_maximized, bool* success);
 
   void GetTabCount(int handle, int* tab_count);
-  void GetType(int handle, int* type_as_int);
   void GetTab(int win_handle, int tab_index, int* tab_handle);
 #if defined(OS_WIN)
   // TODO(port): Replace HWND.
@@ -284,9 +283,7 @@ class AutomationProvider : public base::RefCounted<AutomationProvider>,
                                      int* autocomplete_edit_handle);
 
   // If |show| is true, call Show() on the new window after creating it.
-  void OpenNewBrowserWindow(int type,
-                            bool show,
-                            IPC::Message* reply_message);
+  void OpenNewBrowserWindow(bool show, IPC::Message* reply_message);
 
   void ShowInterstitialPage(int tab_handle,
                             const std::string& html_text,
diff --git a/chrome/browser/browser.h b/chrome/browser/browser.h
index 345ab43..dc465e8 100644
--- a/chrome/browser/browser.h
+++ b/chrome/browser/browser.h
@@ -44,7 +44,6 @@ class Browser : public TabStripModelDelegate,
                 public NotificationObserver,
                 public SelectFileDialog::Listener {
  public:
-  // If you change this, update browser_proxy as well.
   enum Type {
     TYPE_NORMAL = 0,
     TYPE_POPUP = 1,
diff --git a/chrome/browser/defaults.cc b/chrome/browser/defaults.cc
index 916a50f..098c6b9 100644
--- a/chrome/browser/defaults.cc
+++ b/chrome/browser/defaults.cc
@@ -16,7 +16,6 @@ const SessionStartupPref::Type kDefaultSessionStartupType =
     SessionStartupPref::LAST;
 const int kPinnedTabWidth = 64;
 const bool kCanToggleSystemTitleBar = false;
-const bool kRestorePopups = true;
 
 #elif defined(OS_LINUX)
 
@@ -43,7 +42,6 @@ const bool kCanToggleSystemTitleBar = true;
 const SessionStartupPref::Type kDefaultSessionStartupType =
     SessionStartupPref::DEFAULT;
 const int kPinnedTabWidth = 56;
-const bool kRestorePopups = false;
 
 #endif
 
diff --git a/chrome/browser/defaults.h b/chrome/browser/defaults.h
index 45862c2..814a616 100644
--- a/chrome/browser/defaults.h
+++ b/chrome/browser/defaults.h
@@ -34,9 +34,6 @@ extern const SessionStartupPref::Type kDefaultSessionStartupType;
 // Width of pinned tabs.
 extern const int kPinnedTabWidth;
 
-// Should session restore restore popup windows?
-extern const bool kRestorePopups;
-
 }  // namespace browser_defaults
 
 #endif  // CHROME_BROWSER_DEFAULTS_H_
diff --git a/chrome/browser/sessions/session_restore_uitest.cc b/chrome/browser/sessions/session_restore_uitest.cc
index fd25362..5c8157b 100644
--- a/chrome/browser/sessions/session_restore_uitest.cc
+++ b/chrome/browser/sessions/session_restore_uitest.cc
@@ -7,7 +7,6 @@
 #include "base/scoped_ptr.h"
 #include "base/string_util.h"
 #include "chrome/app/chrome_dll_resource.h"
-#include "chrome/browser/defaults.h"
 #include "chrome/common/chrome_paths.h"
 #include "chrome/common/chrome_switches.h"
 #include "chrome/test/automation/tab_proxy.h"
@@ -254,54 +253,6 @@ TEST_F(SessionRestoreUITest, ClosedTabStaysClosed) {
   ASSERT_TRUE(GetActiveTabURL() == url1_);
 }
 
-// Creates a tabbed browser and popup and makes sure we restore both.
-TEST_F(SessionRestoreUITest, NormalAndPopup) {
-  if (!browser_defaults::kRestorePopups)
-    return;  // Test only applicable if restoring popups.
-
-  NavigateToURL(url1_);
-
-  // Make sure we have one window.
-  int window_count;
-  ASSERT_TRUE(automation()->GetBrowserWindowCount(&window_count));
-  ASSERT_EQ(1, window_count);
-
-  // Open a popup.
-  ASSERT_TRUE(automation()->OpenNewBrowserWindow(BrowserProxy::TYPE_POPUP,
-                                                 true));
-  ASSERT_TRUE(automation()->GetBrowserWindowCount(&window_count));
-  ASSERT_EQ(2, window_count);
-
-  // Restart and make sure we have only one window with one tab and the url
-  // is url1_.
-  QuitBrowserAndRestore(1);
-
-  ASSERT_TRUE(automation()->GetBrowserWindowCount(&window_count));
-  ASSERT_EQ(2, window_count);
-
-  scoped_refptr<BrowserProxy> browser_proxy1(
-      automation()->GetBrowserWindow(0));
-  ASSERT_TRUE(browser_proxy1.get());
-
-  scoped_refptr<BrowserProxy> browser_proxy2(
-      automation()->GetBrowserWindow(1));
-  ASSERT_TRUE(browser_proxy2.get());
-
-  BrowserProxy::Type type1, type2;
-  ASSERT_TRUE(browser_proxy1->GetType(&type1));
-  ASSERT_TRUE(browser_proxy2->GetType(&type2));
-
-  // The order of whether the normal window or popup is first depends upon
-  // activation order, which is not necessarily consistant across runs.
-  if (type1 == BrowserProxy::TYPE_NORMAL) {
-    EXPECT_EQ(type2, BrowserProxy::TYPE_POPUP);
-  } else {
-    EXPECT_EQ(type1, BrowserProxy::TYPE_POPUP);
-    EXPECT_EQ(type2, BrowserProxy::TYPE_NORMAL);
-  }
-}
-
-
 #if defined(OS_WIN)
 // Creates a browser, goes incognito, closes browser, launches and make sure
 // we don't restore.
@@ -357,8 +308,7 @@ TEST_F(SessionRestoreUITest, TwoWindowsCloseOneRestoreOnlyOne) {
   ASSERT_EQ(1, window_count);
 
   // Open a second window.
-  ASSERT_TRUE(automation()->OpenNewBrowserWindow(BrowserProxy::TYPE_NORMAL,
-                                                 true));
+  ASSERT_TRUE(automation()->OpenNewBrowserWindow(true));
   ASSERT_TRUE(automation()->GetBrowserWindowCount(&window_count));
   ASSERT_EQ(2, window_count);
 
diff --git a/chrome/browser/sessions/session_service.cc b/chrome/browser/sessions/session_service.cc
index 31c2966..9522cee 100644
--- a/chrome/browser/sessions/session_service.cc
+++ b/chrome/browser/sessions/session_service.cc
@@ -119,14 +119,14 @@ struct PinnedStatePayload {
 
 SessionService::SessionService(Profile* profile)
     : BaseSessionService(SESSION_RESTORE, profile, FilePath()),
-      has_open_trackable_browsers_(false),
+      has_open_tabbed_browsers_(false),
       move_on_new_browser_(false) {
   Init();
 }
 
 SessionService::SessionService(const FilePath& save_path)
     : BaseSessionService(SESSION_RESTORE, NULL, save_path),
-      has_open_trackable_browsers_(false),
+      has_open_tabbed_browsers_(false),
       move_on_new_browser_(false) {
   Init();
 }
@@ -216,7 +216,7 @@ void SessionService::TabClosed(const SessionID& window_id,
     // User closed the last tab in the last tabbed browser. Don't mark the
     // tab closed.
     pending_tab_close_ids_.insert(tab_id.id());
-    has_open_trackable_browsers_ = false;
+    has_open_tabbed_browsers_ = false;
   }
 }
 
@@ -229,12 +229,12 @@ void SessionService::WindowClosing(const SessionID& window_id) {
   //
   // NOTE: if the user chooses the exit menu item session service is destroyed
   // and this code isn't hit.
-  if (has_open_trackable_browsers_) {
-    // Closing a window can never make has_open_trackable_browsers_ go from
-    // false to true, so only update it if already true.
-    has_open_trackable_browsers_ = HasOpenTrackableBrowsers(window_id);
+  if (has_open_tabbed_browsers_) {
+    // Closing a window can never make has_open_tabbed_browsers_ go from false
+    // to true, so only update it if already true.
+    has_open_tabbed_browsers_ = HasOpenTabbedBrowsers(window_id);
   }
-  if (!has_open_trackable_browsers_)
+  if (!has_open_tabbed_browsers_)
     pending_window_close_ids_.insert(window_id.id());
   else
     window_closing_ids_.insert(window_id.id());
@@ -252,8 +252,8 @@ void SessionService::WindowClosed(const SessionID& window_id) {
   } else if (pending_window_close_ids_.find(window_id.id()) ==
              pending_window_close_ids_.end()) {
     // We'll hit this if user closed the last tab in a window.
-    has_open_trackable_browsers_ = HasOpenTrackableBrowsers(window_id);
-    if (!has_open_trackable_browsers_)
+    has_open_tabbed_browsers_ = HasOpenTabbedBrowsers(window_id);
+    if (!has_open_tabbed_browsers_)
       pending_window_close_ids_.insert(window_id.id());
     else
       ScheduleCommand(CreateWindowClosedCommand(window_id.id()));
@@ -271,7 +271,7 @@ void SessionService::SetWindowType(const SessionID& window_id,
   // pending closes.
   CommitPendingCloses();
 
-  has_open_trackable_browsers_ = true;
+  has_open_tabbed_browsers_ = true;
   move_on_new_browser_ = true;
 
   ScheduleCommand(CreateSetWindowTypeCommand(window_id, type));
@@ -409,7 +409,7 @@ void SessionService::Observe(NotificationType type,
         return;
       }
 
-      if (!has_open_trackable_browsers_ && !BrowserInit::InProcessStartup()) {
+      if (!has_open_tabbed_browsers_ && !BrowserInit::InProcessStartup()) {
         // We're going from no tabbed browsers to a tabbed browser (and not in
         // process startup), restore the last session.
         if (move_on_new_browser_) {
@@ -1019,7 +1019,7 @@ void SessionService::ScheduleReset() {
   if (!windows_tracking_.empty()) {
     // We're lazily created on startup and won't get an initial batch of
     // SetWindowType messages. Set these here to make sure our state is correct.
-    has_open_trackable_browsers_ = true;
+    has_open_tabbed_browsers_ = true;
     move_on_new_browser_ = true;
   }
   StartSaveTimer();
@@ -1108,7 +1108,7 @@ bool SessionService::IsOnlyOneTabLeft() {
   }
 
   // NOTE: This uses the original profile so that closing the last non-off the
-  // record window while there are open off the record windows resets state).
+  // record window while there are open off the record window resets state).
   int window_count = 0;
   for (BrowserList::const_iterator i = BrowserList::begin();
        i != BrowserList::end(); ++i) {
@@ -1127,7 +1127,7 @@ bool SessionService::IsOnlyOneTabLeft() {
   return true;
 }
 
-bool SessionService::HasOpenTrackableBrowsers(const SessionID& window_id) {
+bool SessionService::HasOpenTabbedBrowsers(const SessionID& window_id) {
   if (!profile()) {
     // We're testing, always return false.
     return true;
diff --git a/chrome/browser/sessions/session_service.h b/chrome/browser/sessions/session_service.h
index 7fddaaa..b3daf22 100644
--- a/chrome/browser/sessions/session_service.h
+++ b/chrome/browser/sessions/session_service.h
@@ -9,7 +9,6 @@
 
 #include "base/basictypes.h"
 #include "chrome/browser/browser.h"
-#include "chrome/browser/defaults.h"
 #include "chrome/browser/sessions/base_session_service.h"
 #include "chrome/browser/sessions/session_id.h"
 #include "chrome/common/notification_observer.h"
@@ -305,19 +304,16 @@ class SessionService : public BaseSessionService,
   // our profile.
   bool IsOnlyOneTabLeft();
 
-  // Returns true if there are open trackable browser windows whose ids do
-  // match |window_id| with our profile. A trackable window is a window from
-  // which |should_track_changes_for_browser_type| returns true. See
-  // |should_track_changes_for_browser_type| for details.
-  bool HasOpenTrackableBrowsers(const SessionID& window_id);
+  // Returns true if there are no open tabbed browser windows with our profile,
+  // or the only tabbed browser open has a session id of window_id.
+  bool HasOpenTabbedBrowsers(const SessionID& window_id);
 
   // Returns true if changes to tabs in the specified window should be tracked.
   bool ShouldTrackChangesToWindow(const SessionID& window_id);
 
   // Returns true if we track changes to the specified browser type.
   static bool should_track_changes_for_browser_type(Browser::Type type) {
-    return type == Browser::TYPE_NORMAL ||
-        (type == Browser::TYPE_POPUP && browser_defaults::kRestorePopups);
+    return type == Browser::TYPE_NORMAL;
   }
 
   NotificationRegistrar registrar_;
@@ -351,11 +347,11 @@ class SessionService : public BaseSessionService,
   typedef std::set<SessionID::id_type> WindowsTracking;
   WindowsTracking windows_tracking_;
 
-  // Are there any open trackable browsers?
-  bool has_open_trackable_browsers_;
+  // Are there any open open tabbed browsers?
+  bool has_open_tabbed_browsers_;
 
   // If true and a new tabbed browser is created and there are no opened tabbed
-  // browser (has_open_trackable_browsers_ is false), then the current session
+  // browser (has_open_tabbed_browsers_ is false), then the current session
   // is made the previous session. See description above class for details on
   // current/previou session.
   bool move_on_new_browser_;
diff --git a/chrome/browser/sessions/session_service_unittest.cc b/chrome/browser/sessions/session_service_unittest.cc
index 1fec7f3..19997e7 100644
--- a/chrome/browser/sessions/session_service_unittest.cc
+++ b/chrome/browser/sessions/session_service_unittest.cc
@@ -397,9 +397,6 @@ TEST_F(SessionServiceTest, WindowCloseCommittedAfterNavigate) {
 
 // Makes sure we don't track popups.
 TEST_F(SessionServiceTest, IgnorePopups) {
-  if (browser_defaults::kRestorePopups)
-    return;  // This test is only applicable if popups aren't restored.
-
   SessionID window2_id;
   SessionID tab_id;
   SessionID tab2_id;
@@ -434,56 +431,6 @@ TEST_F(SessionServiceTest, IgnorePopups) {
   helper_.AssertNavigationEquals(nav1, tab->navigations[0]);
 }
 
-// Makes sure we track popups.
-TEST_F(SessionServiceTest, RestorePopup) {
-  if (!browser_defaults::kRestorePopups)
-    return;  // This test is only applicable if popups are restored.
-
-  SessionID window2_id;
-  SessionID tab_id;
-  SessionID tab2_id;
-  ASSERT_NE(window2_id.id(), window_id.id());
-
-  service()->SetWindowType(window2_id, Browser::TYPE_POPUP);
-  service()->SetWindowBounds(window2_id, window_bounds, false);
-
-  TabNavigation nav1(0, GURL("http://google.com"), GURL(),
-                     ASCIIToUTF16("abc"), "def",
-                     PageTransition::QUALIFIER_MASK);
-  TabNavigation nav2(0, GURL("http://google2.com"), GURL(),
-                     ASCIIToUTF16("abcd"), "defg",
-                     PageTransition::AUTO_BOOKMARK);
-
-  helper_.PrepareTabInWindow(window_id, tab_id, 0, true);
-  UpdateNavigation(window_id, tab_id, nav1, 0, true);
-
-  helper_.PrepareTabInWindow(window2_id, tab2_id, 0, false);
-  UpdateNavigation(window2_id, tab2_id, nav2, 0, true);
-
-  ScopedVector<SessionWindow> windows;
-  ReadWindows(&(windows.get()));
-
-  ASSERT_EQ(2U, windows->size());
-  int normal_index = windows[0]->type == Browser::TYPE_NORMAL ?
-      0 : 1;
-  int popup_index = normal_index == 0 ? 1 : 0;
-  ASSERT_EQ(0, windows[normal_index]->selected_tab_index);
-  ASSERT_EQ(window_id.id(), windows[normal_index]->window_id.id());
-  ASSERT_EQ(1U, windows[normal_index]->tabs.size());
-
-  SessionTab* tab = windows[normal_index]->tabs[0];
-  helper_.AssertTabEquals(window_id, tab_id, 0, 0, 1, *tab);
-  helper_.AssertNavigationEquals(nav1, tab->navigations[0]);
-
-  ASSERT_EQ(0, windows[popup_index]->selected_tab_index);
-  ASSERT_EQ(window2_id.id(), windows[popup_index]->window_id.id());
-  ASSERT_EQ(1U, windows[popup_index]->tabs.size());
-
-  tab = windows[popup_index]->tabs[0];
-  helper_.AssertTabEquals(window2_id, tab2_id, 0, 0, 1, *tab);
-  helper_.AssertNavigationEquals(nav2, tab->navigations[0]);
-}
-
 // Tests pruning from the front.
 TEST_F(SessionServiceTest, PruneFromFront) {
   const std::string base_url("http://google.com/");
diff --git a/chrome/browser/tab_restore_uitest.cc b/chrome/browser/tab_restore_uitest.cc
index 0f99ad5..491ba8a 100644
--- a/chrome/browser/tab_restore_uitest.cc
+++ b/chrome/browser/tab_restore_uitest.cc
@@ -226,8 +226,7 @@ TEST_F(TabRestoreUITest, FLAKY_RestoreToDifferentWindow) {
   EXPECT_EQ(starting_tab_count + 2, tab_count);
 
   // Create a new browser.
-  ASSERT_TRUE(automation()->OpenNewBrowserWindow(BrowserProxy::TYPE_NORMAL,
-                                                 false));
+  ASSERT_TRUE(automation()->OpenNewBrowserWindow(false));
   int window_count;
   ASSERT_TRUE(automation()->GetBrowserWindowCount(&window_count));
   EXPECT_EQ(2, window_count);
@@ -269,8 +268,7 @@ TEST_F(TabRestoreUITest, MAYBE_BasicRestoreFromClosedWindow) {
   tab_proxy->NavigateToURL(url2_);
 
   // Create a new browser.
-  ASSERT_TRUE(automation()->OpenNewBrowserWindow(BrowserProxy::TYPE_NORMAL,
-                                                 false));
+  ASSERT_TRUE(automation()->OpenNewBrowserWindow(false));
   int window_count;
   ASSERT_TRUE(automation()->GetBrowserWindowCount(&window_count));
   EXPECT_EQ(2, window_count);
@@ -360,8 +358,7 @@ TEST_F(TabRestoreUITest, FLAKY_RestoreWindowAndTab) {
   EXPECT_EQ(starting_tab_count + 2, tab_count);
 
   // Create a new browser.
-  ASSERT_TRUE(automation()->OpenNewBrowserWindow(BrowserProxy::TYPE_NORMAL,
-                                                 false));
+  ASSERT_TRUE(automation()->OpenNewBrowserWindow(false));
   int window_count;
   ASSERT_TRUE(automation()->GetBrowserWindowCount(&window_count));
   EXPECT_EQ(2, window_count);
@@ -409,8 +406,7 @@ TEST_F(TabRestoreUITest, RestoreIntoSameWindow) {
   tab_proxy->NavigateToURL(url2_);
 
   // Create a new browser.
-  ASSERT_TRUE(automation()->OpenNewBrowserWindow(BrowserProxy::TYPE_NORMAL,
-                                                 false));
+  ASSERT_TRUE(automation()->OpenNewBrowserWindow(false));
   int window_count;
   ASSERT_TRUE(automation()->GetBrowserWindowCount(&window_count));
   EXPECT_EQ(2, window_count);
@@ -552,8 +548,7 @@ TEST_F(TabRestoreUITest, RestoreWindow) {
   // Create a new window.
   int window_count;
   ASSERT_TRUE(automation()->GetBrowserWindowCount(&window_count));
-  ASSERT_TRUE(automation()->OpenNewBrowserWindow(BrowserProxy::TYPE_NORMAL,
-                                                 false));
+  ASSERT_TRUE(automation()->OpenNewBrowserWindow(false));
   int new_window_count = 0;
   ASSERT_TRUE(automation()->GetBrowserWindowCount(&new_window_count));
   EXPECT_EQ(++window_count, new_window_count);