Inform the command updater of the tab restore state.

BUG=14428
TEST=TabRestoreUITest.*

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

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

14 files changed, 66 insertions, 35 deletions

diff --git a/chrome/browser/browser.cc b/chrome/browser/browser.cc
index 0687eca..608b963 100644
--- a/chrome/browser/browser.cc
+++ b/chrome/browser/browser.cc
@@ -64,7 +64,6 @@
 #include "chrome/browser/renderer_host/site_instance.h"
 #include "chrome/browser/sessions/session_service.h"
 #include "chrome/browser/sessions/session_types.h"
-#include "chrome/browser/sessions/tab_restore_service.h"
 #include "chrome/browser/status_bubble.h"
 #include "chrome/browser/sync/profile_sync_service.h"
 #include "chrome/browser/sync/sync_ui_util.h"
@@ -232,6 +231,12 @@ Browser::Browser(Type type, Profile* profile)
     use_vertical_tabs_.SetValue(false);
   }
   UpdateTabStripModelInsertionPolicy();
+
+  tab_restore_service_ = profile->GetTabRestoreService();
+  if (tab_restore_service_) {
+    tab_restore_service_->AddObserver(this);
+    TabRestoreServiceChanged(tab_restore_service_);
+  }
 }
 
 Browser::~Browser() {
@@ -275,6 +280,8 @@ Browser::~Browser() {
   // away so they don't try and call back to us.
   if (select_file_dialog_.get())
     select_file_dialog_->ListenerDestroyed();
+
+  TabRestoreServiceDestroyed(tab_restore_service_);
 }
 
 // static
@@ -969,8 +976,7 @@ void Browser::ReplaceRestoredTab(
 }
 
 bool Browser::CanRestoreTab() {
-  TabRestoreService* service = profile_->GetTabRestoreService();
-  return service && !service->entries().empty();
+  return command_updater_.IsCommandEnabled(IDC_RESTORE_TAB);
 }
 
 bool Browser::NavigateToIndexWithDisposition(int index,
@@ -3005,6 +3011,7 @@ void Browser::InitCommandState() {
   command_updater_.UpdateCommandEnabled(IDC_NEW_TAB, true);
   command_updater_.UpdateCommandEnabled(IDC_CLOSE_TAB, true);
   command_updater_.UpdateCommandEnabled(IDC_DUPLICATE_TAB, true);
+  command_updater_.UpdateCommandEnabled(IDC_RESTORE_TAB, false);
   command_updater_.UpdateCommandEnabled(IDC_FULLSCREEN, true);
   command_updater_.UpdateCommandEnabled(IDC_EXIT, true);
 
@@ -3111,8 +3118,6 @@ void Browser::InitCommandState() {
   command_updater_.UpdateCommandEnabled(IDC_SELECT_TAB_6, normal_window);
   command_updater_.UpdateCommandEnabled(IDC_SELECT_TAB_7, normal_window);
   command_updater_.UpdateCommandEnabled(IDC_SELECT_LAST_TAB, normal_window);
-  command_updater_.UpdateCommandEnabled(IDC_RESTORE_TAB,
-                                        !profile_->IsOffTheRecord());
 
   // Page-related commands
   command_updater_.UpdateCommandEnabled(IDC_BOOKMARK_PAGE, normal_window);
@@ -3881,6 +3886,20 @@ bool Browser::RunUnloadEventsHelper(TabContents* contents) {
   return false;
 }
 
+void Browser::TabRestoreServiceChanged(TabRestoreService* service) {
+  command_updater_.UpdateCommandEnabled(IDC_RESTORE_TAB,
+                                        !service->entries().empty());
+}
+
+void Browser::TabRestoreServiceDestroyed(TabRestoreService* service) {
+  if (!tab_restore_service_)
+    return;
+
+  DCHECK_EQ(tab_restore_service_, service);
+  tab_restore_service_->RemoveObserver(this);
+  tab_restore_service_ = NULL;
+}
+
 bool Browser::IsPinned(TabContents* source) {
   int index = tabstrip_model_.GetIndexOfTabContents(source);
   if (index == TabStripModel::kNoTab) {
diff --git a/chrome/browser/browser.h b/chrome/browser/browser.h
index 4fe3694..68fcfe1 100644
--- a/chrome/browser/browser.h
+++ b/chrome/browser/browser.h
@@ -15,6 +15,7 @@
 #include "chrome/browser/command_updater.h"
 #include "chrome/browser/pref_member.h"
 #include "chrome/browser/sessions/session_id.h"
+#include "chrome/browser/sessions/tab_restore_service.h"
 #include "chrome/browser/shell_dialogs.h"
 #include "chrome/browser/tabs/tab_strip_model.h"
 #include "chrome/browser/tab_contents/page_navigator.h"
@@ -48,7 +49,8 @@ class Browser : public TabStripModelDelegate,
                 public PageNavigator,
                 public CommandUpdater::CommandUpdaterDelegate,
                 public NotificationObserver,
-                public SelectFileDialog::Listener {
+                public SelectFileDialog::Listener,
+                public TabRestoreService::Observer {
  public:
   // If you change the values in this enum you'll need to update browser_proxy.
   // TODO(sky): move into a common place that is referenced by both ui_tests
@@ -401,9 +403,6 @@ class Browser : public TabStripModelDelegate,
       bool from_last_session,
       const std::string& extension_app_id);
 
-  // Returns true if a tab can be restored.
-  virtual bool CanRestoreTab();
-
   // Navigate to an index in the tab history, opening a new tab depending on the
   // disposition.
   bool NavigateToIndexWithDisposition(int index, WindowOpenDisposition disp);
@@ -452,7 +451,6 @@ class Browser : public TabStripModelDelegate,
   void SelectNumberedTab(int index);
   void SelectLastTab();
   void DuplicateTab();
-  void RestoreTab();
   void WriteCurrentURLToClipboard();
   void ConvertPopupToTabbedBrowser();
   // In kiosk mode, the first toggle is valid, the rest is discarded.
@@ -609,6 +607,10 @@ class Browser : public TabStripModelDelegate,
   // Helper function to run unload listeners on a TabContents.
   static bool RunUnloadEventsHelper(TabContents* contents);
 
+  // TabRestoreService::Observer ///////////////////////////////////////////////
+  virtual void TabRestoreServiceChanged(TabRestoreService* service);
+  virtual void TabRestoreServiceDestroyed(TabRestoreService* service);
+
  private:
   FRIEND_TEST(BrowserTest, NoTabsInPopups);
 
@@ -655,6 +657,8 @@ class Browser : public TabStripModelDelegate,
   virtual void BookmarkAllTabs();
   virtual bool UseVerticalTabs() const;
   virtual void ToggleUseVerticalTabs();
+  virtual bool CanRestoreTab();
+  virtual void RestoreTab();
 
   // Overridden from TabStripModelObserver:
   virtual void TabInsertedAt(TabContents* contents,
@@ -1037,6 +1041,10 @@ class Browser : public TabStripModelDelegate,
   // Tracks the display mode of the tabstrip.
   mutable BooleanPrefMember use_vertical_tabs_;
 
+  // The profile's tab restore service. The service is owned by the profile,
+  // and we install ourselves as an observer.
+  TabRestoreService* tab_restore_service_;
+
   DISALLOW_COPY_AND_ASSIGN(Browser);
 };
 
diff --git a/chrome/browser/chromeos/status/browser_status_area_view.cc b/chrome/browser/chromeos/status/browser_status_area_view.cc
index a3489c3..a0b43b1 100644
--- a/chrome/browser/chromeos/status/browser_status_area_view.cc
+++ b/chrome/browser/chromeos/status/browser_status_area_view.cc
@@ -145,8 +145,6 @@ bool BrowserStatusAreaView::IsCommandIdChecked(int command_id) const {
 
 bool BrowserStatusAreaView::IsCommandIdEnabled(int command_id) const {
   Browser* browser = browser_view_->browser();
-  if (command_id == IDC_RESTORE_TAB)
-    return browser->CanRestoreTab();
   return browser->command_updater()->IsCommandEnabled(command_id);
 }
 
diff --git a/chrome/browser/cocoa/browser_window_controller.mm b/chrome/browser/cocoa/browser_window_controller.mm
index f64d48c..962b96b 100644
--- a/chrome/browser/cocoa/browser_window_controller.mm
+++ b/chrome/browser/cocoa/browser_window_controller.mm
@@ -869,11 +869,6 @@
           // one tab.
           enable &= [self numberOfTabs] > 1 && [[self window] isKeyWindow];
           break;
-        case IDC_RESTORE_TAB:
-          // We have to ask the Browser manually if we can restore. The
-          // command updater doesn't know.
-          enable &= browser_->CanRestoreTab();
-          break;
         case IDC_FULLSCREEN: {
           enable &= [self supportsFullscreen];
           if ([static_cast<NSObject*>(item) isKindOfClass:[NSMenuItem class]]) {
diff --git a/chrome/browser/cocoa/history_menu_bridge.mm b/chrome/browser/cocoa/history_menu_bridge.mm
index 76daaab..82c8beb 100644
--- a/chrome/browser/cocoa/history_menu_bridge.mm
+++ b/chrome/browser/cocoa/history_menu_bridge.mm
@@ -22,6 +22,7 @@
 #include "grit/app_resources.h"
 #include "grit/theme_resources.h"
 #include "skia/ext/skia_utils_mac.h"
+#include "third_party/skia/include/core/SkBitmap.h"
 
 namespace {
 
diff --git a/chrome/browser/jumplist.cc b/chrome/browser/jumplist.cc
index bb9e506..58b592c 100644
--- a/chrome/browser/jumplist.cc
+++ b/chrome/browser/jumplist.cc
@@ -20,6 +20,7 @@
 #include "base/scoped_comptr_win.h"
 #include "base/string_util.h"
 #include "base/thread.h"
+#include "base/utf_string_conversions.h"
 #include "base/win_util.h"
 #include "chrome/browser/chrome_thread.h"
 #include "chrome/browser/favicon_service.h"
diff --git a/chrome/browser/sessions/session_restore.cc b/chrome/browser/sessions/session_restore.cc
index 86b668d..0d60867 100644
--- a/chrome/browser/sessions/session_restore.cc
+++ b/chrome/browser/sessions/session_restore.cc
@@ -357,7 +357,7 @@ class SessionRestoreImpl : public NotificationObserver {
         }
       }
       if (!browser) {
-        browser = new Browser((*i)->type, profile_);
+        browser = new Browser(static_cast<Browser::Type>((*i)->type), profile_);
         browser->set_override_bounds((*i)->bounds);
         browser->set_maximized_state((*i)->is_maximized ?
             Browser::MAXIMIZED_STATE_MAXIMIZED :
diff --git a/chrome/browser/sessions/session_service.cc b/chrome/browser/sessions/session_service.cc
index f8d422a..ca5f183 100644
--- a/chrome/browser/sessions/session_service.cc
+++ b/chrome/browser/sessions/session_service.cc
@@ -803,7 +803,8 @@ void SessionService::SortTabsBasedOnVisualOrderAndPrune(
   std::map<int, SessionWindow*>::iterator i = windows->begin();
   while (i != windows->end()) {
     if (i->second->tabs.empty() || i->second->is_constrained ||
-        !should_track_changes_for_browser_type(i->second->type)) {
+        !should_track_changes_for_browser_type(
+            static_cast<Browser::Type>(i->second->type))) {
       delete i->second;
       windows->erase(i++);
     } else {
diff --git a/chrome/browser/sessions/session_types.cc b/chrome/browser/sessions/session_types.cc
index 27caf057..6cf193c 100644
--- a/chrome/browser/sessions/session_types.cc
+++ b/chrome/browser/sessions/session_types.cc
@@ -5,6 +5,7 @@
 #include "chrome/browser/sessions/session_types.h"
 
 #include "base/string_util.h"
+#include "chrome/browser/browser.h"
 #include "chrome/browser/tab_contents/navigation_controller.h"
 #include "chrome/browser/tab_contents/navigation_entry.h"
 
@@ -37,3 +38,16 @@ void TabNavigation::SetFromNavigationEntry(const NavigationEntry& entry) {
   transition_ = entry.transition_type();
   type_mask_ = entry.has_post_data() ? TabNavigation::HAS_POST_DATA : 0;
 }
+
+// SessionWindow ---------------------------------------------------------------
+
+SessionWindow::SessionWindow()
+    : selected_tab_index(-1),
+      type(Browser::TYPE_NORMAL),
+      is_constrained(true),
+      is_maximized(false) {
+}
+
+SessionWindow::~SessionWindow() {
+  STLDeleteElements(&tabs);
+}
diff --git a/chrome/browser/sessions/session_types.h b/chrome/browser/sessions/session_types.h
index 475b99b..f7962da 100644
--- a/chrome/browser/sessions/session_types.h
+++ b/chrome/browser/sessions/session_types.h
@@ -10,7 +10,7 @@
 
 #include "base/stl_util-inl.h"
 #include "base/string16.h"
-#include "chrome/browser/browser.h"
+#include "chrome/browser/profile.h"
 #include "chrome/browser/sessions/session_id.h"
 #include "chrome/common/page_transition_types.h"
 #include "gfx/rect.h"
@@ -149,12 +149,8 @@ struct SessionTab {
 
 // Describes a saved window.
 struct SessionWindow {
-  SessionWindow()
-      : selected_tab_index(-1),
-        type(Browser::TYPE_NORMAL),
-        is_constrained(true),
-        is_maximized(false) {}
-  ~SessionWindow() { STLDeleteElements(&tabs); }
+  SessionWindow();
+  ~SessionWindow();
 
   // Identifier of the window.
   SessionID window_id;
@@ -173,7 +169,8 @@ struct SessionWindow {
 
   // Type of the browser. Currently we only store browsers of type
   // TYPE_NORMAL and TYPE_POPUP.
-  Browser::Type type;
+  // This would be Browser::Type, but that would cause a circular dependency.
+  int type;
 
   // If true, the window is constrained.
   //
diff --git a/chrome/browser/tab_contents/tab_contents.cc b/chrome/browser/tab_contents/tab_contents.cc
index d1c61bc..86ad0c5 100644
--- a/chrome/browser/tab_contents/tab_contents.cc
+++ b/chrome/browser/tab_contents/tab_contents.cc
@@ -23,6 +23,7 @@
 #include "chrome/browser/autofill/autofill_manager.h"
 #include "chrome/browser/blocked_popup_container.h"
 #include "chrome/browser/bookmarks/bookmark_model.h"
+#include "chrome/browser/browser.h"
 #include "chrome/browser/browser_process.h"
 #include "chrome/browser/browser_shutdown.h"
 #include "chrome/browser/cert_store.h"
diff --git a/chrome/browser/tab_restore_uitest.cc b/chrome/browser/tab_restore_uitest.cc
index 4939c8c..f471b5b 100644
--- a/chrome/browser/tab_restore_uitest.cc
+++ b/chrome/browser/tab_restore_uitest.cc
@@ -301,9 +301,7 @@ TEST_F(TabRestoreUITest, MAYBE_BasicRestoreFromClosedWindow) {
 }
 
 // Restore a tab then make sure it doesn't restore again.
-// Disabled because the command updater doesn't know the proper state of
-// the tab restore command. http://crbug.com/14428.
-TEST_F(TabRestoreUITest, DISABLED_DontLoadRestoredTab) {
+TEST_F(TabRestoreUITest, DontLoadRestoredTab) {
   scoped_refptr<BrowserProxy> browser_proxy(automation()->GetBrowserWindow(0));
   ASSERT_TRUE(browser_proxy.get());
   CheckActiveWindow(browser_proxy.get());
diff --git a/chrome/browser/unload_uitest.cc b/chrome/browser/unload_uitest.cc
index e0185c8..d83ff25 100644
--- a/chrome/browser/unload_uitest.cc
+++ b/chrome/browser/unload_uitest.cc
@@ -187,7 +187,7 @@ class UnloadTest : public UITest {
 // we don't get confused and think we're closing the tab.
 //
 // This test is flaky on the valgrind UI bots. http://crbug.com/39057
-TEST_F(UnloadTest, FLAKY_CrossSiteInfiniteUnloadAsync) {
+TEST_F(UnloadTest, DISABLED_CrossSiteInfiniteUnloadAsync) {
   // Tests makes no sense in single-process mode since the renderer is hung.
   if (CommandLine::ForCurrentProcess()->HasSwitch(switches::kSingleProcess))
     return;
@@ -219,8 +219,8 @@ TEST_F(UnloadTest, CrossSiteInfiniteUnloadSync) {
 // Then an async crosssite request followed by an input event to ensure that
 // the short unload timeout (not the long input event timeout) is used.
 // See crbug.com/11007.
-TEST_F(UnloadTest, DISABLED_CrossSiteInfiniteUnloadAsyncInputEvent) {
-  // Tests makes no sense in single-process mode since the renderer is hung.
+TEST_F(UnloadTest, FLAKY_CrossSiteInfiniteUnloadAsyncInputEvent) {
+  // Tests makes no sense in single-process mode since the renderer is hung.c
   if (CommandLine::ForCurrentProcess()->HasSwitch(switches::kSingleProcess))
     return;
 
diff --git a/chrome/browser/views/frame/browser_view.cc b/chrome/browser/views/frame/browser_view.cc
index 3dc71af..3f7b353 100644
--- a/chrome/browser/views/frame/browser_view.cc
+++ b/chrome/browser/views/frame/browser_view.cc
@@ -1382,8 +1382,6 @@ bool BrowserView::IsCommandIdChecked(int command_id) const {
 }
 
 bool BrowserView::IsCommandIdEnabled(int command_id) const {
-  if (command_id == IDC_RESTORE_TAB)
-    return browser_->CanRestoreTab();
   return browser_->command_updater()->IsCommandEnabled(command_id);
 }