[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 15 matching lines @@
 #include "grit/generated_resources.h"
 #include "grit/google_chrome_strings.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 {
 
+namespace {
+// The amount of additional space that a link takes up as it has a border if
+// it's focused. TODO(gcasto): Does OSX do this as well?
+const int kLinkBorderSize = 1;
+const int kPasswordSectionHeight = 62;
+}
+
 base::WeakPtr<PasswordGenerationPopupControllerImpl>
 PasswordGenerationPopupControllerImpl::GetOrCreate(
     base::WeakPtr<PasswordGenerationPopupControllerImpl> previous,
     const gfx::RectF& bounds,
     const PasswordForm& form,
     int max_length,
     password_manager::PasswordManager* password_manager,
     PasswordGenerationPopupObserver* observer,
     content::WebContents* web_contents,
     gfx::NativeView container_view) {
@@ skipping 106 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;
 
   // 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) {

[Evan Stade, 2014/06/19 00:16:35]

size calculations belong in view implementations. The size calculations should
be easy; given a hard-coded width, you can lean on GetHeightForWidth() (for
views anyway, I of course have no idea how cocoa works).

[Garrett Casto, 2014/06/20 23:47:00]

One problem with this is that you need to know the height of the popup in order
to determine where on the screen that it's placed, and that code is currently
cross platform. I'm sure we can change this so we can let each implementation
tell us their size before doing actual layout, but that's going to take me a
while to do especially on the Mac side. Are you okay with me adding a bug to
track this and deal with it in a separate CL? I'm hoping to get something
checked in early next week for a cafe study.

[Evan Stade, 2014/06/23 18:20:40]

what platform are you using for the study? cros? Could you do it the right way
on that platform and TODO to fix Mac?

[Garrett Casto, 2014/06/24 01:15:11]

I actually don't know if it's CrOS or Windows. I took a stab at refactoring
this. The actual height calculation is certainly cleaner. It does mean that Mac
is going to not work at all for now, but I suppose it would have been broken in
either case.

-  // 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.
+  // Calculating the height of the help text is complicated by the fact that
+  // we won't add a line break in the middle of a link. Note that this currently
+  // doesn't appropriately wrap at word boundries either, but that is less of
+  // a problem at the moment given the size of the link and it's placement on
+  // the line.
+  // TODO(gcasto): I really wish that there was a better cross platform way of
+  // calculating this, but it's not clear that there is.
+  // gfx::ElideRectangleText() is close, but it doesn't understand embedded
+  // links.
   int total_length = gfx::GetStringWidth(HelpText(), font_list_);
-  int usable_width = width - 2 * kHorizontalPadding;
+  int before_link_length =
+      gfx::GetStringWidth(HelpText().substr(0, HelpTextLinkRange().start()),
+                          font_list_);
+  int after_link_length =
+      gfx::GetStringWidth(HelpText().substr(0, HelpTextLinkRange().end()),
+                          font_list_);
+  // Even though the link will only be on one line, this is how the usable width
+  // is calculated on StyledLabel.
+  int usable_width =
+      width - 2 * kHorizontalPadding - 2 * kLinkBorderSize;
+  int num_lines;
+  if (before_link_length / usable_width != after_link_length / usable_width) {
+    // Link would be wrapped, line break before the link starts.
+    int link_and_after_length =
+        gfx::GetStringWidth(HelpText().substr(HelpTextLinkRange().start()),
+                            font_list_);
+    num_lines = (before_link_length/usable_width +
+                 link_and_after_length/usable_width +
+                 2);
+  } else {
+    // Link shouldn't cross a line break.
+    num_lines = total_length/usable_width + 1;
+  }
+
   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;
+      ((num_lines-1) *

[Evan Stade, 2014/06/19 00:16:35]

spaces around binary operators

[Garrett Casto, 2014/06/20 23:47:00]

Done.

+       (font_list_.GetHeight() +
+        PasswordGenerationPopupView::kHelpSectionAdditionalSpacing)) +
+      font_list_.GetBaseline();
+  // The vertical padding on the last line should be from the base of the
+  // characters not the lowest level of a character, so subtract the descent.
+  int font_descent = font_list_.GetHeight() - font_list_.GetBaseline();
+  int help_section_height =
+      text_height + 2 * kHelpVerticalPadding - font_descent;
 
   int password_section_height = 0;
   if (display_password_) {
-    password_section_height =
-        font_list_.GetFontSize() + 2 * kPasswordVerticalPadding;
+    // Minimum width is wide enought that we shouldn't have to do any text
+    // wrapping in this section.
+    password_section_height = kPasswordSectionHeight;
   }
 
   return (2 * kPopupBorderThickness +
           help_section_height +
           password_section_height);
 }
 
 void PasswordGenerationPopupControllerImpl::CalculateBounds() {
   int popup_width  = GetDesiredWidth();
   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.
   if (display_password_) {
     password_bounds_ =  gfx::Rect(
         kPopupBorderThickness,
         kPopupBorderThickness,
         sub_view_width,
-        font_list_.GetFontSize() + 2 * kPasswordVerticalPadding);
+        kPasswordSectionHeight);
 
     divider_bounds_ = gfx::Rect(kPopupBorderThickness,
                                 password_bounds_.bottom(),
                                 sub_view_width,
                                 1 /* divider heigth*/);
   } else {
     password_bounds_ = gfx::Rect();
     divider_bounds_ = gfx::Rect();
   }
 
@@ skipping 121 matching lines @@
 
 const base::string16& PasswordGenerationPopupControllerImpl::HelpText() {
   return help_text_;
 }
 
 const gfx::Range& PasswordGenerationPopupControllerImpl::HelpTextLinkRange() {
   return link_range_;
 }
 
 }  // namespace autofill