[Password Generation] Update UI to match final mocks.

Specifically
  - Change the text of the generation popup.
  - Add a suggestion label.
  - Show editing popup when password is focused after generation.
  - Display password when it is focused after generation.
  - Various other small UI cleanups.

BUG=318977

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

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

16 files changed, 359 insertions, 123 deletions

diff --git a/chrome/app/generated_resources.grd b/chrome/app/generated_resources.grd
index 2d22ce0..909a344 100644
--- a/chrome/app/generated_resources.grd
+++ b/chrome/app/generated_resources.grd
@@ -14681,12 +14681,12 @@ Do you accept?
     </message>
 
     <!-- Password generation strings -->
-      <message name="IDS_PASSWORD_GENERATION_PROMPT" desc="Autofill dropdown text describing password generation. Will be merged with the next string. Triple quotes necessary to preserve trailing space.">
-        '''This password was generated just for you. Plus we'll remember it so you don't have to. '''
-      </message>
-      <message name="IDS_PASSWORD_GENERATION_LEARN_MORE_LINK" desc="Link text to modify generated password. This is appended to the previous string.">
-        Learn more &#0187;
-      </message>
+    <message name="IDS_PASSWORD_GENERATION_SUGGESTION" desc="Text shown next to a generated password describing it as a suggestion.">
+      Suggested
+    </message>
+    <message name="IDS_PASSWORD_GENERATION_PROMPT" desc="Autofill dropdown text describing password generation. The text inside |bars| is link text.">
+      Chrome will add this to your |saved passwords|.
+    </message>
 
     <message name="IDS_PASSWORD_GENERATION_BUBBLE_TITLE" desc="The title of the bubble asking users if they would like Chrome to generate a password for them on an account creation page.">
       Password Suggestion
