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),
+      open_tabs_delegate_(nullptr),
       last_local_model_index_(kHistorySeparatorIndex),
       default_favicon_(
           ui::ResourceBundle::GetSharedInstance().GetNativeImageNamed(
               IDR_DEFAULT_FAVICON)),
+#if !defined(OS_MACOSX)
+      tab_restore_service_observer_(this),
+      sync_observer_(this),
+#endif  // !defined(OS_MACOSX)
       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().
+// Mac doesn't support the dynamic menu.
 #if !defined(OS_MACOSX)
-    service->AddObserver(this);
+    tab_restore_service_observer_.Add(service);
 #endif
   }
 
+// Mac doesn't support the dynamic menu.
+#if !defined(OS_MACOSX)
+  browser_sync::ProfileSyncService* sync_service =
+      ProfileSyncServiceFactory::GetInstance()->GetForProfile(
+          browser_->profile());
+  if (sync_service && sync_service->IsSyncActive())

[pavely, 2017/03/24 01:20:05]

Is this line executed once per process or every time menu is opened? If former
then it could catch sync_service before it is initialized and therefore not
subscribe for notifications.

[Peter Kasting, 2017/03/24 01:35:21]

Looks like it's called each time the menu is opened.

However, that makes me wonder: can this conditional be false when the menu is
opened, but become true later while the menu is still open, at which point we'd
ideally like to be receiving updates?  If yes, how do we best fix this?  Is it
safe to assume that |sync_service| would always be non-null in such a case, but
IsSyncActive() might not be, yet?  Should we just remove the check for
IsSyncActive() here, or is there something more complex to do?

[pavely, 2017/03/24 04:13:22]

Yeah, we should remove call to IsSyncActive(). If sync is enabled then
sync_service is not nullptr and you can subscribe for notifications regardless
of ISyncActive() state. You should still keep if(sync_service) check though, but
if it is nullptr it is guaranteed to stay nullptr.

+    sync_observer_.Add(sync_service);
+#endif  // !defined(OS_MACOSX)
+
   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,
                                                        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() {
-  sessions::TabRestoreService* service =
-      TabRestoreServiceFactory::GetForProfile(browser_->profile());
-  if (service)
-    service->RemoveObserver(this);
-}
+RecentTabsSubMenuModel::~RecentTabsSubMenuModel() {}
 
 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 ||
       IsDeviceNameCommandId(command_id)) {
@@ skipping 217 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);
+
+  other_devices_tab_cancelable_task_tracker_.TryCancelAll();
+
+  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::OnSyncCycleCompleted(syncer::SyncService* sync) {}
+
+void RecentTabsSubMenuModel::OnForeignSessionUpdated(
+    syncer::SyncService* sync) {
+  ClearTabsFromOtherDevices();
+
+  BuildTabsFromOtherDevices();
+
+  ui::MenuModelDelegate* menu_model_delegate = GetMenuModelDelegate();
+  if (menu_model_delegate)
+    menu_model_delegate->OnMenuStructureChanged();
+}