This is relanding of https://codereview.chromium.org/22975006/ which was...

This is relanding of https://codereview.chromium.org/22975006/ which was reverted in https://codereview.chromium.org/24024006.

Save password functionality added to the save password bubble behind flag. The buttons now let the user save and blacklist passwords.

BUG=261628
TBR=pkasting@chromium.org

Review URL: https://chromiumcodereview.appspot.com/23980003

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@222034 0039d316-1c4b-4281-b951-d872f2087c98

chrome/browser/content_settings/tab_specific_content_settings.cc

@@ -99,6 +99,18 @@ TabSpecificContentSettings::~TabSpecificContentSettings() {
       SiteDataObserver, observer_list_, ContentSettingsDestroyed());
 }
 
+bool TabSpecificContentSettings::PasswordAccepted() {
+  DCHECK(form_to_save_.get());
+  form_to_save_->SavePassword();
+  return true;
+}
+
+bool TabSpecificContentSettings::PasswordFormBlacklisted() {
+  DCHECK(form_to_save_.get());
+  form_to_save_->BlacklistPassword();
+  return true;
+}
+
 TabSpecificContentSettings* TabSpecificContentSettings::Get(
     int render_process_id, int render_view_id) {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
@@ -463,9 +475,9 @@ void TabSpecificContentSettings::OnGeolocationPermissionSet(
       content::NotificationService::NoDetails());
 }
 
-// TODO(npentrel): Save the password when user accepts the prompt
 void TabSpecificContentSettings::OnPasswordSubmitted(
       PasswordFormManager* form_to_save) {
+  form_to_save_.reset(form_to_save);
   OnContentAllowed(CONTENT_SETTINGS_TYPE_SAVE_PASSWORD);
   NotifySiteDataObservers();
 }
