diff options
24 files changed, 513 insertions, 97 deletions
diff --git a/chrome/app/chrome_command_ids.h b/chrome/app/chrome_command_ids.h index 75a3a44..fdff03d 100644 --- a/chrome/app/chrome_command_ids.h +++ b/chrome/app/chrome_command_ids.h @@ -87,6 +87,7 @@ #define IDC_ADVANCED_PRINT 35007 #define IDC_PRINT_TO_DESTINATION 35008 #define IDC_TRANSLATE_PAGE 35009 +#define IDC_MANAGE_PASSWORDS_FOR_PAGE 35010 // When adding a new encoding to this list, be sure to append it to the // EncodingMenuController::kValidEncodingIds array in diff --git a/chrome/browser/ui/browser_command_controller.cc b/chrome/browser/ui/browser_command_controller.cc index a6978e7..354932e 100644 --- a/chrome/browser/ui/browser_command_controller.cc +++ b/chrome/browser/ui/browser_command_controller.cc @@ -559,6 +559,11 @@ void BrowserCommandController::ExecuteCommandWithDisposition( case IDC_TRANSLATE_PAGE: Translate(browser_); break; + case IDC_MANAGE_PASSWORDS_FOR_PAGE: + ManagePasswordsForPage(browser_); + break; + + // Page encoding commands case IDC_ENCODING_AUTO_DETECT: browser_->ToggleEncodingAutoDetect(); break; @@ -917,6 +922,7 @@ void BrowserCommandController::InitCommandState() { // Page-related commands command_updater_.UpdateCommandEnabled(IDC_EMAIL_PAGE_LOCATION, true); + command_updater_.UpdateCommandEnabled(IDC_MANAGE_PASSWORDS_FOR_PAGE, true); command_updater_.UpdateCommandEnabled(IDC_ENCODING_AUTO_DETECT, true); command_updater_.UpdateCommandEnabled(IDC_ENCODING_UTF8, true); command_updater_.UpdateCommandEnabled(IDC_ENCODING_UTF16LE, true); diff --git a/chrome/browser/ui/browser_commands.cc b/chrome/browser/ui/browser_commands.cc index f24bec6..b266077 100644 --- a/chrome/browser/ui/browser_commands.cc +++ b/chrome/browser/ui/browser_commands.cc @@ -775,6 +775,18 @@ void Translate(Browser* browser) { web_contents, step, TranslateErrors::NONE); } +void ManagePasswordsForPage(Browser* browser) { +// TODO(mkwst): Implement this feature on Mac: http://crbug.com/261628 +#if !defined(OS_MACOSX) + if (!browser->window()->IsActive()) + return; + + WebContents* web_contents = + browser->tab_strip_model()->GetActiveWebContents(); + chrome::ShowManagePasswordsBubble(web_contents); +#endif +} + void TogglePagePinnedToStartScreen(Browser* browser) { #if defined(OS_WIN) MetroPinTabHelper::FromWebContents( diff --git a/chrome/browser/ui/browser_commands.h b/chrome/browser/ui/browser_commands.h index 1e6fe26..fe63b0e 100644 --- a/chrome/browser/ui/browser_commands.h +++ b/chrome/browser/ui/browser_commands.h @@ -96,6 +96,7 @@ bool CanBookmarkCurrentPage(const Browser* browser); void BookmarkAllTabs(Browser* browser); bool CanBookmarkAllTabs(const Browser* browser); void Translate(Browser* browser); +void ManagePasswordsForPage(Browser* browser); void TogglePagePinnedToStartScreen(Browser* browser); void SavePage(Browser* browser); bool CanSavePage(const Browser* browser); diff --git a/chrome/browser/ui/browser_dialogs.h b/chrome/browser/ui/browser_dialogs.h index f812a13..ac688d6 100644 --- a/chrome/browser/ui/browser_dialogs.h +++ b/chrome/browser/ui/browser_dialogs.h @@ -114,6 +114,13 @@ void ShowSignedCertificateTimestampsViewer( content::WebContents* web_contents, const content::SignedCertificateTimestampIDStatusList& sct_ids_list); +#if !defined(OS_MACOSX) +// Shows the ManagePasswords bubble for a particular |web_contents|. +// +// TODO(mkwst): Implement this feature on Mac: http://crbug.com/261628 +void ShowManagePasswordsBubble(content::WebContents* web_contents); +#endif + } // namespace chrome #endif // CHROME_BROWSER_UI_BROWSER_DIALOGS_H_ diff --git a/chrome/browser/ui/passwords/manage_passwords_bubble_ui_controller.cc b/chrome/browser/ui/passwords/manage_passwords_bubble_ui_controller.cc index 987077b..88c76e6 100644 --- a/chrome/browser/ui/passwords/manage_passwords_bubble_ui_controller.cc +++ b/chrome/browser/ui/passwords/manage_passwords_bubble_ui_controller.cc @@ -170,6 +170,8 @@ void ManagePasswordsBubbleUIController::UpdateIconAndBubbleState( if (manage_passwords_bubble_needs_showing_) { DCHECK(state == ManagePasswordsIcon::PENDING_STATE); + // TODO(mkwst): Replace this with execution of a browser command once we + // can pipe a CommandUpdater down here. icon->ShowBubbleWithoutUserInteraction(); manage_passwords_bubble_needs_showing_ = false; } diff --git a/chrome/browser/ui/passwords/manage_passwords_icon.cc b/chrome/browser/ui/passwords/manage_passwords_icon.cc index 0914bba..c9758b5 100644 --- a/chrome/browser/ui/passwords/manage_passwords_icon.cc +++ b/chrome/browser/ui/passwords/manage_passwords_icon.cc @@ -14,5 +14,5 @@ void ManagePasswordsIcon::SetState(State state) { if (state_ == state) return; state_ = state; - SetStateInternal(state); + UpdateVisibleUI(); } diff --git a/chrome/browser/ui/passwords/manage_passwords_icon.h b/chrome/browser/ui/passwords/manage_passwords_icon.h index 0a44406..004c8cd 100644 --- a/chrome/browser/ui/passwords/manage_passwords_icon.h +++ b/chrome/browser/ui/passwords/manage_passwords_icon.h @@ -42,8 +42,9 @@ class ManagePasswordsIcon { ManagePasswordsIcon(); ~ManagePasswordsIcon(); - // Called from SetState() iff the icon's state has changed. - virtual void SetStateInternal(State state) = 0; + // Called from SetState() iff the icon's state has changed in order to do + // whatever platform-specific UI work is necessary given the new state. + virtual void UpdateVisibleUI() = 0; private: State state_; diff --git a/chrome/browser/ui/passwords/manage_passwords_icon_mock.cc b/chrome/browser/ui/passwords/manage_passwords_icon_mock.cc index 64bdb16..4b575e8 100644 --- a/chrome/browser/ui/passwords/manage_passwords_icon_mock.cc +++ b/chrome/browser/ui/passwords/manage_passwords_icon_mock.cc @@ -14,5 +14,5 @@ void ManagePasswordsIconMock::ShowBubbleWithoutUserInteraction() { ++bubble_shown_count_; } -void ManagePasswordsIconMock::SetStateInternal(ManagePasswordsIcon::State) { +void ManagePasswordsIconMock::UpdateVisibleUI() { } diff --git a/chrome/browser/ui/passwords/manage_passwords_icon_mock.h b/chrome/browser/ui/passwords/manage_passwords_icon_mock.h index 3781e6a..de4f814 100644 --- a/chrome/browser/ui/passwords/manage_passwords_icon_mock.h +++ b/chrome/browser/ui/passwords/manage_passwords_icon_mock.h @@ -18,7 +18,8 @@ class ManagePasswordsIconMock : public ManagePasswordsIcon { int bubble_shown_count() const { return bubble_shown_count_; } protected: - virtual void SetStateInternal(State state) OVERRIDE; + // ManagePasswordsIcon: + virtual void UpdateVisibleUI() OVERRIDE; private: int bubble_shown_count_; diff --git a/chrome/browser/ui/views/location_bar/location_bar_view.cc b/chrome/browser/ui/views/location_bar/location_bar_view.cc index 42cfd48..14b85c6 100644 --- a/chrome/browser/ui/views/location_bar/location_bar_view.cc +++ b/chrome/browser/ui/views/location_bar/location_bar_view.cc @@ -377,8 +377,8 @@ void LocationBarView::Init() { open_pdf_in_reader_view_ = new OpenPDFInReaderView(); AddChildView(open_pdf_in_reader_view_); - manage_passwords_icon_view_ = new ManagePasswordsIconView(delegate_); - manage_passwords_icon_view_->SetState(ManagePasswordsIcon::INACTIVE_STATE); + manage_passwords_icon_view_ = + new ManagePasswordsIconView(delegate_, command_updater()); AddChildView(manage_passwords_icon_view_); translate_icon_view_ = new TranslateIconView(command_updater()); diff --git a/chrome/browser/ui/views/passwords/manage_passwords_bubble_view.cc b/chrome/browser/ui/views/passwords/manage_passwords_bubble_view.cc index 654c114..e379e67 100644 --- a/chrome/browser/ui/views/passwords/manage_passwords_bubble_view.cc +++ b/chrome/browser/ui/views/passwords/manage_passwords_bubble_view.cc @@ -4,12 +4,12 @@ #include "chrome/browser/ui/views/passwords/manage_passwords_bubble_view.h" -#include "base/metrics/histogram.h" #include "chrome/browser/chrome_notification_types.h" #include "chrome/browser/ui/browser.h" #include "chrome/browser/ui/browser_finder.h" #include "chrome/browser/ui/browser_window.h" #include "chrome/browser/ui/passwords/manage_passwords_bubble_model.h" +#include "chrome/browser/ui/passwords/manage_passwords_bubble_ui_controller.h" #include "chrome/browser/ui/views/frame/browser_view.h" #include "chrome/browser/ui/views/location_bar/location_bar_view.h" #include "chrome/browser/ui/views/passwords/manage_password_item_view.h" @@ -45,33 +45,24 @@ int GetFieldWidth(FieldType type) { : kPasswordFieldSize); } -class SavePasswordRefusalComboboxModel : public ui::ComboboxModel { - public: - enum { INDEX_NOPE = 0, INDEX_NEVER_FOR_THIS_SITE = 1, }; +} // namespace - SavePasswordRefusalComboboxModel() { - items_.push_back( - l10n_util::GetStringUTF16(IDS_PASSWORD_MANAGER_CANCEL_BUTTON)); - items_.push_back( - l10n_util::GetStringUTF16(IDS_PASSWORD_MANAGER_BLACKLIST_BUTTON)); - } - virtual ~SavePasswordRefusalComboboxModel() {} - - private: - // Overridden from ui::ComboboxModel: - virtual int GetItemCount() const OVERRIDE { return items_.size(); } - virtual base::string16 GetItemAt(int index) OVERRIDE { return items_[index]; } - virtual bool IsItemSeparatorAt(int index) OVERRIDE { - return items_[index].empty(); - } - virtual int GetDefaultIndex() const OVERRIDE { return 0; } - std::vector<base::string16> items_; +// Globals -------------------------------------------------------------------- - DISALLOW_COPY_AND_ASSIGN(SavePasswordRefusalComboboxModel); -}; +namespace chrome { -} // namespace +void ShowManagePasswordsBubble(content::WebContents* web_contents) { + ManagePasswordsBubbleUIController* controller = + ManagePasswordsBubbleUIController::FromWebContents(web_contents); + ManagePasswordsBubbleView::ShowBubble( + web_contents, + controller->manage_passwords_bubble_needs_showing() ? + ManagePasswordsBubbleView::AUTOMATIC : + ManagePasswordsBubbleView::USER_ACTION); +} + +} // namespace chrome // ManagePasswordsBubbleView -------------------------------------------------- @@ -262,8 +253,8 @@ void ManagePasswordsBubbleView::Init() { layout->StartRow(0, SINGLE_VIEW_COLUMN_SET); layout->AddView(item); - refuse_combobox_ = - new views::Combobox(new SavePasswordRefusalComboboxModel()); + combobox_model_.reset(new SavePasswordRefusalComboboxModel()); + refuse_combobox_.reset(new views::Combobox(combobox_model_.get())); refuse_combobox_->set_listener(this); refuse_combobox_->SetStyle(views::Combobox::STYLE_ACTION); @@ -273,7 +264,7 @@ void ManagePasswordsBubbleView::Init() { layout->StartRowWithPadding( 0, DOUBLE_BUTTON_COLUMN_SET, 0, views::kRelatedControlVerticalSpacing); layout->AddView(save_button_); - layout->AddView(refuse_combobox_); + layout->AddView(refuse_combobox_.get()); layout->AddPaddingRow(0, views::kRelatedControlVerticalSpacing); } else { // If we have a list of passwords to store for the current site, display diff --git a/chrome/browser/ui/views/passwords/manage_passwords_bubble_view.h b/chrome/browser/ui/views/passwords/manage_passwords_bubble_view.h index 39e1e50..07609c2 100644 --- a/chrome/browser/ui/views/passwords/manage_passwords_bubble_view.h +++ b/chrome/browser/ui/views/passwords/manage_passwords_bubble_view.h @@ -7,6 +7,7 @@ #include "base/basictypes.h" #include "chrome/browser/ui/passwords/manage_passwords_bubble.h" +#include "chrome/browser/ui/views/passwords/save_password_refusal_combobox_model.h" #include "ui/views/bubble/bubble_delegate.h" #include "ui/views/controls/button/button.h" #include "ui/views/controls/combobox/combobox.h" @@ -106,13 +107,18 @@ class ManagePasswordsBubbleView : public ManagePasswordsBubble, // shown twice at the same time. static ManagePasswordsBubbleView* manage_passwords_bubble_; - // The buttons that are shown in the bubble. + // The views that are shown in the bubble. views::BlueButton* save_button_; - views::Combobox* refuse_combobox_; - views::Link* manage_link_; views::LabelButton* done_button_; + // The combobox doesn't take ownership of it's model. If we created a combobox + // we need to ensure that we delete the model here, and because the combobox + // uses the model in it's destructor, we need to make sure we delete the model + // _after_ the combobox itself is deleted. + scoped_ptr<SavePasswordRefusalComboboxModel> combobox_model_; + scoped_ptr<views::Combobox> refuse_combobox_; + DISALLOW_COPY_AND_ASSIGN(ManagePasswordsBubbleView); }; diff --git a/chrome/browser/ui/views/passwords/manage_passwords_bubble_view_browsertest.cc b/chrome/browser/ui/views/passwords/manage_passwords_bubble_view_browsertest.cc new file mode 100644 index 0000000..11f4e87 --- /dev/null +++ b/chrome/browser/ui/views/passwords/manage_passwords_bubble_view_browsertest.cc @@ -0,0 +1,117 @@ +// Copyright 2014 The Chromium Authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +#include "chrome/browser/ui/views/passwords/manage_passwords_bubble_view.h" + +#include "base/metrics/histogram_samples.h" +#include "chrome/browser/ui/browser.h" +#include "chrome/browser/ui/tabs/tab_strip_model.h" +#include "chrome/browser/ui/views/passwords/manage_passwords_view_test.h" +#include "components/password_manager/core/browser/password_manager_metrics_util.h" +#include "components/password_manager/core/browser/stub_password_manager_client.h" +#include "testing/gtest/include/gtest/gtest.h" + +namespace { + +const char kDisplayDispositionMetric[] = "PasswordBubble.DisplayDisposition"; + +} // namespace + +typedef ManagePasswordsViewTest ManagePasswordsBubbleViewTest; + +IN_PROC_BROWSER_TEST_F(ManagePasswordsBubbleViewTest, BasicOpenAndClose) { + EXPECT_FALSE(ManagePasswordsBubbleView::IsShowing()); + ManagePasswordsBubbleView::ShowBubble( + browser()->tab_strip_model()->GetActiveWebContents(), + ManagePasswordsBubble::USER_ACTION); + EXPECT_TRUE(ManagePasswordsBubbleView::IsShowing()); + ManagePasswordsBubbleView::CloseBubble(); + EXPECT_FALSE(ManagePasswordsBubbleView::IsShowing()); + + // And, just for grins, ensure that we can re-open the bubble. + ManagePasswordsBubbleView::ShowBubble( + browser()->tab_strip_model()->GetActiveWebContents(), + ManagePasswordsBubble::USER_ACTION); + EXPECT_TRUE(ManagePasswordsBubbleView::IsShowing()); + ManagePasswordsBubbleView::CloseBubble(); + EXPECT_FALSE(ManagePasswordsBubbleView::IsShowing()); +} + +// Same as 'BasicOpenAndClose', but use the command rather than the static +// method directly. +IN_PROC_BROWSER_TEST_F(ManagePasswordsBubbleViewTest, CommandControlsBubble) { + // The command only works if the icon is visible, so get into management mode. + SetupManagingPasswords(); + EXPECT_FALSE(ManagePasswordsBubbleView::IsShowing()); + ExecuteManagePasswordsCommand(); + EXPECT_TRUE(ManagePasswordsBubbleView::IsShowing()); + ManagePasswordsBubbleView::CloseBubble(); + EXPECT_FALSE(ManagePasswordsBubbleView::IsShowing()); + + // And, just for grins, ensure that we can re-open the bubble. + ExecuteManagePasswordsCommand(); + EXPECT_TRUE(ManagePasswordsBubbleView::IsShowing()); + ManagePasswordsBubbleView::CloseBubble(); + EXPECT_FALSE(ManagePasswordsBubbleView::IsShowing()); +} + +IN_PROC_BROWSER_TEST_F(ManagePasswordsBubbleViewTest, + CommandExecutionInManagingState) { + SetupManagingPasswords(); + ExecuteManagePasswordsCommand(); + + scoped_ptr<base::HistogramSamples> samples( + GetSamples(kDisplayDispositionMetric)); + EXPECT_EQ( + 0, + samples->GetCount( + password_manager::metrics_util::AUTOMATIC_WITH_PASSWORD_PENDING)); + EXPECT_EQ(0, + samples->GetCount( + password_manager::metrics_util::MANUAL_WITH_PASSWORD_PENDING)); + EXPECT_EQ(1, + samples->GetCount( + password_manager::metrics_util::MANUAL_MANAGE_PASSWORDS)); +} + +IN_PROC_BROWSER_TEST_F(ManagePasswordsBubbleViewTest, + CommandExecutionInAutomaticState) { + // Open with pending password: automagical! + SetupPendingPassword(); + + scoped_ptr<base::HistogramSamples> samples( + GetSamples(kDisplayDispositionMetric)); + EXPECT_EQ( + 1, + samples->GetCount( + password_manager::metrics_util::AUTOMATIC_WITH_PASSWORD_PENDING)); + EXPECT_EQ(0, + samples->GetCount( + password_manager::metrics_util::MANUAL_WITH_PASSWORD_PENDING)); + EXPECT_EQ(0, + samples->GetCount( + password_manager::metrics_util::MANUAL_MANAGE_PASSWORDS)); +} + +IN_PROC_BROWSER_TEST_F(ManagePasswordsBubbleViewTest, + CommandExecutionInPendingState) { + // Open once with pending password: automagical! + SetupPendingPassword(); + ManagePasswordsBubbleView::CloseBubble(); + // This opening should be measured as manual. + ExecuteManagePasswordsCommand(); + + scoped_ptr<base::HistogramSamples> samples( + GetSamples(kDisplayDispositionMetric)); + EXPECT_EQ( + 1, + samples->GetCount( + password_manager::metrics_util::AUTOMATIC_WITH_PASSWORD_PENDING)); + EXPECT_EQ(1, + samples->GetCount( + password_manager::metrics_util::MANUAL_WITH_PASSWORD_PENDING)); + EXPECT_EQ(0, + samples->GetCount( + password_manager::metrics_util::MANUAL_MANAGE_PASSWORDS)); +} diff --git a/chrome/browser/ui/views/passwords/manage_passwords_icon_view.cc b/chrome/browser/ui/views/passwords/manage_passwords_icon_view.cc index 23a5950..73368045 100644 --- a/chrome/browser/ui/views/passwords/manage_passwords_icon_view.cc +++ b/chrome/browser/ui/views/passwords/manage_passwords_icon_view.cc @@ -4,6 +4,8 @@ #include "chrome/browser/ui/views/passwords/manage_passwords_icon_view.h" +#include "chrome/app/chrome_command_ids.h" +#include "chrome/browser/command_updater.h" #include "chrome/browser/ui/passwords/manage_passwords_bubble_ui_controller.h" #include "chrome/browser/ui/view_ids.h" #include "chrome/browser/ui/views/passwords/manage_passwords_bubble_view.h" @@ -13,38 +15,45 @@ #include "ui/base/resource/resource_bundle.h" ManagePasswordsIconView::ManagePasswordsIconView( - LocationBarView::Delegate* location_bar_delegate) - : location_bar_delegate_(location_bar_delegate) { + LocationBarView::Delegate* location_bar_delegate, + CommandUpdater* command_updater) + : BubbleIconView(command_updater, IDC_MANAGE_PASSWORDS_FOR_PAGE), + location_bar_delegate_(location_bar_delegate), + command_updater_(command_updater), + icon_id_(0), + tooltip_text_id_(0) { set_id(VIEW_ID_MANAGE_PASSWORDS_ICON_BUTTON); SetAccessibilityFocusable(true); - SetState(ManagePasswordsIcon::INACTIVE_STATE); + UpdateVisibleUI(); } ManagePasswordsIconView::~ManagePasswordsIconView() {} -void ManagePasswordsIconView::SetStateInternal( - ManagePasswordsIcon::State state) { - if (state == ManagePasswordsIcon::INACTIVE_STATE) { +void ManagePasswordsIconView::UpdateVisibleUI() { + // If the icon is inactive: clear out it's image and tooltip, hide the icon, + // close any active bubble, and exit early. + if (state() == ManagePasswordsIcon::INACTIVE_STATE) { + icon_id_ = 0; + tooltip_text_id_ = 0; + SetVisible(false); if (ManagePasswordsBubbleView::IsShowing()) ManagePasswordsBubbleView::CloseBubble(); return; } - // Start with the correct values for ManagePasswordsIcon::MANAGE_STATE, and - // adjust things accordingly if we're either in BLACKLISTED_STATE or - // PENDING_STATE. - int which_icon = IDR_SAVE_PASSWORD; - int which_text = IDS_PASSWORD_MANAGER_TOOLTIP_MANAGE; - if (state == ManagePasswordsIcon::BLACKLISTED_STATE) - which_icon = IDR_SAVE_PASSWORD_BLACKLISTED; - else if (state == ManagePasswordsIcon::PENDING_STATE) - which_text = IDS_PASSWORD_MANAGER_TOOLTIP_SAVE; + // Otherwise, start with the correct values for MANAGE_STATE, and adjust + // things accordingly if we're either in BLACKLISTED_STATE or PENDING_STATE. + icon_id_ = IDR_SAVE_PASSWORD; + tooltip_text_id_ = IDS_PASSWORD_MANAGER_TOOLTIP_MANAGE; + if (state() == ManagePasswordsIcon::BLACKLISTED_STATE) + icon_id_ = IDR_SAVE_PASSWORD_BLACKLISTED; + else if (state() == ManagePasswordsIcon::PENDING_STATE) + tooltip_text_id_ = IDS_PASSWORD_MANAGER_TOOLTIP_SAVE; SetVisible(true); - SetImage( - ui::ResourceBundle::GetSharedInstance().GetImageSkiaNamed(which_icon)); - SetTooltipText(l10n_util::GetStringUTF16(which_text)); + SetImage(ui::ResourceBundle::GetSharedInstance().GetImageSkiaNamed(icon_id_)); + SetTooltipText(l10n_util::GetStringUTF16(tooltip_text_id_)); } void ManagePasswordsIconView::ShowBubbleWithoutUserInteraction() { @@ -52,36 +61,13 @@ void ManagePasswordsIconView::ShowBubbleWithoutUserInteraction() { if (location_bar_delegate_->GetToolbarModel()->input_in_progress()) return; - ManagePasswordsBubbleView::ShowBubble( - location_bar_delegate_->GetWebContents(), - ManagePasswordsBubbleView::AUTOMATIC); -} - -bool ManagePasswordsIconView::GetTooltipText(const gfx::Point& p, - base::string16* tooltip) const { - // Don't show tooltip if the password bubble is displayed. - return !ManagePasswordsBubbleView::IsShowing() && - ImageView::GetTooltipText(p, tooltip); + command_updater_->ExecuteCommand(IDC_MANAGE_PASSWORDS_FOR_PAGE); } -void ManagePasswordsIconView::OnGestureEvent(ui::GestureEvent* event) { - if (event->type() == ui::ET_GESTURE_TAP) { - ManagePasswordsBubbleView::ShowBubble( - location_bar_delegate_->GetWebContents(), - ManagePasswordsBubbleView::USER_ACTION); - event->SetHandled(); - } +bool ManagePasswordsIconView::IsBubbleShowing() const { + return ManagePasswordsBubbleView::IsShowing(); } -bool ManagePasswordsIconView::OnMousePressed(const ui::MouseEvent& event) { - // Do nothing until the mouse button is released. - return true; -} - -void ManagePasswordsIconView::OnMouseReleased(const ui::MouseEvent& event) { - if (event.IsOnlyLeftMouseButton() && HitTestPoint(event.location())) { - ManagePasswordsBubbleView::ShowBubble( - location_bar_delegate_->GetWebContents(), - ManagePasswordsBubbleView::USER_ACTION); - } +void ManagePasswordsIconView::OnExecuting( + BubbleIconView::ExecuteSource source) { } diff --git a/chrome/browser/ui/views/passwords/manage_passwords_icon_view.h b/chrome/browser/ui/views/passwords/manage_passwords_icon_view.h index 7dd791c..efcef5f 100644 --- a/chrome/browser/ui/views/passwords/manage_passwords_icon_view.h +++ b/chrome/browser/ui/views/passwords/manage_passwords_icon_view.h @@ -7,42 +7,60 @@ #include "chrome/browser/ui/passwords/manage_passwords_bubble_model.h" #include "chrome/browser/ui/passwords/manage_passwords_icon.h" +#include "chrome/browser/ui/views/location_bar/bubble_icon_view.h" #include "chrome/browser/ui/views/location_bar/location_bar_view.h" #include "ui/views/controls/image_view.h" +class CommandUpdater; class ManagePasswordsBubbleUIController; // View for the password icon in the Omnibox. class ManagePasswordsIconView : public ManagePasswordsIcon, - public views::ImageView { + public BubbleIconView { public: // Clicking on the ManagePasswordsIconView shows a ManagePasswordsBubbleView, // which requires the current WebContents. Because the current WebContents // changes as the user switches tabs, it cannot be provided in the // constructor. Instead, a LocationBarView::Delegate is passed here so that it // can be queried for the current WebContents as needed. - explicit ManagePasswordsIconView( - LocationBarView::Delegate* location_bar_delegate); + ManagePasswordsIconView(LocationBarView::Delegate* location_bar_delegate, + CommandUpdater* command_updater); virtual ~ManagePasswordsIconView(); // ManagePasswordsIcon: + // + // TODO(mkwst): Remove this once we get rid of the single call to + // ShowBubbleWithoutUserInteraction in ManagePasswordsBubbleUIController. virtual void ShowBubbleWithoutUserInteraction() OVERRIDE; + // BubbleIconView: + virtual bool IsBubbleShowing() const OVERRIDE; + virtual void OnExecuting(BubbleIconView::ExecuteSource source) OVERRIDE; + +#if defined(UNIT_TEST) + int icon_id() const { return icon_id_; } + int tooltip_text_id() const { return tooltip_text_id_; } +#endif + protected: // ManagePasswordsIcon: - virtual void SetStateInternal(ManagePasswordsIcon::State state) OVERRIDE; + virtual void UpdateVisibleUI() OVERRIDE; private: - // views::ImageView: - virtual bool GetTooltipText(const gfx::Point& p, - base::string16* tooltip) const OVERRIDE; - virtual void OnGestureEvent(ui::GestureEvent* event) OVERRIDE; - virtual bool OnMousePressed(const ui::MouseEvent& event) OVERRIDE; - virtual void OnMouseReleased(const ui::MouseEvent& event) OVERRIDE; - // The delegate used to get the currently visible WebContents. LocationBarView::Delegate* location_bar_delegate_; + // The updater used to deliver commands to the browser; we'll use this to + // pop open the bubble when necessary. + // + // TODO(mkwst): Remove this once we get rid of the single call to + // ShowBubbleWithoutUserInteraction in ManagePasswordsBubbleUIController. + CommandUpdater* command_updater_; + + // The ID of the icon and text resources that are currently displayed. + int icon_id_; + int tooltip_text_id_; + DISALLOW_COPY_AND_ASSIGN(ManagePasswordsIconView); }; diff --git a/chrome/browser/ui/views/passwords/manage_passwords_icon_view_browsertest.cc b/chrome/browser/ui/views/passwords/manage_passwords_icon_view_browsertest.cc new file mode 100644 index 0000000..e3c8ca6 --- /dev/null +++ b/chrome/browser/ui/views/passwords/manage_passwords_icon_view_browsertest.cc @@ -0,0 +1,45 @@ +// Copyright 2014 The Chromium Authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +#include "chrome/browser/ui/views/passwords/manage_passwords_icon_view.h" + +#include "chrome/browser/ui/passwords/manage_passwords_bubble_ui_controller_mock.h" +#include "chrome/browser/ui/passwords/manage_passwords_icon.h" +#include "chrome/browser/ui/views/passwords/manage_passwords_view_test.h" +#include "grit/generated_resources.h" +#include "grit/theme_resources.h" +#include "testing/gtest/include/gtest/gtest.h" + +typedef ManagePasswordsViewTest ManagePasswordsIconViewTest; + +IN_PROC_BROWSER_TEST_F(ManagePasswordsIconViewTest, DefaultStateIsInactive) { + EXPECT_EQ(ManagePasswordsIcon::INACTIVE_STATE, view()->state()); + EXPECT_FALSE(view()->visible()); + EXPECT_EQ(0, view()->icon_id()); + EXPECT_EQ(0, view()->tooltip_text_id()); +} + +IN_PROC_BROWSER_TEST_F(ManagePasswordsIconViewTest, PendingState) { + SetupPendingPassword(); + EXPECT_EQ(ManagePasswordsIcon::PENDING_STATE, view()->state()); + EXPECT_TRUE(view()->visible()); + EXPECT_EQ(IDR_SAVE_PASSWORD, view()->icon_id()); + EXPECT_EQ(IDS_PASSWORD_MANAGER_TOOLTIP_SAVE, view()->tooltip_text_id()); +} + +IN_PROC_BROWSER_TEST_F(ManagePasswordsIconViewTest, ManageState) { + SetupManagingPasswords(); + EXPECT_EQ(ManagePasswordsIcon::MANAGE_STATE, view()->state()); + EXPECT_TRUE(view()->visible()); + EXPECT_EQ(IDR_SAVE_PASSWORD, view()->icon_id()); + EXPECT_EQ(IDS_PASSWORD_MANAGER_TOOLTIP_MANAGE, view()->tooltip_text_id()); +} + +IN_PROC_BROWSER_TEST_F(ManagePasswordsIconViewTest, BlacklistedState) { + SetupBlackistedPassword(); + EXPECT_EQ(ManagePasswordsIcon::BLACKLISTED_STATE, view()->state()); + EXPECT_TRUE(view()->visible()); + EXPECT_EQ(IDR_SAVE_PASSWORD_BLACKLISTED, view()->icon_id()); + EXPECT_EQ(IDS_PASSWORD_MANAGER_TOOLTIP_MANAGE, view()->tooltip_text_id()); +} diff --git a/chrome/browser/ui/views/passwords/manage_passwords_view_test.cc b/chrome/browser/ui/views/passwords/manage_passwords_view_test.cc new file mode 100644 index 0000000..4cf4614 --- /dev/null +++ b/chrome/browser/ui/views/passwords/manage_passwords_view_test.cc @@ -0,0 +1,92 @@ +// Copyright 2014 The Chromium Authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +#include "chrome/browser/ui/views/passwords/manage_passwords_view_test.h" + +#include "base/test/statistics_delta_reader.h" +#include "chrome/app/chrome_command_ids.h" +#include "chrome/browser/ui/browser.h" +#include "chrome/browser/ui/browser_command_controller.h" +#include "chrome/browser/ui/passwords/manage_passwords_bubble_ui_controller_mock.h" +#include "chrome/browser/ui/passwords/manage_passwords_icon.h" +#include "chrome/browser/ui/tabs/tab_strip_model.h" +#include "chrome/browser/ui/views/frame/browser_view.h" +#include "chrome/browser/ui/views/passwords/manage_passwords_bubble_view.h" +#include "chrome/browser/ui/views/passwords/manage_passwords_icon_view.h" +#include "chrome/browser/ui/views/toolbar/toolbar_view.h" +#include "chrome/test/base/in_process_browser_test.h" +#include "chrome/test/base/interactive_test_utils.h" +#include "components/autofill/core/common/password_form.h" +#include "components/password_manager/core/browser/mock_password_manager_driver.h" +#include "components/password_manager/core/browser/password_form_manager.h" +#include "components/password_manager/core/browser/password_manager_driver.h" +#include "components/password_manager/core/browser/password_manager_metrics_util.h" +#include "components/password_manager/core/browser/stub_password_manager_client.h" +#include "testing/gtest/include/gtest/gtest.h" + +void ManagePasswordsViewTest::SetUpOnMainThread() { + // Create the test UIController here so that it's bound to the currently + // active WebContents. + new ManagePasswordsBubbleUIControllerMock( + browser()->tab_strip_model()->GetActiveWebContents()); +} + +ManagePasswordsBubbleUIControllerMock* ManagePasswordsViewTest::controller() { + return static_cast<ManagePasswordsBubbleUIControllerMock*>( + ManagePasswordsBubbleUIController::FromWebContents( + browser()->tab_strip_model()->GetActiveWebContents())); +} + +ManagePasswordsIconView* ManagePasswordsViewTest::view() { + BrowserView* browser_view = static_cast<BrowserView*>(browser()->window()); + return browser_view->GetToolbarView()->location_bar()-> + manage_passwords_icon_view(); +} + +void ManagePasswordsViewTest::ExecuteManagePasswordsCommand() { + // Show the window to ensure that it's active. + browser()->window()->Show(); + + CommandUpdater* updater = browser()->command_controller()->command_updater(); + EXPECT_TRUE(updater->IsCommandEnabled(IDC_MANAGE_PASSWORDS_FOR_PAGE)); + EXPECT_TRUE(updater->ExecuteCommand(IDC_MANAGE_PASSWORDS_FOR_PAGE)); + + // Wait for the command execution to pop up the bubble. + content::RunAllPendingInMessageLoop(); +} + +void ManagePasswordsViewTest::SetupManagingPasswords() { + base::string16 kTestUsername = base::ASCIIToUTF16("test_username"); + autofill::PasswordFormMap map; + map[kTestUsername] = test_form(); + controller()->OnPasswordAutofilled(map); + controller()->UpdateIconAndBubbleState(view()); +} + +void ManagePasswordsViewTest::SetupPendingPassword() { + password_manager::StubPasswordManagerClient client; + password_manager::MockPasswordManagerDriver driver; + scoped_ptr<password_manager::PasswordFormManager> test_form_manager( + new password_manager::PasswordFormManager( + NULL, &client, &driver, *test_form(), false)); + controller()->OnPasswordSubmitted(test_form_manager.release()); + + // Wait for the command execution triggered by the automatic popup to pop up + // the bubble. + content::RunAllPendingInMessageLoop(); + controller()->UpdateIconAndBubbleState(view()); +} + +void ManagePasswordsViewTest::SetupBlackistedPassword() { + controller()->OnBlacklistBlockedAutofill(); + controller()->UpdateIconAndBubbleState(view()); +} + +base::HistogramSamples* ManagePasswordsViewTest::GetSamples( + const char* histogram) { + // Ensure that everything has been properly recorded before pulling samples. + content::RunAllPendingInMessageLoop(); + return statistics_reader_.GetHistogramSamplesSinceCreation( + histogram).release(); +} diff --git a/chrome/browser/ui/views/passwords/manage_passwords_view_test.h b/chrome/browser/ui/views/passwords/manage_passwords_view_test.h new file mode 100644 index 0000000..354a192 --- /dev/null +++ b/chrome/browser/ui/views/passwords/manage_passwords_view_test.h @@ -0,0 +1,57 @@ +// Copyright 2014 The Chromium Authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +#ifndef CHROME_BROWSER_UI_VIEWS_PASSWORDS_MANAGE_PASSWORDS_VIEW_TEST_H_ +#define CHROME_BROWSER_UI_VIEWS_PASSWORDS_MANAGE_PASSWORDS_VIEW_TEST_H_ + +#include "base/metrics/histogram_samples.h" +#include "base/test/statistics_delta_reader.h" +#include "chrome/test/base/in_process_browser_test.h" +#include "components/autofill/core/common/password_form.h" +#include "testing/gtest/include/gtest/gtest.h" + +class ManagePasswordsBubbleUIControllerMock; +class ManagePasswordsIconView; + +// Test class for the various password management view bits and pieces. Sets +// up a ManagePasswordsBubbleUIControllerMock, and provides some helper methods +// to poke at the bubble, icon, and controller's state. +class ManagePasswordsViewTest : public InProcessBrowserTest { + public: + ManagePasswordsViewTest() {} + + // InProcessBrowserTest: + virtual void SetUpOnMainThread() OVERRIDE; + + // Get the mock UI controller for the current WebContents. + ManagePasswordsBubbleUIControllerMock* controller(); + + // Get the icon view for the current WebContents. + ManagePasswordsIconView* view(); + + // Execute the browser command to open the manage passwords bubble. + void ExecuteManagePasswordsCommand(); + + // Put the controller, icon, and bubble into a managing-password state. + void SetupManagingPasswords(); + + // Put the controller, icon, and bubble into a pending-password state. + void SetupPendingPassword(); + + // Put the controller, icon, and bubble into a blacklisted state. + void SetupBlackistedPassword(); + + // Get samples for |histogram|. + base::HistogramSamples* GetSamples(const char* histogram); + + autofill::PasswordForm* test_form() { return &test_form_; } + + private: + autofill::PasswordForm test_form_; + base::StatisticsDeltaReader statistics_reader_; + + DISALLOW_COPY_AND_ASSIGN(ManagePasswordsViewTest); +}; + +#endif // CHROME_BROWSER_UI_VIEWS_PASSWORDS_MANAGE_PASSWORDS_VIEW_TEST_H_ diff --git a/chrome/browser/ui/views/passwords/save_password_refusal_combobox_model.cc b/chrome/browser/ui/views/passwords/save_password_refusal_combobox_model.cc new file mode 100644 index 0000000..2ec5998 --- /dev/null +++ b/chrome/browser/ui/views/passwords/save_password_refusal_combobox_model.cc @@ -0,0 +1,33 @@ +// Copyright 2014 The Chromium Authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +#include "chrome/browser/ui/views/passwords/save_password_refusal_combobox_model.h" + +#include "grit/generated_resources.h" +#include "ui/base/l10n/l10n_util.h" + +SavePasswordRefusalComboboxModel::SavePasswordRefusalComboboxModel() { + items_.push_back( + l10n_util::GetStringUTF16(IDS_PASSWORD_MANAGER_CANCEL_BUTTON)); + items_.push_back( + l10n_util::GetStringUTF16(IDS_PASSWORD_MANAGER_BLACKLIST_BUTTON)); +} + +SavePasswordRefusalComboboxModel::~SavePasswordRefusalComboboxModel() {} + +int SavePasswordRefusalComboboxModel::GetItemCount() const { + return items_.size(); +} + +base::string16 SavePasswordRefusalComboboxModel::GetItemAt(int index) { + return items_[index]; +} + +bool SavePasswordRefusalComboboxModel::IsItemSeparatorAt(int index) { + return items_[index].empty(); +} + +int SavePasswordRefusalComboboxModel::GetDefaultIndex() const { + return 0; +} diff --git a/chrome/browser/ui/views/passwords/save_password_refusal_combobox_model.h b/chrome/browser/ui/views/passwords/save_password_refusal_combobox_model.h new file mode 100644 index 0000000..e89f9db --- /dev/null +++ b/chrome/browser/ui/views/passwords/save_password_refusal_combobox_model.h @@ -0,0 +1,33 @@ +// Copyright 2014 The Chromium Authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +#ifndef CHROME_BROWSER_UI_VIEWS_PASSWORDS_SAVE_PASSWORD_REFUSAL_COMBOBOX_MODEL_H_ +#define CHROME_BROWSER_UI_VIEWS_PASSWORDS_SAVE_PASSWORD_REFUSAL_COMBOBOX_MODEL_H_ + +#include <vector> +#include "base/basictypes.h" +#include "base/strings/string16.h" +#include "ui/base/models/combobox_model.h" + +class SavePasswordRefusalComboboxModel : public ui::ComboboxModel { + public: + enum { INDEX_NOPE = 0, INDEX_NEVER_FOR_THIS_SITE = 1 }; + + SavePasswordRefusalComboboxModel(); + virtual ~SavePasswordRefusalComboboxModel(); + + private: + // Overridden from ui::ComboboxModel: + virtual int GetItemCount() const OVERRIDE; + virtual base::string16 GetItemAt(int index) OVERRIDE; + virtual bool IsItemSeparatorAt(int index) OVERRIDE; + virtual int GetDefaultIndex() const OVERRIDE; + + std::vector<base::string16> items_; + + DISALLOW_COPY_AND_ASSIGN(SavePasswordRefusalComboboxModel); +}; + + +#endif // CHROME_BROWSER_UI_VIEWS_PASSWORDS_SAVE_PASSWORD_REFUSAL_COMBOBOX_MODEL_H_ diff --git a/chrome/chrome_browser_ui.gypi b/chrome/chrome_browser_ui.gypi index 5aa3781..8fa7401 100644 --- a/chrome/chrome_browser_ui.gypi +++ b/chrome/chrome_browser_ui.gypi @@ -1824,12 +1824,14 @@ 'browser/ui/views/panels/panel_view.h', 'browser/ui/views/panels/taskbar_window_thumbnailer_win.cc', 'browser/ui/views/panels/taskbar_window_thumbnailer_win.h', + 'browser/ui/views/passwords/manage_password_item_view.cc', + 'browser/ui/views/passwords/manage_password_item_view.h', 'browser/ui/views/passwords/manage_passwords_bubble_view.cc', 'browser/ui/views/passwords/manage_passwords_bubble_view.h', 'browser/ui/views/passwords/manage_passwords_icon_view.cc', 'browser/ui/views/passwords/manage_passwords_icon_view.h', - 'browser/ui/views/passwords/manage_password_item_view.cc', - 'browser/ui/views/passwords/manage_password_item_view.h', + 'browser/ui/views/passwords/save_password_refusal_combobox_model.cc', + 'browser/ui/views/passwords/save_password_refusal_combobox_model.h', 'browser/ui/views/password_generation_bubble_view.cc', 'browser/ui/views/password_generation_bubble_view.h', 'browser/ui/views/pdf_password_dialog.cc', diff --git a/chrome/chrome_tests.gypi b/chrome/chrome_tests.gypi index d6e23b0..cd3f4be 100644 --- a/chrome/chrome_tests.gypi +++ b/chrome/chrome_tests.gypi @@ -128,6 +128,10 @@ 'browser/ui/views/message_center/web_notification_tray_browsertest.cc', 'browser/ui/views/omnibox/omnibox_view_views_browsertest.cc', 'browser/ui/views/panels/panel_view_browsertest.cc', + 'browser/ui/views/passwords/manage_passwords_view_test.cc', + 'browser/ui/views/passwords/manage_passwords_view_test.h', + 'browser/ui/views/passwords/manage_passwords_bubble_view_browsertest.cc', + 'browser/ui/views/passwords/manage_passwords_icon_view_browsertest.cc', 'browser/ui/views/ssl_client_certificate_selector_browsertest.cc', 'browser/ui/views/status_icons/status_tray_state_changer_interactive_uitest_win.cc', 'browser/ui/views/tabs/tab_drag_controller_interactive_uitest.cc', diff --git a/chrome/chrome_tests_unit.gypi b/chrome/chrome_tests_unit.gypi index 0dd87b7..8b98fa2 100644 --- a/chrome/chrome_tests_unit.gypi +++ b/chrome/chrome_tests_unit.gypi @@ -201,6 +201,8 @@ 'browser/ui/fullscreen/fullscreen_controller_state_tests.h', 'browser/ui/fullscreen/fullscreen_controller_test.cc', 'browser/ui/fullscreen/fullscreen_controller_test.h', + 'browser/ui/passwords/manage_passwords_bubble_ui_controller_mock.cc', + 'browser/ui/passwords/manage_passwords_bubble_ui_controller_mock.h', 'browser/ui/test/test_confirm_bubble_model.cc', 'browser/ui/test/test_confirm_bubble_model.h', 'browser/ui/views/find_bar_host_unittest_util_views.cc', @@ -1680,7 +1682,6 @@ 'browser/ui/panels/panel_mouse_watcher_unittest.cc', 'browser/ui/passwords/manage_passwords_icon_mock.cc', 'browser/ui/passwords/manage_passwords_bubble_model_unittest.cc', - 'browser/ui/passwords/manage_passwords_bubble_ui_controller_mock.cc', 'browser/ui/passwords/manage_passwords_bubble_ui_controller_unittest.cc', 'browser/ui/passwords/password_manager_presenter_unittest.cc', 'browser/ui/search_engines/keyword_editor_controller_unittest.cc', |