Dynamic updating recent menu for tabs from other devices.

chrome/browser/ui/toolbar/recent_tabs_sub_menu_model_unittest.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 <memory>
 #include <string>
 #include <vector>
 
 #include "base/command_line.h"
 #include "base/macros.h"
 #include "base/memory/ptr_util.h"
 #include "base/run_loop.h"
+#include "base/strings/utf_string_conversions.h"
 #include "build/build_config.h"
 #include "chrome/app/chrome_command_ids.h"
 #include "chrome/browser/sessions/chrome_tab_restore_service_client.h"
 #include "chrome/browser/sessions/session_service.h"
 #include "chrome/browser/sessions/session_service_factory.h"
 #include "chrome/browser/sessions/tab_restore_service_factory.h"
+#include "chrome/browser/sync/profile_sync_service_factory.h"
 #include "chrome/browser/sync/profile_sync_test_util.h"
 #include "chrome/browser/ui/browser.h"
 #include "chrome/browser/ui/browser_tabstrip.h"
 #include "chrome/browser/ui/sync/browser_synced_window_delegates_getter.h"
 #include "chrome/browser/ui/tabs/tab_strip_model.h"
 #include "chrome/browser/ui/toolbar/recent_tabs_builder_test_helper.h"
 #include "chrome/common/chrome_switches.h"
 #include "chrome/test/base/browser_with_test_window_test.h"
 #include "chrome/test/base/menu_model_test.h"
 #include "chrome/test/base/testing_profile.h"
 #include "components/browser_sync/profile_sync_service_mock.h"
 #include "components/sessions/core/persistent_tab_restore_service.h"
 #include "components/sessions/core/serialized_navigation_entry_test_helper.h"
 #include "components/sessions/core/session_types.h"
 #include "components/sync/base/sync_prefs.h"
 #include "components/sync/device_info/local_device_info_provider_mock.h"
 #include "components/sync/driver/sync_client.h"
 #include "components/sync/model/fake_sync_change_processor.h"
 #include "components/sync/model/sync_error_factory_mock.h"
 #include "components/sync_sessions/sessions_sync_manager.h"
 #include "components/sync_sessions/synced_session.h"
 #include "content/public/browser/browser_thread.h"
 #include "content/public/test/test_utils.h"
 #include "testing/gmock/include/gmock/gmock.h"
 #include "testing/gtest/include/gtest/gtest.h"
 
