[Omnibox] Move result view elements into children.

This CL is a step toward making views children manage their views. This
step moves the data members, but still does the layout in the result
view. A later Cl will move the layout.

Bug: None
Change-Id: I82c498c0ae648b57d750828f687d9064ccee29dd
Reviewed-on: https://chromium-review.googlesource.com/994243
Commit-Queue: Dave Schuyler <dschuyler@chromium.org>
Reviewed-by: Tommy Li <tommycli@chromium.org>
Cr-Commit-Position: refs/heads/master@{#547929}

chrome/browser/ui/views/omnibox/omnibox_result_view.cc

@@ -112,6 +112,12 @@ class OmniboxSeparatedLineView : public views::View {
                                     const gfx::FontList& font_list);
   ~OmniboxSeparatedLineView() override;
 
+  views::ImageView* icon() { return icon_view_; }
+  views::ImageView* image() { return image_view_; }
+  OmniboxTextView* content() { return content_view_; }
+  OmniboxTextView* description() { return description_view_; }
+  OmniboxTextView* separator() { return separator_view_; }
+
   void OnMatchUpdate(const AutocompleteMatch& match);
   void OnHighlightUpdate(const AutocompleteMatch& match);
 
@@ -206,26 +212,17 @@ OmniboxResultView::OmniboxResultView(OmniboxPopupContentsView* model,
       font_height_(std::max(
           font_list.GetHeight(),
           font_list.DeriveWithWeight(gfx::Font::Weight::BOLD).GetHeight())),
-      animation_(new gfx::SlideAnimation(this)),
-      suggestion_icon_view_(AddOmniboxImageView()),
-      suggestion_image_view_(AddOmniboxImageView()),
-      suggestion_content_view_(AddOmniboxTextView(font_list)),
-      suggestion_description_view_(AddOmniboxTextView(font_list)),
-      suggestion_separator_view_(AddOmniboxTextView(font_list)),
-      keyword_icon_view_(AddOmniboxImageView()),
-      keyword_content_view_(AddOmniboxTextView(font_list)),
-      keyword_description_view_(AddOmniboxTextView(font_list)),
-      keyword_separator_view_(AddOmniboxTextView(font_list)) {
+      animation_(new gfx::SlideAnimation(this)) {
   CHECK_GE(model_index, 0);
 
   AddChildView(suggestion_view_ = new OmniboxSuggestionView(this, font_list));
   AddChildView(keyword_view_ = new OmniboxSeparatedLineView(this, font_list));
 
-  keyword_icon_view_->EnableCanvasFlippingForRTLUI(true);
-  keyword_icon_view_->SetImage(gfx::CreateVectorIcon(
+  keyword_view_->icon()->EnableCanvasFlippingForRTLUI(true);
+  keyword_view_->icon()->SetImage(gfx::CreateVectorIcon(
       omnibox::kKeywordSearchIcon, GetLayoutConstant(LOCATION_BAR_ICON_SIZE),
       GetColor(OmniboxPart::RESULTS_ICON)));
-  keyword_icon_view_->SizeToPreferredSize();
+  keyword_view_->icon()->SizeToPreferredSize();
 }
 
 OmniboxResultView::~OmniboxResultView() {}
@@ -241,12 +238,13 @@ void OmniboxResultView::SetMatch(const AutocompleteMatch& match) {
   suggestion_view_->OnMatchUpdate(match_);
   keyword_view_->OnMatchUpdate(match_);
 
-  suggestion_image_view_->SetVisible(false);  // Until SetAnswerImage is called.
-  keyword_icon_view_->SetVisible(match_.associated_keyword.get());
+  suggestion_view_->image()->SetVisible(
+      false);  // Until SetAnswerImage is called.
+  keyword_view_->icon()->SetVisible(match_.associated_keyword.get());
 
   if (OmniboxFieldTrial::InTabSwitchSuggestionWithButtonTrial() &&
       match.type == AutocompleteMatchType::TAB_SEARCH &&
-      !keyword_icon_view_->visible()) {
+      !keyword_view_->icon()->visible()) {
     suggestion_tab_switch_button_ =
         std::make_unique<OmniboxTabSwitchButton>(this, GetTextHeight());
     suggestion_tab_switch_button_->set_owned_by_client();
@@ -284,36 +282,37 @@ void OmniboxResultView::Invalidate() {
   //       SetMatch() once (rather than repeatedly, as happens here). There may
   //       be an optimization opportunity here.
   // TODO(dschuyler): determine whether to optimize the color changes.
-  suggestion_icon_view_->SetImage(GetIcon().ToImageSkia());
-  keyword_icon_view_->SetImage(gfx::CreateVectorIcon(
+  suggestion_view_->icon()->SetImage(GetIcon().ToImageSkia());
+  keyword_view_->icon()->SetImage(gfx::CreateVectorIcon(
       omnibox::kKeywordSearchIcon, GetLayoutConstant(LOCATION_BAR_ICON_SIZE),
       GetColor(OmniboxPart::RESULTS_ICON)));
 
   if (match_.answer) {
-    suggestion_content_view_->SetText(match_.answer->first_line());
-    suggestion_description_view_->SetText(match_.answer->second_line());
+    suggestion_view_->content()->SetText(match_.answer->first_line());
+    suggestion_view_->description()->SetText(match_.answer->second_line());
   } else {
-    suggestion_content_view_->SetText(match_.contents, match_.contents_class);
-    suggestion_description_view_->SetText(match_.description,
+    suggestion_view_->content()->SetText(match_.contents,
+                                         match_.contents_class);
+    suggestion_view_->description()->SetText(match_.description,
                                              match_.description_class);
   }
 
   const base::string16& separator =
       l10n_util::GetStringUTF16(IDS_AUTOCOMPLETE_MATCH_DESCRIPTION_SEPARATOR);
-  suggestion_separator_view_->SetText(separator);
-  suggestion_separator_view_->Dim();
-  keyword_separator_view_->SetText(separator);
-  keyword_separator_view_->Dim();
+  suggestion_view_->separator()->SetText(separator);
+  suggestion_view_->separator()->Dim();
+  keyword_view_->separator()->SetText(separator);
+  keyword_view_->separator()->Dim();
 
   AutocompleteMatch* keyword_match = match_.associated_keyword.get();
-  keyword_content_view_->SetVisible(keyword_match);
-  keyword_description_view_->SetVisible(keyword_match);
+  keyword_view_->content()->SetVisible(keyword_match);
+  keyword_view_->description()->SetVisible(keyword_match);
   if (keyword_match) {
-    keyword_content_view_->SetText(keyword_match->contents,
+    keyword_view_->content()->SetText(keyword_match->contents,
                                       keyword_match->contents_class);
-    keyword_description_view_->SetText(keyword_match->description,
+    keyword_view_->description()->SetText(keyword_match->description,
                                           keyword_match->description_class);
-    keyword_description_view_->Dim();
+    keyword_view_->description()->Dim();
   }
 
   // TODO(dschuyler): without this Layout call the text will shift slightly when
@@ -348,8 +347,8 @@ void OmniboxResultView::OnMatchIconUpdated() {
 }
 
 void OmniboxResultView::SetAnswerImage(const gfx::ImageSkia& image) {
-  suggestion_image_view_->SetImage(image);
-  suggestion_image_view_->SetVisible(true);
+  suggestion_view_->image()->SetImage(image);
+  suggestion_view_->image()->SetVisible(true);
   Layout();
   SchedulePaint();
 }
@@ -479,15 +478,15 @@ int OmniboxResultView::GetAnswerHeight() const {
   const gfx::Image icon = GetIcon();
   int icon_width = icon.Width();
   int answer_icon_size =
-      suggestion_image_view_->visible()
-          ? suggestion_image_view_->height() + kAnswerIconToTextPadding
+      suggestion_view_->image()->visible()
+          ? suggestion_view_->image()->height() + kAnswerIconToTextPadding
           : 0;
   // TODO(dschuyler): The GetIconAlignmentOffset() is applied an extra time to
   // match the math in Layout(). This seems like a (minor) mistake.
   int deduction = (GetIconAlignmentOffset() * 2) + icon_width +
                   (horizontal_padding * 3) + answer_icon_size;
   int description_width = std::max(width() - deduction, 0);
-  return suggestion_description_view_->GetHeightForWidth(description_width) +
+  return suggestion_view_->description()->GetHeightForWidth(description_width) +
          kVerticalPadding;
 }
 
@@ -530,151 +529,163 @@ bool OmniboxResultView::IsSelected() const {
 
 void OmniboxResultView::Layout() {
   views::View::Layout();
+
   const int horizontal_padding =
       GetLayoutConstant(LOCATION_BAR_ELEMENT_PADDING) +
       GetLayoutConstant(LOCATION_BAR_ICON_INTERIOR_PADDING);
+  // NOTE: While animating the keyword match, both matches may be visible.
+  int suggestion_width = width();
+  AutocompleteMatch* keyword_match = match_.associated_keyword.get();
+  if (keyword_match) {
+    const int icon_width = keyword_view_->icon()->width() +
+                           GetIconAlignmentOffset() + (horizontal_padding * 2);
+    const int max_kw_x = width() - icon_width;
+    suggestion_width = animation_->CurrentValueBetween(max_kw_x, 0);
+  }
+  if (suggestion_tab_switch_button_) {
+    const gfx::Size ts_button_size =
+        suggestion_tab_switch_button_->GetPreferredSize();
+    suggestion_tab_switch_button_->SetSize(ts_button_size);
+
+    // It looks nice to have the same margin on top, bottom and right side.
+    const int margin =
+        (suggestion_view_->height() - ts_button_size.height()) / 2;
+    suggestion_width -= ts_button_size.width() + margin;
+    suggestion_tab_switch_button_->SetPosition(
+        gfx::Point(suggestion_width, margin));
+  }
+  keyword_view_->SetBounds(suggestion_width, 0, width() - suggestion_width,
+                           height());
+  suggestion_view_->SetBounds(0, 0, suggestion_width, height());
+
   const int start_x = GetIconAlignmentOffset() + horizontal_padding;
-  int end_x = width();
+  int end_x = suggestion_view_->width();
 
   int text_height = GetTextHeight();
   int row_height = text_height;
   if (IsTwoLineLayout())
     row_height += match_.answer ? GetAnswerHeight() : GetTextHeight();
-  suggestion_separator_view_->SetVisible(false);
-  keyword_separator_view_->SetVisible(false);
+  suggestion_view_->separator()->SetVisible(false);
+  keyword_view_->separator()->SetVisible(false);
 
   // TODO(dschuyler): Refactor these if/else's into separate pieces of code to
   // improve readability/maintainability.
 
-  AutocompleteMatch* keyword_match = match_.associated_keyword.get();
   if (keyword_match) {
-    // NOTE: While animating the keyword match, both matches may be visible.
-    const int icon_width = keyword_icon_view_->width() +
-                           GetIconAlignmentOffset() + horizontal_padding * 2;
-    const int max_kw_x = width() - icon_width;
-    int kw_x = animation_->CurrentValueBetween(max_kw_x, 0);
-    end_x = kw_x;
-    kw_x += start_x + BackgroundWith1PxBorder::kLocationBarBorderThicknessDip;
-    keyword_icon_view_->SetPosition(
-        gfx::Point(kw_x, (height() - keyword_icon_view_->height()) / 2));
-    kw_x += keyword_icon_view_->width() + horizontal_padding;
-
-    int content_width = keyword_content_view_->CalculatePreferredSize().width();
+    int kw_x =
+        start_x + BackgroundWith1PxBorder::kLocationBarBorderThicknessDip;
+    keyword_view_->icon()->SetPosition(gfx::Point(
+        kw_x, (keyword_view_->height() - keyword_view_->icon()->height()) / 2));
+    kw_x += keyword_view_->icon()->width() + horizontal_padding;
+
+    int content_width =
+        keyword_view_->content()->CalculatePreferredSize().width();
     int description_width =
-        keyword_description_view_->CalculatePreferredSize().width();
+        keyword_view_->description()->CalculatePreferredSize().width();
     OmniboxPopupModel::ComputeMatchMaxWidths(
-        content_width, keyword_separator_view_->width(), description_width,
+        content_width, keyword_view_->separator()->width(), description_width,
         width(),
         /*description_on_separate_line=*/false,
         !AutocompleteMatch::IsSearchType(match_.type), &content_width,
         &description_width);
     int y = GetVerticalMargin();
-    keyword_content_view_->SetBounds(kw_x, y, content_width, text_height);
+    keyword_view_->content()->SetBounds(kw_x, y, content_width, text_height);
     if (description_width != 0) {
-      kw_x += keyword_content_view_->width();
-      keyword_separator_view_->SetVisible(true);
-      keyword_separator_view_->SetBounds(
-          kw_x, y, keyword_separator_view_->width(), text_height);
-      kw_x += keyword_separator_view_->width();
-      keyword_description_view_->SetBounds(kw_x, y, description_width,
+      kw_x += keyword_view_->content()->width();
+      keyword_view_->separator()->SetVisible(true);
+      keyword_view_->separator()->SetBounds(
+          kw_x, y, keyword_view_->separator()->width(), text_height);
+      kw_x += keyword_view_->separator()->width();
+      keyword_view_->description()->SetBounds(kw_x, y, description_width,
                                               text_height);
     } else if (IsTwoLineLayout()) {
-      keyword_content_view_->SetSize(gfx::Size(content_width, text_height * 2));
+      keyword_view_->content()->SetSize(
+          gfx::Size(content_width, text_height * 2));
     }
   }
 
   const gfx::Image icon = GetIcon();
   const int icon_y = GetVerticalMargin() + (row_height - icon.Height()) / 2;
-  suggestion_icon_view_->SetBounds(
+  suggestion_view_->icon()->SetBounds(
       start_x, icon_y, std::min(end_x, icon.Width()), icon.Height());
 
-  if (suggestion_tab_switch_button_) {
-    const gfx::Size ts_button_size =
-        suggestion_tab_switch_button_->GetPreferredSize();
-    suggestion_tab_switch_button_->SetSize(ts_button_size);
-
-    // It looks nice to have the same margin on top, bottom and right side.
-    const int margin = (height() - ts_button_size.height()) / 2;
-    end_x -= ts_button_size.width() + margin;
-    suggestion_tab_switch_button_->SetPosition(gfx::Point(end_x, margin));
-  }
-
   // NOTE: While animating the keyword match, both matches may be visible.
   int x = start_x;
   int y = GetVerticalMargin();
 
   if (base::FeatureList::IsEnabled(omnibox::kOmniboxRichEntitySuggestions) &&
       match_.answer) {
-    suggestion_icon_view_->SetVisible(false);
+    suggestion_view_->icon()->SetVisible(false);
     int image_edge_length =
-        text_height + suggestion_description_view_->GetLineHeight();
-    suggestion_image_view_->SetImageSize(
+        text_height + suggestion_view_->description()->GetLineHeight();
+    suggestion_view_->image()->SetImageSize(
         gfx::Size(image_edge_length, image_edge_length));
-    suggestion_image_view_->SetBounds(x, y, image_edge_length,
+    suggestion_view_->image()->SetBounds(x, y, image_edge_length,
                                          image_edge_length);
     x += image_edge_length + horizontal_padding;
-    suggestion_content_view_->SetBounds(x, y, end_x - x, text_height);
+    suggestion_view_->content()->SetBounds(x, y, end_x - x, text_height);
     y += text_height;
-    suggestion_description_view_->SetBounds(x, y, end_x - x, text_height);
+    suggestion_view_->description()->SetBounds(x, y, end_x - x, text_height);
     return;
   }
 
-  suggestion_icon_view_->SetVisible(true);
+  suggestion_view_->icon()->SetVisible(true);
   x += icon.Width() + horizontal_padding;
   if (match_.answer) {
-    suggestion_content_view_->SetBounds(x, y, end_x - x, text_height);
+    suggestion_view_->content()->SetBounds(x, y, end_x - x, text_height);
     y += text_height;
-    if (suggestion_image_view_->visible()) {
+    if (suggestion_view_->image()->visible()) {
       // The description may be multi-line. Using the view height results in
       // an image that's too large, so we use the line height here instead.
-      int image_edge_length = suggestion_description_view_->GetLineHeight();
-      suggestion_image_view_->SetBounds(
-          start_x + suggestion_icon_view_->width() + horizontal_padding,
+      int image_edge_length = suggestion_view_->description()->GetLineHeight();
+      suggestion_view_->image()->SetBounds(
+          start_x + suggestion_view_->icon()->width() + horizontal_padding,
           y + (kVerticalPadding / 2), image_edge_length, image_edge_length);
-      suggestion_image_view_->SetImageSize(
+      suggestion_view_->image()->SetImageSize(
           gfx::Size(image_edge_length, image_edge_length));
-      x += suggestion_image_view_->width() + kAnswerIconToTextPadding;
+      x += suggestion_view_->image()->width() + kAnswerIconToTextPadding;
     }
     int description_width = end_x - x;
-    suggestion_description_view_->SetBounds(
+    suggestion_view_->description()->SetBounds(
         x, y, description_width,
-        suggestion_description_view_->GetHeightForWidth(description_width) +
+        suggestion_view_->description()->GetHeightForWidth(description_width) +
             kVerticalPadding);
   } else if (IsTwoLineLayout()) {
-    if (!!suggestion_description_view_->GetContentsBounds().width()) {
+    if (!!suggestion_view_->description()->GetContentsBounds().width()) {
       // A description is present.
-      suggestion_content_view_->SetBounds(x, y, end_x - x, GetTextHeight());
+      suggestion_view_->content()->SetBounds(x, y, end_x - x, GetTextHeight());
       y += GetTextHeight();
       int description_width = end_x - x;
-      suggestion_description_view_->SetBounds(
+      suggestion_view_->description()->SetBounds(
           x, y, description_width,
-          suggestion_description_view_->GetHeightForWidth(description_width) +
+          suggestion_view_->description()->GetHeightForWidth(
+              description_width) +
               kVerticalPadding);
     } else {
       // For no description, shift down halfway to draw contents in middle.
       y += GetTextHeight() / 2;
-      suggestion_content_view_->SetBounds(x, y, end_x - x, GetTextHeight());
+      suggestion_view_->content()->SetBounds(x, y, end_x - x, GetTextHeight());
     }
   } else {
     int content_width =
-        suggestion_content_view_->CalculatePreferredSize().width();
+        suggestion_view_->content()->CalculatePreferredSize().width();
     int description_width =
-        suggestion_description_view_->CalculatePreferredSize().width();
+        suggestion_view_->description()->CalculatePreferredSize().width();
     OmniboxPopupModel::ComputeMatchMaxWidths(
-        content_width, suggestion_separator_view_->width(), description_width,
-        end_x - x,
+        content_width, suggestion_view_->separator()->width(),
+        description_width, end_x - x,
         /*description_on_separate_line=*/false,
         !AutocompleteMatch::IsSearchType(match_.type), &content_width,
         &description_width);
-    suggestion_content_view_->SetBounds(x, y, content_width, text_height);
+    suggestion_view_->content()->SetBounds(x, y, content_width, text_height);
     x += content_width;
     if (description_width) {
-      suggestion_separator_view_->SetVisible(true);
-      suggestion_separator_view_->SetBounds(
-          x, y, suggestion_separator_view_->width(), text_height);
-      x += suggestion_separator_view_->width();
+      suggestion_view_->separator()->SetVisible(true);
+      suggestion_view_->separator()->SetBounds(
+          x, y, suggestion_view_->separator()->width(), text_height);
+      x += suggestion_view_->separator()->width();
     }
-    suggestion_description_view_->SetBounds(x, y, description_width,
+    suggestion_view_->description()->SetBounds(x, y, description_width,
                                                text_height);
   }
 }

chrome/browser/ui/views/omnibox/omnibox_result_view.h

@@ -130,22 +130,8 @@ class OmniboxResultView : public views::View,
   // Weak pointers for easy reference.
   OmniboxSuggestionView* suggestion_view_;  // The leading (or left) view.
   OmniboxSeparatedLineView* keyword_view_;  // The trailing (or right) view.
-
-  // TODO(dschuyler): Move these views into either suggestion_view_ or
-  // keyword_view_. I intend to do so by May, 2018.
-
-  views::ImageView* suggestion_icon_view_;   // Small icon. e.g. favicon.
-  views::ImageView* suggestion_image_view_;  // For rich suggestions.
-  OmniboxTextView* suggestion_content_view_;
-  OmniboxTextView* suggestion_description_view_;
-  OmniboxTextView* suggestion_separator_view_;  // e.g. A hyphen.
   std::unique_ptr<OmniboxTabSwitchButton> suggestion_tab_switch_button_;
 
-  views::ImageView* keyword_icon_view_;  // An icon resembling a '>'.
-  OmniboxTextView* keyword_content_view_;
-  OmniboxTextView* keyword_description_view_;
-  OmniboxTextView* keyword_separator_view_;
-
   DISALLOW_COPY_AND_ASSIGN(OmniboxResultView);
 };