[Omnibox] Don't draw placeholder image unless it will be shown

This CL avoids creating the right suggestion placeholder image for non-
rich suggestions (where the image won't be shown).
It also moves the placeholder code to a more appropriate file. (from
OmniboxResultView to OmniboxMatchCellView.

Bug: None
Change-Id: If90e3ad4d4bd45aa80ae1e67a03b7c23437f616e
Reviewed-on: https://chromium-review.googlesource.com/1043220
Commit-Queue: Dave Schuyler <dschuyler@chromium.org>
Reviewed-by: Justin Donnelly <jdonnelly@chromium.org>
Cr-Commit-Position: refs/heads/master@{#556193}

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

@@ -7,14 +7,17 @@
 #include "base/macros.h"
 #include "base/metrics/field_trial_params.h"
 #include "chrome/browser/ui/layout_constants.h"
+#include "chrome/browser/ui/omnibox/omnibox_theme.h"
 #include "chrome/browser/ui/views/location_bar/location_bar_view.h"
 #include "chrome/browser/ui/views/omnibox/omnibox_popup_contents_view.h"
 #include "chrome/browser/ui/views/omnibox/omnibox_text_view.h"
 #include "chrome/browser/ui/views/omnibox/rounded_omnibox_results_frame.h"
 #include "chrome/grit/generated_resources.h"
 #include "components/omnibox/browser/omnibox_field_trial.h"
+#include "extensions/common/image_util.h"
 #include "ui/base/l10n/l10n_util.h"
 #include "ui/base/material_design/material_design_controller.h"
+#include "ui/gfx/image/canvas_image_source.h"
 #include "ui/views/controls/image_view.h"
 
 namespace {
@@ -93,6 +96,40 @@ int HorizontalPadding() {
          GetLayoutConstant(LOCATION_BAR_ICON_INTERIOR_PADDING);
 }
 
+////////////////////////////////////////////////////////////////////////////////
+// PlaceholderImageSource:
+
+class PlaceholderImageSource : public gfx::CanvasImageSource {
+ public:
+  PlaceholderImageSource(const gfx::Size& canvas_size, SkColor color);
+  ~PlaceholderImageSource() override;
+
+  // CanvasImageSource override:
+  void Draw(gfx::Canvas* canvas) override;
+
+ private:
+  SkColor color_;
+  gfx::Size size_;
+
+  DISALLOW_COPY_AND_ASSIGN(PlaceholderImageSource);
+};
+
+PlaceholderImageSource::PlaceholderImageSource(const gfx::Size& canvas_size,
+                                               SkColor color)
+    : gfx::CanvasImageSource(canvas_size, false),
+      color_(color),
+      size_(canvas_size) {}
+
+PlaceholderImageSource::~PlaceholderImageSource() = default;
+
+void PlaceholderImageSource::Draw(gfx::Canvas* canvas) {
+  cc::PaintFlags flags;
+  flags.setAntiAlias(true);
+  flags.setStyle(cc::PaintFlags::kStrokeAndFill_Style);
+  flags.setColor(color_);
+  canvas->sk_canvas()->drawOval(gfx::RectToSkRect(gfx::Rect(size_)), flags);
+}
+
 }  // namespace
 
 ////////////////////////////////////////////////////////////////////////////////
@@ -136,6 +173,7 @@ gfx::Size OmniboxMatchCellView::CalculatePreferredSize() const {
   }
   return gfx::Size(0, height);
 }
+
 bool OmniboxMatchCellView::CanProcessEventsWithinSubtree() const {
   return false;
 }
@@ -152,7 +190,8 @@ int OmniboxMatchCellView::GetOldStyleAnswerHeight() const {
          kVerticalPadding;
 }
 
-void OmniboxMatchCellView::OnMatchUpdate(const AutocompleteMatch& match) {
+void OmniboxMatchCellView::OnMatchUpdate(const OmniboxResultView* result_view,
+                                         const AutocompleteMatch& match) {
   is_old_style_answer_ = !!match.answer;
   is_rich_suggestion_ =
       (base::FeatureList::IsEnabled(omnibox::kOmniboxNewAnswerLayout) &&
@@ -163,6 +202,16 @@ void OmniboxMatchCellView::OnMatchUpdate(const AutocompleteMatch& match) {
   if (is_old_style_answer_ || is_rich_suggestion_) {
     // Multi-line layout doesn't use the separator.
     separator_view_->SetSize(gfx::Size());
+
+    // Set up default (placeholder) image.
+    SkColor color = result_view->GetColor(OmniboxPart::RESULTS_BACKGROUND);
+    extensions::image_util::ParseHexColorString(match.image_dominant_color,
+                                                &color);
+    color = SkColorSetA(color, 0x40);  // 25% transparency (arbitrary).
+    int image_edge_length = description_view_->GetLineHeight();
+    image_view_->SetImage(
+        gfx::CanvasImageSource::MakeImageSkia<PlaceholderImageSource>(
+            gfx::Size(image_edge_length, image_edge_length), color));
   } else {
     // Single-line layout doesn't use the image.
     image_view_->SetSize(gfx::Size());

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

@@ -32,7 +32,8 @@ class OmniboxMatchCellView : public views::View {
   OmniboxTextView* description() { return description_view_; }
   OmniboxTextView* separator() { return separator_view_; }
 
-  void OnMatchUpdate(const AutocompleteMatch& match);
+  void OnMatchUpdate(const OmniboxResultView* result_view,
+                     const AutocompleteMatch& match);
 
   // views::View:
   gfx::Size CalculatePreferredSize() const override;

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

@@ -34,7 +34,6 @@
 #include "components/omnibox/browser/omnibox_field_trial.h"
 #include "components/omnibox/browser/omnibox_popup_model.h"
 #include "components/omnibox/browser/vector_icons.h"
-#include "extensions/common/image_util.h"
 #include "third_party/skia/include/core/SkColor.h"
 #include "ui/accessibility/ax_node_data.h"
 #include "ui/base/l10n/l10n_util.h"
@@ -42,13 +41,7 @@
 #include "ui/base/resource/resource_bundle.h"
 #include "ui/base/theme_provider.h"
 #include "ui/events/event.h"
-#include "ui/gfx/canvas.h"
-#include "ui/gfx/color_palette.h"
-#include "ui/gfx/color_utils.h"
-#include "ui/gfx/image/canvas_image_source.h"
-#include "ui/gfx/image/image.h"
 #include "ui/gfx/paint_vector_icon.h"
-#include "ui/gfx/text_utils.h"
 
 namespace {
 
@@ -86,37 +79,6 @@ int HorizontalPadding() {
          GetLayoutConstant(LOCATION_BAR_ICON_INTERIOR_PADDING);
 }
 
-class PlaceholderImageSource : public gfx::CanvasImageSource {
- public:
-  PlaceholderImageSource(const gfx::Size& canvas_size, SkColor color);
-  ~PlaceholderImageSource() override;
-
-  // CanvasImageSource override:
-  void Draw(gfx::Canvas* canvas) override;
-
- private:
-  SkColor color_;
-  gfx::Size size_;
-
-  DISALLOW_COPY_AND_ASSIGN(PlaceholderImageSource);
-};
-
-PlaceholderImageSource::PlaceholderImageSource(const gfx::Size& canvas_size,
-                                               SkColor color)
-    : gfx::CanvasImageSource(canvas_size, false),
-      color_(color),
-      size_(canvas_size) {}
-
-PlaceholderImageSource::~PlaceholderImageSource() = default;
-
-void PlaceholderImageSource::Draw(gfx::Canvas* canvas) {
-  cc::PaintFlags flags;
-  flags.setAntiAlias(true);
-  flags.setStyle(cc::PaintFlags::kStrokeAndFill_Style);
-  flags.setColor(color_);
-  canvas->sk_canvas()->drawOval(gfx::RectToSkRect(gfx::Rect(size_)), flags);
-}
-
 }  // namespace
 
 ////////////////////////////////////////////////////////////////////////////////
@@ -156,18 +118,8 @@ void OmniboxResultView::SetMatch(const AutocompleteMatch& match) {
   match_ = match.GetMatchWithContentsAndDescriptionPossiblySwapped();
   animation_->Reset();
   is_hovered_ = false;
-  suggestion_view_->OnMatchUpdate(match_);
-  keyword_view_->OnMatchUpdate(match_);
-
-  // Set up default (placeholder) image.
-  SkColor color = GetColor(OmniboxPart::RESULTS_BACKGROUND);
-  extensions::image_util::ParseHexColorString(match_.image_dominant_color,
-                                              &color);
-  color = SkColorSetA(color, 0x40);  // 25% transparency (arbitrary).
-  int image_edge_length = suggestion_view_->description()->GetLineHeight();
-  suggestion_view_->image()->SetImage(
-      gfx::CanvasImageSource::MakeImageSkia<PlaceholderImageSource>(
-          gfx::Size(image_edge_length, image_edge_length), color));
+  suggestion_view_->OnMatchUpdate(this, match_);
+  keyword_view_->OnMatchUpdate(this, match_);
 
   keyword_view_->icon()->SetVisible(match_.associated_keyword.get());