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) {

[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());
 }
 
 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) {

[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);
   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 61 matching lines @@
 
   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);
 
   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;
 
     // Get windows of session.
     std::vector<const SessionWindow*> windows;
     if (!associator->GetForeignSession(session_tag, &windows) ||
         windows.empty()) {
       continue;
     }
 
@@ skipping 30 matching lines @@
     // 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
 
     ++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(
     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_;
 }