+using testing::_;
+using testing::Invoke;
+using testing::Return;
+
 namespace {
 
 // This copies parts of MenuModelTest::Delegate and combines them with the
 // RecentTabsSubMenuModel since RecentTabsSubMenuModel is a
 // SimpleMenuModel::Delegate and not just derived from SimpleMenuModel.
 class TestRecentTabsSubMenuModel : public RecentTabsSubMenuModel {
  public:
   TestRecentTabsSubMenuModel(ui::AcceleratorProvider* provider,
-                             Browser* browser,
-                             sync_sessions::OpenTabsUIDelegate* delegate)
-      : RecentTabsSubMenuModel(provider, browser, delegate),
+                             Browser* browser)
+      : RecentTabsSubMenuModel(provider, browser),
         execute_count_(0),
         enable_count_(0) {}
 
   // Testing overrides to ui::SimpleMenuModel::Delegate:
   bool IsCommandIdEnabled(int command_id) const override {
     bool val = RecentTabsSubMenuModel::IsCommandIdEnabled(command_id);
     if (val)
       ++enable_count_;
     return val;
   }
@@ skipping 45 matching lines @@
   void StartRoutingTo(
       sync_sessions::LocalSessionEventHandler* handler) override {}
   void Stop() override {}
 };
 
 }  // namespace
 
 class RecentTabsSubMenuModelTest
     : public BrowserWithTestWindowTest {
  public:
-  RecentTabsSubMenuModelTest()
-      : sync_service_(CreateProfileSyncServiceParamsForTest(&testing_profile_)),
-        local_device_(new syncer::LocalDeviceInfoProviderMock(
-            "RecentTabsSubMenuModelTest",
-            "Test Machine",
-            "Chromium 10k",
-            "Chrome 10k",
-            sync_pb::SyncEnums_DeviceType_TYPE_LINUX,
-            "device_id")) {
-    sync_prefs_.reset(new syncer::SyncPrefs(testing_profile_.GetPrefs()));
+  RecentTabsSubMenuModelTest() {}
+
+  void SetUp() override {
+    BrowserWithTestWindowTest::SetUp();
+
+    local_device_ = base::MakeUnique<syncer::LocalDeviceInfoProviderMock>(

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

This seems very heavyweight.  It would be nice to avoid Gmock entirely, perhaps
by simply calling the "foreign session updated" method of the menu model
directly?  See also comments in the sync code.

+        "RecentTabsSubMenuModelTest", "Test Machine", "Chromium 10k",
+        "Chrome 10k", sync_pb::SyncEnums_DeviceType_TYPE_LINUX, "device_id");
+
+    sync_prefs_.reset(new syncer::SyncPrefs(profile()->GetPrefs()));
+
+    mock_sync_service_ = static_cast<browser_sync::ProfileSyncServiceMock*>(
+        ProfileSyncServiceFactory::GetInstance()->SetTestingFactoryAndUse(
+            profile(), BuildMockProfileSyncService));
+
+    EXPECT_CALL(*mock_sync_service_, AddObserver(_))
+        .WillRepeatedly(Invoke(this, &RecentTabsSubMenuModelTest::AddObserver));
+    EXPECT_CALL(*mock_sync_service_, RemoveObserver(_))
+        .WillRepeatedly(
+            Invoke(this, &RecentTabsSubMenuModelTest::RemoveObserver));
+    EXPECT_CALL(*mock_sync_service_, IsSyncActive())
+        .WillRepeatedly(Return(true));
+    EXPECT_CALL(*mock_sync_service_,
+                IsDataTypeControllerRunning(syncer::SESSIONS))
+        .WillRepeatedly(Return(true));
+    EXPECT_CALL(*mock_sync_service_,
+                IsDataTypeControllerRunning(syncer::PROXY_TABS))
+        .WillRepeatedly(Return(true));
+
     manager_.reset(new sync_sessions::SessionsSyncManager(
-        sync_service_.GetSyncClient()->GetSyncSessionsClient(),
+        mock_sync_service_->GetSyncClient()->GetSyncSessionsClient(),
         sync_prefs_.get(), local_device_.get(),
         std::unique_ptr<sync_sessions::LocalSessionEventRouter>(
             new DummyRouter()),
-        base::Closure(), base::Closure()));
+        base::Bind(
+            &browser_sync::ProfileSyncService::NotifyForeignSessionUpdated,
+            base::Unretained(mock_sync_service_)),
+        base::Closure()));
+
+    EXPECT_CALL(*mock_sync_service_, GetOpenTabsUIDelegateMock())
+        .WillRepeatedly(Return(manager_.get()));
+
     manager_->MergeDataAndStartSyncing(
         syncer::SESSIONS, syncer::SyncDataList(),
         std::unique_ptr<syncer::SyncChangeProcessor>(
             new syncer::FakeSyncChangeProcessor),
         std::unique_ptr<syncer::SyncErrorFactory>(
             new syncer::SyncErrorFactoryMock));
   }
 
+  void TearDown() override {
+    manager_.reset();
+    sync_prefs_.reset();
+    local_device_.reset();
+    BrowserWithTestWindowTest::TearDown();
+  }
+
+  void AddObserver(syncer::SyncServiceObserver* observer) {
+    mock_sync_service_->SyncServiceBase::AddObserver(observer);
+  }
+
+  void RemoveObserver(syncer::SyncServiceObserver* observer) {
+    mock_sync_service_->SyncServiceBase::RemoveObserver(observer);
+  }
+
   void WaitForLoadFromLastSession() {
     content::RunAllBlockingPoolTasksUntilIdle();
   }
 
   static std::unique_ptr<KeyedService> GetTabRestoreService(
       content::BrowserContext* browser_context) {
     return base::MakeUnique<sessions::PersistentTabRestoreService>(
         base::WrapUnique(new ChromeTabRestoreServiceClient(
             Profile::FromBrowserContext(browser_context))),
         nullptr);
   }
 
   sync_sessions::OpenTabsUIDelegate* GetOpenTabsDelegate() {
     return manager_.get();
   }
 
   void RegisterRecentTabs(RecentTabsBuilderTestHelper* helper) {
     helper->ExportToSessionsSyncManager(manager_.get());
   }
 
  private:
