Wrench menu reorg phase 2

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/metrics/histogram.h"
 #include "base/prefs/scoped_user_pref_update.h"
 #include "base/strings/string_number_conversions.h"
@@ skipping 51 matching lines @@
 const int kFirstLocalTabCommandId = WrenchMenuModel::kMinRecentTabsCommandId;
 const int kFirstLocalWindowCommandId = 1031;
 const int kFirstOtherDevicesTabCommandId = 1051;
 const int kMinDeviceNameCommandId = 1100;
 const int kMaxDeviceNameCommandId = 1110;
 
 // The maximum number of local recently closed entries (tab or window) to be
 // shown in the menu.
 const int kMaxLocalEntries = 8;
 
+// Index of the separator that follows the history menu item. Used as a
+// reference position for inserting local entries.
+const int kHistorySeparatorIndex = 1;
+
 // Comparator function for use with std::sort that will sort sessions by
 // descending modified_time (i.e., most recent first).
 bool SortSessionsByRecency(const browser_sync::SyncedSession* s1,
                            const browser_sync::SyncedSession* s2) {
   return s1->modified_time > s2->modified_time;
 }
 
 // Comparator function for use with std::sort that will sort tabs by
 // descending timestamp (i.e., most recent first).
 bool SortTabsByRecency(const sessions::SessionTab* t1,
@@ skipping 82 matching lines @@
 const int RecentTabsSubMenuModel::kRecentlyClosedHeaderCommandId = 1120;
 const int RecentTabsSubMenuModel::kDisabledRecentlyClosedHeaderCommandId = 1121;
 
 RecentTabsSubMenuModel::RecentTabsSubMenuModel(
     ui::AcceleratorProvider* accelerator_provider,
     Browser* browser,
     browser_sync::OpenTabsUIDelegate* open_tabs_delegate)
     : ui::SimpleMenuModel(this),
       browser_(browser),
       open_tabs_delegate_(open_tabs_delegate),
-      last_local_model_index_(-1),
+      last_local_model_index_(kHistorySeparatorIndex),
       default_favicon_(ui::ResourceBundle::GetSharedInstance().
                        GetNativeImageNamed(IDR_DEFAULT_FAVICON)),
       weak_ptr_factory_(this) {
   // Invoke asynchronous call to load tabs from local last session, which does
   // nothing if the tabs have already been loaded or they shouldn't be loaded.
   // TabRestoreServiceChanged() will be called after the tabs are loaded.
   TabRestoreService* service =
       TabRestoreServiceFactory::GetForProfile(browser_->profile());
   if (service) {
     service->LoadTabsFromLastSession();
@@ skipping 18 matching lines @@
                                                        accel_data.modifiers);
       break;
     }
   }
 #else
   if (accelerator_provider) {
     accelerator_provider->GetAcceleratorForCommandId(
         IDC_RESTORE_TAB, &reopen_closed_tab_accelerator_);
   }
 #endif  // defined(USE_ASH)
+
+  if (accelerator_provider) {
+    accelerator_provider->GetAcceleratorForCommandId(
+        IDC_SHOW_HISTORY, &show_history_accelerator_);
+  }
 }
 
 RecentTabsSubMenuModel::~RecentTabsSubMenuModel() {
   TabRestoreService* service =
       TabRestoreServiceFactory::GetForProfile(browser_->profile());
   if (service)
     service->RemoveObserver(this);
 }
 
 bool RecentTabsSubMenuModel::IsCommandIdChecked(int command_id) const {
@@ skipping 15 matching lines @@
   // If there are no recently closed items, we show the accelerator beside
   // the header, otherwise, we show it beside the first item underneath it.
   int index_in_menu = GetIndexOfCommandId(command_id);
   int header_index = GetIndexOfCommandId(kRecentlyClosedHeaderCommandId);
   if ((command_id == kDisabledRecentlyClosedHeaderCommandId ||
        (header_index != -1 && index_in_menu == header_index + 1)) &&
       reopen_closed_tab_accelerator_.key_code() != ui::VKEY_UNKNOWN) {
     *accelerator = reopen_closed_tab_accelerator_;
     return true;
   }
+
+  if (command_id == IDC_SHOW_HISTORY) {
+    *accelerator = show_history_accelerator_;
+    return true;
+  }
+
   return false;
 }
 
 void RecentTabsSubMenuModel::ExecuteCommand(int command_id, int event_flags) {
   if (command_id == IDC_SHOW_HISTORY) {
     UMA_HISTOGRAM_ENUMERATION("WrenchMenu.RecentTabsSubMenu", SHOW_MORE,
                               LIMIT_RECENT_TAB_ACTION);
     // We show all "other devices" on the history page.
     chrome::ExecuteCommandWithDisposition(browser_, IDC_SHOW_HISTORY,
         ui::DispositionFromEventFlags(event_flags));
@@ skipping 101 matching lines @@
     const TabNavigationItem& item = (*tab_items)[tab_items_idx];
     *url = item.url.possibly_invalid_spec();
     *title = item.title;
     return true;
   }
   return false;
 }
 
 void RecentTabsSubMenuModel::Build() {
   // The menu contains:
+  // - History to open the full history tab.
+  // - Separator
   // - Recently closed header, then list of local recently closed tabs/windows,
   //   then separator
   // - device 1 section header, then list of tabs from device, then separator
   // - device 2 section header, then list of tabs from device, then separator
   // - device 3 section header, then list of tabs from device, then separator
-  // - More... to open the history tab to get more other devices.
   // |local_tab_navigation_items_| and |other_devices_tab_navigation_items_|
   // only contain navigatable (and hence executable) tab items for local
   // recently closed tabs and tabs from other devices respectively.
   // |local_window_items_| contains the local recently closed windows.
+  InsertItemWithStringIdAt(0, IDC_SHOW_HISTORY, IDS_SHOW_HISTORY);
+  InsertSeparatorAt(1, ui::NORMAL_SEPARATOR);

[Peter Kasting, 2015/06/22 23:14:24]

Can we ever have no items from both the below calls?  If so, having a separator
here would be wrong.  (Of course, without it, we're down to just one item in
this submenu, in which case we probably shouldn't have the submenu at all, and
we should replace it with a top-level "History" item, maybe...)

[edwardjung, 2015/06/24 12:41:59]

No items in both cases would result in the following minimum menu items: 
- Recently closed (disabled, showing shortcut for user education) 
- Separator
- No tabs from other devices (disabled)

   BuildLocalEntries();
   BuildTabsFromOtherDevices();
 }
 
 void RecentTabsSubMenuModel::BuildLocalEntries() {
+  last_local_model_index_ = kHistorySeparatorIndex;
+
   // All local items use InsertItem*At() to append or insert a menu item.
   // We're appending if building the entries for the first time i.e. invoked
   // from Constructor(), inserting when local entries change subsequently i.e.
   // invoked from TabRestoreServiceChanged().
-
-  DCHECK_EQ(last_local_model_index_, -1);
-
   TabRestoreService* service =
       TabRestoreServiceFactory::GetForProfile(browser_->profile());
+
   if (!service || service->entries().size() == 0) {
     // This is to show a disabled restore tab entry with the accelerator to
     // teach users about this command.
     InsertItemWithStringIdAt(++last_local_model_index_,
                              kDisabledRecentlyClosedHeaderCommandId,
                              IDS_RECENTLY_CLOSED);
   } else {
     InsertItemWithStringIdAt(++last_local_model_index_,
                              kRecentlyClosedHeaderCommandId,
                              IDS_RECENTLY_CLOSED);
@@ skipping 19 matching lines @@
       } else  {
         DCHECK_EQ(entry->type, TabRestoreService::WINDOW);
         BuildLocalWindowItem(
             entry->id,
             static_cast<TabRestoreService::Window*>(entry)->tabs.size(),
             ++last_local_model_index_);
       }
       ++added_count;
     }
   }
