Commit 2a9817f6 authored by felt's avatar felt Committed by Commit bot

Use customization mode by default in permission bubbles

Previously: when multiple permissions were requested, they could only
be toggled individually if the user clicked on "Customize."

Now: the bubble will always allow for customization if there are
multiple permissions in the request. it will be the default state.

BUG=440563

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

Cr-Commit-Position: refs/heads/master@{#314731}
parent 4e86d25e
...@@ -41,8 +41,7 @@ class FakePermissionBubbleView : public PermissionBubbleView { ...@@ -41,8 +41,7 @@ class FakePermissionBubbleView : public PermissionBubbleView {
void SetDelegate(Delegate* delegate) override { delegate_ = delegate; } void SetDelegate(Delegate* delegate) override { delegate_ = delegate; }
void Show(const std::vector<PermissionBubbleRequest*>& requests, void Show(const std::vector<PermissionBubbleRequest*>& requests,
const std::vector<bool>& accept_state, const std::vector<bool>& accept_state) override;
bool customization_mode) override;
bool CanAcceptRequestUpdate() override { return false; } bool CanAcceptRequestUpdate() override { return false; }
...@@ -206,8 +205,7 @@ class DownloadRequestLimiterTest : public ChromeRenderViewHostTestHarness { ...@@ -206,8 +205,7 @@ class DownloadRequestLimiterTest : public ChromeRenderViewHostTestHarness {
void FakePermissionBubbleView::Show( void FakePermissionBubbleView::Show(
const std::vector<PermissionBubbleRequest*>& requests, const std::vector<PermissionBubbleRequest*>& requests,
const std::vector<bool>& accept_state, const std::vector<bool>& accept_state) {
bool customization_mode) {
test_->AskAllow(); test_->AskAllow();
int action = test_->GetAction(); int action = test_->GetAction();
if (action == DownloadRequestLimiterTest::ACCEPT) { if (action == DownloadRequestLimiterTest::ACCEPT) {
......
...@@ -24,8 +24,7 @@ class PermissionBubbleCocoa : public PermissionBubbleView { ...@@ -24,8 +24,7 @@ class PermissionBubbleCocoa : public PermissionBubbleView {
// PermissionBubbleView interface. // PermissionBubbleView interface.
void Show(const std::vector<PermissionBubbleRequest*>& requests, void Show(const std::vector<PermissionBubbleRequest*>& requests,
const std::vector<bool>& accept_state, const std::vector<bool>& accept_state) override;
bool customization_mode) override;
void Hide() override; void Hide() override;
bool IsVisible() override; bool IsVisible() override;
void SetDelegate(Delegate* delegate) override; void SetDelegate(Delegate* delegate) override;
......
...@@ -21,8 +21,7 @@ PermissionBubbleCocoa::~PermissionBubbleCocoa() { ...@@ -21,8 +21,7 @@ PermissionBubbleCocoa::~PermissionBubbleCocoa() {
void PermissionBubbleCocoa::Show( void PermissionBubbleCocoa::Show(
const std::vector<PermissionBubbleRequest*>& requests, const std::vector<PermissionBubbleRequest*>& requests,
const std::vector<bool>& accept_state, const std::vector<bool>& accept_state) {
bool customization_mode) {
DCHECK(parent_window_); DCHECK(parent_window_);
if (!bubbleController_) { if (!bubbleController_) {
...@@ -34,8 +33,7 @@ void PermissionBubbleCocoa::Show( ...@@ -34,8 +33,7 @@ void PermissionBubbleCocoa::Show(
[bubbleController_ showAtAnchor:GetAnchorPoint() [bubbleController_ showAtAnchor:GetAnchorPoint()
withDelegate:delegate_ withDelegate:delegate_
forRequests:requests forRequests:requests
acceptStates:accept_state acceptStates:accept_state];
customizationMode:customization_mode];
} }
void PermissionBubbleCocoa::Hide() { void PermissionBubbleCocoa::Hide() {
......
...@@ -17,7 +17,7 @@ class PermissionBubbleRequest; ...@@ -17,7 +17,7 @@ class PermissionBubbleRequest;
BaseBubbleController<NSTextViewDelegate> { BaseBubbleController<NSTextViewDelegate> {
@private @private
// Array of views that are the checkboxes for every requested permission. // Array of views that are the checkboxes for every requested permission.
// Only populated if |customizationMode| is YES when the UI is shown. // Only populated if multiple requests are shown at once.
base::scoped_nsobject<NSMutableArray> checkboxes_; base::scoped_nsobject<NSMutableArray> checkboxes_;
// Delegate to be informed of user actions. // Delegate to be informed of user actions.
...@@ -43,10 +43,6 @@ class PermissionBubbleRequest; ...@@ -43,10 +43,6 @@ class PermissionBubbleRequest;
- (void)showAtAnchor:(NSPoint)anchor - (void)showAtAnchor:(NSPoint)anchor
withDelegate:(PermissionBubbleView::Delegate*)delegate withDelegate:(PermissionBubbleView::Delegate*)delegate
forRequests:(const std::vector<PermissionBubbleRequest*>&)requests forRequests:(const std::vector<PermissionBubbleRequest*>&)requests
acceptStates:(const std::vector<bool>&)acceptStates acceptStates:(const std::vector<bool>&)acceptStates;
customizationMode:(BOOL)customizationMode;
// Called when a menu item is selected.
- (void)onMenuItemClicked:(int)commandId;
@end @end
...@@ -57,9 +57,6 @@ class MenuDelegate : public ui::SimpleMenuModel::Delegate { ...@@ -57,9 +57,6 @@ class MenuDelegate : public ui::SimpleMenuModel::Delegate {
ui::Accelerator* accelerator) override { ui::Accelerator* accelerator) override {
return false; return false;
} }
void ExecuteCommand(int command_id, int event_flags) override {
[bubble_controller_ onMenuItemClicked:command_id];
}
private: private:
PermissionBubbleController* bubble_controller_; // Weak, owns us. PermissionBubbleController* bubble_controller_; // Weak, owns us.
DISALLOW_COPY_AND_ASSIGN(MenuDelegate); DISALLOW_COPY_AND_ASSIGN(MenuDelegate);
...@@ -68,8 +65,8 @@ class MenuDelegate : public ui::SimpleMenuModel::Delegate { ...@@ -68,8 +65,8 @@ class MenuDelegate : public ui::SimpleMenuModel::Delegate {
} // namespace } // namespace
// NSPopUpButton with a menu containing two items: allow and block. // NSPopUpButton with a menu containing two items: allow and block.
// One AllowBlockMenuButton is used for each requested permission, but only when // One AllowBlockMenuButton is used for each requested permission when there are
// the permission bubble is in 'customize' mode. // multiple permissions in the bubble.
@interface AllowBlockMenuButton : NSPopUpButton { @interface AllowBlockMenuButton : NSPopUpButton {
@private @private
scoped_ptr<PermissionMenuModel> menuModel_; scoped_ptr<PermissionMenuModel> menuModel_;
...@@ -183,10 +180,6 @@ class MenuDelegate : public ui::SimpleMenuModel::Delegate { ...@@ -183,10 +180,6 @@ class MenuDelegate : public ui::SimpleMenuModel::Delegate {
// Returns an autoreleased NSView displaying a block button. // Returns an autoreleased NSView displaying a block button.
- (NSView*)blockButton; - (NSView*)blockButton;
// Returns an autoreleased NSView with a block button and a drop-down menu
// with one item, which will change the UI to allow customizing the permissions.
- (NSView*)blockButtonWithCustomizeMenu;
// Returns an autoreleased NSView displaying the close 'x' button. // Returns an autoreleased NSView displaying the close 'x' button.
- (NSView*)closeButton; - (NSView*)closeButton;
...@@ -202,9 +195,6 @@ class MenuDelegate : public ui::SimpleMenuModel::Delegate { ...@@ -202,9 +195,6 @@ class MenuDelegate : public ui::SimpleMenuModel::Delegate {
// Called when the 'close' button is pressed. // Called when the 'close' button is pressed.
- (void)onClose:(id)sender; - (void)onClose:(id)sender;
// Called when the 'customize' button is pressed.
- (void)onCustomize:(id)sender;
// Sets the width of both |viewA| and |viewB| to be the larger of the // Sets the width of both |viewA| and |viewB| to be the larger of the
// two views' widths. Does not change either view's origin or height. // two views' widths. Does not change either view's origin or height.
+ (CGFloat)matchWidthsOf:(NSView*)viewA andOf:(NSView*)viewB; + (CGFloat)matchWidthsOf:(NSView*)viewA andOf:(NSView*)viewB;
...@@ -267,32 +257,31 @@ class MenuDelegate : public ui::SimpleMenuModel::Delegate { ...@@ -267,32 +257,31 @@ class MenuDelegate : public ui::SimpleMenuModel::Delegate {
- (void)showAtAnchor:(NSPoint)anchorPoint - (void)showAtAnchor:(NSPoint)anchorPoint
withDelegate:(PermissionBubbleView::Delegate*)delegate withDelegate:(PermissionBubbleView::Delegate*)delegate
forRequests:(const std::vector<PermissionBubbleRequest*>&)requests forRequests:(const std::vector<PermissionBubbleRequest*>&)requests
acceptStates:(const std::vector<bool>&)acceptStates acceptStates:(const std::vector<bool>&)acceptStates {
customizationMode:(BOOL)customizationMode {
DCHECK(!requests.empty()); DCHECK(!requests.empty());
DCHECK(delegate); DCHECK(delegate);
DCHECK(!customizationMode || (requests.size() == acceptStates.size()));
delegate_ = delegate; delegate_ = delegate;
NSView* contentView = [[self window] contentView]; NSView* contentView = [[self window] contentView];
[contentView setSubviews:@[]]; [contentView setSubviews:@[]];
BOOL singlePermission = requests.size() == 1;
// Create one button to use as a guide for the permissions' y-offsets. // Create one button to use as a guide for the permissions' y-offsets.
base::scoped_nsobject<NSView> allowOrOkButton; base::scoped_nsobject<NSView> allowOrOkButton;
if (customizationMode) { if (singlePermission) {
NSString* okTitle = l10n_util::GetNSString(IDS_OK);
allowOrOkButton.reset([[self buttonWithTitle:okTitle
action:@selector(ok:)] retain]);
} else {
NSString* allowTitle = l10n_util::GetNSString(IDS_PERMISSION_ALLOW); NSString* allowTitle = l10n_util::GetNSString(IDS_PERMISSION_ALLOW);
allowOrOkButton.reset([[self buttonWithTitle:allowTitle allowOrOkButton.reset([[self buttonWithTitle:allowTitle
action:@selector(onAllow:)] retain]); action:@selector(onAllow:)] retain]);
} else {
NSString* okTitle = l10n_util::GetNSString(IDS_OK);
allowOrOkButton.reset([[self buttonWithTitle:okTitle
action:@selector(ok:)] retain]);
} }
CGFloat yOffset = 2 * kVerticalPadding + NSMaxY([allowOrOkButton frame]); CGFloat yOffset = 2 * kVerticalPadding + NSMaxY([allowOrOkButton frame]);
BOOL singlePermission = requests.size() == 1;
base::scoped_nsobject<NSMutableArray> permissionMenus; base::scoped_nsobject<NSMutableArray> permissionMenus;
if (customizationMode) if (!singlePermission)
permissionMenus.reset([[NSMutableArray alloc] init]); permissionMenus.reset([[NSMutableArray alloc] init]);
CGFloat maxPermissionLineWidth = 0; CGFloat maxPermissionLineWidth = 0;
...@@ -305,7 +294,7 @@ class MenuDelegate : public ui::SimpleMenuModel::Delegate { ...@@ -305,7 +294,7 @@ class MenuDelegate : public ui::SimpleMenuModel::Delegate {
[permissionView setFrameOrigin:origin]; [permissionView setFrameOrigin:origin];
[contentView addSubview:permissionView]; [contentView addSubview:permissionView];
if (customizationMode) { if (!singlePermission) {
int index = it - requests.begin(); int index = it - requests.begin();
base::scoped_nsobject<NSView> menu( base::scoped_nsobject<NSView> menu(
[[self menuForRequest:(*it) [[self menuForRequest:(*it)
...@@ -342,7 +331,7 @@ class MenuDelegate : public ui::SimpleMenuModel::Delegate { ...@@ -342,7 +331,7 @@ class MenuDelegate : public ui::SimpleMenuModel::Delegate {
bubbleFrame.size.height = NSMaxY([titleView frame]) + kVerticalPadding + bubbleFrame.size.height = NSMaxY([titleView frame]) + kVerticalPadding +
info_bubble::kBubbleArrowHeight; info_bubble::kBubbleArrowHeight;
if (customizationMode) { if (!singlePermission) {
// Add the maximum menu width to the bubble width. // Add the maximum menu width to the bubble width.
CGFloat maxMenuWidth = 0; CGFloat maxMenuWidth = 0;
for (AllowBlockMenuButton* button in permissionMenus.get()) { for (AllowBlockMenuButton* button in permissionMenus.get()) {
...@@ -380,20 +369,9 @@ class MenuDelegate : public ui::SimpleMenuModel::Delegate { ...@@ -380,20 +369,9 @@ class MenuDelegate : public ui::SimpleMenuModel::Delegate {
[allowOrOkButton setFrameOrigin:NSMakePoint(xOrigin, kVerticalPadding)]; [allowOrOkButton setFrameOrigin:NSMakePoint(xOrigin, kVerticalPadding)];
[contentView addSubview:allowOrOkButton]; [contentView addSubview:allowOrOkButton];
if (customizationMode) { if (singlePermission) {
// Adjust the horizontal origin for each menu so that its right edge
// lines up with the right edge of the ok button.
CGFloat rightEdge = NSMaxX([allowOrOkButton frame]);
for (NSView* view in permissionMenus.get()) {
[view setFrameOrigin:NSMakePoint(rightEdge - NSWidth([view frame]),
NSMinY([view frame]))];
}
} else {
base::scoped_nsobject<NSView> blockButton; base::scoped_nsobject<NSView> blockButton;
if (singlePermission) blockButton.reset([[self blockButton] retain]);
blockButton.reset([[self blockButton] retain]);
else
blockButton.reset([[self blockButtonWithCustomizeMenu] retain]);
CGFloat width = [PermissionBubbleController matchWidthsOf:blockButton CGFloat width = [PermissionBubbleController matchWidthsOf:blockButton
andOf:allowOrOkButton]; andOf:allowOrOkButton];
// Ensure the allow/ok button is still in the correct position. // Ensure the allow/ok button is still in the correct position.
...@@ -403,6 +381,14 @@ class MenuDelegate : public ui::SimpleMenuModel::Delegate { ...@@ -403,6 +381,14 @@ class MenuDelegate : public ui::SimpleMenuModel::Delegate {
xOrigin = NSMinX([allowOrOkButton frame]) - width - kBetweenButtonsPadding; xOrigin = NSMinX([allowOrOkButton frame]) - width - kBetweenButtonsPadding;
[blockButton setFrameOrigin:NSMakePoint(xOrigin, kVerticalPadding)]; [blockButton setFrameOrigin:NSMakePoint(xOrigin, kVerticalPadding)];
[contentView addSubview:blockButton]; [contentView addSubview:blockButton];
} else {
// Adjust the horizontal origin for each menu so that its right edge
// lines up with the right edge of the ok button.
CGFloat rightEdge = NSMaxX([allowOrOkButton frame]);
for (NSView* view in permissionMenus.get()) {
[view setFrameOrigin:NSMakePoint(rightEdge - NSWidth([view frame]),
NSMinY([view frame]))];
}
} }
bubbleFrame = [[self window] frameRectForContentRect:bubbleFrame]; bubbleFrame = [[self window] frameRectForContentRect:bubbleFrame];
...@@ -518,17 +504,6 @@ class MenuDelegate : public ui::SimpleMenuModel::Delegate { ...@@ -518,17 +504,6 @@ class MenuDelegate : public ui::SimpleMenuModel::Delegate {
action:@selector(onBlock:)]; action:@selector(onBlock:)];
} }
- (NSView*)blockButtonWithCustomizeMenu {
menuDelegate_.reset(new MenuDelegate(self));
base::scoped_nsobject<SplitBlockButton> blockButton([[SplitBlockButton alloc]
initWithMenuDelegate:menuDelegate_.get()]);
[blockButton sizeToFit];
[blockButton setEnabled:YES];
[blockButton setAction:@selector(onBlock:)];
[blockButton setTarget:self];
return blockButton.autorelease();
}
- (NSView*)closeButton { - (NSView*)closeButton {
int dimension = chrome_style::GetCloseButtonSize(); int dimension = chrome_style::GetCloseButtonSize();
NSRect frame = NSMakeRect(0, 0, dimension, dimension); NSRect frame = NSMakeRect(0, 0, dimension, dimension);
...@@ -559,16 +534,6 @@ class MenuDelegate : public ui::SimpleMenuModel::Delegate { ...@@ -559,16 +534,6 @@ class MenuDelegate : public ui::SimpleMenuModel::Delegate {
delegate_->Closing(); delegate_->Closing();
} }
- (void)onCustomize:(id)sender {
DCHECK(delegate_);
delegate_->SetCustomizationMode();
}
- (void)onMenuItemClicked:(int)commandId {
DCHECK(commandId == 0);
[self onCustomize:nil];
}
- (void)activateTabWithContents:(content::WebContents*)newContents - (void)activateTabWithContents:(content::WebContents*)newContents
previousContents:(content::WebContents*)oldContents previousContents:(content::WebContents*)oldContents
atIndex:(NSInteger)index atIndex:(NSInteger)index
......
...@@ -129,20 +129,6 @@ class PermissionBubbleControllerTest : public CocoaTest, ...@@ -129,20 +129,6 @@ class PermissionBubbleControllerTest : public CocoaTest,
[menu performActionForItemAtIndex:[menu indexOfItem:next_item]]; [menu performActionForItemAtIndex:[menu indexOfItem:next_item]];
} }
NSMenuItem* FindCustomizeMenuItem() {
NSButton* button = FindButtonWithTitle(IDS_PERMISSION_DENY);
if (!button || ![button isKindOfClass:[SplitBlockButton class]])
return nil;
NSString* customize = l10n_util::GetNSString(IDS_PERMISSION_CUSTOMIZE);
SplitBlockButton* block_button =
base::mac::ObjCCast<SplitBlockButton>(button);
for (NSMenuItem* item in [[block_button menu] itemArray]) {
if ([[item title] isEqualToString:customize])
return item;
}
return nil;
}
protected: protected:
PermissionBubbleController* controller_; // Weak; it deletes itself. PermissionBubbleController* controller_; // Weak; it deletes itself.
scoped_ptr<PermissionBubbleCocoa> bridge_; scoped_ptr<PermissionBubbleCocoa> bridge_;
...@@ -160,79 +146,108 @@ TEST_F(PermissionBubbleControllerTest, ShowSinglePermission) { ...@@ -160,79 +146,108 @@ TEST_F(PermissionBubbleControllerTest, ShowSinglePermission) {
[controller_ showAtAnchor:NSZeroPoint [controller_ showAtAnchor:NSZeroPoint
withDelegate:this withDelegate:this
forRequests:requests_ forRequests:requests_
acceptStates:accept_states_ acceptStates:accept_states_];
customizationMode:NO];
EXPECT_TRUE(FindTextFieldWithString(kPermissionA)); EXPECT_TRUE(FindTextFieldWithString(kPermissionA));
EXPECT_TRUE(FindButtonWithTitle(IDS_PERMISSION_ALLOW)); EXPECT_TRUE(FindButtonWithTitle(IDS_PERMISSION_ALLOW));
EXPECT_TRUE(FindButtonWithTitle(IDS_PERMISSION_DENY)); EXPECT_TRUE(FindButtonWithTitle(IDS_PERMISSION_DENY));
EXPECT_FALSE(FindButtonWithTitle(IDS_OK)); EXPECT_FALSE(FindButtonWithTitle(IDS_OK));
EXPECT_FALSE(FindCustomizeMenuItem());
} }
TEST_F(PermissionBubbleControllerTest, ShowMultiplePermissions) { TEST_F(PermissionBubbleControllerTest, ShowMultiplePermissions) {
AddRequest(kPermissionB); AddRequest(kPermissionB);
AddRequest(kPermissionC); AddRequest(kPermissionC);
accept_states_.push_back(true); // A
accept_states_.push_back(true); // B
accept_states_.push_back(true); // C
[controller_ showAtAnchor:NSZeroPoint [controller_ showAtAnchor:NSZeroPoint
withDelegate:this withDelegate:this
forRequests:requests_ forRequests:requests_
acceptStates:accept_states_ acceptStates:accept_states_];
customizationMode:NO];
EXPECT_TRUE(FindTextFieldWithString(kPermissionA)); EXPECT_TRUE(FindTextFieldWithString(kPermissionA));
EXPECT_TRUE(FindTextFieldWithString(kPermissionB)); EXPECT_TRUE(FindTextFieldWithString(kPermissionB));
EXPECT_TRUE(FindTextFieldWithString(kPermissionC)); EXPECT_TRUE(FindTextFieldWithString(kPermissionC));
EXPECT_TRUE(FindButtonWithTitle(IDS_PERMISSION_ALLOW)); EXPECT_FALSE(FindButtonWithTitle(IDS_PERMISSION_ALLOW));
EXPECT_TRUE(FindButtonWithTitle(IDS_PERMISSION_DENY)); EXPECT_FALSE(FindButtonWithTitle(IDS_PERMISSION_DENY));
EXPECT_TRUE(FindCustomizeMenuItem()); EXPECT_TRUE(FindButtonWithTitle(IDS_OK));
EXPECT_FALSE(FindButtonWithTitle(IDS_OK));
} }
TEST_F(PermissionBubbleControllerTest, ShowCustomizationModeAllow) { TEST_F(PermissionBubbleControllerTest, ShowMultiplePermissionsAllow) {
accept_states_.push_back(true); AddRequest(kPermissionB);
accept_states_.push_back(true); // A
accept_states_.push_back(true); // B
[controller_ showAtAnchor:NSZeroPoint [controller_ showAtAnchor:NSZeroPoint
withDelegate:this withDelegate:this
forRequests:requests_ forRequests:requests_
acceptStates:accept_states_ acceptStates:accept_states_];
customizationMode:YES];
// Test that there is one menu, with 'Allow' visible. // Test that all menus have 'Allow' visible.
EXPECT_TRUE(FindMenuButtonWithTitle(IDS_PERMISSION_ALLOW)); EXPECT_TRUE(FindMenuButtonWithTitle(IDS_PERMISSION_ALLOW));
EXPECT_FALSE(FindMenuButtonWithTitle(IDS_PERMISSION_DENY)); EXPECT_FALSE(FindMenuButtonWithTitle(IDS_PERMISSION_DENY));
EXPECT_TRUE(FindButtonWithTitle(IDS_OK)); EXPECT_TRUE(FindButtonWithTitle(IDS_OK));
EXPECT_FALSE(FindButtonWithTitle(IDS_PERMISSION_ALLOW)); EXPECT_FALSE(FindButtonWithTitle(IDS_PERMISSION_ALLOW));
EXPECT_FALSE(FindButtonWithTitle(IDS_PERMISSION_DENY)); EXPECT_FALSE(FindButtonWithTitle(IDS_PERMISSION_DENY));
EXPECT_FALSE(FindCustomizeMenuItem());
} }
TEST_F(PermissionBubbleControllerTest, ShowCustomizationModeBlock) { TEST_F(PermissionBubbleControllerTest, ShowMultiplePermissionsBlock) {
accept_states_.push_back(false); AddRequest(kPermissionB);
accept_states_.push_back(false); // A
accept_states_.push_back(false); // B
[controller_ showAtAnchor:NSZeroPoint [controller_ showAtAnchor:NSZeroPoint
withDelegate:this withDelegate:this
forRequests:requests_ forRequests:requests_
acceptStates:accept_states_ acceptStates:accept_states_];
customizationMode:YES];
// Test that there is one menu, with 'Block' visible. // Test that all menus have 'Block' visible.
EXPECT_TRUE(FindMenuButtonWithTitle(IDS_PERMISSION_DENY)); EXPECT_TRUE(FindMenuButtonWithTitle(IDS_PERMISSION_DENY));
EXPECT_FALSE(FindMenuButtonWithTitle(IDS_PERMISSION_ALLOW)); EXPECT_FALSE(FindMenuButtonWithTitle(IDS_PERMISSION_ALLOW));
EXPECT_TRUE(FindButtonWithTitle(IDS_OK)); EXPECT_TRUE(FindButtonWithTitle(IDS_OK));
EXPECT_FALSE(FindButtonWithTitle(IDS_PERMISSION_ALLOW)); EXPECT_FALSE(FindButtonWithTitle(IDS_PERMISSION_ALLOW));
EXPECT_FALSE(FindButtonWithTitle(IDS_PERMISSION_DENY)); EXPECT_FALSE(FindButtonWithTitle(IDS_PERMISSION_DENY));
EXPECT_FALSE(FindCustomizeMenuItem()); }
TEST_F(PermissionBubbleControllerTest, ShowMultiplePermissionsMixed) {
AddRequest(kPermissionB);
AddRequest(kPermissionC);
accept_states_.push_back(false); // A
accept_states_.push_back(false); // B
accept_states_.push_back(true); // C
[controller_ showAtAnchor:NSZeroPoint
withDelegate:this
forRequests:requests_
acceptStates:accept_states_];
// Test that both 'allow' and 'deny' are visible.
EXPECT_TRUE(FindMenuButtonWithTitle(IDS_PERMISSION_DENY));
EXPECT_TRUE(FindMenuButtonWithTitle(IDS_PERMISSION_ALLOW));
EXPECT_TRUE(FindButtonWithTitle(IDS_OK));
EXPECT_FALSE(FindButtonWithTitle(IDS_PERMISSION_ALLOW));
EXPECT_FALSE(FindButtonWithTitle(IDS_PERMISSION_DENY));
} }
TEST_F(PermissionBubbleControllerTest, OK) { TEST_F(PermissionBubbleControllerTest, OK) {
accept_states_.push_back(true); AddRequest(kPermissionB);
accept_states_.push_back(true); // A
accept_states_.push_back(true); // B
[controller_ showAtAnchor:NSZeroPoint [controller_ showAtAnchor:NSZeroPoint
withDelegate:this withDelegate:this
forRequests:requests_ forRequests:requests_
acceptStates:accept_states_ acceptStates:accept_states_];
customizationMode:YES];
EXPECT_CALL(*this, Closing()).Times(1); EXPECT_CALL(*this, Closing()).Times(1);
[FindButtonWithTitle(IDS_OK) performClick:nil]; [FindButtonWithTitle(IDS_OK) performClick:nil];
...@@ -242,8 +257,7 @@ TEST_F(PermissionBubbleControllerTest, Allow) { ...@@ -242,8 +257,7 @@ TEST_F(PermissionBubbleControllerTest, Allow) {
[controller_ showAtAnchor:NSZeroPoint [controller_ showAtAnchor:NSZeroPoint
withDelegate:this withDelegate:this
forRequests:requests_ forRequests:requests_
acceptStates:accept_states_ acceptStates:accept_states_];
customizationMode:NO];
EXPECT_CALL(*this, Accept()).Times(1); EXPECT_CALL(*this, Accept()).Times(1);
[FindButtonWithTitle(IDS_PERMISSION_ALLOW) performClick:nil]; [FindButtonWithTitle(IDS_PERMISSION_ALLOW) performClick:nil];
...@@ -253,8 +267,7 @@ TEST_F(PermissionBubbleControllerTest, Deny) { ...@@ -253,8 +267,7 @@ TEST_F(PermissionBubbleControllerTest, Deny) {
[controller_ showAtAnchor:NSZeroPoint [controller_ showAtAnchor:NSZeroPoint
withDelegate:this withDelegate:this
forRequests:requests_ forRequests:requests_
acceptStates:accept_states_ acceptStates:accept_states_];
customizationMode:NO];
EXPECT_CALL(*this, Deny()).Times(1); EXPECT_CALL(*this, Deny()).Times(1);
[FindButtonWithTitle(IDS_PERMISSION_DENY) performClick:nil]; [FindButtonWithTitle(IDS_PERMISSION_DENY) performClick:nil];
...@@ -263,14 +276,13 @@ TEST_F(PermissionBubbleControllerTest, Deny) { ...@@ -263,14 +276,13 @@ TEST_F(PermissionBubbleControllerTest, Deny) {
TEST_F(PermissionBubbleControllerTest, ChangePermissionSelection) { TEST_F(PermissionBubbleControllerTest, ChangePermissionSelection) {
AddRequest(kPermissionB); AddRequest(kPermissionB);
accept_states_.push_back(true); accept_states_.push_back(true); // A
accept_states_.push_back(false); accept_states_.push_back(false); // B
[controller_ showAtAnchor:NSZeroPoint [controller_ showAtAnchor:NSZeroPoint
withDelegate:this withDelegate:this
forRequests:requests_ forRequests:requests_
acceptStates:accept_states_ acceptStates:accept_states_];
customizationMode:YES];
EXPECT_CALL(*this, ToggleAccept(0, false)).Times(1); EXPECT_CALL(*this, ToggleAccept(0, false)).Times(1);
EXPECT_CALL(*this, ToggleAccept(1, true)).Times(1); EXPECT_CALL(*this, ToggleAccept(1, true)).Times(1);
...@@ -279,18 +291,3 @@ TEST_F(PermissionBubbleControllerTest, ChangePermissionSelection) { ...@@ -279,18 +291,3 @@ TEST_F(PermissionBubbleControllerTest, ChangePermissionSelection) {
ChangePermissionMenuSelection(menu_a, IDS_PERMISSION_DENY); ChangePermissionMenuSelection(menu_a, IDS_PERMISSION_DENY);
ChangePermissionMenuSelection(menu_b, IDS_PERMISSION_ALLOW); ChangePermissionMenuSelection(menu_b, IDS_PERMISSION_ALLOW);
} }
TEST_F(PermissionBubbleControllerTest, ClickCustomize) {
AddRequest(kPermissionB);
[controller_ showAtAnchor:NSZeroPoint
withDelegate:this
forRequests:requests_
acceptStates:accept_states_
customizationMode:NO];
EXPECT_CALL(*this, SetCustomizationMode()).Times(1);
NSMenuItem* customize_item = FindCustomizeMenuItem();
EXPECT_TRUE(customize_item);
NSMenu* menu = [customize_item menu];
[menu performActionForItemAtIndex:[menu indexOfItem:customize_item]];
}
...@@ -133,43 +133,10 @@ void PermissionCombobox::PermissionChanged( ...@@ -133,43 +133,10 @@ void PermissionCombobox::PermissionChanged(
index_, permission.setting == CONTENT_SETTING_ALLOW); index_, permission.setting == CONTENT_SETTING_ALLOW);
} }
// A combobox originating on the Allow button allowing for customization
// of permissions.
class CustomizeAllowComboboxModel : public ui::ComboboxModel {
public:
enum Item {
INDEX_ALLOW = 0,
INDEX_CUSTOMIZE = 1
};
CustomizeAllowComboboxModel() {}
~CustomizeAllowComboboxModel() override {}
int GetItemCount() const override;
base::string16 GetItemAt(int index) override;
int GetDefaultIndex() const override;
};
int CustomizeAllowComboboxModel::GetItemCount() const {
return 2;
}
base::string16 CustomizeAllowComboboxModel::GetItemAt(int index) {
if (index == INDEX_ALLOW)
return l10n_util::GetStringUTF16(IDS_PERMISSION_ALLOW);
else
return l10n_util::GetStringUTF16(IDS_PERMISSION_CUSTOMIZE);
}
int CustomizeAllowComboboxModel::GetDefaultIndex() const {
return INDEX_ALLOW;
}
/////////////////////////////////////////////////////////////////////////////// ///////////////////////////////////////////////////////////////////////////////
// View implementation for the permissions bubble. // View implementation for the permissions bubble.
class PermissionsBubbleDelegateView : public views::BubbleDelegateView, class PermissionsBubbleDelegateView : public views::BubbleDelegateView,
public views::ButtonListener, public views::ButtonListener,
public views::ComboboxListener,
public PermissionCombobox::Listener { public PermissionCombobox::Listener {
public: public:
PermissionsBubbleDelegateView( PermissionsBubbleDelegateView(
...@@ -177,8 +144,7 @@ class PermissionsBubbleDelegateView : public views::BubbleDelegateView, ...@@ -177,8 +144,7 @@ class PermissionsBubbleDelegateView : public views::BubbleDelegateView,
PermissionBubbleViewViews* owner, PermissionBubbleViewViews* owner,
const std::string& languages, const std::string& languages,
const std::vector<PermissionBubbleRequest*>& requests, const std::vector<PermissionBubbleRequest*>& requests,
const std::vector<bool>& accept_state, const std::vector<bool>& accept_state);
bool customization_mode);
~PermissionsBubbleDelegateView() override; ~PermissionsBubbleDelegateView() override;
void Close(); void Close();
...@@ -194,9 +160,6 @@ class PermissionsBubbleDelegateView : public views::BubbleDelegateView, ...@@ -194,9 +160,6 @@ class PermissionsBubbleDelegateView : public views::BubbleDelegateView,
// ButtonListener: // ButtonListener:
void ButtonPressed(views::Button* button, const ui::Event& event) override; void ButtonPressed(views::Button* button, const ui::Event& event) override;
// ComboboxListener:
void OnPerformAction(views::Combobox* combobox) override;
// PermissionCombobox::Listener: // PermissionCombobox::Listener:
void PermissionSelectionChanged(int index, bool allowed) override; void PermissionSelectionChanged(int index, bool allowed) override;
...@@ -217,8 +180,7 @@ PermissionsBubbleDelegateView::PermissionsBubbleDelegateView( ...@@ -217,8 +180,7 @@ PermissionsBubbleDelegateView::PermissionsBubbleDelegateView(
PermissionBubbleViewViews* owner, PermissionBubbleViewViews* owner,
const std::string& languages, const std::string& languages,
const std::vector<PermissionBubbleRequest*>& requests, const std::vector<PermissionBubbleRequest*>& requests,
const std::vector<bool>& accept_state, const std::vector<bool>& accept_state)
bool customization_mode)
: views::BubbleDelegateView(anchor, views::BubbleBorder::TOP_LEFT), : views::BubbleDelegateView(anchor, views::BubbleBorder::TOP_LEFT),
owner_(owner), owner_(owner),
allow_(NULL), allow_(NULL),
...@@ -244,8 +206,8 @@ PermissionsBubbleDelegateView::PermissionsBubbleDelegateView( ...@@ -244,8 +206,8 @@ PermissionsBubbleDelegateView::PermissionsBubbleDelegateView(
for (size_t index = 0; index < requests.size(); index++) { for (size_t index = 0; index < requests.size(); index++) {
DCHECK(index < accept_state.size()); DCHECK(index < accept_state.size());
// The row is laid out containing a leading-aligned label area and a // The row is laid out containing a leading-aligned label area and a
// trailing column which will be filled during customization with a // trailing column which will be filled if there are multiple permission
// combobox. // requests.
views::View* row = new views::View(); views::View* row = new views::View();
views::GridLayout* row_layout = new views::GridLayout(row); views::GridLayout* row_layout = new views::GridLayout(row);
row->SetLayoutManager(row_layout); row->SetLayoutManager(row_layout);
...@@ -272,7 +234,7 @@ PermissionsBubbleDelegateView::PermissionsBubbleDelegateView( ...@@ -272,7 +234,7 @@ PermissionsBubbleDelegateView::PermissionsBubbleDelegateView(
label_container->AddChildView(label); label_container->AddChildView(label);
row_layout->AddView(label_container); row_layout->AddView(label_container);
if (customization_mode) { if (requests.size() > 1) {
PermissionCombobox* combobox = new PermissionCombobox( PermissionCombobox* combobox = new PermissionCombobox(
this, this,
index, index,
...@@ -293,8 +255,8 @@ PermissionsBubbleDelegateView::PermissionsBubbleDelegateView( ...@@ -293,8 +255,8 @@ PermissionsBubbleDelegateView::PermissionsBubbleDelegateView(
button_row->SetLayoutManager(button_layout); button_row->SetLayoutManager(button_layout);
AddChildView(button_row); AddChildView(button_row);
// Customization case: just an "OK" button // For multiple permissions: just an "OK" button.
if (customization_mode) { if (requests.size() > 1) {
columns->AddColumn(views::GridLayout::TRAILING, views::GridLayout::FILL, columns->AddColumn(views::GridLayout::TRAILING, views::GridLayout::FILL,
100, views::GridLayout::USE_PREF, 0, 0); 100, views::GridLayout::USE_PREF, 0, 0);
button_layout->StartRowWithPadding(0, 0, 0, 4); button_layout->StartRowWithPadding(0, 0, 0, 4);
...@@ -308,8 +270,7 @@ PermissionsBubbleDelegateView::PermissionsBubbleDelegateView( ...@@ -308,8 +270,7 @@ PermissionsBubbleDelegateView::PermissionsBubbleDelegateView(
return; return;
} }
// No customization: lay out the Deny/Allow buttons. // For a single permission: lay out the Deny/Allow buttons.
columns->AddColumn(views::GridLayout::TRAILING, views::GridLayout::FILL, columns->AddColumn(views::GridLayout::TRAILING, views::GridLayout::FILL,
100, views::GridLayout::USE_PREF, 0, 0); 100, views::GridLayout::USE_PREF, 0, 0);
columns->AddPaddingColumn(0, kItemMajorSpacing - (2*kButtonBorderSize)); columns->AddPaddingColumn(0, kItemMajorSpacing - (2*kButtonBorderSize));
...@@ -317,26 +278,11 @@ PermissionsBubbleDelegateView::PermissionsBubbleDelegateView( ...@@ -317,26 +278,11 @@ PermissionsBubbleDelegateView::PermissionsBubbleDelegateView(
0, views::GridLayout::USE_PREF, 0, 0); 0, views::GridLayout::USE_PREF, 0, 0);
button_layout->StartRow(0, 0); button_layout->StartRow(0, 0);
// Allow button is a regular button when there's only one option, and a
// STYLE_ACTION Combobox when there are more than one option and
// customization is an option.
base::string16 allow_text = l10n_util::GetStringUTF16(IDS_PERMISSION_ALLOW); base::string16 allow_text = l10n_util::GetStringUTF16(IDS_PERMISSION_ALLOW);
if (requests.size() == 1) { views::LabelButton* allow_button = new views::LabelButton(this, allow_text);
views::LabelButton* allow_button = new views::LabelButton(this, allow_text); allow_button->SetStyle(views::Button::STYLE_BUTTON);
allow_button->SetStyle(views::Button::STYLE_BUTTON); button_layout->AddView(allow_button);
button_layout->AddView(allow_button); allow_ = allow_button;
allow_ = allow_button;
} else {
views::Combobox* allow_combobox = new views::Combobox(
new CustomizeAllowComboboxModel());
allow_combobox->set_listener(this);
allow_combobox->SetStyle(views::Combobox::STYLE_ACTION);
allow_combobox->SetAccessibleName(
l10n_util::GetStringUTF16(IDS_PERMISSION_ALLOW_COMBOBOX));
button_layout->AddView(allow_combobox);
allow_combobox_ = allow_combobox;
}
base::string16 deny_text = l10n_util::GetStringUTF16(IDS_PERMISSION_DENY); base::string16 deny_text = l10n_util::GetStringUTF16(IDS_PERMISSION_DENY);
views::LabelButton* deny_button = new views::LabelButton(this, deny_text); views::LabelButton* deny_button = new views::LabelButton(this, deny_text);
...@@ -403,18 +349,6 @@ void PermissionsBubbleDelegateView::PermissionSelectionChanged( ...@@ -403,18 +349,6 @@ void PermissionsBubbleDelegateView::PermissionSelectionChanged(
owner_->Toggle(index, allowed); owner_->Toggle(index, allowed);
} }
void PermissionsBubbleDelegateView::OnPerformAction(
views::Combobox* combobox) {
if (combobox == allow_combobox_) {
if (combobox->selected_index() ==
CustomizeAllowComboboxModel::INDEX_CUSTOMIZE)
owner_->SetCustomizationMode();
else if (combobox->selected_index() ==
CustomizeAllowComboboxModel::INDEX_ALLOW)
owner_->Accept();
}
}
////////////////////////////////////////////////////////////////////////////// //////////////////////////////////////////////////////////////////////////////
// PermissionBubbleViewViews // PermissionBubbleViewViews
...@@ -437,14 +371,13 @@ void PermissionBubbleViewViews::SetDelegate(Delegate* delegate) { ...@@ -437,14 +371,13 @@ void PermissionBubbleViewViews::SetDelegate(Delegate* delegate) {
void PermissionBubbleViewViews::Show( void PermissionBubbleViewViews::Show(
const std::vector<PermissionBubbleRequest*>& requests, const std::vector<PermissionBubbleRequest*>& requests,
const std::vector<bool>& values, const std::vector<bool>& values) {
bool customization_mode) {
if (bubble_delegate_ != NULL) if (bubble_delegate_ != NULL)
bubble_delegate_->Close(); bubble_delegate_->Close();
bubble_delegate_ = bubble_delegate_ =
new PermissionsBubbleDelegateView(anchor_view_, this, languages_, new PermissionsBubbleDelegateView(anchor_view_, this, languages_,
requests, values, customization_mode); requests, values);
views::BubbleDelegateView::CreateBubble(bubble_delegate_)->Show(); views::BubbleDelegateView::CreateBubble(bubble_delegate_)->Show();
bubble_delegate_->SizeToContents(); bubble_delegate_->SizeToContents();
} }
...@@ -485,8 +418,3 @@ void PermissionBubbleViewViews::Deny() { ...@@ -485,8 +418,3 @@ void PermissionBubbleViewViews::Deny() {
if (delegate_) if (delegate_)
delegate_->Deny(); delegate_->Deny();
} }
void PermissionBubbleViewViews::SetCustomizationMode() {
if (delegate_)
delegate_->SetCustomizationMode();
}
...@@ -26,8 +26,7 @@ class PermissionBubbleViewViews : public PermissionBubbleView { ...@@ -26,8 +26,7 @@ class PermissionBubbleViewViews : public PermissionBubbleView {
// PermissionBubbleView: // PermissionBubbleView:
void SetDelegate(Delegate* delegate) override; void SetDelegate(Delegate* delegate) override;
void Show(const std::vector<PermissionBubbleRequest*>& requests, void Show(const std::vector<PermissionBubbleRequest*>& requests,
const std::vector<bool>& accept_state, const std::vector<bool>& accept_state) override;
bool customization_mode) override;
bool CanAcceptRequestUpdate() override; bool CanAcceptRequestUpdate() override;
void Hide() override; void Hide() override;
bool IsVisible() override; bool IsVisible() override;
...@@ -36,7 +35,6 @@ class PermissionBubbleViewViews : public PermissionBubbleView { ...@@ -36,7 +35,6 @@ class PermissionBubbleViewViews : public PermissionBubbleView {
void Toggle(int index, bool value); void Toggle(int index, bool value);
void Accept(); void Accept();
void Deny(); void Deny();
void SetCustomizationMode();
private: private:
views::View* anchor_view_; views::View* anchor_view_;
......
...@@ -83,7 +83,6 @@ PermissionBubbleManager::PermissionBubbleManager( ...@@ -83,7 +83,6 @@ PermissionBubbleManager::PermissionBubbleManager(
bubble_showing_(false), bubble_showing_(false),
view_(NULL), view_(NULL),
request_url_has_loaded_(false), request_url_has_loaded_(false),
customization_mode_(false),
weak_factory_(this) {} weak_factory_(this) {}
PermissionBubbleManager::~PermissionBubbleManager() { PermissionBubbleManager::~PermissionBubbleManager() {
...@@ -271,12 +270,6 @@ void PermissionBubbleManager::ToggleAccept(int request_index, bool new_value) { ...@@ -271,12 +270,6 @@ void PermissionBubbleManager::ToggleAccept(int request_index, bool new_value) {
accept_states_[request_index] = new_value; accept_states_[request_index] = new_value;
} }
void PermissionBubbleManager::SetCustomizationMode() {
customization_mode_ = true;
if (view_)
view_->Show(requests_, accept_states_, customization_mode_);
}
void PermissionBubbleManager::Accept() { void PermissionBubbleManager::Accept() {
std::vector<PermissionBubbleRequest*>::iterator requests_iter; std::vector<PermissionBubbleRequest*>::iterator requests_iter;
std::vector<bool>::iterator accepts_iter = accept_states_.begin(); std::vector<bool>::iterator accepts_iter = accept_states_.begin();
...@@ -352,7 +345,7 @@ void PermissionBubbleManager::TriggerShowBubble() { ...@@ -352,7 +345,7 @@ void PermissionBubbleManager::TriggerShowBubble() {
// Note: this should appear above Show() for testing, since in that // Note: this should appear above Show() for testing, since in that
// case we may do in-line calling of finalization. // case we may do in-line calling of finalization.
bubble_showing_ = true; bubble_showing_ = true;
view_->Show(requests_, accept_states_, customization_mode_); view_->Show(requests_, accept_states_);
} }
void PermissionBubbleManager::FinalizeBubble() { void PermissionBubbleManager::FinalizeBubble() {
......
...@@ -85,7 +85,6 @@ class PermissionBubbleManager ...@@ -85,7 +85,6 @@ class PermissionBubbleManager
// PermissionBubbleView::Delegate: // PermissionBubbleView::Delegate:
void ToggleAccept(int request_index, bool new_value) override; void ToggleAccept(int request_index, bool new_value) override;
void SetCustomizationMode() override;
void Accept() override; void Accept() override;
void Deny() override; void Deny() override;
void Closing() override; void Closing() override;
...@@ -137,7 +136,6 @@ class PermissionBubbleManager ...@@ -137,7 +136,6 @@ class PermissionBubbleManager
bool request_url_has_loaded_; bool request_url_has_loaded_;
std::vector<bool> accept_states_; std::vector<bool> accept_states_;
bool customization_mode_;
base::WeakPtrFactory<PermissionBubbleManager> weak_factory_; base::WeakPtrFactory<PermissionBubbleManager> weak_factory_;
}; };
......
...@@ -32,8 +32,7 @@ class MockView : public PermissionBubbleView { ...@@ -32,8 +32,7 @@ class MockView : public PermissionBubbleView {
void SetDelegate(Delegate* delegate) override { delegate_ = delegate; } void SetDelegate(Delegate* delegate) override { delegate_ = delegate; }
void Show(const std::vector<PermissionBubbleRequest*>& requests, void Show(const std::vector<PermissionBubbleRequest*>& requests,
const std::vector<bool>& accept_state, const std::vector<bool>& accept_state) override {
bool customization_state_) override {
shown_ = true; shown_ = true;
permission_requests_ = requests; permission_requests_ = requests;
permission_states_ = accept_state; permission_states_ = accept_state;
......
...@@ -23,7 +23,6 @@ class PermissionBubbleView { ...@@ -23,7 +23,6 @@ class PermissionBubbleView {
virtual ~Delegate() {} virtual ~Delegate() {}
virtual void ToggleAccept(int index, bool new_value) = 0; virtual void ToggleAccept(int index, bool new_value) = 0;
virtual void SetCustomizationMode() = 0;
virtual void Accept() = 0; virtual void Accept() = 0;
virtual void Deny() = 0; virtual void Deny() = 0;
virtual void Closing() = 0; virtual void Closing() = 0;
...@@ -43,8 +42,7 @@ class PermissionBubbleView { ...@@ -43,8 +42,7 @@ class PermissionBubbleView {
// in this call. // in this call.
virtual void Show( virtual void Show(
const std::vector<PermissionBubbleRequest*>& requests, const std::vector<PermissionBubbleRequest*>& requests,
const std::vector<bool>& accept_state, const std::vector<bool>& accept_state) = 0;
bool customization_mode) = 0;
// Returns true if the view can accept a new Show() command to coalesce // Returns true if the view can accept a new Show() command to coalesce
// requests. Currently the policy is that this should return true if the view // requests. Currently the policy is that this should return true if the view
......
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