Implement the password generation prompt for the editing state.

The user gets a feedback that the password is saved during editing.

TBR=rouslan@chromium.org

Bug: 851021
Change-Id: I2d254b4b366d67fee5283898b769362902008b15
Reviewed-on: https://chromium-review.googlesource.com/1110229
Commit-Queue: Vasilii Sukhanov <vasilii@chromium.org>
Reviewed-by: Dominic Battré <battre@chromium.org>
Cr-Commit-Position: refs/heads/master@{#569597}

chrome/app/generated_resources.grd

@@ -4344,6 +4344,9 @@ Keep your key file in a safe place. You will need it to create new versions of y
       </message>
       <message name="IDS_PASSWORD_GENERATION_SUGGESTION" desc="Text shown next to a generated password describing it as a suggestion.">
         Use suggested password
+      </message>
+       <message name="IDS_PASSWORD_GENERATION_EDITING_SUGGESTION" desc="Notification text next to the generated password assuring the user that the password has been saved.">
+        Saved password
       </message>
       <message name="IDS_SAVE_PASSWORD" desc="The title of the save password bubble when a password can be saved.">
         Do you want <ph name="PASSWORD_MANAGER_BRAND">$1<ex>Google Chrome</ex></ph> to save your password for this site?

chrome/browser/password_manager/chrome_password_manager_client.cc

@@ -661,7 +661,7 @@ ChromePasswordManagerClient::GetAutofillManagerForMainFrame() {
 }
 
 void ChromePasswordManagerClient::SetTestObserver(
-    autofill::PasswordGenerationPopupObserver* observer) {
+    PasswordGenerationPopupObserver* observer) {
   observer_ = observer;
 }
 
@@ -711,14 +711,16 @@ void ChromePasswordManagerClient::ShowPasswordEditingPopup(
       password_manager_client_bindings_.GetCurrentTargetFrame());
   DCHECK(driver);
   gfx::RectF element_bounds_in_screen_space = GetBoundsInScreenSpace(bounds);
-  popup_controller_ =
-      autofill::PasswordGenerationPopupControllerImpl::GetOrCreate(
-          popup_controller_, element_bounds_in_screen_space, form,
-          base::string16(),  // No generation_element needed for editing.
-          0,                 // Unspecified max length.
-          &password_manager_, driver->AsWeakPtr(), observer_, web_contents(),
-          web_contents()->GetNativeView());
-  popup_controller_->Show(false /* display_password */);
+  popup_controller_ = PasswordGenerationPopupControllerImpl::GetOrCreate(
+      popup_controller_, element_bounds_in_screen_space, form,
+      base::string16(),  // No generation_element needed for editing.
+      0,                 // Unspecified max length.
+      &password_manager_, driver->AsWeakPtr(), observer_, web_contents(),
+      web_contents()->GetNativeView());
+  DCHECK(!form.password_value.empty());
+  popup_controller_->UpdatePassword(form.password_value);
+  popup_controller_->Show(
+      PasswordGenerationPopupController::kEditGeneratedPassword);
 }
 
 void ChromePasswordManagerClient::GenerationAvailableForForm(
@@ -734,6 +736,9 @@ void ChromePasswordManagerClient::PasswordGenerationRejectedByTyping() {
 
 void ChromePasswordManagerClient::PresaveGeneratedPassword(
     const autofill::PasswordForm& password_form) {
+  if (popup_controller_)
+    popup_controller_->UpdatePassword(password_form.password_value);
+
   password_manager_.OnPresaveGeneratedPassword(password_form);
 }
 
@@ -881,13 +886,12 @@ void ChromePasswordManagerClient::ShowPasswordGenerationPopup(
   gfx::RectF element_bounds_in_screen_space =
       GetBoundsInScreenSpace(ui_data.bounds);
 
-  popup_controller_ =
-      autofill::PasswordGenerationPopupControllerImpl::GetOrCreate(
-          popup_controller_, element_bounds_in_screen_space,
-          ui_data.password_form, ui_data.generation_element, ui_data.max_length,
-          &password_manager_, driver->AsWeakPtr(), observer_, web_contents(),
-          web_contents()->GetNativeView());
-  popup_controller_->Show(true /* display_password */);
+  popup_controller_ = PasswordGenerationPopupControllerImpl::GetOrCreate(
+      popup_controller_, element_bounds_in_screen_space, ui_data.password_form,
+      ui_data.generation_element, ui_data.max_length, &password_manager_,
+      driver->AsWeakPtr(), observer_, web_contents(),
+      web_contents()->GetNativeView());
+  popup_controller_->Show(PasswordGenerationPopupController::kOfferGeneration);
 }
 
 password_manager::PasswordManager*

chrome/browser/password_manager/chrome_password_manager_client.h

@@ -29,16 +29,15 @@
 #include "content/public/browser/web_contents_user_data.h"
 #include "ui/gfx/geometry/rect.h"
 
-class Profile;
-
-namespace autofill {
 class PasswordGenerationPopupObserver;
 class PasswordGenerationPopupControllerImpl;
+class Profile;
 
+namespace autofill {
 namespace password_generation {
 struct PasswordGenerationUIData;
-}
-}
+}  // namespace password_generation
+}  // namespace autofill
 
 namespace content {
 class WebContents;
@@ -155,7 +154,7 @@ class ChromePasswordManagerClient
       autofill::AutofillClient* autofill_client);
 
   // Observer for PasswordGenerationPopup events. Used for testing.
-  void SetTestObserver(autofill::PasswordGenerationPopupObserver* observer);
+  void SetTestObserver(PasswordGenerationPopupObserver* observer);
 
   static void BindCredentialManager(
       password_manager::mojom::CredentialManagerRequest request,
@@ -234,11 +233,10 @@ class ChromePasswordManagerClient
       password_manager_client_bindings_;
 
   // Observer for password generation popup.
-  autofill::PasswordGenerationPopupObserver* observer_;
+  PasswordGenerationPopupObserver* observer_;
 
   // Controls the popup
-  base::WeakPtr<
-    autofill::PasswordGenerationPopupControllerImpl> popup_controller_;
+  base::WeakPtr<PasswordGenerationPopupControllerImpl> popup_controller_;
 
   // Set to false to disable password saving (will no longer ask if you
   // want to save passwords and also won't fill the passwords).

chrome/browser/password_manager/password_generation_interactive_uitest.cc

@@ -29,26 +29,25 @@
 
 namespace {
 
-class TestPopupObserver : public autofill::PasswordGenerationPopupObserver {
+class TestPopupObserver : public PasswordGenerationPopupObserver {
  public:
-  TestPopupObserver()
-      : popup_showing_(false),
-        password_visible_(false) {}
-  virtual ~TestPopupObserver() {}
-
-  void OnPopupShown(bool password_visible) override {
+  void OnPopupShown(
+      PasswordGenerationPopupController::GenerationState state) override {
     popup_showing_ = true;
-    password_visible_ = password_visible;
+    state_ = state;
   }
 
   void OnPopupHidden() override { popup_showing_ = false; }
 
-  bool popup_showing() { return popup_showing_; }
-  bool password_visible() { return password_visible_; }
+  bool popup_showing() const { return popup_showing_; }
+  PasswordGenerationPopupController::GenerationState state() const {
+    return state_;
+  }
 
  private:
-  bool popup_showing_;
-  bool password_visible_;
+  bool popup_showing_ = false;
+  PasswordGenerationPopupController::GenerationState state_ =
+      PasswordGenerationPopupController::kOfferGeneration;
 };
 
 }  // namespace
@@ -130,11 +129,15 @@ class PasswordGenerationInteractiveTest :
   }
 
   bool GenerationPopupShowing() {
-    return observer_.popup_showing() && observer_.password_visible();
+    return observer_.popup_showing() &&
+           observer_.state() ==
+               PasswordGenerationPopupController::kOfferGeneration;
   }
 
   bool EditingPopupShowing() {
-    return observer_.popup_showing() && !observer_.password_visible();
+    return observer_.popup_showing() &&
+           observer_.state() ==
+               PasswordGenerationPopupController::kEditGeneratedPassword;
   }
 
  private:

chrome/browser/ui/android/autofill/password_generation_popup_view_android.cc

@@ -21,8 +21,6 @@
 using base::android::JavaParamRef;
 using base::android::ScopedJavaLocalRef;
 
-namespace autofill {
-
 PasswordGenerationPopupViewAndroid::PasswordGenerationPopupViewAndroid(
     PasswordGenerationPopupController* controller)
     : controller_(controller) {}
@@ -62,7 +60,7 @@ void PasswordGenerationPopupViewAndroid::Show() {
   if (view.is_null())
     return;
   JNIEnv* env = base::android::AttachCurrentThread();
-  java_object_.Reset(Java_PasswordGenerationPopupBridge_create(
+  java_object_.Reset(autofill::Java_PasswordGenerationPopupBridge_create(
       env, view, reinterpret_cast<intptr_t>(this),
       view_android->GetWindowAndroid()->GetJavaObject()));
 
@@ -73,7 +71,7 @@ void PasswordGenerationPopupViewAndroid::Hide() {
   controller_ = NULL;
   JNIEnv* env = base::android::AttachCurrentThread();
   if (!java_object_.is_null()) {
-    Java_PasswordGenerationPopupBridge_hide(env, java_object_);
+    autofill::Java_PasswordGenerationPopupBridge_hide(env, java_object_);
   } else {
     // Hide() should delete |this| either via Java dismiss or directly.
     delete this;
@@ -108,8 +106,10 @@ void PasswordGenerationPopupViewAndroid::UpdateBoundsAndRedrawPopup() {
   ScopedJavaLocalRef<jstring> help =
       base::android::ConvertUTF16ToJavaString(env, controller_->HelpText());
 
-  Java_PasswordGenerationPopupBridge_show(
-      env, java_object_, controller_->IsRTL(), controller_->display_password(),
+  autofill::Java_PasswordGenerationPopupBridge_show(
+      env, java_object_, controller_->IsRTL(),
+      controller_->state() ==
+          PasswordGenerationPopupController::kOfferGeneration,
       password, suggestion, help, controller_->HelpTextLinkRange().start(),
       controller_->HelpTextLinkRange().end());
 }
@@ -127,5 +127,3 @@ PasswordGenerationPopupView* PasswordGenerationPopupView::Create(
     PasswordGenerationPopupController* controller) {
   return new PasswordGenerationPopupViewAndroid(controller);
 }
-
-}  // namespace autofill

chrome/browser/ui/android/autofill/password_generation_popup_view_android.h

@@ -13,8 +13,6 @@
 #include "chrome/browser/ui/passwords/password_generation_popup_view.h"
 #include "ui/android/view_android.h"
 
-namespace autofill {
-
 class PasswordGenerationPopupController;
 
 // The android implementation of the password generation UI.
@@ -61,6 +59,4 @@ class PasswordGenerationPopupViewAndroid : public PasswordGenerationPopupView {
   DISALLOW_COPY_AND_ASSIGN(PasswordGenerationPopupViewAndroid);
 };
 
-}  // namespace autofill
-
 #endif  // CHROME_BROWSER_UI_ANDROID_AUTOFILL_PASSWORD_GENERATION_POPUP_VIEW_ANDROID_H_

chrome/browser/ui/passwords/password_generation_popup_controller.cc

@@ -4,16 +4,3 @@
 
 #include "chrome/browser/ui/passwords/password_generation_popup_controller.h"
 
-namespace autofill {
-
-// static
-constexpr int PasswordGenerationPopupController::kHelpVerticalPadding = 15;
-
-// static
-constexpr int PasswordGenerationPopupController::kHorizontalPadding = 16;
-
-// static
-constexpr int PasswordGenerationPopupController::kPopupPasswordSectionHeight =
-    62;
-
-}  // namespace autofill

chrome/browser/ui/passwords/password_generation_popup_controller.h

@@ -12,18 +12,15 @@ namespace gfx {
 class Range;
 }
 
-namespace autofill {
-
-class PasswordGenerationPopupController : public AutofillPopupViewDelegate {
+class PasswordGenerationPopupController
+    : public autofill::AutofillPopupViewDelegate {
  public:
-  // Space above and below help section.
-  static const int kHelpVerticalPadding;
-
-  // Spacing between the border of the popup and any text.
-  static const int kHorizontalPadding;
-
-  // Desired height of the password section.
-  static const int kPopupPasswordSectionHeight;
+  enum GenerationState {
+    // Generated password is offered in the popup but not filled yet.
+    kOfferGeneration,
+    // The generated password was accepted.
+    kEditGeneratedPassword,
+  };
 
   // Called by the view when the password was accepted.
   virtual void PasswordAccepted() = 0;
@@ -35,9 +32,9 @@ class PasswordGenerationPopupController : public AutofillPopupViewDelegate {
   virtual int GetMinimumWidth() = 0;
 
   // Accessors
-  virtual bool display_password() const = 0;
+  virtual GenerationState state() const = 0;
   virtual bool password_selected() const = 0;
-  virtual base::string16 password() const = 0;
+  virtual const base::string16& password() const = 0;
 
   // Translated strings
   virtual base::string16 SuggestedText() = 0;
@@ -45,9 +42,7 @@ class PasswordGenerationPopupController : public AutofillPopupViewDelegate {
   virtual const gfx::Range& HelpTextLinkRange() = 0;
 
  protected:
-  ~PasswordGenerationPopupController() override {}
+  ~PasswordGenerationPopupController() override = default;
 };
 
-}  // namespace autofill
-
 #endif  // CHROME_BROWSER_UI_PASSWORDS_PASSWORD_GENERATION_POPUP_CONTROLLER_H_

chrome/browser/ui/passwords/password_generation_popup_controller_impl.cc

@@ -44,13 +44,11 @@
 #include "chrome/browser/android/preferences/preferences_launcher.h"
 #endif
 
-namespace autofill {
-
 base::WeakPtr<PasswordGenerationPopupControllerImpl>
 PasswordGenerationPopupControllerImpl::GetOrCreate(
     base::WeakPtr<PasswordGenerationPopupControllerImpl> previous,
     const gfx::RectF& bounds,
-    const PasswordForm& form,
+    const autofill::PasswordForm& form,
     const base::string16& generation_element,
     uint32_t max_length,
     password_manager::PasswordManager* password_manager,
@@ -76,14 +74,14 @@ PasswordGenerationPopupControllerImpl::GetOrCreate(
 
 PasswordGenerationPopupControllerImpl::PasswordGenerationPopupControllerImpl(
     const gfx::RectF& bounds,
-    const PasswordForm& form,
+    const autofill::PasswordForm& form,
     const base::string16& generation_element,
     uint32_t max_length,
     const base::WeakPtr<password_manager::PasswordManagerDriver>& driver,
     PasswordGenerationPopupObserver* observer,
     content::WebContents* web_contents,
     gfx::NativeView container_view)
-    : view_(NULL),
+    : view_(nullptr),
       form_(form),
       driver_(driver),
       observer_(observer),
@@ -95,7 +93,7 @@ PasswordGenerationPopupControllerImpl::PasswordGenerationPopupControllerImpl(
       // TODO(estade): use correct text direction.
       controller_common_(bounds, base::i18n::LEFT_TO_RIGHT, container_view),
       password_selected_(false),
-      display_password_(false),
+      state_(kOfferGeneration),
       web_contents_(web_contents),
       weak_ptr_factory_(this) {
   base::string16 link =
@@ -144,7 +142,7 @@ bool PasswordGenerationPopupControllerImpl::PossiblyAcceptPassword() {
 }
 
 void PasswordGenerationPopupControllerImpl::PasswordSelected(bool selected) {
-  if (!display_password_ || selected == password_selected_)
+  if (state_ == kEditGeneratedPassword || selected == password_selected_)
     return;
 
   password_selected_ = selected;
@@ -152,7 +150,7 @@ void PasswordGenerationPopupControllerImpl::PasswordSelected(bool selected) {
 }
 
 void PasswordGenerationPopupControllerImpl::PasswordAccepted() {
-  if (!display_password_)
+  if (state_ != kOfferGeneration)
     return;
 
   driver_->GeneratedPasswordAccepted(current_password_);
@@ -176,14 +174,16 @@ void PasswordGenerationPopupControllerImpl::CalculateBounds() {
       container_view(), IsRTL());
 }
 
-void PasswordGenerationPopupControllerImpl::Show(bool display_password) {
-  display_password_ = display_password;
-  if (display_password_ && current_password_.empty()) {
+void PasswordGenerationPopupControllerImpl::Show(GenerationState state) {
+  // When switching from editing to generation state, regenerate the password.
+  if (state == kOfferGeneration &&
+      (state_ != state || current_password_.empty())) {
     current_password_ =
         driver_->GetPasswordGenerationManager()->GeneratePassword(
             web_contents_->GetLastCommittedURL().GetOrigin(), form_signature_,
             field_signature_, max_length_);
   }
+  state_ = state;
 
   if (!view_) {
     view_ = PasswordGenerationPopupView::Create(this);
@@ -202,13 +202,20 @@ void PasswordGenerationPopupControllerImpl::Show(bool display_password) {
     view_->UpdateBoundsAndRedrawPopup();
   }
 
-  static_cast<ContentAutofillDriver*>(driver_->GetAutofillDriver())
+  static_cast<autofill::ContentAutofillDriver*>(driver_->GetAutofillDriver())
       ->RegisterKeyPressHandler(base::BindRepeating(
           &PasswordGenerationPopupControllerImpl::HandleKeyPressEvent,
           base::Unretained(this)));
 
   if (observer_)
-    observer_->OnPopupShown(display_password_);
+    observer_->OnPopupShown(state_);
+}
+
+void PasswordGenerationPopupControllerImpl::UpdatePassword(
+    base::string16 new_password) {
+  current_password_ = std::move(new_password);
+  if (view_)
+    view_->UpdatePasswordValue();
 }
 
 void PasswordGenerationPopupControllerImpl::HideAndDestroy() {
@@ -217,7 +224,7 @@ void PasswordGenerationPopupControllerImpl::HideAndDestroy() {
 
 void PasswordGenerationPopupControllerImpl::Hide() {
   if (driver_) {
-    static_cast<ContentAutofillDriver*>(driver_->GetAutofillDriver())
+    static_cast<autofill::ContentAutofillDriver*>(driver_->GetAutofillDriver())
         ->RemoveKeyPressHandler();
   }
 
@@ -305,20 +312,23 @@ int PasswordGenerationPopupControllerImpl::GetElidedLabelWidthForRow(int row) {
 }
 #endif
 
-bool PasswordGenerationPopupControllerImpl::display_password() const {
-  return display_password_;
+PasswordGenerationPopupController::GenerationState
+PasswordGenerationPopupControllerImpl::state() const {
+  return state_;
 }
 
 bool PasswordGenerationPopupControllerImpl::password_selected() const {
   return password_selected_;
 }
 
-base::string16 PasswordGenerationPopupControllerImpl::password() const {
+const base::string16& PasswordGenerationPopupControllerImpl::password() const {
   return current_password_;
 }
 
 base::string16 PasswordGenerationPopupControllerImpl::SuggestedText() {
-  return l10n_util::GetStringUTF16(IDS_PASSWORD_GENERATION_SUGGESTION);
+  return l10n_util::GetStringUTF16(
+      state_ == kOfferGeneration ? IDS_PASSWORD_GENERATION_SUGGESTION
+                                 : IDS_PASSWORD_GENERATION_EDITING_SUGGESTION);
 }
 
 const base::string16& PasswordGenerationPopupControllerImpl::HelpText() {
@@ -328,5 +338,3 @@ const base::string16& PasswordGenerationPopupControllerImpl::HelpText() {
 const gfx::Range& PasswordGenerationPopupControllerImpl::HelpTextLinkRange() {
   return link_range_;
 }
-
-}  // namespace autofill

chrome/browser/ui/passwords/password_generation_popup_controller_impl.h

@@ -33,10 +33,12 @@ class PasswordManagerDriver;
 }  // namespace password_manager
 
 namespace autofill {
+struct PasswordForm;
+struct Suggestion;
+}  // namespace autofill
 
 class PasswordGenerationPopupObserver;
 class PasswordGenerationPopupView;
-struct Suggestion;
 
 // This class controls a PasswordGenerationPopupView. It is responsible for
 // determining the location of the popup, handling keypress events while the
@@ -57,7 +59,7 @@ class PasswordGenerationPopupControllerImpl
   static base::WeakPtr<PasswordGenerationPopupControllerImpl> GetOrCreate(
       base::WeakPtr<PasswordGenerationPopupControllerImpl> previous,
       const gfx::RectF& bounds,
-      const PasswordForm& form,
+      const autofill::PasswordForm& form,
       const base::string16& generation_element,
       uint32_t max_length,
       password_manager::PasswordManager* password_manager,
@@ -68,10 +70,10 @@ class PasswordGenerationPopupControllerImpl
   ~PasswordGenerationPopupControllerImpl() override;
 
   // Create a PasswordGenerationPopupView if one doesn't already exist.
-  // If |display_password| is true, a generated password is shown that can be
-  // selected by the user. Otherwise just the text explaining generated
-  // passwords is shown. Idempotent.
-  void Show(bool display_password);
+  void Show(GenerationState state);
+
+  // Update the password to be displayed in the UI.
+  void UpdatePassword(base::string16 new_password);
 
   // Hides the popup and destroys |this|.
   void HideAndDestroy();
@@ -79,7 +81,7 @@ class PasswordGenerationPopupControllerImpl
  protected:
   PasswordGenerationPopupControllerImpl(
       const gfx::RectF& bounds,
-      const PasswordForm& form,
+      const autofill::PasswordForm& form,
       const base::string16& generation_element,
       uint32_t max_length,
       const base::WeakPtr<password_manager::PasswordManagerDriver>& driver,
@@ -112,9 +114,9 @@ class PasswordGenerationPopupControllerImpl
   int GetElidedLabelWidthForRow(int row) override;
 #endif
 
-  bool display_password() const override;
+  GenerationState state() const override;
   bool password_selected() const override;
-  base::string16 password() const override;
+  const base::string16& password() const override;
   base::string16 SuggestedText() override;
   const base::string16& HelpText() override;
   const gfx::Range& HelpTextLinkRange() override;
@@ -133,7 +135,8 @@ class PasswordGenerationPopupControllerImpl
   // wrapping.
   void CalculateBounds();
 
-  PasswordForm form_;
+  autofill::PasswordForm form_;
+
   base::WeakPtr<password_manager::PasswordManagerDriver> driver_;
 
   // May be NULL.
@@ -150,23 +153,25 @@ class PasswordGenerationPopupControllerImpl
   uint32_t max_length_;
 
   // Contains common popup data.
-  const PopupControllerCommon controller_common_;
+  const autofill::PopupControllerCommon controller_common_;
 
   // Help text and the range in the text that corresponds to the saved passwords
   // link.
   base::string16 help_text_;
   gfx::Range link_range_;
 
+  // The password value to be displayed in the UI.
   base::string16 current_password_;
+  // Whether the row with the password is currently selected/highlighted.
   bool password_selected_;
 
-  // If a password will be shown in this popup.
-  bool display_password_;
+  // The state of the generation popup.
+  GenerationState state_;
 
   // Bounds for all the elements of the popup.
   gfx::Rect popup_bounds_;
 
-  PopupViewCommon view_common_;
+  autofill::PopupViewCommon view_common_;
 
   content::WebContents* const web_contents_;
 
@@ -175,6 +180,4 @@ class PasswordGenerationPopupControllerImpl
   DISALLOW_COPY_AND_ASSIGN(PasswordGenerationPopupControllerImpl);
 };
 
-}  // namespace autofill
-
 #endif  // CHROME_BROWSER_UI_PASSWORDS_PASSWORD_GENERATION_POPUP_CONTROLLER_IMPL_H_

chrome/browser/ui/passwords/password_generation_popup_observer.h

@@ -5,15 +5,14 @@
 #ifndef CHROME_BROWSER_UI_PASSWORDS_PASSWORD_GENERATION_POPUP_OBSERVER_H_
 #define CHROME_BROWSER_UI_PASSWORDS_PASSWORD_GENERATION_POPUP_OBSERVER_H_
 
-namespace autofill {
+#include "chrome/browser/ui/passwords/password_generation_popup_controller.h"
 
 // Observer for PasswordGenerationPopup events. Currently only used for testing.
 class PasswordGenerationPopupObserver {
  public:
-  virtual void OnPopupShown(bool password_visible) = 0;
+  virtual void OnPopupShown(
+      PasswordGenerationPopupController::GenerationState state) = 0;
   virtual void OnPopupHidden() = 0;
 };
 
-}  // namespace autofill
-
 #endif  // CHROME_BROWSER_UI_PASSWORDS_PASSWORD_GENERATION_POPUP_OBSERVER_H_

chrome/browser/ui/passwords/password_generation_popup_view.cc

@@ -4,15 +4,3 @@
 
 #include "chrome/browser/ui/passwords/password_generation_popup_view.h"
 
-namespace autofill {
-
-const SkColor PasswordGenerationPopupView::kPasswordTextColor =
-    SkColorSetRGB(0x33, 0x33, 0x33);
-const SkColor PasswordGenerationPopupView::kExplanatoryTextBackgroundColor =
-    SkColorSetRGB(0xF5, 0xF5, 0xF5);
-const SkColor PasswordGenerationPopupView::kExplanatoryTextColor =
-    SkColorSetRGB(0x66, 0x66, 0x66);
-const SkColor PasswordGenerationPopupView::kDividerColor =
-    SkColorSetRGB(0xE9, 0xE9, 0xE9);
-
-}  // namespace autofill

chrome/browser/ui/passwords/password_generation_popup_view.h

@@ -12,16 +12,11 @@ class Point;
 class Size;
 }  // namespace gfx
 
-namespace autofill {
-
 class PasswordGenerationPopupController;
 
 // Interface for creating and controlling a platform dependent view.
 class PasswordGenerationPopupView {
  public:
-  // Number of pixels added in between lines of the help section.
-  static const int kHelpSectionAdditionalSpacing = 3;
-
   // Display the popup.
   virtual void Show() = 0;
 
@@ -35,6 +30,9 @@ class PasswordGenerationPopupView {
   // The layout should be recreated.
   virtual void UpdateState() = 0;
 
+  // The password was edited, the popup should show the new value.
+  virtual void UpdatePasswordValue() {}
+
   // Updates layout information from the controller and performs the layout.
   virtual void UpdateBoundsAndRedrawPopup() = 0;
 
@@ -47,13 +45,6 @@ class PasswordGenerationPopupView {
   // when Hide() is called.
   static PasswordGenerationPopupView* Create(
       PasswordGenerationPopupController* controller);
-
-  static const SkColor kPasswordTextColor;
-  static const SkColor kExplanatoryTextBackgroundColor;
-  static const SkColor kExplanatoryTextColor;
-  static const SkColor kDividerColor;
 };
 
-}  // namespace autofill
-
 #endif  // CHROME_BROWSER_UI_PASSWORDS_PASSWORD_GENERATION_POPUP_VIEW_H_

chrome/browser/ui/passwords/password_generation_popup_view_browsertest.cc

@@ -63,7 +63,7 @@ IN_PROC_BROWSER_TEST_F(PasswordGenerationPopupViewTest,
                        MouseMovementInEditingPopup) {
   controller_ = new autofill::TestPasswordGenerationPopupController(
       GetWebContents(), GetNativeView());
-  controller_->Show(false /* display_password */);
+  controller_->Show(PasswordGenerationPopupController::kEditGeneratedPassword);
 
   gfx::Point center_point =
       static_cast<PasswordGenerationPopupController*>(controller_)
@@ -80,7 +80,7 @@ IN_PROC_BROWSER_TEST_F(PasswordGenerationPopupViewTest,
 IN_PROC_BROWSER_TEST_F(PasswordGenerationPopupViewTest, InvalidContainerView) {
   controller_ = new autofill::TestPasswordGenerationPopupController(
       GetWebContents(), NULL);
-  controller_->Show(true /* display password */);
+  controller_->Show(PasswordGenerationPopupController::kOfferGeneration);
 }
 
 // Verify that destroying web contents with visible popup does not crash.
@@ -88,7 +88,7 @@ IN_PROC_BROWSER_TEST_F(PasswordGenerationPopupViewTest,
                        CloseWebContentsWithVisiblePopup) {
   controller_ = new autofill::TestPasswordGenerationPopupController(
       GetWebContents(), GetNativeView());
-  controller_->Show(false);
+  controller_->Show(PasswordGenerationPopupController::kEditGeneratedPassword);
   GetWebContents()->Close();
 }
 

chrome/browser/ui/passwords/password_generation_popup_view_tester.h

@@ -9,8 +9,6 @@
 
 #include "ui/gfx/geometry/point.h"
 
-namespace autofill {
-
 class PasswordGenerationPopupView;
 
 // Helps test a PasswordGenerationPopupView.
@@ -24,6 +22,4 @@ class PasswordGenerationPopupViewTester {
   virtual void SimulateMouseMovementAt(const gfx::Point& point) = 0;
 };
 
-}  // namespace autofill
-
 #endif  // CHROME_BROWSER_UI_PASSWORDS_PASSWORD_GENERATION_POPUP_VIEW_TESTER_H_

chrome/browser/ui/views/passwords/password_generation_popup_view_tester_views.cc

@@ -7,8 +7,6 @@
 #include "chrome/browser/ui/views/passwords/password_generation_popup_view_views.h"
 #include "ui/events/event_utils.h"
 
-namespace autofill {
-
 std::unique_ptr<PasswordGenerationPopupViewTester>
 PasswordGenerationPopupViewTester::For(PasswordGenerationPopupView* view) {
   return std::make_unique<PasswordGenerationPopupViewTesterViews>(
@@ -28,5 +26,3 @@ void PasswordGenerationPopupViewTesterViews::SimulateMouseMovementAt(
                             ui::EventTimeForNow(), 0, 0);
   static_cast<views::View*>(view_)->OnMouseMoved(mouse_down);
 }
-
-}  // namespace autofill

chrome/browser/ui/views/passwords/password_generation_popup_view_tester_views.h

@@ -8,8 +8,6 @@
 #include "base/macros.h"
 #include "chrome/browser/ui/passwords/password_generation_popup_view_tester.h"
 
-namespace autofill {
-
 class PasswordGenerationPopupViewViews;
 
 class PasswordGenerationPopupViewTesterViews
@@ -28,6 +26,4 @@ class PasswordGenerationPopupViewTesterViews
   DISALLOW_COPY_AND_ASSIGN(PasswordGenerationPopupViewTesterViews);
 };
 
-}  // namespace autofill
-
 #endif  // CHROME_BROWSER_UI_VIEWS_PASSWORDS_PASSWORD_GENERATION_POPUP_VIEW_TESTER_VIEWS_H_

chrome/browser/ui/views/passwords/password_generation_popup_view_views.cc

@@ -24,8 +24,6 @@
 #include "ui/views/layout/grid_layout.h"
 #include "ui/views/widget/widget.h"
 
-namespace autofill {
-
 namespace {
 
 // Background color of the bottom part of the prompt.
@@ -45,21 +43,30 @@ class PasswordGenerationPopupViewViews::GeneratedPasswordBox
   ~GeneratedPasswordBox() override = default;
 
   // |password| is the generated password, |suggestion| is the text to the left
-  // of it.
-  void Init(const base::string16& password, const base::string16& suggestion) {
+  // of it. |generating_state| means that the generated password is offered.
+  void Init(const base::string16& password,
+            const base::string16& suggestion,
+            PasswordGenerationPopupController::GenerationState state) {
     views::GridLayout* layout =
         SetLayoutManager(std::make_unique<views::GridLayout>(this));
     BuildColumnSet(layout);
     layout->StartRow(0, 0);
 
-    views::Label* suggestion_label =
-        new views::Label(suggestion, ChromeTextContext::CONTEXT_BODY_TEXT_LARGE,
-                         views::style::STYLE_PRIMARY);
+    views::Label* suggestion_label = new views::Label(
+        suggestion, ChromeTextContext::CONTEXT_BODY_TEXT_LARGE,
+        state == PasswordGenerationPopupController::kOfferGeneration
+            ? views::style::STYLE_PRIMARY
+            : STYLE_SECONDARY);
     layout->AddView(suggestion_label);
 
-    views::Label* password_label = new views::Label(
+    DCHECK(!password_label_);
+    password_label_ = new views::Label(
         password, ChromeTextContext::CONTEXT_BODY_TEXT_LARGE, STYLE_SECONDARY);
-    layout->AddView(password_label);
+    layout->AddView(password_label_);
+  }
+
+  void UpdatePassword(const base::string16& password) {
+    password_label_->SetText(password);
   }
 
   // views::View:
@@ -79,6 +86,8 @@ class PasswordGenerationPopupViewViews::GeneratedPasswordBox
                           views::GridLayout::USE_PREF, 0, 0);
   }
 
+  views::Label* password_label_ = nullptr;
+
   DISALLOW_COPY_AND_ASSIGN(GeneratedPasswordBox);
 };
 
@@ -110,13 +119,14 @@ gfx::Size PasswordGenerationPopupViewViews::GetPreferredSizeOfPasswordView() {
 }
 
 void PasswordGenerationPopupViewViews::UpdateState() {
-  if (static_cast<bool>(password_view_) != controller_->display_password()) {
-    // The state of the drop-down can change from editing generated password
-    // mode back to generation mode.
-    RemoveAllChildViews(true);
-    password_view_ = nullptr;
-    CreateLayoutAndChildren();
-  }
+  RemoveAllChildViews(true);
+  password_view_ = nullptr;
+  CreateLayoutAndChildren();
+}
+
+void PasswordGenerationPopupViewViews::UpdatePasswordValue() {
+  password_view_->UpdatePassword(controller_->password());
+  Layout();
 }
 
 void PasswordGenerationPopupViewViews::UpdateBoundsAndRedrawPopup() {
@@ -160,14 +170,15 @@ void PasswordGenerationPopupViewViews::CreateLayoutAndChildren() {
       provider->GetDistanceMetric(DISTANCE_TOAST_LABEL_VERTICAL);
   const int kHorizontalMargin =
       provider->GetDistanceMetric(DISTANCE_UNRELATED_CONTROL_HORIZONTAL);
-  if (controller_->display_password()) {
-    password_view_ = new GeneratedPasswordBox();
-    password_view_->SetBorder(
-        views::CreateEmptyBorder(kVerticalPadding, kHorizontalMargin,
-                                 kVerticalPadding, kHorizontalMargin));
-    password_view_->Init(controller_->password(), controller_->SuggestedText());
-    AddChildView(password_view_);
-  }
+
+  password_view_ = new GeneratedPasswordBox();
+  password_view_->SetBorder(
+      views::CreateEmptyBorder(kVerticalPadding, kHorizontalMargin,
+                               kVerticalPadding, kHorizontalMargin));
+  password_view_->Init(controller_->password(), controller_->SuggestedText(),
+                       controller_->state());
+  AddChildView(password_view_);
+  PasswordSelectionUpdated();
 
   views::StyledLabel* help_label =
       new views::StyledLabel(controller_->HelpText(), this);
@@ -227,5 +238,3 @@ PasswordGenerationPopupView* PasswordGenerationPopupView::Create(
 
   return new PasswordGenerationPopupViewViews(controller, observing_widget);
 }
-
-}  // namespace autofill

chrome/browser/ui/views/passwords/password_generation_popup_view_views.h

@@ -10,8 +10,6 @@
 #include "chrome/browser/ui/views/autofill/autofill_popup_base_view.h"
 #include "ui/views/controls/styled_label_listener.h"
 
-namespace autofill {
-
 class PasswordGenerationPopupController;
 
 class PasswordGenerationPopupViewViews : public autofill::AutofillPopupBaseView,
@@ -27,6 +25,7 @@ class PasswordGenerationPopupViewViews : public autofill::AutofillPopupBaseView,
   void Hide() override;
   gfx::Size GetPreferredSizeOfPasswordView() override;
   void UpdateState() override;
+  void UpdatePasswordValue() override;
   void UpdateBoundsAndRedrawPopup() override;
   void PasswordSelectionUpdated() override;
   bool IsPointInPasswordBounds(const gfx::Point& point) override;
@@ -47,7 +46,7 @@ class PasswordGenerationPopupViewViews : public autofill::AutofillPopupBaseView,
                               const gfx::Range& range,
                               int event_flags) override;
 
-  // Sub view that displays the actual password to be saved.
+  // Sub view that displays the actual generated password.
   GeneratedPasswordBox* password_view_ = nullptr;
 
   // Controller for this view. Weak reference.
@@ -56,6 +55,4 @@ class PasswordGenerationPopupViewViews : public autofill::AutofillPopupBaseView,
   DISALLOW_COPY_AND_ASSIGN(PasswordGenerationPopupViewViews);
 };
 
-}  // namespace autofill
-
 #endif  // CHROME_BROWSER_UI_VIEWS_PASSWORDS_PASSWORD_GENERATION_POPUP_VIEW_VIEWS_H_

components/autofill/content/renderer/password_generation_agent.cc

@@ -672,7 +672,7 @@ void PasswordGenerationAgent::ShowEditingPopup() {
   GetPasswordManagerClient()->ShowPasswordEditingPopup(
       render_frame()->GetRenderView()->ElementBoundsInWindow(
           generation_element_),
-      *generation_form_data_->form);
+      *CreatePasswordFormToPresave());
   editing_popup_shown_ = true;
 }