From 8c1b23970f6d8efe9e68f6dfdadbbf7b0e833417 Mon Sep 17 00:00:00 2001
From: anthonyvd <anthonyvd@chromium.org>
Date: Fri, 24 Apr 2015 11:05:54 -0700
Subject: [PATCH] Fix crash in -[ProfileMenuController validateMenuItem:].

This CL also instruments the function to better understand which edge case
triggers the issue in the first place.

BUG=471586

Review URL: https://codereview.chromium.org/1054113004

Cr-Commit-Position: refs/heads/master@{#326835}
---
 .../ui/cocoa/profiles/profile_menu_controller.mm   | 42 ++++++++++++++++++++--
 tools/metrics/histograms/histograms.xml            | 36 +++++++++++++++++++
 2 files changed, 76 insertions(+), 2 deletions(-)

diff --git a/chrome/browser/ui/cocoa/profiles/profile_menu_controller.mm b/chrome/browser/ui/cocoa/profiles/profile_menu_controller.mm
index 437bed15fe87..5ee576fe66a8 100644
--- a/chrome/browser/ui/cocoa/profiles/profile_menu_controller.mm
+++ b/chrome/browser/ui/cocoa/profiles/profile_menu_controller.mm
@@ -5,6 +5,7 @@
 #import "chrome/browser/ui/cocoa/profiles/profile_menu_controller.h"
 
 #include "base/mac/scoped_nsobject.h"
+#include "base/metrics/histogram_macros.h"
 #include "base/strings/sys_string_conversions.h"
 #include "chrome/browser/browser_process.h"
 #include "chrome/browser/profiles/avatar_menu.h"
@@ -25,6 +26,20 @@
 #include "ui/base/l10n/l10n_util_mac.h"
 #include "ui/gfx/image/image.h"
 
+namespace {
+
+// Used in UMA histogram macros, shouldn't be reordered or renumbered
+enum ValidateMenuItemSelector {
+  UNKNOWN_SELECTOR = 0,
+  NEW_PROFILE,
+  EDIT_PROFILE,
+  SWITCH_PROFILE_MENU,
+  SWITCH_PROFILE_DOCK,
+  MAX_VALIDATE_MENU_SELECTOR,
+};
+
+}  // namespace
+
 @interface ProfileMenuController (Private)
 - (void)initializeMenu;
 @end
@@ -169,8 +184,31 @@ class Observer : public chrome::BrowserListObserver,
            [menuItem action] != @selector(editProfile:);
   }
 
-  const AvatarMenu::Item& itemData = avatarMenu_->GetItemAt(
-      avatarMenu_->GetActiveProfileIndex());
+  size_t index = avatarMenu_->GetActiveProfileIndex();
+  if (avatarMenu_->GetNumberOfItems() <= index) {
+    ValidateMenuItemSelector currentSelector = UNKNOWN_SELECTOR;
+    if ([menuItem action] == @selector(newProfile:))
+      currentSelector = NEW_PROFILE;
+    else if ([menuItem action] == @selector(editProfile:))
+      currentSelector = EDIT_PROFILE;
+    else if ([menuItem action] == @selector(switchToProfileFromMenu:))
+      currentSelector = SWITCH_PROFILE_MENU;
+    else if ([menuItem action] == @selector(switchToProfileFromDock:))
+      currentSelector = SWITCH_PROFILE_DOCK;
+    UMA_HISTOGRAM_BOOLEAN("Profile.ValidateMenuItemInvalidIndex.IsGuest",
+                          activeProfile->IsGuestSession());
+    UMA_HISTOGRAM_CUSTOM_COUNTS(
+        "Profile.ValidateMenuItemInvalidIndex.ProfileCount",
+        avatarMenu_->GetNumberOfItems(),
+        1, 20, 20);
+    UMA_HISTOGRAM_ENUMERATION("Profile.ValidateMenuItemInvalidIndex.Selector",
+                              currentSelector,
+                              MAX_VALIDATE_MENU_SELECTOR);
+
+    return NO;
+  }
+
+  const AvatarMenu::Item& itemData = avatarMenu_->GetItemAt(index);
   if ([menuItem action] == @selector(switchToProfileFromDock:) ||
       [menuItem action] == @selector(switchToProfileFromMenu:)) {
     if (!itemData.legacy_supervised)
diff --git a/tools/metrics/histograms/histograms.xml b/tools/metrics/histograms/histograms.xml
index 9d6e4ab99686..28de8b8e7677 100644
--- a/tools/metrics/histograms/histograms.xml
+++ b/tools/metrics/histograms/histograms.xml
@@ -30860,6 +30860,31 @@ Therefore, the affected-histogram name has to have at least one dot in it.
   </summary>
 </histogram>
 
+<histogram name="Profile.ValidateMenuItemInvalidIndex.IsGuest" enum="Boolean">
+  <owner>anthonyvd@chromium.org</owner>
+  <summary>
+    Whether the active profile is a guest profile when -validateMenuItem in the
+    ProfileMenuController gets an invalid value for the current profile's index.
+  </summary>
+</histogram>
+
+<histogram name="Profile.ValidateMenuItemInvalidIndex.ProfileCount">
+  <owner>anthonyvd@chromium.org</owner>
+  <summary>
+    The count of profiles in the avatar menu when -validateMenuItem in the
+    ProfileMenuController gets an invalid value for the current profile's index.
+  </summary>
+</histogram>
+
+<histogram name="Profile.ValidateMenuItemInvalidIndex.Selector"
+    enum="ValidateMenuItemSelectorType">
+  <owner>anthonyvd@chromium.org</owner>
+  <summary>
+    The selector associated with the menu item when -validateMenuItem in the
+    ProfileMenuController gets an invalid value for the current profile's index.
+  </summary>
+</histogram>
+
 <histogram name="Profile.VisitedLinksSize" units="MB">
   <owner>Please list the metric's owners. Add more owner tags as needed.</owner>
   <summary>Size of the visited links database.</summary>
@@ -63716,6 +63741,17 @@ To add a new entry, add it with any value and run test to compute valid value.
   <int value="6" label="CHECKSUM_MISMATCH">Reject due to checksum mismatch</int>
 </enum>
 
+<enum name="ValidateMenuItemSelectorType" type="int">
+  <int value="0"
+      label="The menu items' associated action is an unknown selector."/>
+  <int value="1" label="The menu item's associated action is -newProfile."/>
+  <int value="2" label="The menu item's associated action is -editProfile."/>
+  <int value="3"
+      label="The menu item's associated action is -switchToProfileFromMenu."/>
+  <int value="4"
+      label="The menu item's associated action is -switchToProfileFromDock."/>
+</enum>
+
 <enum name="ValidationFailures" type="int">
   <int value="0" label="DBus"/>
   <int value="1" label="Load Key"/>
-- 
2.11.4.GIT