diff --git a/chrome/browser/password_manager/password_generation_interactive_uitest.cc b/chrome/browser/password_manager/password_generation_interactive_uitest.cc
index 947a9cc..dbd948d 100644
--- a/chrome/browser/password_manager/password_generation_interactive_uitest.cc
+++ b/chrome/browser/password_manager/password_generation_interactive_uitest.cc
@@ -25,11 +25,13 @@ namespace {
 class TestPopupObserver : public autofill::PasswordGenerationPopupObserver {
  public:
   TestPopupObserver()
-      : popup_showing_(false) {}
+      : popup_showing_(false),
+        password_visible_(false) {}
   virtual ~TestPopupObserver() {}
 
-  virtual void OnPopupShown() OVERRIDE {
+  virtual void OnPopupShown(bool password_visible) OVERRIDE {
     popup_showing_ = true;
+    password_visible_ = password_visible;
   }
 
   virtual void OnPopupHidden() OVERRIDE {
@@ -37,9 +39,11 @@ class TestPopupObserver : public autofill::PasswordGenerationPopupObserver {
   }
 
   bool popup_showing() { return popup_showing_; }
+  bool password_visible() { return password_visible_; }
 
  private:
   bool popup_showing_;
+  bool password_visible_;
 };
 
 }  // namespace
@@ -57,7 +61,7 @@ class PasswordGenerationInteractiveTest : public InProcessBrowserTest {
 
   virtual void SetUpOnMainThread() OVERRIDE {
     // Disable Autofill requesting access to AddressBook data. This will cause
-    // test tests to hang on Mac.
+    // the tests to hang on Mac.
     autofill::test::DisableSystemServices(browser()->profile());
 
     // Set observer for popup.
@@ -72,7 +76,7 @@ class PasswordGenerationInteractiveTest : public InProcessBrowserTest {
   }
 
   virtual void CleanUpOnMainThread() OVERRIDE {
-    // Cleanup UI.
+    // Clean up UI.
     PasswordGenerationManager* generation_manager =
         ChromePasswordManagerClient::GetGenerationManagerFromWebContents(
             GetWebContents());
@@ -97,6 +101,16 @@ class PasswordGenerationInteractiveTest : public InProcessBrowserTest {
     return value;
   }
 
+  std::string GetFocusedElement() {
+    std::string focused_element;
+    EXPECT_TRUE(content::ExecuteScriptAndExtractString(
+        GetRenderViewHost(),
+        "window.domAutomationController.send("
+        "    document.activeElement.id)",
+        &focused_element));
+    return focused_element;
+  }
+
   void FocusPasswordField() {
     ASSERT_TRUE(content::ExecuteScript(
         GetRenderViewHost(),
@@ -110,8 +124,12 @@ class PasswordGenerationInteractiveTest : public InProcessBrowserTest {
     GetRenderViewHost()->ForwardKeyboardEvent(event);
   }
 
-  bool popup_showing() {
-    return observer_.popup_showing();
+  bool GenerationPopupShowing() {
+    return observer_.popup_showing() && observer_.password_visible();
+  }
+
+  bool EditingPopupShowing() {
+    return observer_.popup_showing() && !observer_.password_visible();
   }
 
  private:
@@ -119,7 +137,7 @@ class PasswordGenerationInteractiveTest : public InProcessBrowserTest {
 };
 
 #if defined(USE_AURA)
-// Enabled on these platforms
+// Enabled on these platforms.
 #define MAYBE_PopupShownAndPasswordSelected PopupShownAndPasswordSelected
 #define MAYBE_PopupShownAndDismissed PopupShownAndDismissed
 #else
@@ -131,20 +149,29 @@ class PasswordGenerationInteractiveTest : public InProcessBrowserTest {
 IN_PROC_BROWSER_TEST_F(PasswordGenerationInteractiveTest,
                        MAYBE_PopupShownAndPasswordSelected) {
   FocusPasswordField();
-  EXPECT_TRUE(popup_showing());
+  EXPECT_TRUE(GenerationPopupShowing());
   SendKeyToPopup(ui::VKEY_DOWN);
   SendKeyToPopup(ui::VKEY_RETURN);
 
+  // Selecting the password should fill the field and move focus to the
+  // submit button.
   EXPECT_FALSE(GetFieldValue("password_field").empty());
+  EXPECT_FALSE(GenerationPopupShowing());
+  EXPECT_FALSE(EditingPopupShowing());
+  EXPECT_EQ("input_submit_button", GetFocusedElement());
+
+  // Re-focusing the password field should show the editing popup.
+  FocusPasswordField();
+  EXPECT_TRUE(EditingPopupShowing());
 }
 
 IN_PROC_BROWSER_TEST_F(PasswordGenerationInteractiveTest,
                        MAYBE_PopupShownAndDismissed) {
   FocusPasswordField();
-  EXPECT_TRUE(popup_showing());
+  EXPECT_TRUE(GenerationPopupShowing());
 
   SendKeyToPopup(ui::VKEY_ESCAPE);
 
   // Popup is dismissed.
-  EXPECT_FALSE(popup_showing());
+  EXPECT_FALSE(GenerationPopupShowing());
 }
diff --git a/chrome/browser/password_manager/password_generation_manager.cc b/chrome/browser/password_manager/password_generation_manager.cc
index e030b3b..2d895d3 100644
--- a/chrome/browser/password_manager/password_generation_manager.cc
+++ b/chrome/browser/password_manager/password_generation_manager.cc
@@ -113,13 +113,29 @@ void PasswordGenerationManager::OnShowPasswordGenerationPopup(
           observer_,
           web_contents_,
           web_contents_->GetView()->GetNativeView());
-  popup_controller_->Show();
+  popup_controller_->Show(true /* display_password */);
 #endif  // #if defined(USE_AURA)
 }
 
 void PasswordGenerationManager::OnShowPasswordEditingPopup(
-    const gfx::RectF& bounds) {
-  // TODO(gcasto): Enable this.
+    const gfx::RectF& bounds,
+    const autofill::PasswordForm& form) {
+  // Only implemented for Aura right now.
+#if defined(USE_AURA)
+  gfx::RectF element_bounds_in_screen_space = GetBoundsInScreenSpace(bounds);
+
+  popup_controller_ =
+      autofill::PasswordGenerationPopupControllerImpl::GetOrCreate(
+          popup_controller_,
+          element_bounds_in_screen_space,
+          form,
+          password_generator_.get(),
+          driver_->GetPasswordManager(),
+          observer_,
+          web_contents_,
+          web_contents_->GetView()->GetNativeView());
+  popup_controller_->Show(false /* display_password */);
+#endif  // #if defined(USE_AURA)
 }
 
 void PasswordGenerationManager::OnHidePasswordGenerationPopup() {
diff --git a/chrome/browser/password_manager/password_generation_manager.h b/chrome/browser/password_manager/password_generation_manager.h
index 5097a3d..df21285 100644
--- a/chrome/browser/password_manager/password_generation_manager.h
+++ b/chrome/browser/password_manager/password_generation_manager.h
@@ -73,7 +73,8 @@ class PasswordGenerationManager {
                                      const autofill::PasswordForm& form);
 
   // Causes the password editing UI to be shown anchored at |element_bounds|.
-  void OnShowPasswordEditingPopup(const gfx::RectF& element_bounds);
+  void OnShowPasswordEditingPopup(const gfx::RectF& element_bounds,
+                                  const autofill::PasswordForm& form);
 
   // Hides any visible UI.
   void OnHidePasswordGenerationPopup();
diff --git a/chrome/browser/ui/autofill/password_generation_popup_controller.h b/chrome/browser/ui/autofill/password_generation_popup_controller.h
index 91bbe9f..e157c93 100644
--- a/chrome/browser/ui/autofill/password_generation_popup_controller.h
+++ b/chrome/browser/ui/autofill/password_generation_popup_controller.h
@@ -8,32 +8,40 @@
 #include "base/strings/string16.h"
 #include "chrome/browser/ui/autofill/autofill_popup_view_delegate.h"
 
+namespace gfx {
+class FontList;
+class Range;
+}
+
 namespace autofill {
 
 class PasswordGenerationPopupController : public AutofillPopupViewDelegate {
  public:
   // Space above and below the password section.
-  static const int kPasswordVerticalPadding = 14;
+  static const int kPasswordVerticalPadding = 16;
 
   // Space above and below help section.
   static const int kHelpVerticalPadding = 15;
 
   // Spacing between the border of the popup and any text.
-  static const int kHorizontalPadding = 9;
+  static const int kHorizontalPadding = 10;
 
-  // Called by the view when the help link is clicked.
-  virtual void OnHelpLinkClicked() = 0;
+  // Called by the view when the saved passwords link is clicked.
+  virtual void OnSavedPasswordsLinkClicked() = 0;
 
   // Accessors
+  virtual const gfx::FontList& font_list() const = 0;
   virtual const gfx::Rect& password_bounds() const = 0;
   virtual const gfx::Rect& divider_bounds() const = 0;
   virtual const gfx::Rect& help_bounds() const = 0;
+  virtual bool display_password() const = 0;
   virtual bool password_selected() const = 0;
   virtual base::string16 password() const = 0;
 
   // Translated strings
-  virtual base::string16 HelpText() = 0;
-  virtual base::string16 LearnMoreLink() = 0;
+  virtual base::string16 SuggestedText() = 0;
+  virtual const base::string16& HelpText() = 0;
+  virtual const gfx::Range& HelpTextLinkRange() = 0;
 
  protected:
   virtual ~PasswordGenerationPopupController() {}
diff --git a/chrome/browser/ui/autofill/password_generation_popup_controller_impl.cc b/chrome/browser/ui/autofill/password_generation_popup_controller_impl.cc
index fe7319c..d670cd3 100644
--- a/chrome/browser/ui/autofill/password_generation_popup_controller_impl.cc
+++ b/chrome/browser/ui/autofill/password_generation_popup_controller_impl.cc
@@ -6,6 +6,8 @@
 
 #include <math.h>
 
+#include "base/strings/string_split.h"
+#include "base/strings/string_util.h"
 #include "base/strings/utf_string_conversion_utils.h"
 #include "base/strings/utf_string_conversions.h"
 #include "chrome/browser/password_manager/password_manager.h"
@@ -22,15 +24,13 @@
 #include "content/public/browser/web_contents.h"
 #include "grit/generated_resources.h"
 #include "ui/base/l10n/l10n_util.h"
+#include "ui/base/resource/resource_bundle.h"
 #include "ui/events/keycodes/keyboard_codes.h"
 #include "ui/gfx/rect_conversions.h"
 #include "ui/gfx/text_utils.h"
 
 namespace autofill {
 
-const int kMinimumWidth = 60;
-const int kDividerHeight = 1;
-
 base::WeakPtr<PasswordGenerationPopupControllerImpl>
 PasswordGenerationPopupControllerImpl::GetOrCreate(
     base::WeakPtr<PasswordGenerationPopupControllerImpl> previous,
@@ -45,7 +45,7 @@ PasswordGenerationPopupControllerImpl::GetOrCreate(
       previous->element_bounds() == bounds &&
       previous->web_contents() == web_contents &&
       previous->container_view() == container_view) {
-    // TODO(gcasto): Should we clear state here?
+    // TODO(gcasto): Any state that we should clear here?
     return previous;
   }
 
@@ -78,12 +78,24 @@ PasswordGenerationPopupControllerImpl::PasswordGenerationPopupControllerImpl(
       observer_(observer),
       controller_common_(bounds, container_view, web_contents),
       view_(NULL),
-      current_password_(base::ASCIIToUTF16(generator->Generate())),
+      font_list_(ResourceBundle::GetSharedInstance().GetFontList(
+          ResourceBundle::SmallFont)),
       password_selected_(false),
+      display_password_(false),
       weak_ptr_factory_(this) {
   controller_common_.SetKeyPressCallback(
       base::Bind(&PasswordGenerationPopupControllerImpl::HandleKeyPressEvent,
                  base::Unretained(this)));
+
+  std::vector<base::string16> pieces;
+  base::SplitStringDontTrim(
+      l10n_util::GetStringUTF16(IDS_PASSWORD_GENERATION_PROMPT),
+      '|',  // separator
+      &pieces);
+  DCHECK_EQ(3u, pieces.size());
+  link_range_ = gfx::Range(pieces[0].size(),
+                           pieces[0].size() + pieces[1].size());
+  help_text_ = JoinString(pieces, base::string16());
 }
 
 PasswordGenerationPopupControllerImpl::~PasswordGenerationPopupControllerImpl()
@@ -124,11 +136,18 @@ bool PasswordGenerationPopupControllerImpl::PossiblyAcceptPassword() {
 }
 
 void PasswordGenerationPopupControllerImpl::PasswordSelected(bool selected) {
+  if (!display_password_)
+    return;
+
   password_selected_ = selected;
+  view_->PasswordSelectionUpdated();
   view_->UpdateBoundsAndRedrawPopup();
 }
 
 void PasswordGenerationPopupControllerImpl::PasswordAccepted() {
+  if (!display_password_)
+    return;
+
   web_contents()->GetRenderViewHost()->Send(
       new AutofillMsg_GeneratedPasswordAccepted(
           web_contents()->GetRenderViewHost()->GetRoutingID(),
@@ -138,15 +157,22 @@ void PasswordGenerationPopupControllerImpl::PasswordAccepted() {
 }
 
 int PasswordGenerationPopupControllerImpl::GetDesiredWidth() {
-  // Minimum width we want to display the password.
-  int minimum_length_for_text =
-      2 * kHorizontalPadding +
-      font_list_.GetExpectedTextWidth(kMinimumWidth) +
-      2 * kPopupBorderThickness;
+  // Minimum width in pixels.
+  const int minimum_required_width = 300;
 
   // If the width of the field is longer than the minimum, use that instead.
-  return std::max(minimum_length_for_text,
-                  controller_common_.RoundedElementBounds().width());
+  int width = std::max(minimum_required_width,
+                       controller_common_.RoundedElementBounds().width());
+
+  if (display_password_) {
+    // Make sure that the width will always be large enough to display the
+    // password and suggestion on one line.
+    width = std::max(width,
+                     gfx::GetStringWidth(current_password_ + SuggestedText(),
+                                         font_list_) + 2 * kHorizontalPadding);
+  }
+
+  return width;
 }
 
 int PasswordGenerationPopupControllerImpl::GetDesiredHeight(int width) {
@@ -154,16 +180,18 @@ int PasswordGenerationPopupControllerImpl::GetDesiredHeight(int width) {
   // line break in the middle of the link, but as long as the link isn't longer
   // than given width this shouldn't affect the height calculated here. The
   // default width should be wide enough to prevent this from being an issue.
-  int total_length = gfx::GetStringWidth(HelpText() + LearnMoreLink(),
-                                         font_list_);
+  int total_length = gfx::GetStringWidth(HelpText(), font_list_);
   int usable_width = width - 2 * kHorizontalPadding;
   int text_height =
       static_cast<int>(ceil(static_cast<double>(total_length)/usable_width)) *
-      font_list_.GetHeight();
+      font_list_.GetFontSize();
   int help_section_height = text_height + 2 * kHelpVerticalPadding;
 
-  int password_section_height =
-      font_list_.GetHeight() + 2 * kPasswordVerticalPadding;
+  int password_section_height = 0;
+  if (display_password_) {
+    password_section_height =
+        font_list_.GetFontSize() + 2 * kPasswordVerticalPadding;
+  }
 
   return (2 * kPopupBorderThickness +
           help_section_height +
@@ -175,41 +203,54 @@ void PasswordGenerationPopupControllerImpl::CalculateBounds() {
   int popup_height = GetDesiredHeight(popup_width);
 
   popup_bounds_ = controller_common_.GetPopupBounds(popup_height, popup_width);
+  int sub_view_width = popup_bounds_.width() - 2 * kPopupBorderThickness;
 
   // Calculate the bounds for the rest of the elements given the bounds of
   // the popup.
-  password_bounds_ =  gfx::Rect(
-      kPopupBorderThickness,
-      kPopupBorderThickness,
-      popup_bounds_.width() - 2 * kPopupBorderThickness,
-      font_list_.GetHeight() + 2 * kPasswordVerticalPadding);
-
-  divider_bounds_ = gfx::Rect(kPopupBorderThickness,
-                              password_bounds_.bottom(),
-                              password_bounds_.width(),
-                              kDividerHeight);
+  if (display_password_) {
+    password_bounds_ =  gfx::Rect(
+        kPopupBorderThickness,
+        kPopupBorderThickness,
+        sub_view_width,
+        font_list_.GetFontSize() + 2 * kPasswordVerticalPadding);
+
+    divider_bounds_ = gfx::Rect(kPopupBorderThickness,
+                                password_bounds_.bottom(),
+                                sub_view_width,
+                                1 /* divider heigth*/);
+  } else {
+    password_bounds_ = gfx::Rect();
+    divider_bounds_ = gfx::Rect();
+  }
 
+  int help_y = std::max(kPopupBorderThickness, divider_bounds_.bottom());
   int help_height =
-      popup_bounds_.height() - divider_bounds_.bottom() - kPopupBorderThickness;
+      popup_bounds_.height() - help_y - kPopupBorderThickness;
   help_bounds_ = gfx::Rect(
       kPopupBorderThickness,
-      divider_bounds_.bottom(),
-      password_bounds_.width(),
+      help_y,
+      sub_view_width,
       help_height);
 }
 
-void PasswordGenerationPopupControllerImpl::Show() {
+void PasswordGenerationPopupControllerImpl::Show(bool display_password) {
+  display_password_ = display_password;
+  if (display_password_)
+    current_password_ = base::ASCIIToUTF16(generator_->Generate());
+
   CalculateBounds();
 
   if (!view_) {
     view_ = PasswordGenerationPopupView::Create(this);
     view_->Show();
+  } else {
+    view_->UpdateBoundsAndRedrawPopup();
   }
 
   controller_common_.RegisterKeyPressCallback();
 
   if (observer_)
-    observer_->OnPopupShown();
+    observer_->OnPopupShown(display_password_);
 }
 
 void PasswordGenerationPopupControllerImpl::HideAndDestroy() {
@@ -234,7 +275,9 @@ void PasswordGenerationPopupControllerImpl::ViewDestroyed() {
   Hide();
 }
 
-void PasswordGenerationPopupControllerImpl::OnHelpLinkClicked() {
+void PasswordGenerationPopupControllerImpl::OnSavedPasswordsLinkClicked() {
+  // TODO(gcasto): Change this to navigate to account central once passwords
+  // are visible there.
   Browser* browser =
       chrome::FindBrowserWithWebContents(controller_common_.web_contents());
   content::OpenURLParams params(
@@ -273,6 +316,10 @@ gfx::NativeView PasswordGenerationPopupControllerImpl::container_view() {
   return controller_common_.container_view();
 }
 
+const gfx::FontList& PasswordGenerationPopupControllerImpl::font_list() const {
+  return font_list_;
+}
+
 const gfx::Rect& PasswordGenerationPopupControllerImpl::popup_bounds() const {
   return popup_bounds_;
 }
@@ -291,6 +338,10 @@ const gfx::Rect& PasswordGenerationPopupControllerImpl::help_bounds() const {
   return help_bounds_;
 }
 
+bool PasswordGenerationPopupControllerImpl::display_password() const {
+  return display_password_;
+}
+
 bool PasswordGenerationPopupControllerImpl::password_selected() const {
   return password_selected_;
 }
@@ -299,12 +350,16 @@ base::string16 PasswordGenerationPopupControllerImpl::password() const {
   return current_password_;
 }
 
-base::string16 PasswordGenerationPopupControllerImpl::HelpText() {
-  return l10n_util::GetStringUTF16(IDS_PASSWORD_GENERATION_PROMPT);
+base::string16 PasswordGenerationPopupControllerImpl::SuggestedText() {
+  return l10n_util::GetStringUTF16(IDS_PASSWORD_GENERATION_SUGGESTION);
+}
+
+const base::string16& PasswordGenerationPopupControllerImpl::HelpText() {
+  return help_text_;
 }
 
-base::string16 PasswordGenerationPopupControllerImpl::LearnMoreLink() {
-  return l10n_util::GetStringUTF16(IDS_PASSWORD_GENERATION_LEARN_MORE_LINK);
+const gfx::Range& PasswordGenerationPopupControllerImpl::HelpTextLinkRange() {
+  return link_range_;
 }
 
 }  // namespace autofill
diff --git a/chrome/browser/ui/autofill/password_generation_popup_controller_impl.h b/chrome/browser/ui/autofill/password_generation_popup_controller_impl.h
index 14386f4..a7e1c9c 100644
--- a/chrome/browser/ui/autofill/password_generation_popup_controller_impl.h
+++ b/chrome/browser/ui/autofill/password_generation_popup_controller_impl.h
@@ -14,6 +14,7 @@
 #include "components/autofill/core/common/password_form.h"
 #include "ui/gfx/font_list.h"
 #include "ui/gfx/native_widget_types.h"
+#include "ui/gfx/range/range.h"
 #include "ui/gfx/rect.h"
 #include "ui/gfx/rect_f.h"
 
@@ -56,8 +57,10 @@ class PasswordGenerationPopupControllerImpl
   virtual ~PasswordGenerationPopupControllerImpl();
 
   // Create a PasswordGenerationPopupView if one doesn't already exist.
-  // Does not update the view if one is already showing.
-  void Show();
+  // 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.
+  void Show(bool display_password);
 
   // Hides the popup and destroys |this|.
   void HideAndDestroy();
@@ -88,16 +91,19 @@ class PasswordGenerationPopupControllerImpl
   virtual void SelectionCleared() OVERRIDE;
   virtual bool ShouldRepostEvent(const ui::MouseEvent& event) OVERRIDE;
   virtual bool ShouldHideOnOutsideClick() const OVERRIDE;
-  virtual void OnHelpLinkClicked() OVERRIDE;
+  virtual void OnSavedPasswordsLinkClicked() OVERRIDE;
   virtual gfx::NativeView container_view() OVERRIDE;
+  virtual const gfx::FontList& font_list() const OVERRIDE;
   virtual const gfx::Rect& popup_bounds() const OVERRIDE;
   virtual const gfx::Rect& password_bounds() const OVERRIDE;
   virtual const gfx::Rect& divider_bounds() const OVERRIDE;
   virtual const gfx::Rect& help_bounds() const OVERRIDE;
+  virtual bool display_password() const OVERRIDE;
   virtual bool password_selected() const OVERRIDE;
   virtual base::string16 password() const OVERRIDE;
-  virtual base::string16 HelpText() OVERRIDE;
-  virtual base::string16 LearnMoreLink() OVERRIDE;
+  virtual base::string16 SuggestedText() OVERRIDE;
+  virtual const base::string16& HelpText() OVERRIDE;
+  virtual const gfx::Range& HelpTextLinkRange() OVERRIDE;
 
   base::WeakPtr<PasswordGenerationPopupControllerImpl> GetWeakPtr();
 
@@ -122,6 +128,7 @@ class PasswordGenerationPopupControllerImpl
   PasswordForm form_;
   PasswordGenerator* generator_;
   PasswordManager* password_manager_;
+  // May be NULL.
   PasswordGenerationPopupObserver* observer_;
 
   // Contains common popup functionality.
@@ -131,11 +138,19 @@ class PasswordGenerationPopupControllerImpl
   PasswordGenerationPopupView* view_;
 
   // Font list used in the popup.
-  gfx::FontList font_list_;
+  const gfx::FontList& font_list_;
+
+  // Help text and the range in the text that corresponds to the saved passwords
+  // link.
+  base::string16 help_text_;
+  gfx::Range link_range_;
 
   base::string16 current_password_;
   bool password_selected_;
 
+  // If a password will be shown in this popup.
+  bool display_password_;
+
   // Bounds for all the elements of the popup.
   gfx::Rect popup_bounds_;
   gfx::Rect password_bounds_;
diff --git a/chrome/browser/ui/autofill/password_generation_popup_observer.h b/chrome/browser/ui/autofill/password_generation_popup_observer.h
index 07a2edb..cfacb3a 100644
--- a/chrome/browser/ui/autofill/password_generation_popup_observer.h
+++ b/chrome/browser/ui/autofill/password_generation_popup_observer.h
@@ -10,7 +10,7 @@ namespace autofill {
 // Observer for PasswordGenerationPopup events. Currently only used for testing.
 class PasswordGenerationPopupObserver {
  public:
-  virtual void OnPopupShown() = 0;
+  virtual void OnPopupShown(bool password_visible) = 0;
   virtual void OnPopupHidden() = 0;
 };
 
diff --git a/chrome/browser/ui/autofill/password_generation_popup_view.h b/chrome/browser/ui/autofill/password_generation_popup_view.h
index db895a4..eb4ee00 100644
--- a/chrome/browser/ui/autofill/password_generation_popup_view.h
+++ b/chrome/browser/ui/autofill/password_generation_popup_view.h
@@ -12,6 +12,11 @@ class PasswordGenerationPopupController;
 // Interface for creating and controlling a platform dependent view.
 class PasswordGenerationPopupView {
  public:
+  // This is the amount of vertical whitespace that is left above and below the
+  // password when it is highlighted.
+  static const int kPasswordVerticalInset = 7;
+
+  // Display the popup.
   virtual void Show() = 0;
 
   // This will cause the popup to be deleted.
@@ -20,6 +25,9 @@ class PasswordGenerationPopupView {
   // Updates layout information from the controller.
   virtual void UpdateBoundsAndRedrawPopup() = 0;
 
+  // Called when the password selection state has changed.
+  virtual void PasswordSelectionUpdated() = 0;
+
   // Note that PasswordGenerationPopupView owns itself, and will only be deleted
   // when Hide() is called.
   static PasswordGenerationPopupView* Create(
diff --git a/chrome/browser/ui/views/autofill/password_generation_popup_view_views.cc b/chrome/browser/ui/views/autofill/password_generation_popup_view_views.cc
index 4fbf85b..9f99758 100644
--- a/chrome/browser/ui/views/autofill/password_generation_popup_view_views.cc
+++ b/chrome/browser/ui/views/autofill/password_generation_popup_view_views.cc
@@ -11,6 +11,7 @@
 #include "ui/views/border.h"
 #include "ui/views/controls/label.h"
 #include "ui/views/controls/styled_label.h"
+#include "ui/views/layout/box_layout.h"
 #include "ui/views/widget/widget.h"
 
 namespace autofill {
@@ -18,59 +19,98 @@ namespace autofill {
 namespace {
 
 const SkColor kExplanatoryTextBackground = SkColorSetRGB(0xF5, 0xF5, 0xF5);
-const SkColor kExplanatoryTextColor = SkColorSetRGB(0x93, 0x93, 0x93);
-const SkColor kDividerColor = SkColorSetRGB(0xE7, 0xE7, 0xE7);
-
-// This is the amount of vertical whitespace that is left above and below the
-// password when it is highlighted.
-const int kPasswordVerticalInset = 7;
+const SkColor kExplanatoryTextColor = SkColorSetRGB(0x7F, 0x7F, 0x7F);
+const SkColor kDividerColor = SkColorSetRGB(0xE9, 0xE9, 0xE9);
 
 // The amount of whitespace that is present when there is no padding. Used
 // to get the proper spacing in the help section.
 const int kHelpVerticalOffset = 3;
 
+// Class that shows the password and the suggestion side-by-side.
+class PasswordRow : public views::View {
+ public:
+  PasswordRow(const base::string16& password,
+              const base::string16& suggestion,
+              const gfx::FontList& font_list,
+              int horizontal_border) {
+    set_clip_insets(gfx::Insets(
+        PasswordGenerationPopupView::kPasswordVerticalInset, 0,
+        PasswordGenerationPopupView::kPasswordVerticalInset, 0));
+    views::BoxLayout* box_layout = new views::BoxLayout(
+        views::BoxLayout::kHorizontal, horizontal_border, 0, 0);
+    box_layout->set_spread_blank_space(true);
+    SetLayoutManager(box_layout);
+
+    password_label_ = new views::Label(password, font_list);
+    password_label_->SetHorizontalAlignment(gfx::ALIGN_LEFT);
+    AddChildView(password_label_);
+
+    suggestion_label_ = new views::Label(suggestion, font_list);
+    suggestion_label_->SetHorizontalAlignment(gfx::ALIGN_RIGHT);
+    suggestion_label_->SetEnabledColor(kExplanatoryTextColor);
+    AddChildView(suggestion_label_);
+  }
+  virtual ~PasswordRow() {}
+
+  virtual bool HitTestRect(const gfx::Rect& rect) const OVERRIDE {
+    // Have parent do event handling.
+    return false;
+  }
+
+ private:
+  // Child views. Not owned.
+  views::Label* suggestion_label_;
+  views::Label* password_label_;
+
+  DISALLOW_COPY_AND_ASSIGN(PasswordRow);
+};
+
 }  // namespace
 
 PasswordGenerationPopupViewViews::PasswordGenerationPopupViewViews(
     PasswordGenerationPopupController* controller,
     views::Widget* observing_widget)
     : AutofillPopupBaseView(controller, observing_widget),
+      password_view_(NULL),
       controller_(controller) {
-  password_label_ = new views::Label(controller->password());
-  password_label_->SetBoundsRect(controller->password_bounds());
-  password_label_->SetHorizontalAlignment(gfx::ALIGN_LEFT);
-  password_label_->set_clip_insets(gfx::Insets(
-      kPasswordVerticalInset, 0, kPasswordVerticalInset, 0));
-  password_label_->SetBorder(views::Border::CreateEmptyBorder(
-      0, controller_->kHorizontalPadding, 0, controller_->kHorizontalPadding));
-  AddChildView(password_label_);
-
-  base::string16 help_text = controller->HelpText();
-  base::string16 learn_more_link_text = controller->LearnMoreLink();
-  views::StyledLabel* help_label =
-      new views::StyledLabel(help_text + learn_more_link_text, this);
+  if (controller_->display_password())
+    CreatePasswordView();
+
+  help_label_ = new views::StyledLabel(controller_->HelpText(), this);
+  help_label_->SetBaseFontList(controller_->font_list());
   views::StyledLabel::RangeStyleInfo default_style;
   default_style.color = kExplanatoryTextColor;
-  help_label->SetDefaultStyle(default_style);
-  help_label->AddStyleRange(
-      gfx::Range(help_text.size(),
-                 help_text.size() + learn_more_link_text.size()),
+  help_label_->SetDefaultStyle(default_style);
+  help_label_->AddStyleRange(
+      controller_->HelpTextLinkRange(),
       views::StyledLabel::RangeStyleInfo::CreateForLink());
-  help_label->SetBoundsRect(controller->help_bounds());
-  help_label->set_background(
+
+  help_label_->SetBoundsRect(controller_->help_bounds());
+  help_label_->set_background(
       views::Background::CreateSolidBackground(kExplanatoryTextBackground));
-  help_label->SetBorder(views::Border::CreateEmptyBorder(
+  help_label_->SetBorder(views::Border::CreateEmptyBorder(
       controller_->kHelpVerticalPadding - kHelpVerticalOffset,
       controller_->kHorizontalPadding,
       0,
       controller_->kHorizontalPadding));
-  AddChildView(help_label);
+  AddChildView(help_label_);
 
   set_background(views::Background::CreateSolidBackground(kPopupBackground));
 }
 
 PasswordGenerationPopupViewViews::~PasswordGenerationPopupViewViews() {}
 
+void PasswordGenerationPopupViewViews::CreatePasswordView() {
+  if (password_view_)
+    return;
+
+  password_view_ = new PasswordRow(controller_->password(),
+                                   controller_->SuggestedText(),
+                                   controller_->font_list(),
+                                   controller_->kHorizontalPadding);
+  AddChildView(password_view_);
+}
+
 void PasswordGenerationPopupViewViews::Show() {
   DoShow();
 }
@@ -83,32 +123,49 @@ void PasswordGenerationPopupViewViews::Hide() {
 }
 
 void PasswordGenerationPopupViewViews::UpdateBoundsAndRedrawPopup() {
+  // Currently the UI can change from not offering a password to offering
+  // a password (e.g. the user is editing a generated password and deletes it),
+  // but it can't change the other way around.
+  if (controller_->display_password())
+    CreatePasswordView();
+
   DoUpdateBoundsAndRedrawPopup();
 }
 
+void PasswordGenerationPopupViewViews::PasswordSelectionUpdated() {
+  if (!password_view_)
+    return;
+
+  password_view_->set_background(
+      views::Background::CreateSolidBackground(
+          controller_->password_selected() ?
+          kHoveredBackgroundColor :
+          kPopupBackground));
+}
+
+void PasswordGenerationPopupViewViews::Layout() {
+  if (password_view_)
+    password_view_->SetBoundsRect(controller_->password_bounds());
+
+  help_label_->SetBoundsRect(controller_->help_bounds());
+}
+
 void PasswordGenerationPopupViewViews::OnPaint(gfx::Canvas* canvas) {
   if (!controller_)
     return;
 
-  if (controller_->password_selected()) {
-    password_label_->set_background(
-        views::Background::CreateSolidBackground(kHoveredBackgroundColor));
-  } else {
-    password_label_->set_background(
-        views::Background::CreateSolidBackground(kPopupBackground));
-  }
-
   // Draw border and background.
   views::View::OnPaint(canvas);
 
   // Divider line needs to be drawn after OnPaint() otherwise the background
   // will overwrite the divider.
-  canvas->FillRect(controller_->divider_bounds(), kDividerColor);
+  if (password_view_)
+    canvas->FillRect(controller_->divider_bounds(), kDividerColor);
 }
 
 void PasswordGenerationPopupViewViews::StyledLabelLinkClicked(
     const gfx::Range& range, int event_flags) {
-  controller_->OnHelpLinkClicked();
+  controller_->OnSavedPasswordsLinkClicked();
 }
 
 PasswordGenerationPopupView* PasswordGenerationPopupView::Create(
diff --git a/chrome/browser/ui/views/autofill/password_generation_popup_view_views.h b/chrome/browser/ui/views/autofill/password_generation_popup_view_views.h
index 9fe5d44..28f07cc 100644
--- a/chrome/browser/ui/views/autofill/password_generation_popup_view_views.h
+++ b/chrome/browser/ui/views/autofill/password_generation_popup_view_views.h
@@ -10,7 +10,8 @@
 #include "ui/views/controls/styled_label_listener.h"
 
 namespace views {
-class Label;
+class StyledLabel;
+class View;
 }
 
 namespace autofill {
@@ -29,20 +30,25 @@ class PasswordGenerationPopupViewViews : public AutofillPopupBaseView,
   virtual void Show() OVERRIDE;
   virtual void Hide() OVERRIDE;
   virtual void UpdateBoundsAndRedrawPopup() OVERRIDE;
+  virtual void PasswordSelectionUpdated() OVERRIDE;
 
  private:
   virtual ~PasswordGenerationPopupViewViews();
 
+  // Helper function to create |password_view_|.
+  void CreatePasswordView();
+
   // views:Views implementation.
+  virtual void Layout() OVERRIDE;
   virtual void OnPaint(gfx::Canvas* canvas) OVERRIDE;
 
   // views::StyledLabelListener implementation
   virtual void StyledLabelLinkClicked(const gfx::Range& range,
                                       int event_flags) OVERRIDE;
 
-  // Label for the generated password. Used to change the background color when
-  // the password is selected/deselected. Weak reference.
-  views::Label* password_label_;
+  // Sub views. Used to change bounds when updating. Weak references.
+  views::View* password_view_;
+  views::StyledLabel* help_label_;
 
   // Controller for this view. Weak reference.
   PasswordGenerationPopupController* controller_;
diff --git a/components/autofill/content/common/autofill_messages.h b/components/autofill/content/common/autofill_messages.h
index 54fd250..3eb70c8 100644
--- a/components/autofill/content/common/autofill_messages.h
+++ b/components/autofill/content/common/autofill_messages.h
@@ -241,9 +241,11 @@ IPC_MESSAGE_ROUTED3(AutofillHostMsg_ShowPasswordGenerationPopup,
                     autofill::PasswordForm)
 
 // Instructs the browser to show the popup for editing a generated password.
-// The location should be specified in the renderers coordinate system.
-IPC_MESSAGE_ROUTED1(AutofillHostMsg_ShowPasswordEditingPopup,
-                    gfx::RectF /* source location */)
+// The location should be specified in the renderers coordinate system. Form
+// is the form associated with the password field.
+IPC_MESSAGE_ROUTED2(AutofillHostMsg_ShowPasswordEditingPopup,
+                    gfx::RectF /* source location */,
+                    autofill::PasswordForm)
 
 // Instructs the browser to hide any password generation popups.
 IPC_MESSAGE_ROUTED0(AutofillHostMsg_HidePasswordGenerationPopup)
diff --git a/components/autofill/content/renderer/password_generation_agent.cc b/components/autofill/content/renderer/password_generation_agent.cc
index f7e01d1..4949f8e 100644
--- a/components/autofill/content/renderer/password_generation_agent.cc
+++ b/components/autofill/content/renderer/password_generation_agent.cc
@@ -282,7 +282,9 @@ void PasswordGenerationAgent::DetermineGenerationElement() {
 }
 
 void PasswordGenerationAgent::FocusedNodeChanged(const blink::WebNode& node) {
-  // TODO(gcasto): Re-hide generation_element text.
+  if (!generation_element_.isNull())
+    generation_element_.setShouldRevealPassword(false);
+
   if (node.isNull() || !node.isElementNode())
     return;
 
@@ -295,7 +297,7 @@ void PasswordGenerationAgent::FocusedNodeChanged(const blink::WebNode& node) {
     return;
 
   if (password_is_generated_) {
-    // TODO(gcasto): Make characters visible.
+    generation_element_.setShouldRevealPassword(true);
     ShowEditingPopup();
   }
 
@@ -319,8 +321,11 @@ bool PasswordGenerationAgent::TextDidChangeInTextField(
           password_generation::PASSWORD_DELETED);
     }
 
+    // Do not treat the password as generated.
     // TODO(gcasto): Set PasswordForm::type in the browser to TYPE_NORMAL.
     password_is_generated_ = false;
+    generation_element_.setShouldRevealPassword(false);
+
     // Offer generation again.
     ShowGenerationPopup();
   } else if (!password_is_generated_) {
@@ -359,8 +364,10 @@ void PasswordGenerationAgent::ShowEditingPopup() {
       GetScaledBoundingBox(render_view_->GetWebView()->pageScaleFactor(),
                            &generation_element_);
 
-  Send(new AutofillHostMsg_ShowPasswordEditingPopup(routing_id(),
-                                                    bounding_box_scaled));
+  Send(new AutofillHostMsg_ShowPasswordEditingPopup(
+      routing_id(),
+      bounding_box_scaled,
+      *possible_account_creation_form_));
 
   password_generation::LogPasswordGenerationEvent(
       password_generation::EDITING_POPUP_SHOWN);
diff --git a/ui/views/controls/styled_label.cc b/ui/views/controls/styled_label.cc
index 4ca729c..d707b6f 100644
--- a/ui/views/controls/styled_label.cc
+++ b/ui/views/controls/styled_label.cc
@@ -23,13 +23,15 @@ namespace {
 
 // Calculates the height of a line of text. Currently returns the height of
 // a label.
-int CalculateLineHeight() {
+int CalculateLineHeight(const gfx::FontList& font_list) {
   Label label;
+  label.SetFontList(font_list);
   return label.GetPreferredSize().height();
 }
 
 scoped_ptr<Label> CreateLabelRange(
     const base::string16& text,
+    const gfx::FontList& font_list,
     const StyledLabel::RangeStyleInfo& style_info,
     views::LinkListener* link_listener) {
   scoped_ptr<Label> result;
@@ -44,6 +46,7 @@ scoped_ptr<Label> CreateLabelRange(
   }
 
   result->SetEnabledColor(style_info.color);
+  result->SetFontList(font_list);
 
   if (!style_info.tooltip.empty())
     result->SetTooltipText(style_info.tooltip);
@@ -106,6 +109,11 @@ void StyledLabel::SetText(const base::string16& text) {
   PreferredSizeChanged();
 }
 
+void StyledLabel::SetBaseFontList(const gfx::FontList& font_list) {
+  font_list_ = font_list;
+  PreferredSizeChanged();
+}
+
 void StyledLabel::AddStyleRange(const gfx::Range& range,
                                 const RangeStyleInfo& style_info) {
   DCHECK(!range.is_reversed());
@@ -179,7 +187,7 @@ gfx::Size StyledLabel::CalculateAndDoLayout(int width, bool dry_run) {
   if (width <= 0 || text_.empty())
     return gfx::Size();
 
-  const int line_height = CalculateLineHeight();
+  const int line_height = CalculateLineHeight(font_list_);
   // The index of the line we're on.
   int line = 0;
   // The x position (in pixels) of the line we're on, relative to content
@@ -205,7 +213,7 @@ gfx::Size StyledLabel::CalculateAndDoLayout(int width, bool dry_run) {
 
     const gfx::Rect chunk_bounds(x, 0, width - x, 2 * line_height);
     std::vector<base::string16> substrings;
-    gfx::FontList text_font_list;
+    gfx::FontList text_font_list = font_list_;
     // If the start of the remaining text is inside a styled range, the font
     // style may differ from the base font. The font specified by the range
     // should be used when eliding text.
@@ -255,7 +263,7 @@ gfx::Size StyledLabel::CalculateAndDoLayout(int width, bool dry_run) {
 
       chunk = chunk.substr(0, std::min(chunk.size(), range.end() - position));
 
-      label = CreateLabelRange(chunk, style_info, this);
+      label = CreateLabelRange(chunk, font_list_, style_info, this);
 
       if (style_info.is_link && !dry_run)
         link_targets_[label.get()] = range;
@@ -266,7 +274,7 @@ gfx::Size StyledLabel::CalculateAndDoLayout(int width, bool dry_run) {
       // This chunk is normal text.
       if (position + chunk.size() > range.start())
         chunk = chunk.substr(0, range.start() - position);
-      label = CreateLabelRange(chunk, default_style_info_, this);
+      label = CreateLabelRange(chunk, font_list_, default_style_info_, this);
     }
 
     if (displayed_on_background_color_set_)
diff --git a/ui/views/controls/styled_label.h b/ui/views/controls/styled_label.h
index f600159..d74def2 100644
--- a/ui/views/controls/styled_label.h
+++ b/ui/views/controls/styled_label.h
@@ -11,6 +11,7 @@
 #include "base/basictypes.h"
 #include "base/strings/string16.h"
 #include "third_party/skia/include/core/SkColor.h"
+#include "ui/gfx/font_list.h"
 #include "ui/gfx/range/range.h"
 #include "ui/gfx/size.h"
 #include "ui/views/controls/link_listener.h"
@@ -60,6 +61,10 @@ class VIEWS_EXPORT StyledLabel : public View, public LinkListener {
   // Sets the text to be displayed, and clears any previous styling.
   void SetText(const base::string16& text);
 
+  // Sets the fonts used by all labels. Can be augemented by styling set by
+  // AddStyleRange and SetDefaultStyle.
+  void SetBaseFontList(const gfx::FontList& font_list);
+
   // Marks the given range within |text_| with style defined by |style_info|.
   // |range| must be contained in |text_|.
   void AddStyleRange(const gfx::Range& range, const RangeStyleInfo& style_info);
@@ -114,6 +119,9 @@ class VIEWS_EXPORT StyledLabel : public View, public LinkListener {
   // The text to display.
   base::string16 text_;
 
+  // Fonts used to display text. Can be augmented by RangeStyleInfo.
+  gfx::FontList font_list_;
+
   // The default style to use for any part of the text that isn't within
   // a range in |style_ranges_|.
   RangeStyleInfo default_style_info_;
diff --git a/ui/views/controls/styled_label_unittest.cc b/ui/views/controls/styled_label_unittest.cc
index e3d0b02..2200c0b 100644
--- a/ui/views/controls/styled_label_unittest.cc
+++ b/ui/views/controls/styled_label_unittest.cc
@@ -381,6 +381,24 @@ TEST_F(StyledLabelTest, StyledRangeWithTooltip) {
   EXPECT_EQ(ASCIIToUTF16("tooltip"), tooltip);
 }
 
+TEST_F(StyledLabelTest, SetBaseFontList) {
+  const std::string text("This is a test block of text.");
+  InitStyledLabel(text);
+  std::string font_name("arial");
+  gfx::Font font(font_name, 30);
+  styled()->SetBaseFontList(gfx::FontList(font));
+  Label label(ASCIIToUTF16(text), gfx::FontList(font));
+
+  styled()->SetBounds(0,
+                      0,
+                      label.GetPreferredSize().width(),
+                      label.GetPreferredSize().height());
+
+  // Make sure we have the same sizing as a label.
+  EXPECT_EQ(label.GetPreferredSize().height(), styled()->height());
+  EXPECT_EQ(label.GetPreferredSize().width(), styled()->width());
+}
+
 TEST_F(StyledLabelTest, HandleEmptyLayout) {
   const std::string text("This is a test block of text.");
   InitStyledLabel(text);