Omnibox Material Refresh: Search Provider Favicon Scaffolding

For Material Refresh, we want to place the default search provider's
favicon into the location bar while the user is typing search queries.

This CL adds some scaffolding necessary for OmniboxView to provide the
favicon. The actual favicon fetching must be asynchronous, so we need
this extra infrastructure.

I expect this CL to be Part 1/4 or so of the primary implementation.

Bug: 823535
Change-Id: Ia7d6979222fd18e4a34fd1473f00164ecc8a1c7b
Reviewed-on: https://chromium-review.googlesource.com/1022500
Commit-Queue: Tommy Li <tommycli@chromium.org>
Reviewed-by: Peter Kasting <pkasting@chromium.org>
Cr-Commit-Position: refs/heads/master@{#552933}

chrome/browser/ui/cocoa/location_bar/location_bar_view_mac.mm

@@ -84,6 +84,11 @@ const int kMinURLWidth = 120;
 // SkColorSetARGB(0xCC, 0xFF, 0xFF 0xFF);
 const SkColor kMaterialDarkVectorIconColor = SK_ColorWHITE;
 
+void NotReached(const gfx::Image& image) {
+  // Mac Cocoa version should not receive asynchronously delivered favicons.
+  NOTREACHED();
+}
+
 }  // namespace
 
 // TODO(shess): This code is mostly copied from the gtk
