AutofillPopupController clarifications + simplifications.

chrome/browser/ui/autofill/autofill_popup_controller_impl.cc

Index: chrome/browser/ui/autofill/autofill_popup_controller_impl.cc
diff --git a/chrome/browser/ui/autofill/autofill_popup_controller_impl.cc b/chrome/browser/ui/autofill/autofill_popup_controller_impl.cc
index d7231c2a20ba59c4a9e538b90a2d9e4c47f6ac35..fc058cb52a79f14de9fda76cbe793cdfbaee91ee 100644
--- a/chrome/browser/ui/autofill/autofill_popup_controller_impl.cc
+++ b/chrome/browser/ui/autofill/autofill_popup_controller_impl.cc
@@ -20,7 +20,7 @@ namespace {
 // Used to indicate that no line is currently selected by the user.
 const int kNoSelection = -1;
 
-// Size difference between value text and label text in pixels.
+// Size difference between name and subtext in pixels.
 const int kLabelFontSizeDelta = -2;
 
 // The vertical height of each row in pixels.
@@ -30,9 +30,9 @@ const size_t kRowHeight = 24;
 const size_t kSeparatorHeight = 1;
 
 // The amount of minimum padding between the Autofill value and label in pixels.

[Ilya Sherman, 2012/12/20 23:11:55]

nit: Please update this comment.

[Evan Stade, 2012/12/20 23:15:01]

Done.

-const size_t kLabelPadding = 15;
+const size_t kNamePadding = 15;
 
-// The maximum amount of characters to display from either the label or value.
+// The maximum amount of characters to display from either the name or subtext.
 const size_t kMaxTextLength = 15;
 
 #if !defined(OS_ANDROID)
@@ -94,7 +94,7 @@ AutofillPopupControllerImpl::AutofillPopupControllerImpl(
       delete_icon_hovered_(false),
       is_hiding_(false) {
 #if !defined(OS_ANDROID)
-  label_font_ = value_font_.DeriveFont(kLabelFontSizeDelta);
+  subtext_font_ = name_font_.DeriveFont(kLabelFontSizeDelta);
 #endif
 }
 
@@ -104,24 +104,24 @@ AutofillPopupControllerImpl::~AutofillPopupControllerImpl() {
 }
 
 void AutofillPopupControllerImpl::Show(
-    const std::vector<string16>& autofill_values,
-    const std::vector<string16>& autofill_labels,
-    const std::vector<string16>& autofill_icons,
-    const std::vector<int>& autofill_unique_ids) {
-  autofill_values_ = autofill_values;
-  autofill_labels_ = autofill_labels;
-  autofill_icons_ = autofill_icons;
-  autofill_unique_ids_ = autofill_unique_ids;
+    const std::vector<string16>& names,
+    const std::vector<string16>& subtexts,
+    const std::vector<string16>& icons,
+    const std::vector<int>& identifiers) {
+  names_ = names;
+  subtexts_ = subtexts;
+  icons_ = icons;
+  identifiers_ = identifiers;
 
 #if !defined(OS_ANDROID)
   // Android displays the long text with ellipsis using the view attributes.
 
   // TODO(csharp): Fix crbug.com/156163 and use better logic when clipping.
-  for (size_t i = 0; i < autofill_values_.size(); ++i) {
-    if (autofill_values_[i].length() > 15)
-      autofill_values_[i].erase(15);
-    if (autofill_labels[i].length() > 15)
-      autofill_labels_[i].erase(15);
+  for (size_t i = 0; i < names_.size(); ++i) {
+    if (names_[i].length() > 15)
+      names_[i].erase(15);
+    if (subtexts[i].length() > 15)
+      subtexts_[i].erase(15);
   }
 #endif
 
@@ -151,10 +151,8 @@ void AutofillPopupControllerImpl::UpdateBoundsAndRedrawPopup() {
   view_->UpdateBoundsAndRedrawPopup();
 }
 
-void AutofillPopupControllerImpl::SetSelectedPosition(int x, int y) {
-  int line = LineFromY(y);
-
-  SetSelectedLine(line);
+void AutofillPopupControllerImpl::MouseHovered(int x, int y) {
+  SetSelectedLine(LineFromY(y));
 
   bool delete_icon_hovered = DeleteIconIsUnder(x, y);
   if (delete_icon_hovered != delete_icon_hovered_) {
@@ -163,26 +161,23 @@ void AutofillPopupControllerImpl::SetSelectedPosition(int x, int y) {
   }
 }
 
-bool AutofillPopupControllerImpl::AcceptAutofillSuggestion(
-    const string16& value,
-    int unique_id,
-    unsigned index) {
-  return delegate_->DidAcceptAutofillSuggestion(value, unique_id, index);
-}
-
-void AutofillPopupControllerImpl::AcceptSelectedPosition(int x, int y) {
-  DCHECK_EQ(selected_line(), LineFromY(y));
+void AutofillPopupControllerImpl::MouseClicked(int x, int y) {
+  MouseHovered(x, y);
 
-  if (DeleteIconIsUnder(x, y))
+  if (delete_icon_hovered_)
     RemoveSelectedLine();
   else
     AcceptSelectedLine();
 }
 
-void AutofillPopupControllerImpl::ClearSelectedLine() {
+void AutofillPopupControllerImpl::MouseExitedPopup() {
   SetSelectedLine(kNoSelection);
 }
 
+void AutofillPopupControllerImpl::AcceptSuggestion(size_t index) {
+  delegate_->DidAcceptSuggestion(names_[index], identifiers_[index]);
+}
+
 int AutofillPopupControllerImpl::GetIconResourceID(
     const string16& resource_name) {
   for (size_t i = 0; i < arraysize(kDataResources); ++i) {
@@ -193,7 +188,8 @@ int AutofillPopupControllerImpl::GetIconResourceID(
   return -1;
 }
 
-bool AutofillPopupControllerImpl::CanDelete(int id) {
+bool AutofillPopupControllerImpl::CanDelete(size_t index) {
+  int id = identifiers_[index];
   return id > 0 ||
       id == WebAutofillClient::MenuItemIDAutocompleteEntry ||
       id == WebAutofillClient::MenuItemIDPasswordEntry;
@@ -201,27 +197,27 @@ bool AutofillPopupControllerImpl::CanDelete(int id) {
 
 #if !defined(OS_ANDROID)
 int AutofillPopupControllerImpl::GetPopupRequiredWidth() {
-  if (value_font_.platform_font() == NULL ||
-      label_font_.platform_font() == NULL) {
+  if (name_font_.platform_font() == NULL ||
+      subtext_font_.platform_font() == NULL) {
     // We can't calculate the size of the popup if the fonts
     // aren't present.
     return 0;
   }
 
   int popup_width = element_bounds().width();
-  DCHECK_EQ(autofill_values().size(), autofill_labels().size());
-  for (size_t i = 0; i < autofill_values().size(); ++i) {
+  DCHECK_EQ(names().size(), subtexts().size());
+  for (size_t i = 0; i < names().size(); ++i) {
     int row_size = kEndPadding +
-        value_font_.GetStringWidth(autofill_values()[i]) +
-        kLabelPadding +
-        label_font_.GetStringWidth(autofill_labels()[i]);
+        name_font_.GetStringWidth(names()[i]) +
+        kNamePadding +
+        subtext_font_.GetStringWidth(subtexts()[i]);
 
     // Add the Autofill icon size, if required.
-    if (!autofill_icons()[i].empty())
+    if (!icons()[i].empty())
       row_size += kAutofillIconWidth + kIconPadding;
 
     // Add delete icon, if required.
-    if (CanDelete(autofill_unique_ids()[i]))
+    if (CanDelete(i))
       row_size += kDeleteIconWidth + kIconPadding;
 
     // Add the padding at the end
@@ -236,29 +232,25 @@ int AutofillPopupControllerImpl::GetPopupRequiredWidth() {
 int AutofillPopupControllerImpl::GetPopupRequiredHeight() {
   int popup_height = 0;
 
-  for (size_t i = 0; i < autofill_unique_ids().size(); ++i) {
-    popup_height += GetRowHeightFromId(autofill_unique_ids()[i]);
+  for (size_t i = 0; i < identifiers().size(); ++i) {
+    popup_height += GetRowHeightFromId(identifiers()[i]);
   }
 
   return popup_height;
 }
 #endif  // !defined(OS_ANDROID)
 
-int AutofillPopupControllerImpl::GetRowHeightFromId(int unique_id) {
-  if (unique_id == WebAutofillClient::MenuItemIDSeparator)
-    return kSeparatorHeight;
-
-  return kRowHeight;
-}
-
-gfx::Rect AutofillPopupControllerImpl::GetRectForRow(size_t row, int width) {
+gfx::Rect AutofillPopupControllerImpl::GetRowBounds(size_t index) {
   int top = 0;
-  for (size_t i = 0; i < row; ++i) {
-    top += GetRowHeightFromId(autofill_unique_ids()[i]);
+  for (size_t i = 0; i < index; ++i) {
+    top += GetRowHeightFromId(identifiers()[i]);
   }
 
   return gfx::Rect(
-      0, top, width, GetRowHeightFromId(autofill_unique_ids()[row]));
+      0,
+      top,
+      popup_bounds_.width(),
+      GetRowHeightFromId(identifiers()[index]));
 }
 
 void AutofillPopupControllerImpl::SetPopupBounds(const gfx::Rect& bounds) {
@@ -278,33 +270,29 @@ const gfx::Rect& AutofillPopupControllerImpl::element_bounds() const {
   return element_bounds_;
 }
 
-const std::vector<string16>& AutofillPopupControllerImpl::
-    autofill_values() const {
-  return autofill_values_;
+const std::vector<string16>& AutofillPopupControllerImpl::names() const {
+  return names_;
 }
 
-const std::vector<string16>& AutofillPopupControllerImpl::
-    autofill_labels() const {
-  return autofill_labels_;
+const std::vector<string16>& AutofillPopupControllerImpl::subtexts() const {
+  return subtexts_;
 }
 
-const std::vector<string16>& AutofillPopupControllerImpl::
-    autofill_icons() const {
-  return autofill_icons_;
+const std::vector<string16>& AutofillPopupControllerImpl::icons() const {
+  return icons_;
 }
 
-const std::vector<int>& AutofillPopupControllerImpl::
-    autofill_unique_ids() const {
-  return autofill_unique_ids_;
+const std::vector<int>& AutofillPopupControllerImpl::identifiers() const {
+  return identifiers_;
 }
 
 #if !defined(OS_ANDROID)
-const gfx::Font& AutofillPopupControllerImpl::label_font() const {
-  return label_font_;
+const gfx::Font& AutofillPopupControllerImpl::name_font() const {
+  return name_font_;
 }
 
-const gfx::Font& AutofillPopupControllerImpl::value_font() const {
-  return value_font_;
+const gfx::Font& AutofillPopupControllerImpl::subtext_font() const {
+  return subtext_font_;
 }
 #endif
 
@@ -329,7 +317,7 @@ bool AutofillPopupControllerImpl::HandleKeyPressEvent(
       SetSelectedLine(0);
       return true;
     case ui::VKEY_NEXT:  // Page down.
-      SetSelectedLine(autofill_values().size() - 1);
+      SetSelectedLine(names().size() - 1);
       return true;
     case ui::VKEY_ESCAPE:
       HideInternal();
@@ -368,8 +356,7 @@ void AutofillPopupControllerImpl::SetSelectedLine(int selected_line) {
   selected_line_ = selected_line;
 
   if (selected_line_ != kNoSelection) {
-    delegate_->SelectAutofillSuggestion(
-        autofill_unique_ids_[selected_line_]);
+    delegate_->DidSelectSuggestion(identifiers_[selected_line_]);
   }
 }
 
@@ -377,12 +364,12 @@ void AutofillPopupControllerImpl::SelectNextLine() {
   int new_selected_line = selected_line_ + 1;
 
   // Skip over any lines that can't be selected.
-  while (static_cast<size_t>(new_selected_line) < autofill_values_.size() &&
-         !CanAccept(autofill_unique_ids()[new_selected_line])) {
+  while (static_cast<size_t>(new_selected_line) < names_.size() &&
+         !CanAccept(identifiers()[new_selected_line])) {
     ++new_selected_line;
   }
 
-  if (new_selected_line == static_cast<int>(autofill_values_.size()))
+  if (new_selected_line == static_cast<int>(names_.size()))
     new_selected_line = 0;
 
   SetSelectedLine(new_selected_line);
@@ -393,12 +380,12 @@ void AutofillPopupControllerImpl::SelectPreviousLine() {
 
   // Skip over any lines that can't be selected.
   while (new_selected_line > kNoSelection &&
-         !CanAccept(autofill_unique_ids()[new_selected_line])) {
+         !CanAccept(identifiers()[new_selected_line])) {
     --new_selected_line;
   }
 
   if (new_selected_line <= kNoSelection)
-    new_selected_line = autofill_values_.size() - 1;
+    new_selected_line = names_.size() - 1;
 
   SetSelectedLine(new_selected_line);
 }
@@ -408,15 +395,13 @@ bool AutofillPopupControllerImpl::AcceptSelectedLine() {
     return false;
 
   DCHECK_GE(selected_line_, 0);
-  DCHECK_LT(selected_line_, static_cast<int>(autofill_values_.size()));
+  DCHECK_LT(selected_line_, static_cast<int>(names_.size()));
 
-  if (!CanAccept(autofill_unique_ids_[selected_line_]))
+  if (!CanAccept(identifiers_[selected_line_]))
     return false;
 
-  return AcceptAutofillSuggestion(
-      autofill_values_[selected_line_],
-      autofill_unique_ids_[selected_line_],
-      selected_line_);
+  AcceptSuggestion(selected_line_);
+  return true;
 }
 
 bool AutofillPopupControllerImpl::RemoveSelectedLine() {
@@ -424,24 +409,19 @@ bool AutofillPopupControllerImpl::RemoveSelectedLine() {
     return false;
 
   DCHECK_GE(selected_line_, 0);
-  DCHECK_LT(selected_line_, static_cast<int>(autofill_values_.size()));
+  DCHECK_LT(selected_line_, static_cast<int>(names_.size()));
 
-  if (!CanDelete(autofill_unique_ids_[selected_line_]))
+  if (!CanDelete(selected_line_))
     return false;
 
-  if (autofill_unique_ids_[selected_line_] > 0) {
-    delegate_->RemoveAutofillProfileOrCreditCard(
-        autofill_unique_ids_[selected_line_]);
-  } else {
-    delegate_->RemoveAutocompleteEntry(
-        autofill_values_[selected_line_]);
-  }
+  delegate_->RemoveSuggestion(names_[selected_line_],
+                              identifiers_[selected_line_]);
 
   // Remove the deleted element.
-  autofill_values_.erase(autofill_values_.begin() + selected_line_);
-  autofill_labels_.erase(autofill_labels_.begin() + selected_line_);
-  autofill_icons_.erase(autofill_icons_.begin() + selected_line_);
-  autofill_unique_ids_.erase(autofill_unique_ids_.begin() + selected_line_);
+  names_.erase(names_.begin() + selected_line_);
+  subtexts_.erase(subtexts_.begin() + selected_line_);
+  icons_.erase(icons_.begin() + selected_line_);
+  identifiers_.erase(identifiers_.begin() + selected_line_);
 
   SetSelectedLine(kNoSelection);
 
@@ -458,15 +438,22 @@ bool AutofillPopupControllerImpl::RemoveSelectedLine() {
 int AutofillPopupControllerImpl::LineFromY(int y) {
   int current_height = 0;
 
-  for (size_t i = 0; i < autofill_unique_ids().size(); ++i) {
-    current_height += GetRowHeightFromId(autofill_unique_ids()[i]);
+  for (size_t i = 0; i < identifiers().size(); ++i) {
+    current_height += GetRowHeightFromId(identifiers()[i]);
 
     if (y <= current_height)
       return i;
   }
 
   // The y value goes beyond the popup so stop the selection at the last line.
-  return autofill_unique_ids().size() - 1;
+  return identifiers().size() - 1;
+}
+
+int AutofillPopupControllerImpl::GetRowHeightFromId(int identifier) {
+  if (identifier == WebAutofillClient::MenuItemIDSeparator)
+    return kSeparatorHeight;
+
+  return kRowHeight;
 }
 
 bool AutofillPopupControllerImpl::DeleteIconIsUnder(int x, int y) {
@@ -478,7 +465,7 @@ bool AutofillPopupControllerImpl::DeleteIconIsUnder(int x, int y) {
 
   int row_start_y = 0;
   for (int i = 0; i < selected_line(); ++i) {
-    row_start_y += GetRowHeightFromId(autofill_unique_ids()[i]);
+    row_start_y += GetRowHeightFromId(identifiers()[i]);
   }
 
   gfx::Rect delete_icon_bounds = gfx::Rect(
@@ -492,16 +479,17 @@ bool AutofillPopupControllerImpl::DeleteIconIsUnder(int x, int y) {
 }
 
 bool AutofillPopupControllerImpl::CanAccept(int id) {
-  return id != WebAutofillClient::MenuItemIDSeparator;
+  return id != WebAutofillClient::MenuItemIDSeparator &&
+      id != WebAutofillClient::MenuItemIDWarningMessage;
 }
 
 bool AutofillPopupControllerImpl::HasAutofillEntries() {
-  return autofill_values_.size() != 0 &&
-      (autofill_unique_ids_[0] > 0 ||
-       autofill_unique_ids_[0] ==
+  return names_.size() != 0 &&
+      (identifiers_[0] > 0 ||
+       identifiers_[0] ==
            WebAutofillClient::MenuItemIDAutocompleteEntry ||
-       autofill_unique_ids_[0] == WebAutofillClient::MenuItemIDPasswordEntry ||
-       autofill_unique_ids_[0] == WebAutofillClient::MenuItemIDDataListEntry);
+       identifiers_[0] == WebAutofillClient::MenuItemIDPasswordEntry ||
+       identifiers_[0] == WebAutofillClient::MenuItemIDDataListEntry);
 }
 
 void AutofillPopupControllerImpl::ShowView() {