[Password Generation] Update Aura UI

chrome/browser/ui/autofill/password_generation_popup_controller_impl.cc

 // Copyright 2014 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "chrome/browser/ui/autofill/password_generation_popup_controller_impl.h"
 
 #include <math.h>
 
 #include "base/strings/string_split.h"
 #include "base/strings/string_util.h"
@@ skipping 141 matching lines @@
   web_contents()->GetRenderViewHost()->Send(
       new AutofillMsg_GeneratedPasswordAccepted(
           web_contents()->GetRenderViewHost()->GetRoutingID(),
           current_password_));
   password_manager_->SetFormHasGeneratedPassword(form_);
   Hide();
 }
 
 int PasswordGenerationPopupControllerImpl::GetDesiredWidth() {
   // Minimum width in pixels.
-  const int minimum_required_width = 300;
+  const int minimum_required_width = 350;

[Evan Stade, 2014/06/24 21:16:58]

it's ok to have a minimum width defined in some shared location, but the rest of
the calculations in this function belong in the view

[Garrett Casto, 2014/06/24 23:51:30]

The first calculation requires information that is currently only known in
shared code (the size of the element that is hosting this popup). The second
actually doesn't make sense anymore.

[Evan Stade, 2014/06/25 00:02:41]

You could expose a MinimumWidth() method to the views, which does:

return std::max(350, controller_common_.RoundedElementBounds().width());

Doing this also makes the views code cleaner because it can consult the
controller for the minimum width any time it needs to (i.e. during Layout()).

[Garrett Casto, 2014/06/25 19:38:14]

To be clear, you're suggesting that in CalculateBounds(), instead of directly
calling this function, or something like it, I should call view_->GetBound(),
which in turn just returns controller_->MinimumWidth()? I'm not sure that I see
the point, but I can do it if you want. It does change
controller_->popup_bounds().width() to controller_->MinimumWidth() in the view,
but the later doesn't seem obviously better to me.

[Evan Stade, 2014/06/25 21:27:29]

view_->GetBounds() uses controller_->MinimumWidth() to help calculate the
height, yes, but it doesn't just return the width.
In Layout() you should use bounds() (as in this->bounds()).

[Garrett Casto, 2014/06/25 21:54:55]

Done.
 
Done.

 
   // If the width of the field is longer than the minimum, use that instead.
   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) {
-  // Note that this wrapping isn't exactly what the popup will do. It shouldn't
-  // 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(), font_list_);
-  int usable_width = width - 2 * kHorizontalPadding;
-  int text_height =
-      static_cast<int>(ceil(static_cast<double>(total_length)/usable_width)) *
-      font_list_.GetFontSize();
-  int help_section_height = text_height + 2 * kHelpVerticalPadding;
-
-  int password_section_height = 0;
-  if (display_password_) {
-    password_section_height =
-        font_list_.GetFontSize() + 2 * kPasswordVerticalPadding;
-  }
-
-  return (2 * kPopupBorderThickness +
-          help_section_height +
-          password_section_height);
-}
-
 void PasswordGenerationPopupControllerImpl::CalculateBounds() {
   int popup_width  = GetDesiredWidth();
-  int popup_height = GetDesiredHeight(popup_width);
+  int popup_height = view_->SetBoundsForWidth(popup_width);

[Evan Stade, 2014/06/24 21:16:58]

I think this should be GetHeight(). Just calculate bounds, don't actually set
them. Also the desired width is determined in the view so no need for
"ForWidth".

[Garrett Casto, 2014/06/24 23:51:30]

Done.

 
   popup_bounds_ = controller_common_.GetPopupBounds(popup_height, popup_width);

[Evan Stade, 2014/06/25 21:27:29]

width should always come before height >:(

[Garrett Casto, 2014/06/25 21:54:55]

Switched.

[Evan Stade, 2014/06/26 00:06:04]

thank you

-  int sub_view_width = popup_bounds_.width() - 2 * kPopupBorderThickness;
-
-  // Calculate the bounds for the rest of the elements given the bounds of
-  // the popup.
-  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() - help_y - kPopupBorderThickness;
-  help_bounds_ = gfx::Rect(
-      kPopupBorderThickness,
-      help_y,
-      sub_view_width,
-      help_height);
 }
 
 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);
+    CalculateBounds();
     view_->Show();
   } else {
+    CalculateBounds();
     view_->UpdateBoundsAndRedrawPopup();
   }
 
   controller_common_.RegisterKeyPressCallback();
 
   if (observer_)
     observer_->OnPopupShown(display_password_);
 }
 
 void PasswordGenerationPopupControllerImpl::HideAndDestroy() {
@@ skipping 24 matching lines @@
   Browser* browser =
       chrome::FindBrowserWithWebContents(controller_common_.web_contents());
   content::OpenURLParams params(
       GURL(chrome::kAutoPasswordGenerationLearnMoreURL), content::Referrer(),
       NEW_FOREGROUND_TAB, content::PAGE_TRANSITION_LINK, false);
   browser->OpenURL(params);
 }
 
 void PasswordGenerationPopupControllerImpl::SetSelectionAtPoint(
     const gfx::Point& point) {
-  PasswordSelected(password_bounds_.Contains(point));
+  PasswordSelected(view_->IsPointInPasswordBounds(point));
 }
 
 bool PasswordGenerationPopupControllerImpl::AcceptSelectedLine() {
   if (!password_selected_)
     return false;
 
   PasswordAccepted();
   return true;
 }
 
 void PasswordGenerationPopupControllerImpl::SelectionCleared() {
   PasswordSelected(false);
 }
 
 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_;
 }
 
-const gfx::Rect& PasswordGenerationPopupControllerImpl::password_bounds()
-    const {
-  return password_bounds_;
-}
-
-const gfx::Rect& PasswordGenerationPopupControllerImpl::divider_bounds()
-    const {
-  return divider_bounds_;
-}
-
-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_;
 }
 
 base::string16 PasswordGenerationPopupControllerImpl::password() const {
   return current_password_;
 }
 
 base::string16 PasswordGenerationPopupControllerImpl::SuggestedText() {
   return l10n_util::GetStringUTF16(IDS_PASSWORD_GENERATION_SUGGESTION);
 }
 
 const base::string16& PasswordGenerationPopupControllerImpl::HelpText() {
   return help_text_;
 }
 
 const gfx::Range& PasswordGenerationPopupControllerImpl::HelpTextLinkRange() {
   return link_range_;
 }
 
 }  // namespace autofill