Dynamic updating recent menu for tabs from 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 <stddef.h>
 
 #include "base/bind.h"
 #include "base/metrics/histogram_macros.h"
@@ skipping 161 matching lines @@
   SessionID::id_type tab_id;  // -1 for invalid, >= 0 otherwise.
   base::string16 title;
   GURL url;
 };
 
 const int RecentTabsSubMenuModel::kRecentlyClosedHeaderCommandId = 1120;
 const int RecentTabsSubMenuModel::kDisabledRecentlyClosedHeaderCommandId = 1121;
 
 RecentTabsSubMenuModel::RecentTabsSubMenuModel(
     ui::AcceleratorProvider* accelerator_provider,
-    Browser* browser,
-    sync_sessions::OpenTabsUIDelegate* open_tabs_delegate)
+    Browser* browser)
     : ui::SimpleMenuModel(this),
       browser_(browser),
-      open_tabs_delegate_(open_tabs_delegate),
       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.
   sessions::TabRestoreService* service =
       TabRestoreServiceFactory::GetForProfile(browser_->profile());
   if (service) {
     service->LoadTabsFromLastSession();
 
   // TODO(sail): enable this when mac implements the dynamic menu, together with
   // MenuModelDelegate::MenuStructureChanged().
 #if !defined(OS_MACOSX)
     service->AddObserver(this);
 #endif
   }
 
+#if !defined(OS_MACOSX)

[Peter Kasting, 2017/03/23 04:48:34]

Nit: Add note about why this isn't on for Mac

+  browser_sync::ProfileSyncService* sync_service =
+      ProfileSyncServiceFactory::GetInstance()->GetForProfile(
+          browser_->profile());
+  if (sync_service && sync_service->IsSyncActive()) {

[Peter Kasting, 2017/03/23 04:48:34]

Nit: No {}

+    sync_service->AddObserver(this);
+  }
+#endif
+
   Build();
 
   // Retrieve accelerator key for IDC_RESTORE_TAB now, because on ASH, it's not
   // defined in |accelerator_provider|, but in shell, so simply retrieve it now
   // for all ASH and non-ASH for use in |GetAcceleratorForCommandId|.
 #if defined(USE_ASH)
   for (size_t i = 0; i < ash::kAcceleratorDataLength; ++i) {
     const ash::AcceleratorData& accel_data = ash::kAcceleratorData[i];
     if (accel_data.action == ash::RESTORE_TAB) {
       reopen_closed_tab_accelerator_ = ui::Accelerator(accel_data.keycode,
@@ skipping 12 matching lines @@
     accelerator_provider->GetAcceleratorForCommandId(
         IDC_SHOW_HISTORY, &show_history_accelerator_);
   }
 }
 
 RecentTabsSubMenuModel::~RecentTabsSubMenuModel() {
   sessions::TabRestoreService* service =
       TabRestoreServiceFactory::GetForProfile(browser_->profile());
   if (service)
     service->RemoveObserver(this);
+
+#if !defined(OS_MACOSX)
+  browser_sync::ProfileSyncService* sync_service =
+      ProfileSyncServiceFactory::GetInstance()->GetForProfile(
+          browser_->profile());
+  if (sync_service) {
+    sync_service->RemoveObserver(this);

[Peter Kasting, 2017/03/23 04:48:34]

Can this be done with a ScopedObserver class member instead of manual Add/Remove
calls?  Or is it possible that the sync service will be torn down while the menu
is open?  (I would think a ScopedObserver would work, and for the
TabRestoreService as well.)

+  }
+#endif
 }
 
 bool RecentTabsSubMenuModel::IsCommandIdChecked(int command_id) const {
   return false;
 }
 
 bool RecentTabsSubMenuModel::IsCommandIdEnabled(int command_id) const {
   if (command_id == kRecentlyClosedHeaderCommandId ||
       command_id == kDisabledRecentlyClosedHeaderCommandId ||
       command_id == IDC_RECENT_TABS_NO_DEVICE_TABS ||
@@ skipping 218 matching lines @@
         }
       }
       ++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.
+  // a menu item, because they take always place in the end of menu.
 
   sync_sessions::OpenTabsUIDelegate* open_tabs = GetOpenTabsUIDelegate();
   std::vector<const sync_sessions::SyncedSession*> sessions;
   if (!open_tabs || !open_tabs->GetAllForeignSessions(&sessions)) {
     AddSeparator(ui::NORMAL_SEPARATOR);
     AddItemWithStringId(IDC_RECENT_TABS_NO_DEVICE_TABS,
                         IDS_RECENT_TABS_NO_DEVICE_TABS);
     return;
   }
 
@@ skipping 212 matching lines @@
   // 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();
 }
 
+void RecentTabsSubMenuModel::ClearTabsFromOtherDevices() {
+  DCHECK_GE(last_local_model_index_, 0);
+  int count = GetItemCount();
+  for (int index = count - 1; index > last_local_model_index_; --index)
+    RemoveItemAt(index);
+
+  // Cancel asynchronous FaviconService::GetFaviconImageForPageURL() tasks of

[Peter Kasting, 2017/03/23 04:48:34]

Nit: These comments (here and below) add nothing to the code, remove them.

+  // all tabs from the other devices.
+  other_devices_tab_cancelable_task_tracker_.TryCancelAll();
+
+  // Remove all tab from the other devices navigation items.
+  other_devices_tab_navigation_items_.clear();
+}
+
 sync_sessions::OpenTabsUIDelegate*
 RecentTabsSubMenuModel::GetOpenTabsUIDelegate() {
   if (!open_tabs_delegate_) {
     browser_sync::ProfileSyncService* service =
         ProfileSyncServiceFactory::GetInstance()->GetForProfile(
             browser_->profile());
     // Only return the delegate if it exists and it is done syncing sessions.
     if (service && service->IsSyncActive())
       open_tabs_delegate_ = service->GetOpenTabsUIDelegate();
   }
   return open_tabs_delegate_;
 }
 
 void RecentTabsSubMenuModel::TabRestoreServiceChanged(
     sessions::TabRestoreService* service) {
   ClearLocalEntries();
 
   BuildLocalEntries();
 
   ui::MenuModelDelegate* menu_model_delegate = GetMenuModelDelegate();
   if (menu_model_delegate)
     menu_model_delegate->OnMenuStructureChanged();
 }
 
 void RecentTabsSubMenuModel::TabRestoreServiceDestroyed(
     sessions::TabRestoreService* service) {
   TabRestoreServiceChanged(service);
 }
+
+void RecentTabsSubMenuModel::OnForeignSessionUpdated(
+    syncer::SyncService* sync) {
+  ClearTabsFromOtherDevices();
+
+  BuildTabsFromOtherDevices();
+
+  ui::MenuModelDelegate* menu_model_delegate = GetMenuModelDelegate();
+  if (menu_model_delegate)
+    menu_model_delegate->OnMenuStructureChanged();
+}