-
   DCHECK_GE(last_local_model_index_, 0);
 }
 
 void RecentTabsSubMenuModel::BuildTabsFromOtherDevices() {
   // All other devices' items (device headers or tabs) use AddItem*() to append
   // a menu item, because they are always only built once (i.e. invoked from
   // Constructor()) and don't change after that.
 
   browser_sync::OpenTabsUIDelegate* open_tabs = GetOpenTabsUIDelegate();
   std::vector<const browser_sync::SyncedSession*> sessions;
@@ skipping 59 matching lines @@
          k < std::min(tabs_in_session.size(), kMaxTabsPerSessionToShow);
          ++k) {
       BuildOtherDevicesTabItem(session_tag, *tabs_in_session[k]);
     }  // for all tabs in one session
 
     ++num_sessions_added;
   }  // for all sessions
 
   // We are not supposed to get here unless at least some items were added.
   DCHECK_GT(GetItemCount(), 0);
-  AddSeparator(ui::NORMAL_SEPARATOR);
-  AddItemWithStringId(IDC_SHOW_HISTORY, IDS_RECENT_TABS_MORE);
 }
 
 void RecentTabsSubMenuModel::BuildLocalTabItem(int session_id,
                                                const base::string16& title,
                                                const GURL& url,
                                                int curr_model_index) {
   TabNavigationItem item(std::string(), session_id, title, url);
   int command_id = TabVectorIndexToCommandId(
       local_tab_navigation_items_.size(), kFirstLocalTabCommandId);
   // See comments in BuildLocalEntries() about usage of InsertItem*At().
@@ skipping 123 matching lines @@
   if (command_id >= kFirstOtherDevicesTabCommandId) {
     *tab_items = &other_devices_tab_navigation_items_;
     return command_id - kFirstOtherDevicesTabCommandId;
   }
   *tab_items = &local_tab_navigation_items_;
   return command_id - kFirstLocalTabCommandId;
 }
 
 void RecentTabsSubMenuModel::ClearLocalEntries() {
   // Remove local items (recently closed tabs and windows) from menumodel.
-  while (last_local_model_index_ >= 0)
+  while (last_local_model_index_ > kHistorySeparatorIndex)
     RemoveItemAt(last_local_model_index_--);
 
   // Cancel asynchronous FaviconService::GetFaviconImageForPageURL() tasks of
-  // all
-  // local tabs.
+  // all local tabs.
   local_tab_cancelable_task_tracker_.TryCancelAll();
 
   // Remove all local tab navigation items.
   local_tab_navigation_items_.clear();
 
   // Remove all local window items.
   local_window_items_.clear();
 }
 
 browser_sync::OpenTabsUIDelegate*
@@ skipping 16 matching lines @@
 
   ui::MenuModelDelegate* menu_model_delegate = GetMenuModelDelegate();
   if (menu_model_delegate)
     menu_model_delegate->OnMenuStructureChanged();
 }
 
 void RecentTabsSubMenuModel::TabRestoreServiceDestroyed(
     TabRestoreService* service) {
   TabRestoreServiceChanged(service);
 }