Enable disabled tests in c/b/ui/toolbar.

TBR=oshima, dbeam

Bug: none
Change-Id: Idf7858096414e43b9d067b55627bb6af97839ab1
Reviewed-on: https://chromium-review.googlesource.com/c/1351796
Reviewed-by: Peter Kasting <pkasting@chromium.org>
Reviewed-by: Robert Sesek <rsesek@chromium.org>
Commit-Queue: Peter Kasting <pkasting@chromium.org>
Cr-Commit-Position: refs/heads/master@{#619441}

chrome/app/theme/theme_resources.grd

@@ -164,11 +164,6 @@
       <structure type="chrome_scaled_image" name="IDR_INFOBAR_3D_BLOCKED" file="common/infobar_3d_blocked.png" />
       <structure type="chrome_scaled_image" name="IDR_INPUT_ALERT" file="common/input_alert.png" />
       <structure type="chrome_scaled_image" name="IDR_INPUT_ALERT_MENU" file="common/input_alert_menu.png" />
-      <if expr="is_macosx">
-        <structure type="chrome_scaled_image" name="IDR_LAPTOP_FAVICON" file="legacy/favicon_laptop.png" />
-        <structure type="chrome_scaled_image" name="IDR_PHONE_FAVICON" file="legacy/favicon_phone.png" />
-        <structure type="chrome_scaled_image" name="IDR_TABLET_FAVICON" file="legacy/favicon_tablet.png" />
-      </if>
       <if expr="enable_service_discovery">
         <structure type="chrome_scaled_image" name="IDR_LOCAL_DISCOVERY_CLOUDPRINT_ICON" file="common/cloudprint.png" />
       </if>

chrome/browser/browser_resources.grd

@@ -559,9 +559,6 @@
         <include name="IDR_MD_USER_MANAGER_TUTORIAL_HTML" file="resources\md_user_manager\user_manager_tutorial.html" type="BINDATA" />
         <include name="IDR_MD_USER_MANAGER_TUTORIAL_JS" file="resources\md_user_manager\user_manager_tutorial.js" type="BINDATA" />
       </if>
-      <if expr="is_macosx">
-        <include name="IDR_RECENTLY_CLOSED_WINDOW" file="resources\ntp4\images\closed_window.png" type="BINDATA" />
-      </if>
       <if expr="not is_android">
         <include name="IDR_IDENTITY_INTERNALS_HTML" file="resources\identity_internals.html" type="BINDATA" />
         <include name="IDR_IDENTITY_INTERNALS_CSS" file="resources\identity_internals.css" type="BINDATA" />

chrome/browser/ui/toolbar/recent_tabs_sub_menu_model.cc

@@ -15,6 +15,7 @@
 #include "base/strings/utf_string_conversions.h"
 #include "build/build_config.h"
 #include "chrome/app/chrome_command_ids.h"
+#include "chrome/app/vector_icons/vector_icons.h"
 #include "chrome/browser/favicon/favicon_service_factory.h"
 #include "chrome/browser/profiles/profile.h"
 #include "chrome/browser/search/search.h"
@@ -28,9 +29,7 @@
 #include "chrome/browser/ui/in_product_help/reopen_tab_in_product_help_factory.h"
 #include "chrome/browser/ui/tabs/tab_strip_model.h"
 #include "chrome/browser/ui/toolbar/app_menu_model.h"
-#include "chrome/grit/browser_resources.h"
 #include "chrome/grit/generated_resources.h"
-#include "chrome/grit/theme_resources.h"
 #include "components/favicon_base/favicon_types.h"
 #include "components/feature_engagement/buildflags.h"
 #include "components/prefs/scoped_user_pref_update.h"
@@ -42,13 +41,9 @@
 #include "ui/base/accelerators/accelerator.h"
 #include "ui/base/l10n/l10n_util.h"
 #include "ui/base/resource/resource_bundle.h"
-#include "ui/resources/grit/ui_resources.h"
-
-#if !defined(OS_MACOSX)
-#include "chrome/app/vector_icons/vector_icons.h"
 #include "ui/gfx/color_palette.h"
 #include "ui/gfx/paint_vector_icon.h"
-#endif
+#include "ui/resources/grit/ui_resources.h"
 
 namespace {
 
@@ -80,10 +75,6 @@ const int kMaxDeviceNameCommandId = 1110;
 // 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 sync_sessions::SyncedSession* s1,
@@ -131,11 +122,9 @@ int CommandIdToWindowVectorIndex(int command_id) {
   return command_id - kFirstLocalWindowCommandId;
 }
 
-#if !defined(OS_MACOSX)
 gfx::Image CreateFavicon(const gfx::VectorIcon& icon) {
   return gfx::Image(gfx::CreateVectorIcon(icon, 16, gfx::kChromeIconGrey));
 }
-#endif
 
 }  // namespace
 
@@ -172,9 +161,6 @@ struct RecentTabsSubMenuModel::TabNavigationItem {
   GURL url;
 };
 
-const int RecentTabsSubMenuModel::kRecentlyClosedHeaderCommandId = 1120;
-const int RecentTabsSubMenuModel::kDisabledRecentlyClosedHeaderCommandId = 1121;
-
 RecentTabsSubMenuModel::RecentTabsSubMenuModel(
     ui::AcceleratorProvider* accelerator_provider,
     Browser* browser)
@@ -183,30 +169,19 @@ RecentTabsSubMenuModel::RecentTabsSubMenuModel(
       session_sync_service_(
           SessionSyncServiceFactory::GetInstance()->GetForProfile(
               browser->profile())),
-      last_local_model_index_(kHistorySeparatorIndex),
       default_favicon_(
           ui::ResourceBundle::GetSharedInstance().GetNativeImageNamed(
-              IDR_DEFAULT_FAVICON)),
-#if !defined(OS_MACOSX)
-      tab_restore_service_observer_(this),
-#endif  // !defined(OS_MACOSX)
-      weak_ptr_factory_(this) {
+              IDR_DEFAULT_FAVICON)) {
   // 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();
-
-// Mac doesn't support the dynamic menu.
-#if !defined(OS_MACOSX)
     tab_restore_service_observer_.Add(service);
-#endif
   }
 
-// Mac doesn't support the dynamic menu.
-#if !defined(OS_MACOSX)
   if (session_sync_service_) {
     // Using a weak pointer below for simplicity although, strictly speaking,
     // it's not needed because the subscription itself should take care.
@@ -216,7 +191,6 @@ RecentTabsSubMenuModel::RecentTabsSubMenuModel(
                 &RecentTabsSubMenuModel::OnForeignSessionUpdated,
                 weak_ptr_factory_.GetWeakPtr()));
   }
