Add new option to open history page to show more other devices

chrome/browser/ui/toolbar/recent_tabs_sub_menu_model.cc

Index: chrome/browser/ui/toolbar/recent_tabs_sub_menu_model.cc
diff --git a/chrome/browser/ui/toolbar/recent_tabs_sub_menu_model.cc b/chrome/browser/ui/toolbar/recent_tabs_sub_menu_model.cc
index d8d5204575afe9a1a9644f67fa4f559b63d30c57..84d235b92342649f1e3d1e2820e81a9c8f19b083 100644
--- a/chrome/browser/ui/toolbar/recent_tabs_sub_menu_model.cc
+++ b/chrome/browser/ui/toolbar/recent_tabs_sub_menu_model.cc
@@ -149,6 +149,8 @@ bool RecentTabsSubMenuModel::IsCommandIdEnabled(int command_id) const {
   if (command_id == kDisabledCommandId ||
       command_id == IDC_RECENT_TABS_NO_DEVICE_TABS) {
     return false;
+  } else if (command_id == IDC_RECENT_TABS_MORE) {

[sky, 2013/03/22 03:55:56]

nit: no else for returns (see style guide), just as 149 is not an else.

[MAD, 2013/03/22 14:17:41]

Ho, yes, sorry, I keep forgetting about that one... :-(

+    return true;
   }
   int model_index = CommandIdToModelIndex(command_id);
   return model_index >= 0 && model_index < static_cast<int>(model_.size());
@@ -188,6 +190,11 @@ void RecentTabsSubMenuModel::ExecuteCommand(int command_id, int event_flags) {
     chrome::ExecuteCommandWithDisposition(browser_, command_id,
         ui::DispositionFromEventFlags(event_flags));
     return;
+  } else if (command_id == IDC_RECENT_TABS_MORE) {

[sky, 2013/03/22 03:55:56]

same comment as 152.

[MAD, 2013/03/22 14:17:41]

Done.

+    // We show all "other devices" on the history page.
+    chrome::ExecuteCommandWithDisposition(browser_, IDC_SHOW_HISTORY,
+        ui::DispositionFromEventFlags(event_flags));

[sky, 2013/03/22 03:55:56]

Does this do something more than just show history?

[MAD, 2013/03/22 14:17:41]

No, why?

+    return;
   }
 
   DCHECK_NE(kDisabledCommandId, command_id);
@@ -269,9 +276,8 @@ void RecentTabsSubMenuModel::BuildDevices() {
 
   const size_t kMaxSessionsToShow = 3;
   size_t num_sessions_added = 0;
-  for (size_t i = 0;
-       i < sessions.size() && num_sessions_added < kMaxSessionsToShow;
-       ++i) {
+  size_t i = 0;

[sky, 2013/03/22 03:55:56]

Why are you moving i outside the loop?

[MAD, 2013/03/22 14:17:41]

Oups, sorry about that... I should have brought it back as it was... I did that
for a previous version of that change...

+  for (; i < sessions.size() && num_sessions_added < kMaxSessionsToShow; ++i) {
     const browser_sync::SyncedSession* session = sessions[i];
     const std::string& session_tag = session->session_tag;
 
@@ -322,6 +328,9 @@ void RecentTabsSubMenuModel::BuildDevices() {
 
     ++num_sessions_added;
   }  // for all sessions
+
+  AddSeparator(ui::NORMAL_SEPARATOR);

[sky, 2013/03/22 03:55:56]

Is it possible to get here and nothing has been added?

[MAD, 2013/03/22 14:17:41]

Added a comment... Thanks!

+  AddItemWithStringId(IDC_RECENT_TABS_MORE, IDS_RECENT_TABS_MORE);
 }
 
 void RecentTabsSubMenuModel::BuildForeignTabItem(