[Autofill] Refactor AutofillPopupControllerImpl and AutofillPopupBaseView

This CL attempts to do the following:-
- Move methods unused for password generation from AutofillPopupViewDelegate to AutofillPopupController
- Remove elided values from AutofillPopupBaseView and its subclasses

Bug: 1045401
Change-Id: I1200513085c72d75c0fbe63a1d08d82f3e257929
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2071937
Commit-Queue: Vidhan Jain <vidhanj@google.com>
Reviewed-by: Vasilii Sukhanov <vasilii@chromium.org>
Cr-Commit-Position: refs/heads/master@{#746773}

chrome/browser/autofill/mock_autofill_popup_controller.cc

@@ -9,7 +9,7 @@ namespace autofill {
 MockAutofillPopupController::MockAutofillPopupController() {
   gfx::FontList::SetDefaultFontDescription("Arial, Times New Roman, 15px");
   layout_model_ = std::make_unique<autofill::AutofillPopupLayoutModel>(
-      this, false /* is_credit_card_field */);
+      false /* is_credit_card_field */);
 }
 
 MockAutofillPopupController::~MockAutofillPopupController() = default;

chrome/browser/autofill/mock_autofill_popup_controller.h

@@ -40,17 +40,13 @@ class MockAutofillPopupController
     return *bounds;
   }
   MOCK_CONST_METHOD0(IsRTL, bool());
-  const std::vector<autofill::Suggestion> GetSuggestions() override {
-    return suggestions_;
-  }
-#if !defined(OS_ANDROID)
-  MOCK_METHOD1(GetElidedValueWidthForRow, int(int row));
-  MOCK_METHOD1(GetElidedLabelWidthForRow, int(int row));
-#endif
 
   // AutofillPopupController
   MOCK_METHOD0(OnSuggestionsChanged, void());
   MOCK_METHOD1(AcceptSuggestion, void(int index));
+  const std::vector<autofill::Suggestion> GetSuggestions() override {
+    return suggestions_;
+  }
 
   int GetLineCount() const override { return suggestions_.size(); }
 

chrome/browser/ui/autofill/autofill_popup_controller.h

