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

chrome/browser/ui/toolbar/recent_tabs_sub_menu_model.cc

 // Copyright 2012 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/ui/toolbar/recent_tabs_sub_menu_model.h"
 
 #include "base/bind.h"
 #include "base/strings/string_number_conversions.h"
 #include "base/utf_string_conversions.h"
 #include "chrome/app/chrome_command_ids.h"
@@ skipping 131 matching lines @@
 bool RecentTabsSubMenuModel::IsCommandIdChecked(int command_id) const {
   return false;
 }
 
 bool RecentTabsSubMenuModel::IsCommandIdEnabled(int command_id) const {
   if (command_id == IDC_RESTORE_TAB)
     return chrome::IsCommandEnabled(browser_, command_id);
   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());
 }
 
 bool RecentTabsSubMenuModel::GetAcceleratorForCommandId(
     int command_id, ui::Accelerator* accelerator) {
   if (command_id == IDC_RESTORE_TAB &&
       reopen_closed_tab_accelerator_.key_code() != ui::VKEY_UNKNOWN) {
     *accelerator = reopen_closed_tab_accelerator_;
@@ skipping 19 matching lines @@
     }
   }
   return l10n_util::GetStringUTF16(string_id);
 }
 
 void RecentTabsSubMenuModel::ExecuteCommand(int command_id, int event_flags) {
   if (command_id == IDC_RESTORE_TAB) {
     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);
   DCHECK_NE(IDC_RECENT_TABS_NO_DEVICE_TABS, command_id);
 
   int model_idx = CommandIdToModelIndex(command_id);
   DCHECK(model_idx >= 0 && model_idx < static_cast<int>(model_.size()));
   const NavigationItem& item = model_[model_idx];
   DCHECK(item.tab_id > -1 && item.url.is_valid());
 
@@ skipping 59 matching lines @@
   if (!associator)
     return;
 
   std::vector<const browser_sync::SyncedSession*> sessions;
   if (!associator->GetAllForeignSessions(&sessions))
     return;
 
   // 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;
 
     // Get windows of session.
     std::vector<const SessionWindow*> windows;
     if (!associator->GetForeignSession(session_tag, &windows) ||
         windows.empty()) {
       continue;
     }
 
@@ skipping 27 matching lines @@
     AddItem(kDisabledCommandId, UTF8ToUTF16(session->session_name));
     AddDeviceFavicon(GetItemCount() - 1, session->device_type);
 
     // Build tab menu items from sorted session tabs.
     const size_t kMaxTabsPerSessionToShow = 4;
     for (size_t k = 0;
          k < std::min(tabs_in_session.size(), kMaxTabsPerSessionToShow);
          ++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(
     const std::string& session_tag,
     const SessionTab& tab) {
   const TabNavigation& current_navigation =
       tab.navigations.at(tab.normalized_navigation_index());
   NavigationItem item(session_tag, tab.tab_id.id(),
                       current_navigation.virtual_url());
   int command_id = ModelIndexToCommandId(model_.size());
@@ skipping 89 matching lines @@
     RecentTabsSubMenuModel::GetModelAssociator() {
   if (!associator_) {
     ProfileSyncService* service = ProfileSyncServiceFactory::GetInstance()->
         GetForProfile(browser_->profile());
     // Only return the associator if it exists and it is done syncing sessions.
     if (service && service->ShouldPushChanges())
       associator_ = service->GetSessionModelAssociator();
   }
   return associator_;
 }