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..2f2bb372d25fdd0a050fcc440e83450777973d6f 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_DEVICES) {
+    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_DEVICES) {
+    // We show all "other devices" on the history page.
+    chrome::ExecuteCommandWithDisposition(browser_, IDC_SHOW_HISTORY,
+        ui::DispositionFromEventFlags(event_flags));
+    return;
   }
 
   DCHECK_NE(kDisabledCommandId, command_id);
@@ -267,11 +274,11 @@ void RecentTabsSubMenuModel::BuildDevices() {
   // Sort sessions from most recent to least recent.
   std::sort(sessions.begin(), sessions.end(), SortSessionsByRecency);
 
+  bool show_more = false;
   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;
+  for (; i < sessions.size() && num_sessions_added < kMaxSessionsToShow; ++i) {
     const browser_sync::SyncedSession* session = sessions[i];
     const std::string& session_tag = session->session_tag;
 
@@ -319,9 +326,16 @@ void RecentTabsSubMenuModel::BuildDevices() {
          ++k) {
       BuildForeignTabItem(session_tag, *tabs_in_session[k]);
     }  // for all tabs in one session
+    if (tabs_in_session.size() > kMaxTabsPerSessionToShow)
+      show_more = true;
 
     ++num_sessions_added;
   }  // for all sessions

[Alexei Svitkine (slow), 2013/03/19 13:48:21]

Nit: Add a blank line after this.

[MAD, 2013/03/19 14:26:14]

Done.

+  if (show_more || i < sessions.size()) {

[Alexei Svitkine (slow), 2013/03/19 13:48:21]

Shouldn't this be checking |num_sessions_added| rather than |i|? (Since the loop
above can skip certain i's).

[MAD, 2013/03/19 14:26:14]

This is what I was doing initially, but I changed it to i since we could have
added less tabs than size() yet still reached the end of the list when we
skipped some and comparing it to kMaxSessionsToShow doesn't tell us much either
since we may have reached the max on the last i. Unless I'm missing something,
this seems more robust, I added a comment to explaining, let me know if it's
clear enough.

Thanks!

+    AddSeparator(ui::NORMAL_SEPARATOR);
+    AddItemWithStringId(IDC_RECENT_TABS_MORE_DEVICES,
+                        IDS_RECENT_TABS_MORE_DEVICES);
+  }
 }
 
 void RecentTabsSubMenuModel::BuildForeignTabItem(