@@ -454,15 +459,17 @@ void LocationBarViewMac::UpdateWithoutTabRestore() {
 
 void LocationBarViewMac::UpdateLocationIcon() {
   SkColor vector_icon_color = GetLocationBarIconColor();
-  const gfx::VectorIcon& vector_icon_id =
-      GetPageInfoVerboseType() == PageInfoVerboseType::kEVCert
-          ? toolbar::kHttpsValidIcon
-          : omnibox_view_->GetVectorIcon();
+  gfx::ImageSkia image_skia;
+  if (GetPageInfoVerboseType() == PageInfoVerboseType::kEVCert) {
+    image_skia = gfx::CreateVectorIcon(toolbar::kHttpsValidIcon,
+                                       kDefaultIconSize, vector_icon_color);
+  } else {
+    image_skia = omnibox_view_->GetIcon(kDefaultIconSize, vector_icon_color,
+                                        base::BindOnce(&NotReached));
+  }
 
   NSImage* image = NSImageFromImageSkiaWithColorSpace(
-      gfx::CreateVectorIcon(vector_icon_id, kDefaultIconSize,
-                            vector_icon_color),
-      base::mac::GetSRGBColorSpace());
+      image_skia, base::mac::GetSRGBColorSpace());
   page_info_decoration_->SetImage(image);
   page_info_decoration_->SetLabelColor(vector_icon_color);
 

chrome/browser/ui/views/location_bar/location_bar_view.cc

@@ -193,8 +193,7 @@ LocationBarView::LocationBarView(Browser* browser,
       browser_(browser),
       delegate_(delegate),
       is_popup_mode_(is_popup_mode),
-      tint_(GetTintForProfile(profile)),
-      focus_ring_(nullptr) {
+      tint_(GetTintForProfile(profile)) {
   edit_bookmarks_enabled_.Init(
       bookmarks::prefs::kEditBookmarksEnabled, profile->GetPrefs(),
       base::Bind(&LocationBarView::UpdateWithoutTabRestore,
@@ -846,11 +845,24 @@ void LocationBarView::RefreshBackground() {
 }
 
 void LocationBarView::RefreshLocationIcon() {
+  // Cancel any previous outstanding icon requests, as they are now outdated.
+  icon_fetch_weak_ptr_factory_.InvalidateWeakPtrs();
+
   security_state::SecurityLevel security_level =
       GetToolbarModel()->GetSecurityLevel(false);
-  location_icon_view_->SetImage(gfx::CreateVectorIcon(
-      omnibox_view_->GetVectorIcon(), GetLayoutConstant(LOCATION_BAR_ICON_SIZE),
-      GetSecurityChipColor(security_level)));
+
+  gfx::ImageSkia icon = omnibox_view_->GetIcon(
+      GetLayoutConstant(LOCATION_BAR_ICON_SIZE),
+      GetSecurityChipColor(security_level),
+      base::BindOnce(&LocationBarView::OnLocationIconFetched,
+                     icon_fetch_weak_ptr_factory_.GetWeakPtr()));
+
+  location_icon_view_->SetImage(icon);
+  location_icon_view_->Update();
+}
+
+void LocationBarView::OnLocationIconFetched(const gfx::Image& image) {
+  location_icon_view_->SetImage(image.AsImageSkia());
   location_icon_view_->Update();
 }
 

chrome/browser/ui/views/location_bar/location_bar_view.h

@@ -13,6 +13,7 @@
 #include "base/compiler_specific.h"
 #include "base/gtest_prod_util.h"
 #include "base/macros.h"
+#include "base/memory/weak_ptr.h"
 #include "chrome/browser/extensions/extension_context_menu_model.h"
 #include "chrome/browser/ui/location_bar/location_bar.h"
 #include "chrome/browser/ui/omnibox/chrome_omnibox_edit_controller.h"
@@ -29,6 +30,7 @@
 #include "ui/gfx/animation/slide_animation.h"
 #include "ui/gfx/font.h"
 #include "ui/gfx/geometry/rect.h"
+#include "ui/gfx/image/image.h"
 #include "ui/views/controls/button/button.h"
 #include "ui/views/drag_controller.h"
 
@@ -290,6 +292,9 @@ class LocationBarView : public LocationBar,
   // Updates |location_icon_view_| based on the current state and theme.
   void RefreshLocationIcon();
 
+  // Handles the arrival of an asynchronously fetched location bar icon.
+  void OnLocationIconFetched(const gfx::Image& image);
+
   // Updates the visibility state of the Content Blocked icons to reflect what
   // is actually blocked on the current page. Returns true if the visibility
   // of at least one of the views in |content_setting_views_| changed.
@@ -463,7 +468,7 @@ class LocationBarView : public LocationBar,
   bool show_focus_rect_ = false;
 
   // The focus ring view.
-  views::View* focus_ring_;
+  views::View* focus_ring_ = nullptr;
 
   // Tracks this preference to determine whether bookmark editing is allowed.
   BooleanPrefMember edit_bookmarks_enabled_;
@@ -476,6 +481,12 @@ class LocationBarView : public LocationBar,
   security_state::SecurityLevel last_update_security_level_ =
       security_state::NONE;
 
+  // Used to scope the lifetime of asynchronous icon fetch callbacks to the
+  // lifetime of the object. Weak pointers issued by this factory are
+  // invalidated whenever we start a new icon fetch, so don't use this weak
+  // factory for any other purposes.
+  base::WeakPtrFactory<LocationBarView> icon_fetch_weak_ptr_factory_{this};
+
   DISALLOW_COPY_AND_ASSIGN(LocationBarView);
 };
 

components/omnibox/browser/omnibox_view.cc

@@ -21,6 +21,11 @@
 #include "components/toolbar/toolbar_model.h"
 #include "extensions/common/constants.h"
 #include "ui/base/l10n/l10n_util.h"
+#include "ui/base/material_design/material_design_controller.h"
+
+#if !defined(OS_IOS)
+#include "ui/gfx/paint_vector_icon.h"
+#endif
 
 // static
 base::string16 OmniboxView::StripJavascriptSchemas(const base::string16& text) {
@@ -96,14 +101,39 @@ bool OmniboxView::IsEditingOrEmpty() const {
       (GetOmniboxTextLength() == 0);
 }
 
-const gfx::VectorIcon& OmniboxView::GetVectorIcon() const {
-  if (!IsEditingOrEmpty())
-    return controller_->GetToolbarModel()->GetVectorIcon();
+gfx::ImageSkia OmniboxView::GetIcon(int dip_size,
+                                    SkColor color,
+                                    IconFetchedCallback on_icon_fetched) const {
+#if defined(OS_IOS)
+  // OmniboxViewIOS provides its own icon logic. The iOS build also does not
+  // link in the vector icon rendering code.
+  return gfx::ImageSkia();
+#else   // !defined(OS_IOS)
+  if (!IsEditingOrEmpty()) {
+    return gfx::CreateVectorIcon(
+        controller_->GetToolbarModel()->GetVectorIcon(), dip_size, color);
+  }
+
+  // For Material Refresh, display the favicon of the default search engine.
+  const auto type = model_ ? model_->CurrentTextType()
+                           : AutocompleteMatchType::URL_WHAT_YOU_TYPED;
+  if (ui::MaterialDesignController::IsNewerMaterialUi() &&
+      AutocompleteMatch::IsSearchType(type)) {
+    // TODO(tommycli): Implement fetching the default search provider and
+    // starting the async request for its favicon here. This will also need
+    // caching to provide good performance.
+
+    // Fall through to provide the vector icon until we receive the favicon.
+    // Note that the FaviconService can fail to fetch the favicon, in which
+    // the default vector icon we provide below should remain.
+  }
 
-  return AutocompleteMatch::TypeToVectorIcon(
-      model_ ? model_->CurrentTextType()
-             : AutocompleteMatchType::URL_WHAT_YOU_TYPED,
-      /*is_bookmark=*/false, /*is_tab_match=*/false);
+  const gfx::VectorIcon& vector_icon =
+      AutocompleteMatch::TypeToVectorIcon(type,
+                                          /*is_bookmark=*/false,
+                                          /*is_tab_match=*/false);
+  return gfx::CreateVectorIcon(vector_icon, dip_size, color);
+#endif  // defined(OS_IOS)
 }
 
 void OmniboxView::SetUserText(const base::string16& text) {

components/omnibox/browser/omnibox_view.h

@@ -23,7 +23,9 @@
 #include "base/strings/utf_string_conversions.h"
 #include "components/omnibox/browser/autocomplete_match.h"
 #include "components/omnibox/browser/omnibox_client.h"
+#include "third_party/skia/include/core/SkColor.h"
 #include "ui/base/window_open_disposition.h"
+#include "ui/gfx/image/image_skia.h"
 #include "ui/gfx/native_widget_types.h"
 #include "ui/gfx/range/range.h"
 
@@ -34,6 +36,8 @@ class OmniboxEditModel;
 
 class OmniboxView {
  public:
+  using IconFetchedCallback = base::OnceCallback<void(const gfx::Image& icon)>;
+
   // Represents the changes between two State objects.  This is used by the
   // model to determine how its internal state should be updated after the view
   // state changes.  See OmniboxEditModel::OnAfterPossibleChange().
@@ -84,8 +88,11 @@ class OmniboxView {
   // the field is empty.
   bool IsEditingOrEmpty() const;
 
-  // Returns the vector icon to display as the location icon.
-  const gfx::VectorIcon& GetVectorIcon() const;
+  // Returns the icon to display as the location icon. If a favicon is
+  // available, |on_icon_fetched| may be called later asynchronously.
+  gfx::ImageSkia GetIcon(int dip_size,
+                         SkColor color,
+                         IconFetchedCallback on_icon_fetched) const;
 
   // The user text is the text the user has manually keyed in.  When present,
   // this is shown in preference to the permanent text; hitting escape will