-#endif  // !defined(OS_MACOSX)
 
   Build();
 
@@ -235,13 +209,10 @@ bool RecentTabsSubMenuModel::IsCommandIdChecked(int command_id) const {
 }
 
 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)) {
-    return false;
-  }
-  return true;
+  return command_id != kRecentlyClosedHeaderCommandId &&
+         command_id != kDisabledRecentlyClosedHeaderCommandId &&
+         command_id != IDC_RECENT_TABS_NO_DEVICE_TABS &&
+         !IsDeviceNameCommandId(command_id);
 }
 
 bool RecentTabsSubMenuModel::GetAcceleratorForCommandId(
@@ -433,13 +404,7 @@ void RecentTabsSubMenuModel::BuildLocalEntries() {
     InsertItemWithStringIdAt(++last_local_model_index_,
                              kRecentlyClosedHeaderCommandId,
                              IDS_RECENTLY_CLOSED);
-#if defined(OS_MACOSX)
-    ui::ResourceBundle& rb = ui::ResourceBundle::GetSharedInstance();
-    SetIcon(last_local_model_index_,
-            rb.GetNativeImageNamed(IDR_RECENTLY_CLOSED_WINDOW));
-#else
     SetIcon(last_local_model_index_, CreateFavicon(kTabIcon));
-#endif
 
     int added_count = 0;
     for (const auto& entry : service->entries()) {
@@ -549,12 +514,7 @@ void RecentTabsSubMenuModel::BuildLocalWindowItem(SessionID window_id,
   // See comments in BuildLocalEntries() about usage of InsertItem*At().
   InsertItemAt(curr_model_index, command_id, l10n_util::GetPluralStringFUTF16(
       IDS_RECENTLY_CLOSED_WINDOW, num_tabs));
-#if defined(OS_MACOSX)
-  ui::ResourceBundle& rb = ui::ResourceBundle::GetSharedInstance();
-  SetIcon(curr_model_index, rb.GetNativeImageNamed(IDR_RECENTLY_CLOSED_WINDOW));
-#else
   SetIcon(curr_model_index, CreateFavicon(kTabIcon));
-#endif
   local_window_items_.push_back(window_id);
 }
 
@@ -580,30 +540,6 @@ void RecentTabsSubMenuModel::BuildOtherDevicesTabItem(
 void RecentTabsSubMenuModel::AddDeviceFavicon(
     int index_in_menu,
     sync_pb::SyncEnums::DeviceType device_type) {
-#if defined(OS_MACOSX)
-  int favicon_id = -1;
-  switch (device_type) {
-    case sync_pb::SyncEnums::TYPE_PHONE:
-      favicon_id = IDR_PHONE_FAVICON;
-      break;
-
-    case sync_pb::SyncEnums::TYPE_TABLET:
-      favicon_id = IDR_TABLET_FAVICON;
-      break;
-
-    case sync_pb::SyncEnums::TYPE_CROS:
-    case sync_pb::SyncEnums::TYPE_WIN:
-    case sync_pb::SyncEnums::TYPE_MAC:
-    case sync_pb::SyncEnums::TYPE_LINUX:
-    case sync_pb::SyncEnums::TYPE_OTHER:
-    case sync_pb::SyncEnums::TYPE_UNSET:
-      favicon_id = IDR_LAPTOP_FAVICON;
-      break;
-  }
-
-  ui::ResourceBundle& rb = ui::ResourceBundle::GetSharedInstance();
-  SetIcon(index_in_menu, rb.GetNativeImageNamed(favicon_id));
-#else
   const gfx::VectorIcon* favicon = nullptr;
   switch (device_type) {
     case sync_pb::SyncEnums::TYPE_PHONE:
@@ -625,7 +561,6 @@ void RecentTabsSubMenuModel::AddDeviceFavicon(
   }
 
   SetIcon(index_in_menu, CreateFavicon(*favicon));
-#endif
 }
 
 void RecentTabsSubMenuModel::AddTabFavicon(int command_id, const GURL& url) {

chrome/browser/ui/toolbar/recent_tabs_sub_menu_model.h

@@ -56,8 +56,8 @@ class RecentTabsSubMenuModel : public ui::SimpleMenuModel,
  public:
   // Command Id for recently closed items header or disabled item to which the
   // accelerator string will be appended.
-  static const int kRecentlyClosedHeaderCommandId;
-  static const int kDisabledRecentlyClosedHeaderCommandId;
+  static constexpr int kRecentlyClosedHeaderCommandId = 1120;
+  static constexpr int kDisabledRecentlyClosedHeaderCommandId = 1121;
 
   // Exposed for tests only: return the Command Id for the first entry in the
   // recently closed window items list.
@@ -82,9 +82,12 @@ class RecentTabsSubMenuModel : public ui::SimpleMenuModel,
 
  private:
   struct TabNavigationItem;
-  typedef std::vector<TabNavigationItem> TabNavigationItems;
+  using TabNavigationItems = std::vector<TabNavigationItem>;
+  using WindowItems = std::vector<SessionID>;
 
-  typedef std::vector<SessionID> WindowItems;
+  // Index of the separator that follows the history menu item. Used as a
+  // reference position for inserting local entries.
+  static constexpr int kHistorySeparatorIndex = 1;
 
   // Build the menu items by populating the menumodel.
   void Build();
@@ -174,7 +177,7 @@ class RecentTabsSubMenuModel : public ui::SimpleMenuModel,
 
   // Index of the last local entry (recently closed tab or window) in the
   // menumodel.
-  int last_local_model_index_;
+  int last_local_model_index_ = kHistorySeparatorIndex;
 
   gfx::Image default_favicon_;
 
@@ -184,16 +187,13 @@ class RecentTabsSubMenuModel : public ui::SimpleMenuModel,
   // Time the menu is open for until a recent tab is selected.
   base::ElapsedTimer menu_opened_timer_;
 
-// Mac doesn't support the dynamic menu.
-#if !defined(OS_MACOSX)
   ScopedObserver<sessions::TabRestoreService, RecentTabsSubMenuModel>
-      tab_restore_service_observer_;
-#endif
+      tab_restore_service_observer_{this};
 
   std::unique_ptr<base::CallbackList<void()>::Subscription>
       foreign_session_updated_subscription_;
 
-  base::WeakPtrFactory<RecentTabsSubMenuModel> weak_ptr_factory_;
+  base::WeakPtrFactory<RecentTabsSubMenuModel> weak_ptr_factory_{this};
 
   DISALLOW_COPY_AND_ASSIGN(RecentTabsSubMenuModel);
 };

chrome/browser/ui/toolbar/recent_tabs_sub_menu_model_unittest.cc

@@ -271,17 +271,8 @@ TEST_F(RecentTabsSubMenuModelTest, RecentlyClosedTabsFromCurrentSession) {
   EXPECT_FALSE(model.GetURLAndTitleForItemAtIndex(6, &url, &title));
 }
 
-// TODO(sail): enable this test when dynamic model is enabled in
-// RecentTabsSubMenuModel.
-#if defined(OS_MACOSX)
-#define MAYBE_RecentlyClosedTabsAndWindowsFromLastSession \
-    DISABLED_RecentlyClosedTabsAndWindowsFromLastSession
-#else
-#define MAYBE_RecentlyClosedTabsAndWindowsFromLastSession \
-    RecentlyClosedTabsAndWindowsFromLastSession
-#endif
 TEST_F(RecentTabsSubMenuModelTest,
-       MAYBE_RecentlyClosedTabsAndWindowsFromLastSession) {
+       RecentlyClosedTabsAndWindowsFromLastSession) {
   DisableSync();
 
   TabRestoreServiceFactory::GetInstance()->SetTestingFactory(
@@ -508,13 +499,7 @@ TEST_F(RecentTabsSubMenuModelTest, OtherDevices) {
   EXPECT_TRUE(model.GetURLAndTitleForItemAtIndex(12, &url, &title));
 }
 
-// Mac doesn't support the dynamic menu.
-#if defined(OS_MACOSX)
-#define MAYBE_OtherDevicesDynamicUpdate DISABLED_OtherDevicesDynamicUpdate
-#else
-#define MAYBE_OtherDevicesDynamicUpdate OtherDevicesDynamicUpdate
-#endif
-TEST_F(RecentTabsSubMenuModelTest, MAYBE_OtherDevicesDynamicUpdate) {
+TEST_F(RecentTabsSubMenuModelTest, OtherDevicesDynamicUpdate) {
   // Create menu with disabled synchronization.
   DisableSync();