Commit 8c1b2397 authored by anthonyvd's avatar anthonyvd Committed by Commit bot

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}
parent f29a78fc
...@@ -5,6 +5,7 @@ ...@@ -5,6 +5,7 @@
#import "chrome/browser/ui/cocoa/profiles/profile_menu_controller.h" #import "chrome/browser/ui/cocoa/profiles/profile_menu_controller.h"
#include "base/mac/scoped_nsobject.h" #include "base/mac/scoped_nsobject.h"
#include "base/metrics/histogram_macros.h"
#include "base/strings/sys_string_conversions.h" #include "base/strings/sys_string_conversions.h"
#include "chrome/browser/browser_process.h" #include "chrome/browser/browser_process.h"
#include "chrome/browser/profiles/avatar_menu.h" #include "chrome/browser/profiles/avatar_menu.h"
...@@ -25,6 +26,20 @@ ...@@ -25,6 +26,20 @@
#include "ui/base/l10n/l10n_util_mac.h" #include "ui/base/l10n/l10n_util_mac.h"
#include "ui/gfx/image/image.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) @interface ProfileMenuController (Private)
- (void)initializeMenu; - (void)initializeMenu;
@end @end
...@@ -169,8 +184,31 @@ class Observer : public chrome::BrowserListObserver, ...@@ -169,8 +184,31 @@ class Observer : public chrome::BrowserListObserver,
[menuItem action] != @selector(editProfile:); [menuItem action] != @selector(editProfile:);
} }
const AvatarMenu::Item& itemData = avatarMenu_->GetItemAt( size_t index = avatarMenu_->GetActiveProfileIndex();
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:) || if ([menuItem action] == @selector(switchToProfileFromDock:) ||
[menuItem action] == @selector(switchToProfileFromMenu:)) { [menuItem action] == @selector(switchToProfileFromMenu:)) {
if (!itemData.legacy_supervised) if (!itemData.legacy_supervised)
......
...@@ -30860,6 +30860,31 @@ Therefore, the affected-histogram name has to have at least one dot in it. ...@@ -30860,6 +30860,31 @@ Therefore, the affected-histogram name has to have at least one dot in it.
</summary> </summary>
</histogram> </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"> <histogram name="Profile.VisitedLinksSize" units="MB">
<owner>Please list the metric's owners. Add more owner tags as needed.</owner> <owner>Please list the metric's owners. Add more owner tags as needed.</owner>
<summary>Size of the visited links database.</summary> <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. ...@@ -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> <int value="6" label="CHECKSUM_MISMATCH">Reject due to checksum mismatch</int>
</enum> </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"> <enum name="ValidationFailures" type="int">
<int value="0" label="DBus"/> <int value="0" label="DBus"/>
<int value="1" label="Load Key"/> <int value="1" label="Load Key"/>
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment