Omnibox UI Refresh: Fix rounded favicons in suggestions.

Favicons for suggestions have been getting incorrectly rounded. This
led to a discrepency in how they appeared on the tab and location-bar
vs. the Omnibox suggestions.

This CL does a few things:

 1) Changes the icon_view_ member of OmniboxMatchCellView from the
    OmniboxImageView class (which rounds corners) to a plain ImageView.
    This fixes the heart of the bug.

 2) Renames OmniboxImageView => RoundedCornerImageView, to make things
    more explicit.

 3) Renames image_view_ to answer_image_view_ to make things more
    explicit. Also updates the comment to clarify that it's used for
    all suggestion answers, not just rich entities.

As a followup, we should investigate eliminating / refactoring
RoundedCornerImageView, as it appears (in the code) to apply the entity
image corner radius to all answers-in-suggest images.

But that's a less noticeable and separate concern.

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

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

@@ -220,11 +220,11 @@ void EncircledImageSource::Draw(gfx::Canvas* canvas) {
 }
 
 ////////////////////////////////////////////////////////////////////////////////
-// OmniboxImageView:
+// RoundedCornerImageView:
 
-class OmniboxImageView : public views::ImageView {
+class RoundedCornerImageView : public views::ImageView {
  public:
-  OmniboxImageView() = default;
+  RoundedCornerImageView() = default;
 
   bool CanProcessEventsWithinSubtree() const override { return false; }
 
@@ -232,10 +232,10 @@ class OmniboxImageView : public views::ImageView {
   // views::ImageView:
   void OnPaint(gfx::Canvas* canvas) override;
 
-  DISALLOW_COPY_AND_ASSIGN(OmniboxImageView);
+  DISALLOW_COPY_AND_ASSIGN(RoundedCornerImageView);
 };
 
-void OmniboxImageView::OnPaint(gfx::Canvas* canvas) {
+void RoundedCornerImageView::OnPaint(gfx::Canvas* canvas) {
   gfx::Path mask;
   mask.addRoundRect(gfx::RectToSkRect(GetImageBounds()),
                     kEntityImageCornerRadius, kEntityImageCornerRadius);
@@ -252,8 +252,8 @@ OmniboxMatchCellView::OmniboxMatchCellView(OmniboxResultView* result_view)
     : is_old_style_answer_(false),
       is_rich_suggestion_(false),
       is_search_type_(false) {
-  AddChildView(icon_view_ = new OmniboxImageView());
-  AddChildView(image_view_ = new OmniboxImageView());
+  AddChildView(icon_view_ = new views::ImageView());
+  AddChildView(answer_image_view_ = new RoundedCornerImageView());
   AddChildView(content_view_ = new OmniboxTextView(result_view));
   AddChildView(description_view_ = new OmniboxTextView(result_view));
   AddChildView(separator_view_ = new OmniboxTextView(result_view));
@@ -262,8 +262,8 @@ OmniboxMatchCellView::OmniboxMatchCellView(OmniboxResultView* result_view)
     icon_view_->SetHorizontalAlignment(views::ImageView::CENTER);
     icon_view_->SetVerticalAlignment(views::ImageView::CENTER);
   }
-  image_view_->SetHorizontalAlignment(views::ImageView::CENTER);
-  image_view_->SetVerticalAlignment(views::ImageView::CENTER);
+  answer_image_view_->SetHorizontalAlignment(views::ImageView::CENTER);
+  answer_image_view_->SetVerticalAlignment(views::ImageView::CENTER);
 
   const base::string16& separator =
       l10n_util::GetStringUTF16(IDS_AUTOCOMPLETE_MATCH_DESCRIPTION_SEPARATOR);
@@ -280,9 +280,9 @@ gfx::Size OmniboxMatchCellView::CalculatePreferredSize() const {
   } else if (is_old_style_answer_) {
     int icon_width = icon_view_->width();
     int answer_image_size =
-        image_view_->GetImage().isNull()
+        answer_image_view_->GetImage().isNull()
             ? 0
-            : image_view_->height() + kAnswerIconToTextPadding;
+            : answer_image_view_->height() + kAnswerIconToTextPadding;
     int deduction = icon_width + (HorizontalPadding() * 3) + answer_image_size;
     int description_width = std::max(width() - deduction, 0);
     height = content_view_->GetLineHeight() +
@@ -335,16 +335,16 @@ void OmniboxMatchCellView::OnMatchUpdate(const OmniboxResultView* result_view,
 
   if (OmniboxFieldTrial::IsNewAnswerLayoutEnabled() &&
       match.type == AutocompleteMatchType::CALCULATOR) {
-    image_view_->SetImage(
+    answer_image_view_->SetImage(
         ui::ResourceBundle::GetSharedInstance().GetImageSkiaNamed(
             IDR_OMNIBOX_CALCULATOR_ROUND));
-    image_view_->SetImageSize(
+    answer_image_view_->SetImageSize(
         gfx::Size(kNewAnswerImageSize, kNewAnswerImageSize));
   } else if (!is_rich_suggestion_) {
-    // An entry with |is_old_style_answer_| may use the image_view_. But it's
-    // set when the image arrives (later).
-    image_view_->SetImage(gfx::ImageSkia());
-    image_view_->SetSize(gfx::Size());
+    // An entry with |is_old_style_answer_| may use the answer_image_view_. But
+    // it's set when the image arrives (later).
+    answer_image_view_->SetImage(gfx::ImageSkia());
+    answer_image_view_->SetSize(gfx::Size());
   } else {
     // Determine if we have a local icon (or else it will be downloaded).
     const gfx::VectorIcon* vector_icon = nullptr;
@@ -378,17 +378,17 @@ void OmniboxMatchCellView::OnMatchUpdate(const OmniboxResultView* result_view,
       }
       if (vector_icon) {
         const auto& icon = gfx::CreateVectorIcon(*vector_icon, SK_ColorWHITE);
-        image_view_->SetImage(
+        answer_image_view_->SetImage(
             gfx::CanvasImageSource::MakeImageSkia<EncircledImageSource>(
                 kNewAnswerImageSize / 2, gfx::kGoogleBlue600, icon));
       } else if (idr_image) {
-        image_view_->SetImage(
+        answer_image_view_->SetImage(
             ui::ResourceBundle::GetSharedInstance().GetImageSkiaNamed(
                 idr_image));
       }
       // Always set the image size so that downloaded images get the correct
       // size (such as Weather answers).
-      image_view_->SetImageSize(
+      answer_image_view_->SetImageSize(
           gfx::Size(kNewAnswerImageSize, kNewAnswerImageSize));
     } else {
       SkColor color = result_view->GetColor(OmniboxPart::RESULTS_BACKGROUND);
@@ -396,8 +396,8 @@ void OmniboxMatchCellView::OnMatchUpdate(const OmniboxResultView* result_view,
                                                   &color);
       color = SkColorSetA(color, 0x40);  // 25% transparency (arbitrary).
       const gfx::Size size = gfx::Size(kEntityImageSize, kEntityImageSize);
-      image_view_->SetImageSize(size);
-      image_view_->SetImage(
+      answer_image_view_->SetImageSize(size);
+      answer_image_view_->SetImage(
           gfx::CanvasImageSource::MakeImageSkia<PlaceholderImageSource>(size,
                                                                         color));
     }
@@ -446,13 +446,14 @@ void OmniboxMatchCellView::LayoutOldStyleAnswer(int icon_view_width,
   x += text_indent;
   content_view_->SetBounds(x, y, width() - x, text_height);
   y += text_height;
-  if (!image_view_->GetImage().isNull()) {
+  if (!answer_image_view_->GetImage().isNull()) {
     constexpr int kImageEdgeLength = 24;
     constexpr int kImagePadding = 2;
-    image_view_->SetBounds(x, y + kImagePadding, kImageEdgeLength,
-                           kImageEdgeLength);
-    image_view_->SetImageSize(gfx::Size(kImageEdgeLength, kImageEdgeLength));
-    x += image_view_->width() + kAnswerIconToTextPadding;
+    answer_image_view_->SetBounds(x, y + kImagePadding, kImageEdgeLength,
+                                  kImageEdgeLength);
+    answer_image_view_->SetImageSize(
+        gfx::Size(kImageEdgeLength, kImageEdgeLength));
+    x += answer_image_view_->width() + kAnswerIconToTextPadding;
   }
   int description_width = width() - x;
   description_view_->SetBounds(
@@ -466,7 +467,7 @@ void OmniboxMatchCellView::LayoutNewStyleTwoLineSuggestion() {
   int y = child_area.y();
   views::ImageView* image_view;
   if (is_rich_suggestion_) {
-    image_view = image_view_;
+    image_view = answer_image_view_;
   } else {
     image_view = icon_view_;
   }

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

@@ -21,7 +21,7 @@ class OmniboxMatchCellView : public views::View {
   ~OmniboxMatchCellView() override;
 
   views::ImageView* icon() { return icon_view_; }
-  views::ImageView* image() { return image_view_; }
+  views::ImageView* answer_image() { return answer_image_view_; }
   OmniboxTextView* content() { return content_view_; }
   OmniboxTextView* description() { return description_view_; }
   OmniboxTextView* separator() { return separator_view_; }
@@ -55,8 +55,10 @@ class OmniboxMatchCellView : public views::View {
   bool should_show_tab_match_ = false;
 
   // Weak pointers for easy reference.
-  views::ImageView* icon_view_;   // An icon representing the type or content.
-  views::ImageView* image_view_;  // For rich suggestions.
+  // An icon representing the type or content.
+  views::ImageView* icon_view_;
+  // The image for answers in suggest and rich entity suggestions.
+  views::ImageView* answer_image_view_;
   OmniboxTextView* content_view_;
   OmniboxTextView* description_view_;
   OmniboxTextView* separator_view_;

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

@@ -230,7 +230,7 @@ void OmniboxResultView::OnMatchIconUpdated() {
 }
 
 void OmniboxResultView::SetRichSuggestionImage(const gfx::ImageSkia& image) {
-  suggestion_view_->image()->SetImage(image);
+  suggestion_view_->answer_image()->SetImage(image);
   Layout();
   SchedulePaint();
 }