@@ -35,6 +35,9 @@ class AutofillPopupController : public AutofillPopupViewDelegate {
   // Returns the number of lines of data that there are.
   virtual int GetLineCount() const = 0;
 
+  // Returns the full set of autofill suggestions, if applicable.
+  virtual const std::vector<autofill::Suggestion> GetSuggestions() = 0;
+
   // Returns the suggestion or pre-elided string at the given row index.
   virtual const autofill::Suggestion& GetSuggestionAt(int row) const = 0;
   virtual const base::string16& GetElidedValueAt(int row) const = 0;

chrome/browser/ui/autofill/autofill_popup_controller_impl.cc

@@ -79,22 +79,20 @@ AutofillPopupControllerImpl::AutofillPopupControllerImpl(
     base::i18n::TextDirection text_direction)
     : controller_common_(element_bounds, text_direction, container_view),
       web_contents_(web_contents),
-      layout_model_(this, delegate->GetPopupType() == PopupType::kCreditCards),
+      layout_model_(delegate->GetPopupType() == PopupType::kCreditCards),
       delegate_(delegate) {
   ClearState();
   delegate->RegisterDeletionCallback(base::BindOnce(
       &AutofillPopupControllerImpl::HideViewAndDie, GetWeakPtr()));
 }
 
-AutofillPopupControllerImpl::~AutofillPopupControllerImpl() {}
+AutofillPopupControllerImpl::~AutofillPopupControllerImpl() = default;
 
 void AutofillPopupControllerImpl::Show(
     const std::vector<Suggestion>& suggestions,
     bool autoselect_first_suggestion,
     PopupType popup_type) {
   SetValues(suggestions);
-  DCHECK_EQ(suggestions_.size(), elided_values_.size());
-  DCHECK_EQ(suggestions_.size(), elided_labels_.size());
 
   bool just_created = false;
   if (!view_) {
@@ -110,20 +108,6 @@ void AutofillPopupControllerImpl::Show(
     just_created = true;
   }
 
-#if !defined(OS_ANDROID)
-  // Android displays the long text with ellipsis using the view attributes.
-
-  layout_model_.UpdatePopupBounds();
-
-  // Elide the name and label strings so that the popup fits in the available
-  // space.
-  for (int i = 0; i < GetLineCount(); ++i) {
-    bool has_label = !suggestions_[i].label.empty();
-    ElideValueAndLabelForRow(
-        i, layout_model_.GetAvailableWidthForRow(i, has_label));
-  }
-#endif
-
   if (just_created) {
 #if defined(OS_ANDROID)
     ManualFillingController::GetOrCreate(web_contents_)
@@ -151,16 +135,11 @@ void AutofillPopupControllerImpl::Show(
                      base::Unretained(this)));
   pinned_until_update_ = false;
   delegate_->OnPopupShown();
-
-  DCHECK_EQ(suggestions_.size(), elided_values_.size());
-  DCHECK_EQ(suggestions_.size(), elided_labels_.size());
 }
 
 void AutofillPopupControllerImpl::UpdateDataListValues(
     const std::vector<base::string16>& values,
     const std::vector<base::string16>& labels) {
-  DCHECK_EQ(suggestions_.size(), elided_values_.size());
-  DCHECK_EQ(suggestions_.size(), elided_labels_.size());
 
   selected_line_.reset();
   // Remove all the old data list values, which should always be at the top of
@@ -168,8 +147,6 @@ void AutofillPopupControllerImpl::UpdateDataListValues(
   while (!suggestions_.empty() &&
          suggestions_[0].frontend_id == POPUP_ITEM_ID_DATALIST_ENTRY) {
     suggestions_.erase(suggestions_.begin());
-    elided_values_.erase(elided_values_.begin());
-    elided_labels_.erase(elided_labels_.begin());
   }
 
   // If there are no new data list values, exit (clearing the separator if there
@@ -178,8 +155,6 @@ void AutofillPopupControllerImpl::UpdateDataListValues(
     if (!suggestions_.empty() &&
         suggestions_[0].frontend_id == POPUP_ITEM_ID_SEPARATOR) {
       suggestions_.erase(suggestions_.begin());
-      elided_values_.erase(elided_values_.begin());
-      elided_labels_.erase(elided_labels_.begin());
     }
 
      // The popup contents have changed, so either update the bounds or hide it.
@@ -196,29 +171,17 @@ void AutofillPopupControllerImpl::UpdateDataListValues(
       suggestions_[0].frontend_id != POPUP_ITEM_ID_SEPARATOR) {
     suggestions_.insert(suggestions_.begin(), Suggestion());
     suggestions_[0].frontend_id = POPUP_ITEM_ID_SEPARATOR;
-    elided_values_.insert(elided_values_.begin(), base::string16());
-    elided_labels_.insert(elided_labels_.begin(), base::string16());
   }
 
   // Prepend the parameters to the suggestions we already have.
   suggestions_.insert(suggestions_.begin(), values.size(), Suggestion());
-  elided_values_.insert(elided_values_.begin(), values.size(),
-                        base::string16());
-  elided_labels_.insert(elided_labels_.begin(), values.size(),
-                        base::string16());
   for (size_t i = 0; i < values.size(); i++) {
     suggestions_[i].value = values[i];
     suggestions_[i].label = labels[i];
     suggestions_[i].frontend_id = POPUP_ITEM_ID_DATALIST_ENTRY;
-
-    // TODO(brettw) it looks like these should be elided.
-    elided_values_[i] = values[i];
-    elided_labels_[i] = labels[i];
   }
 
   OnSuggestionsChanged();
-  DCHECK_EQ(suggestions_.size(), elided_values_.size());
-  DCHECK_EQ(suggestions_.size(), elided_labels_.size());
 }
 
 void AutofillPopupControllerImpl::PinViewUntilUpdate() {
@@ -294,13 +257,7 @@ bool AutofillPopupControllerImpl::HandleKeyPressEvent(
 }
 
 void AutofillPopupControllerImpl::OnSuggestionsChanged() {
-#if !defined(OS_ANDROID)
-  // TODO(csharp): Since UpdatePopupBounds can change the position of the popup,
-  // the popup could end up jumping from above the element to below it.
-  // It is unclear if it is better to keep the popup where it was, or if it
-  // should try and move to its desired position.
-  layout_model_.UpdatePopupBounds();
-#else
+#if defined(OS_ANDROID)
   // Assume that suggestions are (still) available. If this is wrong, the method
   // |HideViewAndDie| will be called soon after and will hide all suggestions.
   ManualFillingController::GetOrCreate(web_contents_)
@@ -351,18 +308,6 @@ const std::vector<Suggestion> AutofillPopupControllerImpl::GetSuggestions() {
   return suggestions_;
 }
 
-#if !defined(OS_ANDROID)
-int AutofillPopupControllerImpl::GetElidedValueWidthForRow(int row) {
-  return gfx::GetStringWidth(GetElidedValueAt(row),
-                             layout_model_.GetValueFontListForRow(row));
-}
-
-int AutofillPopupControllerImpl::GetElidedLabelWidthForRow(int row) {
-  return gfx::GetStringWidth(GetElidedLabelAt(row),
-                             layout_model_.GetLabelFontListForRow(row));
-}
-#endif
-
 int AutofillPopupControllerImpl::GetLineCount() const {
   return suggestions_.size();
 }
@@ -373,12 +318,12 @@ const Suggestion& AutofillPopupControllerImpl::GetSuggestionAt(int row) const {
 
 const base::string16& AutofillPopupControllerImpl::GetElidedValueAt(
     int row) const {
-  return elided_values_[row];
+  return suggestions_[row].value;
 }
 
 const base::string16& AutofillPopupControllerImpl::GetElidedLabelAt(
     int row) const {
-  return elided_labels_[row];
+  return suggestions_[row].label;
 }
 
 bool AutofillPopupControllerImpl::GetRemovalConfirmationText(
@@ -398,8 +343,6 @@ bool AutofillPopupControllerImpl::RemoveSuggestion(int list_index) {
 
   // Remove the deleted element.
   suggestions_.erase(suggestions_.begin() + list_index);
-  elided_values_.erase(elided_values_.begin() + list_index);
-  elided_labels_.erase(elided_labels_.begin() + list_index);
 
   selected_line_.reset();
 
@@ -503,46 +446,12 @@ bool AutofillPopupControllerImpl::HasSuggestions() {
 void AutofillPopupControllerImpl::SetValues(
     const std::vector<Suggestion>& suggestions) {
   suggestions_ = suggestions;
-  elided_values_.resize(suggestions.size());
-  elided_labels_.resize(suggestions.size());
-  for (size_t i = 0; i < suggestions.size(); i++) {
-    elided_values_[i] = suggestions[i].value;
-    elided_labels_[i] = suggestions[i].label;
-  }
 }
 
 WeakPtr<AutofillPopupControllerImpl> AutofillPopupControllerImpl::GetWeakPtr() {
   return weak_ptr_factory_.GetWeakPtr();
 }
 
-#if !defined(OS_ANDROID)
-void AutofillPopupControllerImpl::ElideValueAndLabelForRow(
-    int row,
-    int available_width) {
-  int value_width = gfx::GetStringWidth(
-      suggestions_[row].value, layout_model_.GetValueFontListForRow(row));
-  int label_width = gfx::GetStringWidth(
-      suggestions_[row].label, layout_model_.GetLabelFontListForRow(row));
-  int total_text_length = value_width + label_width;
-
-  // The line can have no strings if it represents a UI element, such as
-  // a separator line.
-  if (total_text_length == 0)
-    return;
-
-  // Each field receives space in proportion to its length.
-  int value_size = available_width * value_width / total_text_length;
-  elided_values_[row] = gfx::ElideText(
-      suggestions_[row].value, layout_model_.GetValueFontListForRow(row),
-      value_size, gfx::ELIDE_TAIL);
-
-  int label_size = available_width * label_width / total_text_length;
-  elided_labels_[row] = gfx::ElideText(
-      suggestions_[row].label, layout_model_.GetLabelFontListForRow(row),
-      label_size, gfx::ELIDE_TAIL);
-}
-#endif
-
 bool AutofillPopupControllerImpl::AcceptSelectedLine() {
   if (!selected_line_)
     return false;
@@ -560,8 +469,6 @@ void AutofillPopupControllerImpl::ClearState() {
   // Don't clear view_, because otherwise the popup will have to get regenerated
   // and this will cause flickering.
   suggestions_.clear();
-  elided_values_.clear();
-  elided_labels_.clear();
 
   selected_line_.reset();
 }

chrome/browser/ui/autofill/autofill_popup_controller_impl.h

@@ -96,10 +96,6 @@ class AutofillPopupControllerImpl : public AutofillPopupController {
   void SetElementBounds(const gfx::RectF& bounds);
   bool IsRTL() const override;
   const std::vector<Suggestion> GetSuggestions() override;
-#if !defined(OS_ANDROID)
-  int GetElidedValueWidthForRow(int row) override;
-  int GetElidedLabelWidthForRow(int row) override;
-#endif
 
   // AutofillPopupController implementation.
   void OnSuggestionsChanged() override;
@@ -151,14 +147,6 @@ class AutofillPopupControllerImpl : public AutofillPopupController {
   virtual ui::AXPlatformNode* GetRootAXPlatformNodeForWebContents();
 
  private:
-#if !defined(OS_ANDROID)
-  FRIEND_TEST_ALL_PREFIXES(AutofillPopupControllerUnitTest, ElideText);
-  // Helper method which elides the value and label for the suggestion at |row|
-  // given the |available_width|. Puts the results in |elided_values_| and
-  // |elided_labels_|.
-  void ElideValueAndLabelForRow(int row, int available_width);
-#endif
-
   // The user has accepted the currently selected line. Returns whether there
   // was a selection to accept.
   bool AcceptSelectedLine();
@@ -187,11 +175,6 @@ class AutofillPopupControllerImpl : public AutofillPopupController {
   // The current Autofill query values.
   std::vector<Suggestion> suggestions_;
 
-  // Elided values and labels corresponding to the suggestions_ vector to
-  // ensure that it fits on the screen.
-  std::vector<base::string16> elided_values_;
-  std::vector<base::string16> elided_labels_;
-
   // The line that is currently selected by the user, null indicates that no
   // line is currently selected.
   base::Optional<int> selected_line_;

chrome/browser/ui/autofill/autofill_popup_controller_unittest.cc

@@ -710,47 +710,6 @@ TEST_F(AutofillPopupControllerUnitTest, DontHideWhenWaitingForData) {
   Mock::VerifyAndClearExpectations(autofill_popup_view());
 }
 
-#if !defined(OS_ANDROID)
-TEST_F(AutofillPopupControllerUnitTest, ElideText) {
-  std::vector<Suggestion> suggestions;
-  suggestions.push_back(
-      Suggestion("Text that will need to be trimmed",
-      "Label that will be trimmed", "genericCC", 0));
-  suggestions.push_back(
-      Suggestion("untrimmed", "Untrimmed", "genericCC", 0));
-
-  autofill_popup_controller_->SetValues(suggestions);
-
-  // Ensure the popup will be too small to display all of the first row.
-  int popup_max_width =
-      gfx::GetStringWidth(
-          suggestions[0].value,
-          autofill_popup_controller_->layout_model().GetValueFontListForRow(
-              0)) +
-      gfx::GetStringWidth(
-          suggestions[0].label,
-          autofill_popup_controller_->layout_model().GetLabelFontListForRow(
-              0)) -
-      25;
-
-  autofill_popup_controller_->ElideValueAndLabelForRow(0, popup_max_width);
-
-  // The first element was long so it should have been trimmed.
-  EXPECT_NE(autofill_popup_controller_->GetSuggestionAt(0).value,
-            autofill_popup_controller_->GetElidedValueAt(0));
-  EXPECT_NE(autofill_popup_controller_->GetSuggestionAt(0).label,
-            autofill_popup_controller_->GetElidedLabelAt(0));
-
-  autofill_popup_controller_->ElideValueAndLabelForRow(1, popup_max_width);
-
-  // The second element was shorter so it should be unchanged.
-  EXPECT_EQ(autofill_popup_controller_->GetSuggestionAt(1).value,
-            autofill_popup_controller_->GetElidedValueAt(1));
-  EXPECT_EQ(autofill_popup_controller_->GetSuggestionAt(1).label,
-            autofill_popup_controller_->GetElidedLabelAt(1));
-}
-#endif
-
 #if !defined(OS_CHROMEOS)
 TEST_F(AutofillPopupControllerAccessibilityUnitTest, FireControlsChangedEvent) {
   StrictMock<MockAxPlatformNodeDelegate> mock_ax_platform_node_delegate;

chrome/browser/ui/autofill/autofill_popup_layout_model.cc

@@ -5,6 +5,9 @@
 #include "chrome/browser/ui/autofill/autofill_popup_layout_model.h"
 
 #include <algorithm>
+#include <memory>
+#include <utility>
+#include <vector>
 
 #include "base/macros.h"
 #include "base/stl_util.h"
@@ -16,17 +19,12 @@
 #include "chrome/browser/ui/autofill/popup_constants.h"
 #include "components/autofill/core/browser/data_model/credit_card.h"
 #include "components/autofill/core/browser/ui/popup_item_ids.h"
-#include "components/autofill/core/browser/ui/suggestion.h"
 #include "components/autofill/core/common/autofill_features.h"
 #include "components/autofill/core/common/autofill_util.h"
 #include "components/grit/components_scaled_resources.h"
 #include "components/strings/grit/components_strings.h"
 #include "third_party/skia/include/core/SkColor.h"
 #include "ui/base/resource/resource_bundle.h"
-#include "ui/gfx/color_palette.h"
-#include "ui/gfx/color_utils.h"
-#include "ui/gfx/font_list.h"
-#include "ui/gfx/geometry/rect_conversions.h"
 #include "ui/gfx/image/image_skia.h"
 #include "ui/gfx/paint_vector_icon.h"
 
@@ -39,16 +37,7 @@ namespace autofill {
 
 namespace {
 
-// The vertical height of each row in pixels.
-const size_t kRowHeight = 24;
-
-// The vertical height of a separator in pixels.
-const size_t kSeparatorHeight = 1;
-
 #if !defined(OS_ANDROID)
-// Size difference between the normal font and the smaller font, in pixels.
-const int kSmallerFontSizeDelta = -1;
-
 // Default sice for icons in the autofill popup.
 constexpr int kIconSize = 16;
 #endif
@@ -91,174 +80,33 @@ const struct {
 #endif  // BUILDFLAG(GOOGLE_CHROME_BRANDING)
 };
 
-int GetRowHeightFromId(int identifier) {
-  if (identifier == POPUP_ITEM_ID_SEPARATOR)
-    return kSeparatorHeight;
-
-  return kRowHeight;
-}
-
 }  // namespace
 
-AutofillPopupLayoutModel::AutofillPopupLayoutModel(
-    AutofillPopupViewDelegate* delegate,
-    bool is_credit_card_popup)
-    : delegate_(delegate), is_credit_card_popup_(is_credit_card_popup) {
-#if !defined(OS_ANDROID)
-  smaller_font_list_ =
-      normal_font_list_.DeriveWithSizeDelta(kSmallerFontSizeDelta);
-  bold_font_list_ = normal_font_list_.DeriveWithWeight(gfx::Font::Weight::BOLD);
-  view_common_ = std::make_unique<PopupViewCommon>();
-#endif
-}
+AutofillPopupLayoutModel::AutofillPopupLayoutModel(bool is_credit_card_popup)
+    : is_credit_card_popup_(is_credit_card_popup) {}
 
-AutofillPopupLayoutModel::~AutofillPopupLayoutModel() {}
+AutofillPopupLayoutModel::~AutofillPopupLayoutModel() = default;
 
 #if !defined(OS_ANDROID)
-int AutofillPopupLayoutModel::GetDesiredPopupHeight() const {
-  std::vector<autofill::Suggestion> suggestions = delegate_->GetSuggestions();
-  int popup_height = 2 * kPopupBorderThickness;
-
-  for (size_t i = 0; i < suggestions.size(); ++i) {
-    popup_height += GetRowHeightFromId(suggestions[i].frontend_id);
-  }
-
-  return popup_height;
-}
-
-int AutofillPopupLayoutModel::GetDesiredPopupWidth() const {
-  std::vector<autofill::Suggestion> suggestions = delegate_->GetSuggestions();
-
-  int popup_width = RoundedElementBounds().width();
-
-  for (size_t i = 0; i < suggestions.size(); ++i) {
-    int label_size = delegate_->GetElidedLabelWidthForRow(i);
-    int row_size = delegate_->GetElidedValueWidthForRow(i) + label_size +
-                   RowWidthWithoutText(i, /* has_subtext= */ label_size > 0);
-
-    popup_width = std::max(popup_width, row_size);
-  }
-
-  return popup_width;
-}
-
-int AutofillPopupLayoutModel::RowWidthWithoutText(int row,
-                                                  bool has_subtext) const {
-  std::vector<autofill::Suggestion> suggestions = delegate_->GetSuggestions();
-  int row_size = 2 * (kEndPadding + kPopupBorderThickness);
-  if (has_subtext)
-    row_size += kNamePadding;
-
-  // Add the Autofill icon size, if required.
-  const std::string& icon = suggestions[row].icon;
-  if (!icon.empty()) {
-    row_size += GetIconImage(row).width() + kIconPadding;
-  }
-  return row_size;
-}
-
-int AutofillPopupLayoutModel::GetAvailableWidthForRow(int row,
-                                                      bool has_subtext) const {
-  return popup_bounds_.width() - RowWidthWithoutText(row, has_subtext);
-}
-
-void AutofillPopupLayoutModel::UpdatePopupBounds() {
-  int popup_width = GetDesiredPopupWidth();
-  int popup_height = GetDesiredPopupHeight();
-
-  popup_bounds_ = view_common_->CalculatePopupBounds(
-      popup_width, popup_height, RoundedElementBounds(),
-      delegate_->container_view(), delegate_->IsRTL());
-}
+// static
+gfx::ImageSkia AutofillPopupLayoutModel::GetIconImage(
+    const autofill::Suggestion& suggestion) {
+  if (!suggestion.custom_icon.IsEmpty())
+    return suggestion.custom_icon.AsImageSkia();
 
-const gfx::FontList& AutofillPopupLayoutModel::GetValueFontListForRow(
-    size_t index) const {
-  std::vector<autofill::Suggestion> suggestions = delegate_->GetSuggestions();
-
-  // Autofill values have positive |frontend_id|.
-  if (suggestions[index].frontend_id > 0)
-    return bold_font_list_;
-
-  // All other message types are defined here.
-  PopupItemId id = static_cast<PopupItemId>(suggestions[index].frontend_id);
-  switch (id) {
-    case POPUP_ITEM_ID_INSECURE_CONTEXT_PAYMENT_DISABLED_MESSAGE:
-    case POPUP_ITEM_ID_CLEAR_FORM:
-    case POPUP_ITEM_ID_CREDIT_CARD_SIGNIN_PROMO:
-    case POPUP_ITEM_ID_AUTOFILL_OPTIONS:
-    case POPUP_ITEM_ID_CREATE_HINT:
-    case POPUP_ITEM_ID_SCAN_CREDIT_CARD:
-    case POPUP_ITEM_ID_SEPARATOR:
-    case POPUP_ITEM_ID_LOADING_SPINNER:
-    case POPUP_ITEM_ID_TITLE:
-    case POPUP_ITEM_ID_PASSWORD_ENTRY:
-    case POPUP_ITEM_ID_ALL_SAVED_PASSWORDS_ENTRY:
-    case POPUP_ITEM_ID_HIDE_AUTOFILL_SUGGESTIONS:
-    case POPUP_ITEM_ID_GENERATE_PASSWORD_ENTRY:
-    case POPUP_ITEM_ID_SHOW_ACCOUNT_CARDS:
-    case POPUP_ITEM_ID_PASSWORD_ACCOUNT_STORAGE_OPTIN:
-    case POPUP_ITEM_ID_USE_VIRTUAL_CARD:
-    case POPUP_ITEM_ID_ONE_TIME_CODE:
-      return normal_font_list_;
-    case POPUP_ITEM_ID_AUTOCOMPLETE_ENTRY:
-    case POPUP_ITEM_ID_DATALIST_ENTRY:
-    case POPUP_ITEM_ID_USERNAME_ENTRY:
-      return bold_font_list_;
-  }
-  NOTREACHED();
-  return normal_font_list_;
-}
-
-const gfx::FontList& AutofillPopupLayoutModel::GetLabelFontListForRow(
-    size_t index) const {
-  return smaller_font_list_;
-}
-
-gfx::ImageSkia AutofillPopupLayoutModel::GetIconImage(size_t index) const {
-  std::vector<autofill::Suggestion> suggestions = delegate_->GetSuggestions();
-  if (!suggestions[index].custom_icon.IsEmpty())
-    return suggestions[index].custom_icon.AsImageSkia();
-
-  return GetIconImageByName(suggestions[index].icon);
+  return GetIconImageByName(suggestion.icon);
 }
 
+// static
 gfx::ImageSkia AutofillPopupLayoutModel::GetStoreIndicatorIconImage(
-    size_t index) const {
-  return GetIconImageByName(
-      delegate_->GetSuggestions()[index].store_indicator_icon);
+    const autofill::Suggestion& suggestion) {
+  return GetIconImageByName(suggestion.store_indicator_icon);
 }
 #endif  // !defined(OS_ANDROID)
 
-int AutofillPopupLayoutModel::LineFromY(int y) const {
-  std::vector<autofill::Suggestion> suggestions = delegate_->GetSuggestions();
-  int current_height = kPopupBorderThickness;
-
-  for (size_t i = 0; i < suggestions.size(); ++i) {
-    current_height += GetRowHeightFromId(suggestions[i].frontend_id);
-
-    if (y <= current_height)
-      return i;
-  }
-
-  // The y value goes beyond the popup so stop the selection at the last line.
-  return suggestions.size() - 1;
-}
-
-gfx::Rect AutofillPopupLayoutModel::GetRowBounds(size_t index) const {
-  std::vector<autofill::Suggestion> suggestions = delegate_->GetSuggestions();
-
-  int top = kPopupBorderThickness;
-  for (size_t i = 0; i < index; ++i) {
-    top += GetRowHeightFromId(suggestions[i].frontend_id);
-  }
-
-  return gfx::Rect(kPopupBorderThickness, top,
-                   popup_bounds_.width() - 2 * kPopupBorderThickness,
-                   GetRowHeightFromId(suggestions[index].frontend_id));
-}
-
+// static
 int AutofillPopupLayoutModel::GetIconResourceID(
-    const std::string& resource_name) const {
+    const std::string& resource_name) {
 #if !BUILDFLAG(GOOGLE_CHROME_BRANDING)
   if (resource_name == "googlePay" || resource_name == "googlePayDark") {
     return 0;
@@ -280,13 +128,10 @@ void AutofillPopupLayoutModel::SetUpForTesting(
   view_common_ = std::move(view_common);
 }
 
-gfx::Rect AutofillPopupLayoutModel::RoundedElementBounds() const {
-  return gfx::ToEnclosingRect(delegate_->element_bounds());
-}
-
 #if !defined(OS_ANDROID)
+// static
 gfx::ImageSkia AutofillPopupLayoutModel::GetIconImageByName(
-    const std::string& icon_str) const {
+    const std::string& icon_str) {
   if (icon_str.empty())
     return gfx::ImageSkia();
 

chrome/browser/ui/autofill/autofill_popup_layout_model.h

@@ -7,10 +7,11 @@
 
 #include <stddef.h>
 
+#include <memory>
 #include <string>
 
-#include "chrome/browser/ui/autofill/autofill_popup_view_delegate.h"
 #include "chrome/browser/ui/autofill/popup_view_common.h"
+#include "components/autofill/core/browser/ui/suggestion.h"
 #include "ui/gfx/font_list.h"
 #include "ui/gfx/geometry/rect.h"
 
@@ -24,100 +25,36 @@ namespace autofill {
 // TODO(mathp): investigate moving ownership of this class to the view.
 class AutofillPopupLayoutModel {
  public:
-  AutofillPopupLayoutModel(AutofillPopupViewDelegate* delegate,
-                           bool is_credit_card_popup);
+  explicit AutofillPopupLayoutModel(bool is_credit_card_popup);
 
   ~AutofillPopupLayoutModel();
 
-  // The minimum amount of padding between the Autofill name and subtext,
-  // in dip.
-  static const int kNamePadding = 15;
-
-  // The amount of padding around icons in dip.
-  static const int kIconPadding = 5;
-
-  // The amount of horizontal padding around icons in dip for the case, when
-  // icon is located on the left side.
-  static const int kPaddingAfterLeadingIcon = 8;
-
-  // The amount of padding at the end of the popup in dip.
-  static const int kEndPadding = 8;
-
 #if !defined(OS_ANDROID)
-  // Calculates the desired height of the popup based on its contents.
-  int GetDesiredPopupHeight() const;
-
-  // Calculates the desired width of the popup based on its contents.
-  int GetDesiredPopupWidth() const;
-
-  // Calculate the width of the row, excluding all the text. This provides
-  // the size of the row that won't be reducible (since all the text can be
-  // elided if there isn't enough space). |with_label| indicates whether a label
-  // is expected to be present.
-  int RowWidthWithoutText(int row, bool with_label) const;
-
-  // Get the available space for the total text width. |with_label| indicates
-  // whether a label is expected to be present.
-  int GetAvailableWidthForRow(int row, bool with_label) const;
-
-  // Calculates and sets the bounds of the popup, including placing it properly
-  // to prevent it from going off the screen.
-  void UpdatePopupBounds();
-
-  // The same font can vary based on the type of data it is showing at the row
-  // |index|.
-  const gfx::FontList& GetValueFontListForRow(size_t index) const;
-  const gfx::FontList& GetLabelFontListForRow(size_t index) const;
+  // Returns the icon image of the given |suggestion|.
+  static gfx::ImageSkia GetIconImage(const autofill::Suggestion& suggestion);
 
-  // Returns the icon image of the item at |index| in the popup.
-  gfx::ImageSkia GetIconImage(size_t index) const;
-
-  // Returns the store indicator icon image of the item at |index| in the popup.
-  gfx::ImageSkia GetStoreIndicatorIconImage(size_t index) const;
+  // Returns the store indicator icon image of the given |suggestion|.
+  static gfx::ImageSkia GetStoreIndicatorIconImage(
+      const autofill::Suggestion& suggestion);
 #endif
 
-  // Convert a y-coordinate to the closest line.
-  int LineFromY(int y) const;
-
-  const gfx::Rect popup_bounds() const { return popup_bounds_; }
-
-  // Returns the bounds of the item at |index| in the popup, relative to
-  // the top left of the popup.
-  gfx::Rect GetRowBounds(size_t index) const;
+  bool is_credit_card_popup() const { return is_credit_card_popup_; }
 
   // Gets the resource value for the given resource, returning 0 if the
   // resource isn't recognized.
-  int GetIconResourceID(const std::string& resource_name) const;
-
-  bool is_credit_card_popup() const { return is_credit_card_popup_; }
+  static int GetIconResourceID(const std::string& resource_name);
 
   // Allows the provision of another implementation of view_common, for use in
   // unit tests where using the real thing could cause crashes.
   void SetUpForTesting(std::unique_ptr<PopupViewCommon> view_common);
 
  private:
-  // Returns the enclosing rectangle for the element_bounds.
-  gfx::Rect RoundedElementBounds() const;
-
 #if !defined(OS_ANDROID)
-  gfx::ImageSkia GetIconImageByName(const std::string& icon_str) const;
-
-  // The fonts for the popup text.
-  // Normal font (readable size, non bold).
-  gfx::FontList normal_font_list_;
-  // Slightly smaller than the normal font.
-  gfx::FontList smaller_font_list_;
-  // Bold version of the normal font.
-  gfx::FontList bold_font_list_;
+  static gfx::ImageSkia GetIconImageByName(const std::string& icon_str);
 #endif
 
-  // The bounds of the Autofill popup.
-  gfx::Rect popup_bounds_;
-
   std::unique_ptr<PopupViewCommon> view_common_;
 
-  AutofillPopupViewDelegate* delegate_;  // Weak reference.
-
   const bool is_credit_card_popup_;
 
   DISALLOW_COPY_AND_ASSIGN(AutofillPopupLayoutModel);

chrome/browser/ui/autofill/autofill_popup_layout_model_unittest.cc

@@ -9,101 +9,25 @@
 #include <memory>
 #include <vector>
 
-#include "chrome/browser/ui/autofill/autofill_popup_view.h"
-#include "chrome/browser/ui/autofill/autofill_popup_view_delegate.h"
-#include "chrome/browser/ui/autofill/popup_constants.h"
 #include "chrome/test/base/chrome_render_view_host_test_harness.h"
-#include "components/autofill/core/browser/ui/popup_item_ids.h"
-#include "components/autofill/core/browser/ui/suggestion.h"
-#include "components/grit/components_scaled_resources.h"
-#include "content/public/browser/web_contents.h"
-#include "testing/gtest/include/gtest/gtest.h"
-#include "ui/base/resource/resource_bundle.h"
-#include "ui/gfx/geometry/point.h"
-#include "ui/gfx/geometry/rect.h"
-#include "ui/gfx/geometry/rect_f.h"
-#include "ui/gfx/image/image_skia.h"
-#include "ui/gfx/native_widget_types.h"
 
 namespace autofill {
 
 namespace {
 
-class TestAutofillPopupViewDelegate : public AutofillPopupViewDelegate {
- public:
-  explicit TestAutofillPopupViewDelegate(content::WebContents* web_contents)
-      : element_bounds_(0.0, 0.0, 100.0, 100.0),
-        container_view_(web_contents->GetNativeView()) {}
-
-  void Hide(PopupHidingReason reason) override {}
-  void ViewDestroyed() override {}
-  void SelectionCleared() override {}
-  gfx::NativeView container_view() const override { return container_view_; }
-  const gfx::RectF& element_bounds() const override { return element_bounds_; }
-  bool IsRTL() const override { return false; }
-
-  const std::vector<autofill::Suggestion> GetSuggestions() override {
-    // Give elements 1 and 3 subtexts and elements 2 and 3 icons, to ensure
-    // all combinations of subtexts and icons.
-    std::vector<Suggestion> suggestions;
-    suggestions.push_back(Suggestion("", "", "", 0));
-    suggestions.push_back(Suggestion("", "x", "", 0));
-    suggestions.push_back(Suggestion("", "", "americanExpressCC", 0));
-    suggestions.push_back(Suggestion("", "x", "genericCC", 0));
-    return suggestions;
-  }
-#if !defined(OS_ANDROID)
-  int GetElidedValueWidthForRow(int row) override { return 0; }
-  int GetElidedLabelWidthForRow(int row) override { return 0; }
-#endif
-
- private:
-  gfx::RectF element_bounds_;
-  gfx::NativeView container_view_;
-};
-
 class AutofillPopupLayoutModelTest : public ChromeRenderViewHostTestHarness {
  public:
   void SetUp() override {
     ChromeRenderViewHostTestHarness::SetUp();
-
-    delegate_ = std::make_unique<TestAutofillPopupViewDelegate>(web_contents());
-    layout_model_ = std::make_unique<AutofillPopupLayoutModel>(
-        delegate_.get(), false /* is_credit_card_field */);
+    layout_model_ = std::make_unique<AutofillPopupLayoutModel>(false);
   }
 
   AutofillPopupLayoutModel* layout_model() { return layout_model_.get(); }
 
  private:
-  std::unique_ptr<TestAutofillPopupViewDelegate> delegate_;
   std::unique_ptr<AutofillPopupLayoutModel> layout_model_;
 };
 
 }  // namespace
 
-#if !defined(OS_ANDROID)
-TEST_F(AutofillPopupLayoutModelTest, RowWidthWithoutText) {
-  int base_size =
-      AutofillPopupLayoutModel::kEndPadding * 2 + kPopupBorderThickness * 2;
-  int subtext_increase = AutofillPopupLayoutModel::kNamePadding;
-
-  // Refer to GetSuggestions() in TestAutofillPopupViewDelegate.
-  EXPECT_EQ(base_size,
-            layout_model()->RowWidthWithoutText(0, /* has_substext= */ false));
-  EXPECT_EQ(base_size + subtext_increase,
-            layout_model()->RowWidthWithoutText(1, /* has_substext= */ true));
-  EXPECT_EQ(base_size + AutofillPopupLayoutModel::kIconPadding +
-                ui::ResourceBundle::GetSharedInstance()
-                    .GetImageNamed(IDR_AUTOFILL_CC_AMEX)
-                    .Width(),
-            layout_model()->RowWidthWithoutText(2, /* has_substext= */ false));
-  EXPECT_EQ(base_size + subtext_increase +
-                AutofillPopupLayoutModel::kIconPadding +
-                ui::ResourceBundle::GetSharedInstance()
-                    .GetImageNamed(IDR_AUTOFILL_CC_GENERIC)
-                    .Width(),
-            layout_model()->RowWidthWithoutText(3, /* has_substext= */ true));
-}
-#endif
-
 }  // namespace autofill

chrome/browser/ui/autofill/autofill_popup_view_delegate.h

@@ -20,8 +20,6 @@ class RectF;
 
 namespace autofill {
 
-struct Suggestion;
-
 // Base class for Controllers of Autofill-style popups. This interface is
 // used by the relevant views to communicate with the controller.
 class AutofillPopupViewDelegate {
@@ -47,15 +45,6 @@ class AutofillPopupViewDelegate {
   // If the current popup should be displayed in RTL mode.
   virtual bool IsRTL() const = 0;
 
-  // Returns the full set of autofill suggestions, if applicable.
-  virtual const std::vector<autofill::Suggestion> GetSuggestions() = 0;
-
-#if !defined(OS_ANDROID)
-  // Returns elided values and labels for the given |row|.
-  virtual int GetElidedValueWidthForRow(int row) = 0;
-  virtual int GetElidedLabelWidthForRow(int row) = 0;
-#endif
-
  protected:
   virtual ~AutofillPopupViewDelegate() {}
 };

chrome/browser/ui/cocoa/touchbar/credit_card_autofill_touch_bar_controller_unittest.mm

@@ -30,8 +30,7 @@ class MockAutofillPopupController : public autofill::AutofillPopupController {
  public:
   MockAutofillPopupController() {
     gfx::FontList::SetDefaultFontDescription("Arial, Times New Roman, 15px");
-    layout_model_ =
-        std::make_unique<autofill::AutofillPopupLayoutModel>(this, false);
+    layout_model_ = std::make_unique<autofill::AutofillPopupLayoutModel>(false);
     suggestions_.push_back(
         autofill::Suggestion("bufflehead", "canvasback", "goldeneye", 1));
     suggestions_.push_back(
@@ -88,7 +87,7 @@ class MockAutofillPopupController : public autofill::AutofillPopupController {
 
   void SetIsCreditCardField(bool is_credit_card_field) {
     layout_model_.reset(
-        new autofill::AutofillPopupLayoutModel(this, is_credit_card_field));
+        new autofill::AutofillPopupLayoutModel(is_credit_card_field));
   }
 
   void set_line_count(int line_count) {

chrome/browser/ui/passwords/password_generation_popup_controller_impl.cc

@@ -319,21 +319,6 @@ bool PasswordGenerationPopupControllerImpl::IsRTL() const {
   return base::i18n::IsRTL();
 }
 
-const std::vector<autofill::Suggestion>
-PasswordGenerationPopupControllerImpl::GetSuggestions() {
-  return std::vector<autofill::Suggestion>();
-}
-
-#if !defined(OS_ANDROID)
-int PasswordGenerationPopupControllerImpl::GetElidedValueWidthForRow(int row) {
-  return 0;
-}
-
-int PasswordGenerationPopupControllerImpl::GetElidedLabelWidthForRow(int row) {
-  return 0;
-}
-#endif
-
 PasswordGenerationPopupController::GenerationUIState
 PasswordGenerationPopupControllerImpl::state() const {
   return state_;

chrome/browser/ui/passwords/password_generation_popup_controller_impl.h

@@ -40,7 +40,6 @@ class PasswordManagerDriver;
 
 namespace autofill {
 struct FormData;
-struct Suggestion;
 namespace password_generation {
 struct PasswordGenerationUIData;
 }  // namespace password_generation
@@ -137,11 +136,6 @@ class PasswordGenerationPopupControllerImpl
   gfx::NativeView container_view() const override;
   const gfx::RectF& element_bounds() const override;
   bool IsRTL() const override;
-  const std::vector<autofill::Suggestion> GetSuggestions() override;
-#if !defined(OS_ANDROID)
-  int GetElidedValueWidthForRow(int row) override;
-  int GetElidedLabelWidthForRow(int row) override;
-#endif
 
   GenerationUIState state() const override;
   bool password_selected() const override;

chrome/browser/ui/views/autofill/autofill_popup_base_view_browsertest.cc

@@ -41,11 +41,6 @@ class MockAutofillPopupViewDelegate : public AutofillPopupViewDelegate {
   MOCK_CONST_METHOD0(container_view, gfx::NativeView());
   MOCK_CONST_METHOD0(element_bounds, gfx::RectF&());
   MOCK_CONST_METHOD0(IsRTL, bool());
-  MOCK_METHOD0(GetSuggestions, const std::vector<autofill::Suggestion>());
-#if !defined(OS_ANDROID)
-  MOCK_METHOD1(GetElidedValueWidthForRow, int(int));
-  MOCK_METHOD1(GetElidedLabelWidthForRow, int(int));
-#endif
 };
 
 }  // namespace

chrome/browser/ui/views/autofill/autofill_popup_view_native_views.cc

@@ -447,8 +447,10 @@ void AutofillPopupItemView::CreateContent() {
   layout_manager->set_cross_axis_alignment(
       views::BoxLayout::CrossAxisAlignment::kCenter);
 
+  std::vector<autofill::Suggestion> suggestions = controller->GetSuggestions();
+
   const gfx::ImageSkia icon =
-      controller->layout_model().GetIconImage(line_number());
+      controller->layout_model().GetIconImage(suggestions[line_number()]);
 
   if (!icon.isNull()) {
     AddIcon(icon);
@@ -489,7 +491,8 @@ void AutofillPopupItemView::CreateContent() {
 
   AddChildView(std::move(all_labels));
   const gfx::ImageSkia store_indicator_icon =
-      controller->layout_model().GetStoreIndicatorIconImage(line_number());
+      controller->layout_model().GetStoreIndicatorIconImage(
+          suggestions[line_number()]);
   if (!store_indicator_icon.isNull()) {
     AddSpacerWithSize(GetHorizontalMargin(),
                       /*resize=*/true, layout_manager);
@@ -719,8 +722,8 @@ void AutofillPopupFooterView::CreateContent() {
   layout_manager->set_cross_axis_alignment(
       views::BoxLayout::CrossAxisAlignment::kStretch);
 
-  const gfx::ImageSkia icon =
-      controller->layout_model().GetIconImage(line_number());
+  const gfx::ImageSkia icon = controller->layout_model().GetIconImage(
+      controller->GetSuggestions()[line_number()]);
 
   // A FooterView shows an icon, if any, on the trailing (right in LTR) side,
   // but the Show Account Cards context is an anomaly. Its icon is on the