Let SearchEngineTabHelper extend FaviconDriverObserver to update the favicon in TemplateURLService.

This CL let iOS SearchEngineTabHelper extend FaviconDriverObserver so that
it has equivalent functionality as the SearchEngineTabHelper in
/chrome/browser/ui/search_engines.

Bug: 433824
Change-Id: If49d1b46d90b2250c2b378d77d9e3cea5cdd96a5
Reviewed-on: https://chromium-review.googlesource.com/c/1329171
Commit-Queue: Yi Su <mrsuyi@chromium.org>
Reviewed-by: Mark Cogan <marq@chromium.org>
Cr-Commit-Position: refs/heads/master@{#606898}

ios/chrome/browser/search_engines/BUILD.gn

@@ -36,6 +36,7 @@ source_set("search_engines") {
   ]
   deps = [
     "//base",
+    "//components/favicon/ios",
     "//components/google/core/browser",
     "//components/history/core/browser",
     "//components/keyed_service/core",
@@ -75,9 +76,11 @@ source_set("unit_tests") {
     ":search_engines",
     "//base:base",
     "//base/test:test_support",
+    "//components/favicon/ios",
     "//components/search_engines",
     "//ios/chrome/browser/browser_state",
     "//ios/chrome/browser/browser_state:test_support",
+    "//ios/chrome/browser/favicon:favicon",
     "//ios/chrome/browser/web:test_support",
     "//ios/web",
     "//ios/web/public/test",

ios/chrome/browser/search_engines/search_engine_tab_helper.h

@@ -6,7 +6,8 @@
 #define IOS_CHROME_BROWSER_SEARCH_ENGINES_SEARCH_ENGINE_TAB_HELPER_H_
 
 #include "base/macros.h"
-#include "base/memory/weak_ptr.h"
+#include "base/scoped_observer.h"
+#include "components/favicon/ios/web_favicon_driver.h"
 #import "ios/web/public/web_state/web_state_observer.h"
 #import "ios/web/public/web_state/web_state_user_data.h"
 
@@ -33,7 +34,8 @@ class WebState;
 //
 class SearchEngineTabHelper
     : public web::WebStateObserver,
-      public web::WebStateUserData<SearchEngineTabHelper> {
+      public web::WebStateUserData<SearchEngineTabHelper>,
+      public favicon::FaviconDriverObserver {
  public:
   ~SearchEngineTabHelper() override;
 
@@ -62,6 +64,17 @@ class SearchEngineTabHelper
                    bool form_in_main_frame,
                    web::WebFrame* sender_frame);
 
+  // favicon::FaviconDriverObserver implementation.
+  void OnFaviconUpdated(favicon::FaviconDriver* driver,
+                        NotificationIconType notification_icon_type,
+                        const GURL& icon_url,
+                        bool icon_url_changed,
+                        const gfx::Image& image) override;
+
+  // Manages observation relationship between |this| and WebFaviconDriver.
+  ScopedObserver<favicon::WebFaviconDriver, favicon::FaviconDriverObserver>
+      favicon_driver_observer_{this};
+
   // WebState this tab helper is attached to.
   web::WebState* web_state_ = nullptr;
 

ios/chrome/browser/search_engines/search_engine_tab_helper.mm

@@ -82,12 +82,31 @@ SearchEngineTabHelper::SearchEngineTabHelper(web::WebState* web_state)
       base::BindRepeating(&SearchEngineTabHelper::OnJsMessage,
                           base::Unretained(this)),
       kCommandPrefix);
+  DCHECK(favicon::WebFaviconDriver::FromWebState(web_state));
+  favicon_driver_observer_.Add(
+      favicon::WebFaviconDriver::FromWebState(web_state));
 }
 
 void SearchEngineTabHelper::WebStateDestroyed(web::WebState* web_state) {
   web_state->RemoveScriptCommandCallback(kCommandPrefix);
   web_state->RemoveObserver(this);
   web_state_ = nullptr;
+  favicon_driver_observer_.RemoveAll();
+}
+
+// When favicon is updated, notify TemplateURLService about the change.
+void SearchEngineTabHelper::OnFaviconUpdated(
+    favicon::FaviconDriver* driver,
+    NotificationIconType notification_icon_type,
+    const GURL& icon_url,
+    bool icon_url_changed,
+    const gfx::Image& image) {
+  ios::ChromeBrowserState* browser_state =
+      ios::ChromeBrowserState::FromBrowserState(web_state_->GetBrowserState());
+  TemplateURLService* url_service =
+      ios::TemplateURLServiceFactory::GetForBrowserState(browser_state);
+  if (url_service && url_service->loaded())
+    url_service->UpdateProviderFavicons(driver->GetActiveURL(), icon_url);
 }
 
 // When the page is loaded, checks if |searchable_url_| has a value generated

ios/chrome/browser/search_engines/search_engine_tab_helper_unittest.mm

@@ -6,9 +6,12 @@
 
 #include "base/strings/utf_string_conversions.h"
 #import "base/test/ios/wait_util.h"
+#include "components/favicon/ios/web_favicon_driver.h"
+#include "components/keyed_service/core/service_access_type.h"
 #include "components/search_engines/template_url_service.h"
 #include "ios/chrome/browser/browser_state/chrome_browser_state.h"
 #include "ios/chrome/browser/browser_state/test_chrome_browser_state.h"
+#include "ios/chrome/browser/favicon/favicon_service_factory.h"
 #include "ios/chrome/browser/search_engines/template_url_service_factory.h"
 #include "ios/chrome/browser/web/chrome_web_test.h"
 #import "ios/web/public/test/web_test_with_web_state.h"
@@ -50,6 +53,10 @@ class SearchEngineTabHelperTest : public ChromeWebTest {
 
   void SetUp() override {
     WebTestWithWebState::SetUp();
+    favicon::WebFaviconDriver::CreateForWebState(
+        web_state(), ios::FaviconServiceFactory::GetForBrowserState(
+                         chrome_browser_state_->GetOriginalChromeBrowserState(),
+                         ServiceAccessType::IMPLICIT_ACCESS));
     SearchEngineTabHelper::CreateForWebState(web_state());
     server_.ServeFilesFromSourceDirectory(".");
     ASSERT_TRUE(server_.Start());

ios/chrome/browser/tabs/tab_helper_util.mm

@@ -103,10 +103,6 @@ void AttachTabHelpers(web::WebState* web_state, bool for_prerender) {
     ImageFetchTabHelper::CreateForWebState(web_state);
   }
 
-  if (base::FeatureList::IsEnabled(kCustomSearchEngines)) {
-    SearchEngineTabHelper::CreateForWebState(web_state);
-  }
-
   ReadingListModel* model =
       ReadingListModelFactory::GetForBrowserState(browser_state);
   ReadingListWebStateObserver::CreateForWebState(web_state, model);
@@ -125,6 +121,11 @@ void AttachTabHelpers(web::WebState* web_state, bool for_prerender) {
 
   AutofillTabHelper::CreateForWebState(web_state, nullptr);
 
+  // Depends on favicon::WebFaviconDriver, must be created after it.
+  if (base::FeatureList::IsEnabled(kCustomSearchEngines)) {
+    SearchEngineTabHelper::CreateForWebState(web_state);
+  }
+
   FormSuggestionTabHelper::CreateForWebState(web_state, @[
     PasswordTabHelper::FromWebState(web_state)->GetSuggestionProvider(),
     AutofillTabHelper::FromWebState(web_state)->GetSuggestionProvider(),