[profile-menu] Update incognito menu

 - Move all incognito code to incognito_menu_view.cc
   (to avoid special casing for ChromeOS)
 - Add "Exit incognito" button
 - Re-enable browser tests

Bug: 995720
Change-Id: I55674e9192065d4a185b712af391a56fdc56095a
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1859962Reviewed-by: David Roger <droger@chromium.org>
Commit-Queue: Thomas Tangl <tangltom@chromium.org>
Cr-Commit-Position: refs/heads/master@{#705544}

chrome/browser/ui/views/profiles/incognito_menu_view.cc

@@ -49,23 +49,40 @@ void IncognitoMenuView::BuildMenu() {
   const SkColor icon_color = provider->GetTypographyProvider().GetColor(
       *this, views::style::CONTEXT_LABEL, views::style::STYLE_PRIMARY);
 
-  auto incognito_icon = std::make_unique<views::ImageView>();
-  incognito_icon->SetImage(
-      gfx::CreateVectorIcon(kIncognitoProfileIcon, icon_color));
+  if (!base::FeatureList::IsEnabled(features::kProfileMenuRevamp)) {
+    auto incognito_icon = std::make_unique<views::ImageView>();
+    incognito_icon->SetImage(
+        gfx::CreateVectorIcon(kIncognitoProfileIcon, icon_color));
 
-  AddMenuGroup(false /* add_separator */);
-  CreateAndAddTitleCard(
-      std::move(incognito_icon),
+    AddMenuGroup(false /* add_separator */);
+    CreateAndAddTitleCard(
+        std::move(incognito_icon),
+        l10n_util::GetStringUTF16(IDS_INCOGNITO_PROFILE_MENU_TITLE),
+        incognito_window_count > 1
+            ? l10n_util::GetPluralStringFUTF16(
+                  IDS_INCOGNITO_WINDOW_COUNT_MESSAGE, incognito_window_count)
+            : base::string16(),
+        base::RepeatingClosure());
+
+    AddMenuGroup();
+    exit_button_ = CreateAndAddButton(
+        gfx::CreateVectorIcon(kCloseAllIcon, 16, gfx::kChromeIconGrey),
+        l10n_util::GetStringUTF16(IDS_INCOGNITO_PROFILE_MENU_CLOSE_BUTTON),
+        base::BindRepeating(&IncognitoMenuView::OnExitButtonClicked,
+                            base::Unretained(this)));
+    return;
+  }
+
+  SetIdentityInfo(
+      ColoredImageForMenu(kIncognitoProfileIcon, icon_color),
+      /*badge=*/gfx::ImageSkia(),
       l10n_util::GetStringUTF16(IDS_INCOGNITO_PROFILE_MENU_TITLE),
       incognito_window_count > 1
           ? l10n_util::GetPluralStringFUTF16(IDS_INCOGNITO_WINDOW_COUNT_MESSAGE,
                                              incognito_window_count)
-          : base::string16(),
-      base::RepeatingClosure());
-
-  AddMenuGroup();
-  exit_button_ = CreateAndAddButton(
-      gfx::CreateVectorIcon(kCloseAllIcon, 16, gfx::kChromeIconGrey),
+          : base::string16());
+  AddFeatureButton(
+      ImageForMenu(kCloseAllIcon),
       l10n_util::GetStringUTF16(IDS_INCOGNITO_PROFILE_MENU_CLOSE_BUTTON),
       base::BindRepeating(&IncognitoMenuView::OnExitButtonClicked,
                           base::Unretained(this)));
@@ -78,6 +95,7 @@ base::string16 IncognitoMenuView::GetAccessibleWindowTitle() const {
 }
 
 void IncognitoMenuView::OnExitButtonClicked() {
+  RecordClick(ActionableItem::kExitProfileButton);
   // Skipping before-unload trigger to give incognito mode users a chance to
   // quickly close all incognito windows without needing to confirm closing the
   // open forms.

chrome/browser/ui/views/profiles/profile_menu_view.cc

@@ -10,7 +10,6 @@
 
 #include "base/feature_list.h"
 #include "base/macros.h"
-#include "base/metrics/histogram_functions.h"
 #include "base/metrics/histogram_macros.h"
 #include "base/metrics/user_metrics.h"
 #include "build/build_config.h"
@@ -178,8 +177,6 @@ void ProfileMenuView::BuildMenu() {
     BuildSyncInfo();
     BuildFeatureButtons();
     BuildAutofillButtons();
-  } else if (profile->IsIncognitoProfile()) {
-    BuildIncognitoIdentity();
   } else if (profile->IsGuestSession()) {
     BuildGuestIdentity();
   } else {
@@ -403,10 +400,6 @@ void ProfileMenuView::OnAddNewProfileButtonClicked() {
                     profiles::USER_MANAGER_OPEN_CREATE_USER_PAGE);
 }
 
-void ProfileMenuView::RecordClick(ActionableItem item) {
-  base::UmaHistogramEnumeration("Profile.Menu.ClickedActionableItem", item);
-}
-
 void ProfileMenuView::BuildIdentity() {
   Profile* profile = browser()->profile();
   signin::IdentityManager* identity_manager =
@@ -438,19 +431,6 @@ void ProfileMenuView::BuildGuestIdentity() {
                   l10n_util::GetStringUTF16(IDS_GUEST_PROFILE_NAME));
 }
 
-void ProfileMenuView::BuildIncognitoIdentity() {
-  int incognito_window_count =
-      BrowserList::GetIncognitoSessionsActiveForProfile(browser()->profile());
-
-  SetIdentityInfo(
-      ImageForMenu(kIncognitoProfileIcon), GetSyncIcon(),
-      l10n_util::GetStringUTF16(IDS_INCOGNITO_PROFILE_MENU_TITLE),
-      incognito_window_count > 1
-          ? l10n_util::GetPluralStringFUTF16(IDS_INCOGNITO_WINDOW_COUNT_MESSAGE,
-                                             incognito_window_count)
-          : base::string16());
-}
-
 gfx::ImageSkia ProfileMenuView::GetSyncIcon() {
   Profile* profile = browser()->profile();
   signin::IdentityManager* identity_manager =

chrome/browser/ui/views/profiles/profile_menu_view.h

@@ -30,29 +30,6 @@ class Browser;
 // It displays a list of profiles and allows users to switch between profiles.
 class ProfileMenuView : public ProfileMenuViewBase, public AvatarMenuObserver {
  public:
-  // These values are persisted to logs. Entries should not be renumbered and
-  // numeric values should never be reused.
-  enum class ActionableItem {
-    kManageGoogleAccountButton = 0,
-    kPasswordsButton = 1,
-    kCreditCardsButton = 2,
-    kAddressesButton = 3,
-    kGuestProfileButton = 4,
-    kManageProfilesButton = 5,
-    kLockButton = 6,
-    kExitProfileButton = 7,
-    kSyncErrorButton = 8,
-    kCurrentProfileCard = 9,
-    kSigninButton = 10,
-    kSigninAccountButton = 11,
-    kSignoutButton = 12,
-    kOtherProfileButton = 13,
-    kCookiesClearedOnExitLink = 14,
-    kAddNewProfileButton = 15,
-    kSyncSettingsButton = 16,
-    kMaxValue = kSyncSettingsButton,
-  };
-
   ProfileMenuView(views::Button* anchor_button,
                      Browser* browser,
                      signin_metrics::AccessPoint access_point);
@@ -90,9 +67,6 @@ class ProfileMenuView : public ProfileMenuViewBase, public AvatarMenuObserver {
   void OnCookiesClearedOnExitLinkClicked();
   void OnAddNewProfileButtonClicked();
 
-  // Should be called inside each button/link action.
-  void RecordClick(ActionableItem item);
-
   // AvatarMenuObserver:
   void OnAvatarMenuChanged(AvatarMenu* avatar_menu) override;
 
@@ -104,7 +78,6 @@ class ProfileMenuView : public ProfileMenuViewBase, public AvatarMenuObserver {
   // Helper methods for building the menu.
   void BuildIdentity();
   void BuildGuestIdentity();
-  void BuildIncognitoIdentity();
   gfx::ImageSkia GetSyncIcon();
   void BuildAutofillButtons();
   void BuildSyncInfo();

chrome/browser/ui/views/profiles/profile_menu_view_base.cc

@@ -10,6 +10,7 @@
 
 #include "base/feature_list.h"
 #include "base/macros.h"
+#include "base/metrics/histogram_functions.h"
 #include "base/metrics/user_metrics.h"
 #include "build/build_config.h"
 #include "chrome/app/vector_icons/vector_icons.h"
@@ -202,29 +203,17 @@ void ProfileMenuViewBase::ShowBubble(
 
   ProfileMenuViewBase* bubble;
 
-  if (base::FeatureList::IsEnabled(features::kProfileMenuRevamp)) {
-#if !defined(OS_CHROMEOS)
-    // On Desktop, all modes(regular, guest, incognito) are handled within
-    // ProfileMenuView.
-    bubble = new ProfileMenuView(anchor_button, browser, access_point);
-#else
-    // On ChromeOS, only the incognito menu is implemented.
+  if (view_mode == profiles::BUBBLE_VIEW_MODE_INCOGNITO) {
     DCHECK(browser->profile()->IsIncognitoProfile());
     bubble = new IncognitoMenuView(anchor_button, browser);
-#endif
   } else {
-    if (view_mode == profiles::BUBBLE_VIEW_MODE_INCOGNITO) {
-      DCHECK(browser->profile()->IsIncognitoProfile());
-      bubble = new IncognitoMenuView(anchor_button, browser);
-    } else {
-      DCHECK_EQ(profiles::BUBBLE_VIEW_MODE_PROFILE_CHOOSER, view_mode);
+    DCHECK_EQ(profiles::BUBBLE_VIEW_MODE_PROFILE_CHOOSER, view_mode);
 #if !defined(OS_CHROMEOS)
-      bubble = new ProfileMenuView(anchor_button, browser, access_point);
+    bubble = new ProfileMenuView(anchor_button, browser, access_point);
 #else
-      NOTREACHED();
-      return;
+    NOTREACHED();
+    return;
 #endif
-    }
   }
 
   views::BubbleDialogDelegateView::CreateBubble(bubble)->Show();
@@ -527,6 +516,11 @@ gfx::ImageSkia ProfileMenuViewBase::ColoredImageForMenu(
   return gfx::CreateVectorIcon(icon, kMaxImageSize, color);
 }
 
+void ProfileMenuViewBase::RecordClick(ActionableItem item) {
+  // TODO(tangltom): Separate metrics for incognito and guest menu.
+  base::UmaHistogramEnumeration("Profile.Menu.ClickedActionableItem", item);
+}
+
 ax::mojom::Role ProfileMenuViewBase::GetAccessibleWindowRole() {
   // Return |ax::mojom::Role::kDialog| which will make screen readers announce
   // the following in the listed order:

chrome/browser/ui/views/profiles/profile_menu_view_base.h

@@ -42,6 +42,30 @@ class ProfileMenuViewBase : public content::WebContentsDelegate,
                             public views::StyledLabelListener,
                             public views::LinkListener {
  public:
+  // Enumeration of all actionable items in the profile menu.
+  // These values are persisted to logs. Entries should not be renumbered and
+  // numeric values should never be reused.
+  enum class ActionableItem {
+    kManageGoogleAccountButton = 0,
+    kPasswordsButton = 1,
+    kCreditCardsButton = 2,
+    kAddressesButton = 3,
+    kGuestProfileButton = 4,
+    kManageProfilesButton = 5,
+    kLockButton = 6,
+    kExitProfileButton = 7,
+    kSyncErrorButton = 8,
+    kCurrentProfileCard = 9,
+    kSigninButton = 10,
+    kSigninAccountButton = 11,
+    kSignoutButton = 12,
+    kOtherProfileButton = 13,
+    kCookiesClearedOnExitLink = 14,
+    kAddNewProfileButton = 15,
+    kSyncSettingsButton = 16,
+    kMaxValue = kSyncSettingsButton,
+  };
+
   // MenuItems struct keeps the menu items and meta data for a group of items in
   // a menu. It takes the ownership of views and passes it to the menu when menu
   // is constructed.
@@ -127,6 +151,8 @@ class ProfileMenuViewBase : public content::WebContentsDelegate,
                               float icon_to_image_ratio = 1.0f);
   gfx::ImageSkia ColoredImageForMenu(const gfx::VectorIcon& icon,
                                      SkColor color);
+  // Should be called inside each button/link action.
+  void RecordClick(ActionableItem item);
 
   // Initializes a new group of menu items. A separator is added before them if
   // |add_separator| is true.