@@ -647,6 +659,8 @@ bool TabSpecificContentSettings::OnMessageReceived(
 void TabSpecificContentSettings::DidNavigateMainFrame(
     const content::LoadCommittedDetails& details,
     const content::FrameNavigateParams& params) {
+  if (form_to_save_)
+    form_to_save_->ApplyChange();
   if (!details.is_in_page) {
     // Clear "blocked" flags.
     ClearBlockedContentSettingsExceptForCookies();

chrome/browser/content_settings/tab_specific_content_settings.h

@@ -222,8 +222,6 @@ class TabSpecificContentSettings
     return midi_usages_state_;
   }
 
-  void OnPasswordSubmitted(PasswordFormManager* form_to_save);
-
   // Call to indicate that there is a protocol handler pending user approval.
   void set_pending_protocol_handler(const ProtocolHandler& handler) {
     pending_protocol_handler_ = handler;
@@ -297,6 +295,14 @@ class TabSpecificContentSettings
   virtual void AppCacheAccessed(const GURL& manifest_url,
                                 bool blocked_by_policy) OVERRIDE;
 
+  // If user clicks on 'save password' this will have the password saved upon
+  // the next navigation.
+  bool PasswordAccepted();
+
+  // If user clicks on 'never save password for this site' this have the
+  // password blacklisted upon the next navigation.
+  bool PasswordFormBlacklisted();
+
   // Message handlers. Public for testing.
   void OnContentBlocked(ContentSettingsType type,
                         const std::string& resource_identifier);
@@ -336,6 +342,11 @@ class TabSpecificContentSettings
       const MediaStreamDevicesController::MediaStreamTypePermissionMap&
           request_permissions);
 
+  // This method is called to pass the |form_to_save| on a successful password
+  // submission. It also updates the status of the save password content
+  // setting.
+  void OnPasswordSubmitted(PasswordFormManager* form_to_save);
+
   // There methods are called to update the status about MIDI access.
   void OnMIDISysExAccessed(const GURL& reqesting_origin);
   void OnMIDISysExAccessBlocked(const GURL& requesting_origin);
@@ -417,6 +428,10 @@ class TabSpecificContentSettings
   // stored here. http://crbug.com/259794
   GURL media_stream_access_origin_;
 
+  // The PasswordFormManager managing the form we're asking the user about,
+  // and should update as per the decision.
+  scoped_ptr<PasswordFormManager> form_to_save_;
+
   DISALLOW_COPY_AND_ASSIGN(TabSpecificContentSettings);
 };
 

chrome/browser/password_manager/password_form_manager.cc

@@ -39,7 +39,9 @@ PasswordFormManager::PasswordFormManager(Profile* profile,
       web_contents_(web_contents),
       manager_action_(kManagerActionNone),
       user_action_(kUserActionNone),
-      submit_result_(kSubmitResultNotSubmitted) {
+      submit_result_(kSubmitResultNotSubmitted),
+      should_save_password_(false),
+      should_blacklist_password_(false) {
   DCHECK(profile_);
   if (observed_form_.origin.is_valid())
     base::SplitString(observed_form_.origin.path(), '/', &form_path_tokens_);
@@ -50,6 +52,10 @@ PasswordFormManager::~PasswordFormManager() {
   UMA_HISTOGRAM_ENUMERATION("PasswordManager.ActionsTaken",
                             GetActionsTaken(),
                             kMaxNumActionsTaken);
+  // In case the tab is closed before the next navigation occurs this will
+  // apply outstanding changes.
+  if (should_save_password_ || should_blacklist_password_)
+    ApplyChange();
 }
 
 int PasswordFormManager::GetActionsTaken() {
@@ -106,6 +112,26 @@ bool PasswordFormManager::DoesManage(const PasswordForm& form,
   return true;
 }
 
+void PasswordFormManager::ApplyChange() {
+  DCHECK(!should_blacklist_password_ || !should_save_password_);
+  if (should_save_password_)
+    Save();
+  else if (should_blacklist_password_)
+    PermanentlyBlacklist();
+  should_blacklist_password_ = false;
+  should_save_password_ = false;
+}
+
+void PasswordFormManager::SavePassword() {
+  should_blacklist_password_ = false;
+  should_save_password_ = true;
+}
+
+void PasswordFormManager::BlacklistPassword() {
+  should_save_password_ = false;
+  should_blacklist_password_ = true;
+}
+
 bool PasswordFormManager::IsBlacklisted() {
   DCHECK_EQ(state_, POST_MATCHING_PHASE);
   if (preferred_match_ && preferred_match_->blacklisted_by_user)

chrome/browser/password_manager/password_form_manager.h

@@ -71,6 +71,18 @@ class PasswordFormManager : public PasswordStoreConsumer {
   // the same thread!
   bool HasCompletedMatching();
 
+  // Sets current password to be saved when ApplyEdits() is called. Will
+  // override a previous call to BlacklistPassword().
+  void SavePassword();
+
+  // Sets current password to be blacklisted when ApplyEdits() is called. Will
+  // override a previous call to SavePassword().
+  void BlacklistPassword();
+
+  // Persist changes from the latest call to either SavePassword() or
+  // BlacklistPassword().
+  void ApplyChange();
+
   // Determines if the user opted to 'never remember' passwords for this form.
   bool IsBlacklisted();
 
@@ -105,6 +117,8 @@ class PasswordFormManager : public PasswordStoreConsumer {
 
   // A user opted to 'never remember' passwords for this form.
   // Blacklist it so that from now on when it is seen we ignore it.
+
+  // TODO: Make this private once we switch to the new UI.
   void PermanentlyBlacklist();
 
   // If the user has submitted observed_form_, provisionally hold on to
@@ -120,6 +134,8 @@ class PasswordFormManager : public PasswordStoreConsumer {
   // Handles save-as-new or update of the form managed by this manager.
   // Note the basic data of updated_credentials must match that of
   // observed_form_ (e.g DoesManage(pending_credentials_) == true).
+
+  // TODO: Make this private once we switch to the new UI.
   void Save();
 
   // Call these if/when we know the form submission worked or failed.
@@ -291,6 +307,8 @@ class PasswordFormManager : public PasswordStoreConsumer {
   ManagerAction manager_action_;
   UserAction user_action_;
   SubmitResult submit_result_;
+  bool should_save_password_;
+  bool should_blacklist_password_;
 
   DISALLOW_COPY_AND_ASSIGN(PasswordFormManager);
 };

chrome/browser/ui/browser_content_setting_bubble_model_delegate.cc

@@ -45,5 +45,10 @@ void BrowserContentSettingBubbleModelDelegate::ShowContentSettingsPage(
     return;
   }
 
+  if (type == CONTENT_SETTINGS_TYPE_SAVE_PASSWORD) {
+    chrome::ShowSettingsSubPage(browser_, chrome::kPasswordManagerSubPage);
+    return;
+  }
+
   chrome::ShowContentSettings(browser_, type);
 }

chrome/browser/ui/content_settings/content_setting_bubble_model.cc

@@ -10,7 +10,6 @@
 #include "chrome/browser/chrome_notification_types.h"
 #include "chrome/browser/content_settings/content_settings_utils.h"
 #include "chrome/browser/content_settings/cookie_settings.h"
-#include "chrome/browser/content_settings/tab_specific_content_settings.h"
 #include "chrome/browser/custom_handlers/protocol_handler_registry.h"
 #include "chrome/browser/custom_handlers/protocol_handler_registry_factory.h"
 #include "chrome/browser/favicon/favicon_tab_helper.h"
@@ -164,6 +163,8 @@ void ContentSettingTitleAndLinkModel::SetManageLink() {
     {CONTENT_SETTINGS_TYPE_PPAPI_BROKER, IDS_PPAPI_BROKER_BUBBLE_MANAGE_LINK},
     {CONTENT_SETTINGS_TYPE_AUTOMATIC_DOWNLOADS, IDS_BLOCKED_DOWNLOADS_LINK},
     {CONTENT_SETTINGS_TYPE_MIDI_SYSEX, IDS_MIDI_SYSEX_BUBBLE_MANAGE_LINK},
+    {CONTENT_SETTINGS_TYPE_SAVE_PASSWORD,
+     IDS_OPTIONS_PASSWORDS_MANAGE_PASSWORDS},
   };
   set_manage_link(l10n_util::GetStringUTF8(
       GetIdForContentType(kLinkIDs, arraysize(kLinkIDs), content_type())));
@@ -573,24 +574,11 @@ void ContentSettingPopupBubbleModel::OnPopupClicked(int index) {
 }
 
 // The model for the save password bubble.
-class SavePasswordBubbleModel : public ContentSettingBubbleModel {
- public:
-  SavePasswordBubbleModel(WebContents* web_contents, Profile* profile);
-  virtual ~SavePasswordBubbleModel() {}
-
- private:
-  // Sets the title of the bubble.
-  void SetTitle();
-
-  TabSpecificContentSettings::PasswordSavingState state_;
-
-  DISALLOW_COPY_AND_ASSIGN(SavePasswordBubbleModel);
-};
-
-SavePasswordBubbleModel::SavePasswordBubbleModel(WebContents* web_contents,
+SavePasswordBubbleModel::SavePasswordBubbleModel(Delegate* delegate,
+                                                 WebContents* web_contents,
                                                  Profile* profile)
-    : ContentSettingBubbleModel(web_contents, profile,
-                                CONTENT_SETTINGS_TYPE_SAVE_PASSWORD),
+    : ContentSettingTitleAndLinkModel(delegate, web_contents, profile,
+                                      CONTENT_SETTINGS_TYPE_SAVE_PASSWORD),
       state_(TabSpecificContentSettings::NO_PASSWORD_TO_BE_SAVED) {
   DCHECK(profile);
   TabSpecificContentSettings* content_settings =
@@ -610,6 +598,18 @@ void SavePasswordBubbleModel::SetTitle() {
   set_title(l10n_util::GetStringUTF8(title_id));
 }
 
+void SavePasswordBubbleModel::OnCancelClicked() {
+  TabSpecificContentSettings* content_settings =
+      TabSpecificContentSettings::FromWebContents(web_contents());
+  content_settings->PasswordFormBlacklisted();
+}
+
+void SavePasswordBubbleModel::OnSaveClicked() {
+  TabSpecificContentSettings* content_settings =
+      TabSpecificContentSettings::FromWebContents(web_contents());
+  content_settings->PasswordAccepted();
+}
+
 // The model of the content settings bubble for media settings.
 class ContentSettingMediaStreamBubbleModel
     : public ContentSettingTitleAndLinkModel {
@@ -1277,7 +1277,7 @@ ContentSettingBubbleModel*
         Profile* profile,
         ContentSettingsType content_type) {
   if (content_type == CONTENT_SETTINGS_TYPE_SAVE_PASSWORD) {
-    return new SavePasswordBubbleModel(web_contents, profile);
+    return new SavePasswordBubbleModel(delegate, web_contents, profile);
   }
   if (content_type == CONTENT_SETTINGS_TYPE_COOKIES) {
     return new ContentSettingCookiesBubbleModel(delegate, web_contents, profile,

chrome/browser/ui/content_settings/content_setting_bubble_model.h

@@ -11,6 +11,7 @@
 #include <vector>
 
 #include "base/compiler_specific.h"
+#include "chrome/browser/content_settings/tab_specific_content_settings.h"
 #include "chrome/common/content_settings.h"
 #include "chrome/common/custom_handlers/protocol_handler.h"
 #include "content/public/browser/notification_observer.h"
@@ -112,10 +113,16 @@ class ContentSettingBubbleModel : public content::NotificationObserver {
   virtual void OnMediaMenuClicked(content::MediaStreamType type,
                                   const std::string& selected_device_id) {}
 
+  // Called by the view code when the cancel button in clicked by the user.
+  virtual void OnCancelClicked() {}
+
   // Called by the view code when the bubble is closed by the user using the
   // Done button.
   virtual void OnDoneClicked() {}
 
+  // Called by the view code when the save button in clicked by the user.
+  virtual void OnSaveClicked() {}
+
  protected:
   ContentSettingBubbleModel(
       content::WebContents* web_contents,
@@ -184,6 +191,23 @@ class ContentSettingTitleAndLinkModel : public ContentSettingBubbleModel {
   Delegate* delegate_;
 };
 
+class SavePasswordBubbleModel : public ContentSettingTitleAndLinkModel {
+ public:
+  SavePasswordBubbleModel(Delegate* delegate,
+                          content::WebContents* web_contents,
+                          Profile* profile);
+  virtual ~SavePasswordBubbleModel() {}
+  virtual void OnCancelClicked() OVERRIDE;
+  virtual void OnSaveClicked() OVERRIDE;
+ private:
+  // Sets the title of the bubble.
+  void SetTitle();
+
+  TabSpecificContentSettings::PasswordSavingState state_;
+
+  DISALLOW_COPY_AND_ASSIGN(SavePasswordBubbleModel);
+};
+
 class ContentSettingRPHBubbleModel : public ContentSettingTitleAndLinkModel {
  public:
   ContentSettingRPHBubbleModel(Delegate* delegate,

chrome/browser/ui/views/content_setting_bubble_contents.cc

@@ -148,7 +148,9 @@ ContentSettingBubbleContents::ContentSettingBubbleContents(
       web_contents_(web_contents),
       custom_link_(NULL),
       manage_link_(NULL),
-      close_button_(NULL) {
+      close_button_(NULL),
+      never_button_(NULL),
+      save_button_(NULL) {
   // Compensate for built-in vertical padding in the anchor view's image.
   set_anchor_view_insets(gfx::Insets(5, 0, 5, 0));
 
@@ -389,22 +391,57 @@ void ContentSettingBubbleContents::Init() {
     layout->AddPaddingRow(0, views::kRelatedControlVerticalSpacing);
   }
 
-  const int kDoubleColumnSetId = 1;
-  views::ColumnSet* double_column_set =
-      layout->AddColumnSet(kDoubleColumnSetId);
-  double_column_set->AddColumn(GridLayout::LEADING, GridLayout::CENTER, 1,
-                        GridLayout::USE_PREF, 0, 0);
-  double_column_set->AddPaddingColumn(
-      0, views::kUnrelatedControlHorizontalSpacing);
-  double_column_set->AddColumn(GridLayout::TRAILING, GridLayout::CENTER, 0,
-                        GridLayout::USE_PREF, 0, 0);
-
-  layout->StartRow(0, kDoubleColumnSetId);
-  manage_link_ = new views::Link(UTF8ToUTF16(bubble_content.manage_link));
-  manage_link_->set_listener(this);
-  layout->AddView(manage_link_);
-  if (content_setting_bubble_model_->content_type() !=
+  if (content_setting_bubble_model_->content_type() ==
       CONTENT_SETTINGS_TYPE_SAVE_PASSWORD) {
+    const int kDoubleColumnSetId = 2;
+    views::ColumnSet* double_column_set =
+        layout->AddColumnSet(kDoubleColumnSetId);
+    double_column_set->AddColumn(GridLayout::TRAILING, GridLayout::CENTER, 1,
+                                 GridLayout::USE_PREF, 0, 0);
+    double_column_set->AddPaddingColumn(
+        0, views::kRelatedControlSmallVerticalSpacing);
+    double_column_set->AddColumn(GridLayout::TRAILING, GridLayout::CENTER, 0,
+                                 GridLayout::USE_PREF, 0, 0);
+
+    const int kSingleColumnRightSetId = 1;
+    views::ColumnSet* right_column_set =
+        layout->AddColumnSet(kSingleColumnRightSetId);
+    right_column_set->AddColumn(GridLayout::TRAILING, GridLayout::FILL, 1,
+                                GridLayout::USE_PREF, 0, 0);
+
+    never_button_ = new views::LabelButton(
+        this, l10n_util::GetStringUTF16(IDS_PASSWORD_MANAGER_BLACKLIST_BUTTON));
+    never_button_->SetStyle(views::Button::STYLE_NATIVE_TEXTBUTTON);
+    save_button_ = new views::LabelButton(
+        this, l10n_util::GetStringUTF16(IDS_PASSWORD_MANAGER_SAVE_BUTTON));
+    save_button_->SetStyle(views::Button::STYLE_NATIVE_TEXTBUTTON);
+    manage_link_ = new views::Link(UTF8ToUTF16(bubble_content.manage_link));
+    manage_link_->set_listener(this);
+
+    // Buttons row
+    layout->StartRow(0, kDoubleColumnSetId);
+    layout->AddView(never_button_);
+    layout->AddView(save_button_);
+
+    // Manage link row
+    layout->StartRow(0, kSingleColumnRightSetId);
+    layout->AddView(manage_link_);
+  } else {
+    const int kDoubleColumnSetId = 1;
+    views::ColumnSet* double_column_set =
+        layout->AddColumnSet(kDoubleColumnSetId);
+    double_column_set->AddColumn(GridLayout::LEADING, GridLayout::CENTER, 1,
+                          GridLayout::USE_PREF, 0, 0);
+    double_column_set->AddPaddingColumn(
+        0, views::kUnrelatedControlHorizontalSpacing);
+    double_column_set->AddColumn(GridLayout::TRAILING, GridLayout::CENTER, 0,
+                          GridLayout::USE_PREF, 0, 0);
+
+    layout->StartRow(0, kDoubleColumnSetId);
+    manage_link_ = new views::Link(UTF8ToUTF16(bubble_content.manage_link));
+    manage_link_->set_listener(this);
+    layout->AddView(manage_link_);
+
     close_button_ =
         new views::LabelButton(this, l10n_util::GetStringUTF16(IDS_DONE));
     close_button_->SetStyle(views::Button::STYLE_NATIVE_TEXTBUTTON);
@@ -414,6 +451,16 @@ void ContentSettingBubbleContents::Init() {
 
 void ContentSettingBubbleContents::ButtonPressed(views::Button* sender,
                                                  const ui::Event& event) {
+  if (sender == save_button_) {
+    content_setting_bubble_model_->OnSaveClicked();
+    StartFade(false);
+    return;
+  }
+  if (sender == never_button_) {
+    content_setting_bubble_model_->OnCancelClicked();
+    StartFade(false);
+    return;
+  }
   if (sender == close_button_) {
     content_setting_bubble_model_->OnDoneClicked();
     StartFade(false);

chrome/browser/ui/views/content_setting_bubble_contents.h

@@ -113,6 +113,8 @@ class ContentSettingBubbleContents : public content::NotificationObserver,
   views::Link* custom_link_;
   views::Link* manage_link_;
   views::LabelButton* close_button_;
+  views::LabelButton* never_button_;
+  views::LabelButton* save_button_;
   scoped_ptr<views::MenuRunner> menu_runner_;
   MediaMenuPartsMap media_menus_;