Answers in Suggest icon downloading

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

 // Copyright (c) 2012 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 // For WinDDK ATL compatibility, these ATL headers must come first.
 #include "build/build_config.h"
 #if defined(OS_WIN)
 #include <atlbase.h>  // NOLINT
 #include <atlwin.h>  // NOLINT
 #endif
 
 #include "chrome/browser/ui/views/omnibox/omnibox_result_view.h"
 
 #include <algorithm>  // NOLINT
 
 #include "base/i18n/bidi_line_iterator.h"
 #include "base/memory/scoped_vector.h"
 #include "base/strings/string_number_conversions.h"
 #include "base/strings/string_util.h"
+#include "chrome/browser/bitmap_fetcher/bitmap_fetcher_service_factory.h"
+#include "chrome/browser/profiles/profile.h"
 #include "chrome/browser/ui/omnibox/omnibox_popup_model.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/grit/generated_resources.h"
 #include "components/omnibox/suggestion_answer.h"
 #include "grit/components_scaled_resources.h"
 #include "grit/theme_resources.h"
 #include "ui/base/l10n/l10n_util.h"
 #include "ui/base/theme_provider.h"
 #include "ui/gfx/canvas.h"
 #include "ui/gfx/color_utils.h"
 #include "ui/gfx/image/image.h"
 #include "ui/gfx/range/range.h"
 #include "ui/gfx/render_text.h"
 #include "ui/gfx/text_utils.h"
 #include "ui/native_theme/native_theme.h"
 
 using ui::NativeTheme;
 
 namespace {
 
+// The observer watches for changes in the image being downloaded.
+class AnswersImageObserverDesktop : public BitmapFetcherService::Observer {
+ public:
+  explicit AnswersImageObserverDesktop(
+      const base::WeakPtr<OmniboxResultView>& view)
+      : view_(view) {}
+
+  void OnImageChanged(BitmapFetcherService::RequestId request_id,
+                      const SkBitmap& image) override;
+
+ private:
+  const base::WeakPtr<OmniboxResultView> view_;
+  DISALLOW_COPY_AND_ASSIGN(AnswersImageObserverDesktop);
+};
+
+void AnswersImageObserverDesktop::OnImageChanged(
+    BitmapFetcherService::RequestId request_id,
+    const SkBitmap& image) {
+  DCHECK(!image.empty());
+  if (view_)
+    view_.get()->SetAnswerImage(gfx::ImageSkia::CreateFrom1xBitmap(image));
+}
+
 // The minimum distance between the top and bottom of the {icon|text} and the
 // top or bottom of the row.
 const int kMinimumIconVerticalPadding = 2;
 const int kMinimumTextVerticalPadding = 3;
 
 // A mapping from OmniboxResultView's ResultViewState/ColorKind types to
 // NativeTheme colors.
 struct TranslationTable {
   ui::NativeTheme::ColorId id;
   OmniboxResultView::ResultViewState state;
@@ skipping 69 matching lines @@
   int center_;
   int right_;
 
   DISALLOW_COPY_AND_ASSIGN(MirroringContext);
 };
 
 OmniboxResultView::OmniboxResultView(OmniboxPopupContentsView* model,
                                      int model_index,
                                      LocationBarView* location_bar_view,
                                      const gfx::FontList& font_list)
-    : edge_item_padding_(LocationBarView::kItemPadding),
+    : request_id_(BitmapFetcherService::REQUEST_ID_INVALID),
+      edge_item_padding_(LocationBarView::kItemPadding),
       item_padding_(LocationBarView::kItemPadding),
       minimum_text_vertical_padding_(kMinimumTextVerticalPadding),
       model_(model),
       model_index_(model_index),
       location_bar_view_(location_bar_view),
       font_list_(font_list),
       font_height_(
           std::max(font_list.GetHeight(),
                    font_list.DeriveWithStyle(gfx::Font::BOLD).GetHeight())),
       mirroring_context_(new MirroringContext()),
       keyword_icon_(new views::ImageView()),
-      animation_(new gfx::SlideAnimation(this)) {
+      animation_(new gfx::SlideAnimation(this)),
+      weak_ptr_factory_(this) {
   CHECK_GE(model_index, 0);
   if (default_icon_size_ == 0) {
     default_icon_size_ =
         location_bar_view_->GetThemeProvider()->GetImageSkiaNamed(
             AutocompleteMatch::TypeToIcon(
                 AutocompleteMatchType::URL_WHAT_YOU_TYPED))->width();
   }
   keyword_icon_->set_owned_by_client();
   keyword_icon_->EnableCanvasFlippingForRTLUI(true);
   keyword_icon_->SetImage(GetKeywordIcon());
   keyword_icon_->SizeToPreferredSize();
+  image_service_ = BitmapFetcherServiceFactory::GetForBrowserContext(
+      location_bar_view_->profile());

[Peter Kasting, 2015/03/03 01:32:36]

Nit: Init this in the initializer list

[dschuyler, 2015/03/03 21:13:56]

Done.

 }
 
 OmniboxResultView::~OmniboxResultView() {
+  if (image_service_)
+    image_service_->CancelRequest(request_id_);
 }
 
 SkColor OmniboxResultView::GetColor(
     ResultViewState state,
     ColorKind kind) const {
   for (size_t i = 0; i < arraysize(kTranslationTable); ++i) {
     if (kTranslationTable[i].state == state &&
         kTranslationTable[i].kind == kind) {
       return GetNativeTheme()->GetSystemColor(kTranslationTable[i].id);
     }
   }
 
   NOTREACHED();
   return SK_ColorRED;
 }
 
 void OmniboxResultView::SetMatch(const AutocompleteMatch& match) {
   match_ = match;
   ResetRenderTexts();
   animation_->Reset();
 
+  answer_image_ = gfx::ImageSkia();
+  if (match_.answer && match_.answer->second_line().image_url().is_valid())
+    GetAnswerIcon();

[Peter Kasting, 2015/03/03 01:32:36]

It seems sort of weird to have caller code that conditionally calls a function,
and have the function immediately do a further conditional check.  Plus, both
sides want to look at match_.answer->second_line().image_url(), so by extracting
the code in GetAnswerIcon() out, we're not really improving any kind of
encapsulation.

I would respond to all this by collapsing together all this code; either call
the function unconditionally here and move the conditionals there, or (probably
better) just inline the contents of that function into here, since this is the
lone callsite and the function body is short.

Then the two separate conditionals can be one statement.

[dschuyler, 2015/03/03 21:13:56]

Done.

+
   AutocompleteMatch* associated_keyword_match = match_.associated_keyword.get();
   if (associated_keyword_match) {
     keyword_icon_->SetImage(GetKeywordIcon());
     if (!keyword_icon_->parent())
       AddChildView(keyword_icon_.get());
   } else if (keyword_icon_->parent()) {
     RemoveChildView(keyword_icon_.get());
   }
 
   Layout();
@@ skipping 27 matching lines @@
 OmniboxResultView::ResultViewState OmniboxResultView::GetState() const {
   if (model_->IsSelectedIndex(model_index_))
     return SELECTED;
   return model_->IsHoveredIndex(model_index_) ? HOVERED : NORMAL;
 }
 
 int OmniboxResultView::GetTextHeight() const {
   return font_height_;
 }
 
-void OmniboxResultView::PaintMatch(
-    const AutocompleteMatch& match,
-    gfx::RenderText* contents,
-    gfx::RenderText* description,
-    gfx::Canvas* canvas,
-    int x) const {
+void OmniboxResultView::PaintMatch(const AutocompleteMatch& match,
+                                   gfx::RenderText* contents,
+                                   gfx::RenderText* description,
+                                   gfx::Canvas* canvas,
+                                   int x) const {
   int y = text_bounds_.y();
 
   if (!separator_rendertext_) {
     const base::string16& separator =
         l10n_util::GetStringUTF16(IDS_AUTOCOMPLETE_MATCH_DESCRIPTION_SEPARATOR);
     separator_rendertext_.reset(CreateRenderText(separator).release());
     separator_rendertext_->SetColor(GetColor(GetState(), DIMMED_TEXT));
     separator_width_ = separator_rendertext_->GetContentWidth();
   }
 
   int contents_max_width, description_max_width;
   OmniboxPopupModel::ComputeMatchMaxWidths(
       contents->GetContentWidth(),
       separator_width_,
       description ? description->GetContentWidth() : 0,
       mirroring_context_->remaining_width(x),
       !AutocompleteMatch::IsSearchType(match.type),
       &contents_max_width,
       &description_max_width);
 
   x = DrawRenderText(match, contents, true, canvas, x, y, contents_max_width);
 
   if (description_max_width != 0) {
     x = DrawRenderText(match, separator_rendertext_.get(), false, canvas, x, y,
                        separator_width_);
+
+    if (!answer_image_.size().IsEmpty()) {
+      canvas->DrawImageInt(answer_image_,
+                           // Source x, y, w, h.
+                           0, 0, answer_image_.width(), answer_image_.height(),
+                           // Destination x, y, w, h.
+                           GetMirroredXInView(x),
+                           y + kMinimumIconVerticalPadding, default_icon_size_,
+                           default_icon_size_, true);
+      x += default_icon_size_ + LocationBarView::kIconInternalPadding;
+    }
+
     DrawRenderText(match, description, false, canvas, x, y,
                    description_max_width);
   }
 }
 
 int OmniboxResultView::DrawRenderText(
     const AutocompleteMatch& match,
     gfx::RenderText* render_text,
     bool contents,
     gfx::Canvas* canvas,
@@ skipping 123 matching lines @@
     render_text->ApplyColor(GetColor(GetState(), color_kind), current_range);
   }
   return render_text.Pass();
 }
 
 int OmniboxResultView::GetMatchContentsWidth() const {
   InitContentsRenderTextIfNecessary();
   return contents_rendertext_ ? contents_rendertext_->GetContentWidth() : 0;
 }
 
+void OmniboxResultView::SetAnswerImage(gfx::ImageSkia image) {
+  answer_image_ = image;
+  SchedulePaint();
+}
+
 // TODO(skanuj): This is probably identical across all OmniboxResultView rows in
 // the omnibox dropdown. Consider sharing the result.
 int OmniboxResultView::GetDisplayOffset(
     const AutocompleteMatch& match,
     bool is_ui_rtl,
     bool is_match_contents_rtl) const {
   if (match.type != AutocompleteMatchType::SEARCH_SUGGEST_TAIL)
     return 0;
 
   const base::string16& input_text =
       base::UTF8ToUTF16(match.GetAdditionalInfo(kACMatchPropertyInputText));
   int contents_start_index = 0;
   base::StringToInt(match.GetAdditionalInfo(kACMatchPropertyContentsStartIndex),
                     &contents_start_index);
 
   scoped_ptr<gfx::RenderText> input_render_text(CreateRenderText(input_text));
   const gfx::Range& glyph_bounds =
       input_render_text->GetGlyphBounds(contents_start_index);
   const int start_padding = is_match_contents_rtl ?
       std::max(glyph_bounds.start(), glyph_bounds.end()) :
       std::min(glyph_bounds.start(), glyph_bounds.end());
 
   return is_ui_rtl ?
       (input_render_text->GetContentWidth() - start_padding) : start_padding;
 }
 
 // static
 int OmniboxResultView::default_icon_size_ = 0;
 
+void OmniboxResultView::GetAnswerIcon() {
+  // The answer images are not bundled with Chrome, so we need to download

[Peter Kasting, 2015/03/03 01:32:36]

Is there a reason the reader would have expected these images to be bundled with
Chrome?  If not, I would probably omit this comment.

[dschuyler, 2015/03/03 21:13:56]

Done.

+  // them.
+  if (image_service_) {
+    image_service_->CancelRequest(request_id_);
+    request_id_ = image_service_->RequestImage(
+        match_.answer->second_line().image_url(),
+        new AnswersImageObserverDesktop(GetWeakPtr()));
+  }
+}
+
 const char* OmniboxResultView::GetClassName() const {
   return "OmniboxResultView";
 }
 
 gfx::ImageSkia OmniboxResultView::GetIcon() const {
   const gfx::Image image = model_->GetIconIfExtensionMatch(model_index_);
   if (!image.IsEmpty())
     return image.AsImageSkia();
 
   int icon = model_->IsStarredMatch(match_) ?
@@ skipping 130 matching lines @@
     }
     PaintMatch(*keyword_match, keyword_contents_rendertext_.get(),
                keyword_description_rendertext_.get(), canvas, x);
   }
 }
 
 void OmniboxResultView::AnimationProgressed(const gfx::Animation* animation) {
   Layout();
   SchedulePaint();
 }