-  TestingProfile testing_profile_;
-  browser_sync::ProfileSyncServiceMock sync_service_;
+  std::unique_ptr<syncer::LocalDeviceInfoProviderMock> local_device_;
   std::unique_ptr<syncer::SyncPrefs> sync_prefs_;
+  browser_sync::ProfileSyncServiceMock* mock_sync_service_ = nullptr;
   std::unique_ptr<sync_sessions::SessionsSyncManager> manager_;
-  std::unique_ptr<syncer::LocalDeviceInfoProviderMock> local_device_;
 };
 
 // Test disabled "Recently closed" header with no foreign tabs.
 TEST_F(RecentTabsSubMenuModelTest, NoTabs) {
-  TestRecentTabsSubMenuModel model(NULL, browser(), NULL);
+  TestRecentTabsSubMenuModel model(nullptr, browser());
 
   // Expected menu:
   // Menu index  Menu items
   // ---------------------------------------------
   // 0           History
   // 1           <separator>
   // 2           Recently closed header (disabled)
   // 3           <separator>
   // 4           No tabs from other Devices
 
@@ skipping 21 matching lines @@
 // Test enabled "Recently closed" header with no foreign tabs.
 TEST_F(RecentTabsSubMenuModelTest, RecentlyClosedTabsFromCurrentSession) {
   TabRestoreServiceFactory::GetInstance()->SetTestingFactory(
       profile(), RecentTabsSubMenuModelTest::GetTabRestoreService);
 
   // Add 2 tabs and close them.
   AddTab(browser(), GURL("http://foo/1"));
   AddTab(browser(), GURL("http://foo/2"));
   browser()->tab_strip_model()->CloseAllTabs();
 
-  TestRecentTabsSubMenuModel model(NULL, browser(), NULL);
+  TestRecentTabsSubMenuModel model(nullptr, browser());
   // Expected menu:
   // Menu index  Menu items
   // --------------------------------------
   // 0           History
   // 1           <separator>
   // 2           Recently closed header
   // 3           <tab for http://foo/2>
   // 4           <tab for http://foo/1>
   // 5           <separator>
   // 6           No tabs from other Devices
@@ skipping 73 matching lines @@
   SessionServiceFactory::GetForProfile(profile())->
       MoveCurrentSessionToLastSession();
 
   // Create a new TabRestoreService so that it'll load the recently closed tabs
   // and windows afresh.
   TabRestoreServiceFactory::GetInstance()->SetTestingFactory(
       profile(), RecentTabsSubMenuModelTest::GetTabRestoreService);
   // Let the shutdown of previous TabRestoreService run.
   content::RunAllBlockingPoolTasksUntilIdle();
 
-  TestRecentTabsSubMenuModel model(NULL, browser(), NULL);
+  TestRecentTabsSubMenuModel model(nullptr, browser());
   TestRecentTabsMenuModelDelegate delegate(&model);
   EXPECT_FALSE(delegate.got_changes());
 
   // Expected menu before tabs/windows from last session are loaded:
   // Menu index  Menu items
   // ----------------------------------------------------------------
   // 0           History
   // 1           <separator>
   // 2           Recently closed header
   // 3           <separator>
@@ skipping 104 matching lines @@
   // 1           <separator>
   // 2           Recently closed header (disabled)
   // 3           <separator>
   // 4           <section header for 1st session>
   // 5-7         <3 tabs of the only window of session 0>
   // 8           <separator>
   // 9           <section header for 2nd session>
   // 10          <the only tab of window 0 of session 1>
   // 11-12       <2 tabs of window 1 of session 2>
 
-  TestRecentTabsSubMenuModel model(NULL, browser(), GetOpenTabsDelegate());
+  TestRecentTabsSubMenuModel model(nullptr, browser());
   int num_items = model.GetItemCount();
   EXPECT_EQ(13, num_items);
   model.ActivatedAt(0);
   EXPECT_TRUE(model.IsEnabledAt(0));
   model.ActivatedAt(1);
   EXPECT_TRUE(model.IsEnabledAt(1));
   model.ActivatedAt(2);
   EXPECT_FALSE(model.IsEnabledAt(2));
   model.ActivatedAt(3);
   EXPECT_TRUE(model.IsEnabledAt(3));
@@ skipping 35 matching lines @@
   EXPECT_FALSE(model.GetURLAndTitleForItemAtIndex(3, &url, &title));
   EXPECT_FALSE(model.GetURLAndTitleForItemAtIndex(4, &url, &title));
   EXPECT_TRUE(model.GetURLAndTitleForItemAtIndex(5, &url, &title));
   EXPECT_TRUE(model.GetURLAndTitleForItemAtIndex(6, &url, &title));
   EXPECT_TRUE(model.GetURLAndTitleForItemAtIndex(7, &url, &title));
   EXPECT_FALSE(model.GetURLAndTitleForItemAtIndex(8, &url, &title));
   EXPECT_FALSE(model.GetURLAndTitleForItemAtIndex(9, &url, &title));
   EXPECT_TRUE(model.GetURLAndTitleForItemAtIndex(10, &url, &title));
   EXPECT_TRUE(model.GetURLAndTitleForItemAtIndex(11, &url, &title));
   EXPECT_TRUE(model.GetURLAndTitleForItemAtIndex(12, &url, &title));
+

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

Nit: Prefer to pull this out into its own test, even if it means duplicating
setup code (but consider refactoring that setup code to avoid duplication), and
give it (and this existing test) descriptive names so it's clear what aspects of
the other devices submenu they're testing.

+// Remove this when dynamic model is enabled in RecentTabsSubMenuModel.

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

Nit: This comment sounds like "remove this block", but I think you mean "remove
the #if".  Try "Enable this for Mac" instead?

+#if !defined(OS_MACOSX)
+  // Make changes dynamically
+  int previous_enable_count = model.enable_count();
+  int previous_execute_count = model.execute_count();
+
+  base::Time update_timestamp =
+      base::Time::Now() - base::TimeDelta::FromMinutes(5);
+
+  // Add tab to the only window in the first session and add tab to the seсond
+  // window in the second session.
+  recent_tabs_builder.AddTabWithInfo(0, 0, update_timestamp, base::string16());
+  recent_tabs_builder.AddTabWithInfo(1, 1, update_timestamp, base::string16());
+
+  RegisterRecentTabs(&recent_tabs_builder);
+
+  // Verify that data is populated correctly in RecentTabsSubMenuModel.
+  // Expected menu:
+  // - first inserted tab is most recent and hence is top
+  // Menu index  Menu items
+  // -----------------------------------------------------
+  // 0           History
+  // 1           <separator>
+  // 2           Recently closed header (disabled)
+  // 3           <separator>
+  // 4           <section header for 1st session>
+  // 5           <new added tab of the only window of session 0>
+  // 6-8         <3 tabs of the only window of session 0>
+  // 9           <separator>
+  // 10          <section header for 2nd session>
+  // 11          <new added tab of window 1 of session 2>
+  // 12          <the only tab of window 0 of session 1>
+  // 13-14       <2 tabs of window 1 of session 2>
+
+  num_items = model.GetItemCount();

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

Do we really need to check everything below in order to verify the change you've
made in this CL?  It would be good to have a minimal test that just says, hey,
we added a tab somewhere, and we picked up on that in the menu, without
retesting everything else.

+  EXPECT_EQ(15, num_items);
+  model.ActivatedAt(0);
+  EXPECT_TRUE(model.IsEnabledAt(0));
+  model.ActivatedAt(1);
+  EXPECT_TRUE(model.IsEnabledAt(1));
+  model.ActivatedAt(2);
+  EXPECT_FALSE(model.IsEnabledAt(2));
+  model.ActivatedAt(3);
+  EXPECT_TRUE(model.IsEnabledAt(3));
+
+  model.ActivatedAt(5);
+  EXPECT_TRUE(model.IsEnabledAt(5));
+  model.ActivatedAt(6);
+  EXPECT_TRUE(model.IsEnabledAt(6));
+  model.ActivatedAt(7);
+  EXPECT_TRUE(model.IsEnabledAt(7));
+  model.ActivatedAt(8);
+  EXPECT_TRUE(model.IsEnabledAt(8));
+
+  model.ActivatedAt(11);
+  EXPECT_TRUE(model.IsEnabledAt(11));
+  model.ActivatedAt(12);
+  EXPECT_TRUE(model.IsEnabledAt(12));
+  model.ActivatedAt(13);
+  EXPECT_TRUE(model.IsEnabledAt(13));
+  model.ActivatedAt(14);
+  EXPECT_TRUE(model.IsEnabledAt(14));
+
+  EXPECT_EQ(previous_enable_count + 9, model.enable_count());
+  EXPECT_EQ(previous_execute_count + 12, model.execute_count());
+
+  EXPECT_EQ(nullptr, model.GetLabelFontListAt(0));
+  EXPECT_EQ(nullptr, model.GetLabelFontListAt(1));
+  EXPECT_EQ(nullptr, model.GetLabelFontListAt(2));
+  EXPECT_EQ(nullptr, model.GetLabelFontListAt(3));
+  EXPECT_TRUE(model.GetLabelFontListAt(4) != nullptr);

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

Nit: Can probably be EXPECT_NE(nullptr, ...); (2 places)

+  EXPECT_EQ(nullptr, model.GetLabelFontListAt(5));
+  EXPECT_EQ(nullptr, model.GetLabelFontListAt(6));
+  EXPECT_EQ(nullptr, model.GetLabelFontListAt(7));
+  EXPECT_EQ(nullptr, model.GetLabelFontListAt(8));
+  EXPECT_EQ(nullptr, model.GetLabelFontListAt(9));
+  EXPECT_TRUE(model.GetLabelFontListAt(10) != nullptr);
+  EXPECT_EQ(nullptr, model.GetLabelFontListAt(11));
+  EXPECT_EQ(nullptr, model.GetLabelFontListAt(12));
+  EXPECT_EQ(nullptr, model.GetLabelFontListAt(13));
+  EXPECT_EQ(nullptr, model.GetLabelFontListAt(14));
+
+  EXPECT_FALSE(model.GetURLAndTitleForItemAtIndex(0, &url, &title));
+  EXPECT_FALSE(model.GetURLAndTitleForItemAtIndex(1, &url, &title));
+  EXPECT_FALSE(model.GetURLAndTitleForItemAtIndex(2, &url, &title));
+  EXPECT_FALSE(model.GetURLAndTitleForItemAtIndex(3, &url, &title));
+  EXPECT_FALSE(model.GetURLAndTitleForItemAtIndex(4, &url, &title));
+  EXPECT_TRUE(model.GetURLAndTitleForItemAtIndex(5, &url, &title));
+  EXPECT_TRUE(model.GetURLAndTitleForItemAtIndex(6, &url, &title));
+  EXPECT_TRUE(model.GetURLAndTitleForItemAtIndex(7, &url, &title));
+  EXPECT_TRUE(model.GetURLAndTitleForItemAtIndex(8, &url, &title));
+  EXPECT_FALSE(model.GetURLAndTitleForItemAtIndex(9, &url, &title));
+  EXPECT_FALSE(model.GetURLAndTitleForItemAtIndex(10, &url, &title));
+  EXPECT_TRUE(model.GetURLAndTitleForItemAtIndex(11, &url, &title));
+  EXPECT_TRUE(model.GetURLAndTitleForItemAtIndex(12, &url, &title));
+  EXPECT_TRUE(model.GetURLAndTitleForItemAtIndex(13, &url, &title));
+  EXPECT_TRUE(model.GetURLAndTitleForItemAtIndex(14, &url, &title));
+#endif  // !defined(OS_MACOSX)
 }
 
 // Per http://crbug.com/603744, MaxSessionsAndRecenty fails intermittently on
 // windows, linux and mac.
 #if defined(OS_WIN) || defined(OS_LINUX) || defined(OS_MACOSX)
 #define MAYBE_MaxSessionsAndRecency DISABLED_MaxSessionsAndRecency
 #else
 #define MAYBE_MaxSessionsAndRecency MaxSessionsAndRecency
 #endif
 TEST_F(RecentTabsSubMenuModelTest, MAYBE_MaxSessionsAndRecency) {
@@ skipping 17 matching lines @@
   // 3           <separator>
   // 4           <section header for 1st session>
   // 5           <the only tab of the only window of session 3>
   // 6           <separator>
   // 7           <section header for 2nd session>
   // 8           <the only tab of the only window of session 2>
   // 9           <separator>
   // 10          <section header for 3rd session>
   // 11          <the only tab of the only window of session 1>
 
-  TestRecentTabsSubMenuModel model(NULL, browser(), GetOpenTabsDelegate());
+  TestRecentTabsSubMenuModel model(nullptr, browser());
   int num_items = model.GetItemCount();
   EXPECT_EQ(12, num_items);
 
   std::vector<base::string16> tab_titles =
       recent_tabs_builder.GetTabTitlesSortedByRecency();
   EXPECT_EQ(tab_titles[0], model.GetLabelAt(5));
   EXPECT_EQ(tab_titles[1], model.GetLabelAt(8));
   EXPECT_EQ(tab_titles[2], model.GetLabelAt(11));
 }
 
@@ skipping 14 matching lines @@
   //   independent of which window they came from.
   // Menu index  Menu items
   // ---------------------------------------------
   // 0           History
   // 1           <separator>
   // 2           Recently closed header (disabled)
   // 3           <separator>
   // 4           <section header for session>
   // 5-8         <4 most-recent tabs of session>
 
-  TestRecentTabsSubMenuModel model(NULL, browser(), GetOpenTabsDelegate());
+  TestRecentTabsSubMenuModel model(nullptr, browser());
   int num_items = model.GetItemCount();
   EXPECT_EQ(9, num_items);
 
   std::vector<base::string16> tab_titles =
       recent_tabs_builder.GetTabTitlesSortedByRecency();
   for (int i = 0; i < 4; ++i)
     EXPECT_EQ(tab_titles[i], model.GetLabelAt(i + 5));
 }
 
 TEST_F(RecentTabsSubMenuModelTest, MaxWidth) {
   // Create 1 session with 1 window and 1 tab.
   RecentTabsBuilderTestHelper recent_tabs_builder;
   recent_tabs_builder.AddSession();
   recent_tabs_builder.AddWindow(0);
   recent_tabs_builder.AddTab(0, 0);
   RegisterRecentTabs(&recent_tabs_builder);
 
   // Menu index  Menu items
   // ----------------------------------------------------------
   // 0           History
   // 1           <separator>
   // 2           Recently closed header (disabled)
   // 3           <separator>
   // 4           <section header for 1st session>
   // 5           <the only tab of the only window of session 1>
 
-  TestRecentTabsSubMenuModel model(NULL, browser(), GetOpenTabsDelegate());
+  TestRecentTabsSubMenuModel model(nullptr, browser());
   EXPECT_EQ(6, model.GetItemCount());
   EXPECT_EQ(-1, model.GetMaxWidthForItemAtIndex(2));
   EXPECT_NE(-1, model.GetMaxWidthForItemAtIndex(3));
   EXPECT_NE(-1, model.GetMaxWidthForItemAtIndex(4));
   EXPECT_NE(-1, model.GetMaxWidthForItemAtIndex(5));
 }
 
 TEST_F(RecentTabsSubMenuModelTest, MaxWidthNoDevices) {
   // Expected menu:
   // Menu index  Menu items
   // --------------------------------------------
   // 0           History
   // 1           <separator>
   // 2           Recently closed heaer (disabled)
   // 3           <separator>
   // 4           No tabs from other Devices
 
-  TestRecentTabsSubMenuModel model(NULL, browser(), NULL);
+  TestRecentTabsSubMenuModel model(nullptr, browser());
   EXPECT_EQ(5, model.GetItemCount());
   EXPECT_EQ(-1, model.GetMaxWidthForItemAtIndex(2));
   EXPECT_NE(-1, model.GetMaxWidthForItemAtIndex(3));
   EXPECT_EQ(-1, model.GetMaxWidthForItemAtIndex(4));
 }