[image_service] Hook up ImageService mojom to History Clusters WebUI

This CL hooks up the ImageService mojom that was added in-component here, to the WebUI. https://chromium-review.googlesource.com/c/chromium/src/+/4265691 It doesn't create a cr_component, as we're going to try just using per-UI surface BrowserProxy instances for now. Bug: b/244507194, 1367485, b/248367751 Change-Id: I0bc5fdccc68df20684167c6ad191ef9271567313 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4251681 Reviewed-by: Alex Gough <ajgo@chromium.org> Reviewed-by: Demetrios Papadopoulos <dpapad@chromium.org> Commit-Queue: Tommy Li <tommycli@chromium.org> Cr-Commit-Position: refs/heads/main@{#1108655}
chromium · Feb 22, 2023 · 789079d · 789079d
1 parent 05c208c
commit 789079d
Show file tree

Hide file tree

Showing 23 changed files with 237 additions and 18 deletions.
diff --git a/chrome/browser/chrome_browser_interface_binders.cc b/chrome/browser/chrome_browser_interface_binders.cc
@@ -182,6 +182,7 @@
 #include "chrome/browser/ui/webui/whats_new/whats_new_ui.h"
 #include "chrome/common/webui_url_constants.h"
 #include "components/commerce/core/mojom/shopping_list.mojom.h"  // nogncheck crbug.com/1125897
+#include "components/image_service/mojom/image_service.mojom.h"
 #include "components/search/ntp_features.h"
 #include "ui/webui/resources/cr_components/color_change_listener/color_change_listener.mojom.h"
 #include "ui/webui/resources/cr_components/customize_themes/customize_themes.mojom.h"
@@ -981,6 +982,12 @@ void PopulateChromeWebUIFrameBinders(
       RegisterWebUIControllerInterfaceBinder<
           history_clusters::mojom::PageHandler, HistoryUI>(map);
     }
+
+    if (history_clusters_service->IsJourneysImagesEnabled()) {
+      RegisterWebUIControllerInterfaceBinder<
+          image_service::mojom::ImageServiceHandler, HistoryUI,
+          HistoryClustersSidePanelUI>(map);
+    }
   }
 
   RegisterWebUIControllerInterfaceBinder<

diff --git a/chrome/browser/ui/webui/cr_components/history_clusters/history_clusters_util.cc b/chrome/browser/ui/webui/cr_components/history_clusters/history_clusters_util.cc
@@ -4,13 +4,15 @@
 
 #include "chrome/browser/ui/webui/cr_components/history_clusters/history_clusters_util.h"
 
+#include "base/feature_list.h"
 #include "chrome/browser/history_clusters/history_clusters_service_factory.h"
 #include "chrome/browser/profiles/profile.h"
 #include "chrome/common/pref_names.h"
 #include "chrome/grit/generated_resources.h"
 #include "components/history_clusters/core/config.h"
 #include "components/history_clusters/core/history_clusters_prefs.h"
 #include "components/history_clusters/core/history_clusters_service.h"
+#include "components/image_service/features.h"
 #include "components/prefs/pref_service.h"
 #include "components/strings/grit/components_strings.h"
 #include "content/public/browser/web_ui_data_source.h"
@@ -39,6 +41,10 @@ void HistoryClustersUtil::PopulateSource(content::WebUIDataSource* source,
                      history_clusters::GetConfig().hide_visits);
   source->AddBoolean("isHideVisitsIconEnabled",
                      history_clusters::GetConfig().hide_visits_icon);
+  source->AddBoolean(
+      "isHistoryClustersImagesEnabled",
+      history_clusters::GetConfig().images &&
+          base::FeatureList::IsEnabled(image_service::kImageService));
 
   static constexpr webui::LocalizedString kHistoryClustersStrings[] = {
       {"actionMenuDescription", IDS_HISTORY_CLUSTERS_ACTION_MENU_DESCRIPTION},

diff --git a/chrome/browser/ui/webui/history/history_ui.cc b/chrome/browser/ui/webui/history/history_ui.cc
@@ -13,8 +13,10 @@
 #include "base/functional/bind.h"
 #include "base/functional/callback_helpers.h"
 #include "base/memory/ref_counted_memory.h"
+#include "base/memory/weak_ptr.h"
 #include "base/values.h"
 #include "build/build_config.h"
+#include "chrome/browser/image_service/image_service_factory.h"
 #include "chrome/browser/profiles/profile.h"
 #include "chrome/browser/signin/identity_manager_factory.h"
 #include "chrome/browser/ui/ui_features.h"
@@ -39,6 +41,8 @@
 #include "components/history_clusters/core/config.h"
 #include "components/history_clusters/core/features.h"
 #include "components/history_clusters/core/history_clusters_prefs.h"
+#include "components/image_service/image_service.h"
+#include "components/image_service/image_service_handler.h"
 #include "components/prefs/pref_service.h"
 #include "components/signin/public/base/signin_pref_names.h"
 #include "components/signin/public/identity_manager/identity_manager.h"
@@ -192,6 +196,19 @@ void HistoryUI::BindInterface(
           web_ui()->GetWebContents());
 }
 
+void HistoryUI::BindInterface(
+    mojo::PendingReceiver<image_service::mojom::ImageServiceHandler>
+        pending_page_handler) {
+  base::WeakPtr<image_service::ImageService> image_service_weak;
+  if (auto* image_service =
+          image_service::ImageServiceFactory::GetForBrowserContext(
+              Profile::FromWebUI(web_ui()))) {
+    image_service_weak = image_service->GetWeakPtr();
+  }
+  image_service_handler_ = std::make_unique<image_service::ImageServiceHandler>(
+      std::move(pending_page_handler), std::move(image_service_weak));
+}
+
 void HistoryUI::UpdateDataSource() {
   CHECK(web_ui());
 

diff --git a/chrome/browser/ui/webui/history/history_ui.h b/chrome/browser/ui/webui/history/history_ui.h
@@ -8,6 +8,7 @@
 #include <memory>
 
 #include "base/gtest_prod_util.h"
+#include "components/image_service/mojom/image_service.mojom-forward.h"
 #include "components/prefs/pref_change_registrar.h"
 #include "mojo/public/cpp/bindings/pending_receiver.h"
 #include "ui/base/layout.h"
@@ -22,6 +23,10 @@ namespace history_clusters {
 class HistoryClustersHandler;
 }
 
+namespace image_service {
+class ImageServiceHandler;
+}
+
 class HistoryUI : public ui::MojoWebUIController {
  public:
   explicit HistoryUI(content::WebUI* web_ui);
@@ -32,11 +37,12 @@ class HistoryUI : public ui::MojoWebUIController {
   static base::RefCountedMemory* GetFaviconResourceBytes(
       ui::ResourceScaleFactor scale_factor);
 
-  // Instantiates the implementor of the history_clusters::mojom::PageHandler
-  // mojo interface passing to it the pending receiver that will be internally
-  // bound.
+  // Instantiates the implementors of mojom interfaces.
   void BindInterface(mojo::PendingReceiver<history_clusters::mojom::PageHandler>
                          pending_page_handler);
+  void BindInterface(
+      mojo::PendingReceiver<image_service::mojom::ImageServiceHandler>
+          pending_page_handler);
 
   // For testing only.
   history_clusters::HistoryClustersHandler*
@@ -47,6 +53,7 @@ class HistoryUI : public ui::MojoWebUIController {
  private:
   std::unique_ptr<history_clusters::HistoryClustersHandler>
       history_clusters_handler_;
+  std::unique_ptr<image_service::ImageServiceHandler> image_service_handler_;
   PrefChangeRegistrar pref_change_registrar_;
 
   void UpdateDataSource();

diff --git a/chrome/browser/ui/webui/side_panel/history_clusters/history_clusters_side_panel_ui.cc b/chrome/browser/ui/webui/side_panel/history_clusters/history_clusters_side_panel_ui.cc
@@ -7,6 +7,7 @@
 #include <string>
 #include <utility>
 
+#include "chrome/browser/image_service/image_service_factory.h"
 #include "chrome/browser/profiles/profile.h"
 #include "chrome/browser/ui/ui_features.h"
 #include "chrome/browser/ui/webui/cr_components/history_clusters/history_clusters_util.h"
@@ -17,6 +18,8 @@
 #include "chrome/grit/side_panel_history_clusters_resources.h"
 #include "chrome/grit/side_panel_history_clusters_resources_map.h"
 #include "components/favicon_base/favicon_url_parser.h"
+#include "components/image_service/image_service.h"
+#include "components/image_service/image_service_handler.h"
 #include "components/strings/grit/components_strings.h"
 #include "content/public/browser/navigation_handle.h"
 #include "content/public/browser/web_ui.h"
@@ -56,6 +59,19 @@ void HistoryClustersSidePanelUI::BindInterface(
   history_clusters_handler_->SetSidePanelUIEmbedder(this->embedder());
 }
 
+void HistoryClustersSidePanelUI::BindInterface(
+    mojo::PendingReceiver<image_service::mojom::ImageServiceHandler>
+        pending_page_handler) {
+  base::WeakPtr<image_service::ImageService> image_service_weak;
+  if (auto* image_service =
+          image_service::ImageServiceFactory::GetForBrowserContext(
+              Profile::FromWebUI(web_ui()))) {
+    image_service_weak = image_service->GetWeakPtr();
+  }
+  image_service_handler_ = std::make_unique<image_service::ImageServiceHandler>(
+      std::move(pending_page_handler), std::move(image_service_weak));
+}
+
 base::WeakPtr<HistoryClustersSidePanelUI>
 HistoryClustersSidePanelUI::GetWeakPtr() {
   return weak_ptr_factory_.GetWeakPtr();

diff --git a/chrome/browser/ui/webui/side_panel/history_clusters/history_clusters_side_panel_ui.h b/chrome/browser/ui/webui/side_panel/history_clusters/history_clusters_side_panel_ui.h
@@ -9,6 +9,7 @@
 
 #include "base/memory/weak_ptr.h"
 #include "chrome/browser/history_clusters/history_clusters_metrics_logger.h"
+#include "components/image_service/mojom/image_service.mojom-forward.h"
 #include "content/public/browser/web_contents_observer.h"
 #include "mojo/public/cpp/bindings/pending_receiver.h"
 #include "ui/webui/mojo_bubble_web_ui_controller.h"
@@ -18,6 +19,10 @@ namespace history_clusters {
 class HistoryClustersHandler;
 }
 
+namespace image_service {
+class ImageServiceHandler;
+}
+
 class HistoryClustersSidePanelUI : public ui::MojoBubbleWebUIController,
                                    public content::WebContentsObserver {
  public:
@@ -31,6 +36,9 @@ class HistoryClustersSidePanelUI : public ui::MojoBubbleWebUIController,
   // interface passing the pending receiver that will be internally bound.
   void BindInterface(mojo::PendingReceiver<history_clusters::mojom::PageHandler>
                          pending_page_handler);
+  void BindInterface(
+      mojo::PendingReceiver<image_service::mojom::ImageServiceHandler>
+          pending_page_handler);
 
   // Gets a weak pointer to this object.
   base::WeakPtr<HistoryClustersSidePanelUI> GetWeakPtr();
@@ -56,6 +64,7 @@ class HistoryClustersSidePanelUI : public ui::MojoBubbleWebUIController,
  private:
   std::unique_ptr<history_clusters::HistoryClustersHandler>
       history_clusters_handler_;
+  std::unique_ptr<image_service::ImageServiceHandler> image_service_handler_;
 
   // The initial state that we have to cache here until the page finishes its
   // navigation to the WebUI host.

diff --git a/chrome/test/data/webui/cr_components/BUILD.gn b/chrome/test/data/webui/cr_components/BUILD.gn
@@ -41,6 +41,7 @@ build_webui_tests("build") {
     "//ui/webui/resources/cr_components/customize_themes:build_ts",
     "//ui/webui/resources/cr_components/help_bubble:build_ts",
     "//ui/webui/resources/cr_components/history_clusters:build_ts",
+    "//ui/webui/resources/cr_components/image_service:build_ts",
     "//ui/webui/resources/cr_components/localized_link:build_ts",
     "//ui/webui/resources/cr_components/managed_dialog:build_ts",
     "//ui/webui/resources/cr_components/managed_footnote:build_ts",

diff --git a/chrome/test/data/webui/cr_components/cr_components_mojo_browsertest.js b/chrome/test/data/webui/cr_components/cr_components_mojo_browsertest.js
@@ -8,6 +8,7 @@
 GEN_INCLUDE(['//chrome/test/data/webui/polymer_browser_test_base.js']);
 
 GEN('#include "build/build_config.h"');
+GEN('#include "components/history_clusters/core/features.h"');
 GEN('#include "content/public/test/browser_test.h"');
 
 /** Test fixture for shared Polymer 3 components using Mojo. */
@@ -59,6 +60,15 @@ var CrComponentsHistoryClustersTest =
   get browsePreload() {
     return 'chrome://history/test_loader.html?module=cr_components/history_clusters_test.js';
   }
+
+  /** @override */
+  get featureList() {
+    return {
+      enabled: [
+        'history_clusters::internal::kJourneysImages',
+      ],
+    };
+  }
 };
 
 TEST_F('CrComponentsHistoryClustersTest', 'All', function() {

diff --git a/chrome/test/data/webui/cr_components/history_clusters_test.ts b/chrome/test/data/webui/cr_components/history_clusters_test.ts
@@ -9,21 +9,36 @@ import {BrowserProxyImpl} from 'chrome://resources/cr_components/history_cluster
 import {HistoryClustersElement} from 'chrome://resources/cr_components/history_clusters/clusters.js';
 import {Cluster, RawVisitData, URLVisit} from 'chrome://resources/cr_components/history_clusters/history_cluster_types.mojom-webui.js';
 import {PageCallbackRouter, PageHandlerRemote, PageRemote, QueryResult} from 'chrome://resources/cr_components/history_clusters/history_clusters.mojom-webui.js';
-import {assertEquals} from 'chrome://webui-test/chai_assert.js';
+import {ImageServiceBrowserProxy} from 'chrome://resources/cr_components/image_service/browser_proxy.js';
+import {ClientId as ImageServiceClientId, ImageServiceHandlerRemote} from 'chrome://resources/cr_components/image_service/image_service.mojom-webui.js';
+import {loadTimeData} from 'chrome://resources/js/load_time_data.js';
+import {assertEquals, assertTrue} from 'chrome://webui-test/chai_assert.js';
 import {flushTasks} from 'chrome://webui-test/polymer_test_util.js';
 import {TestMock} from 'chrome://webui-test/test_mock.js';
 
 let handler: TestMock<PageHandlerRemote>&PageHandlerRemote;
 let callbackRouterRemote: PageRemote;
+let imageServiceHandler: TestMock<ImageServiceHandlerRemote>&
+    ImageServiceHandlerRemote;
 
 function createBrowserProxy() {
   handler = TestMock.fromClass(PageHandlerRemote);
   const callbackRouter = new PageCallbackRouter();
   BrowserProxyImpl.setInstance(new BrowserProxyImpl(handler, callbackRouter));
   callbackRouterRemote = callbackRouter.$.bindNewPipeAndPassRemote();
+
+  imageServiceHandler = TestMock.fromClass(ImageServiceHandlerRemote);
+  ImageServiceBrowserProxy.setInstance(
+      new ImageServiceBrowserProxy(imageServiceHandler));
 }
 
 suite('history-clusters', () => {
+  suiteSetup(() => {
+    loadTimeData.overrideValues({
+      isHistoryClustersImagesEnabled: true,
+    });
+  });
+
   setup(() => {
     document.body.innerHTML = window.trustedTypes!.emptyHTML;
 
@@ -197,4 +212,37 @@ suite('history-clusters', () => {
     assertEquals(true, openHistoryClusterArgs[1].shiftKey);
     assertEquals(1, handler.getCallCount('openHistoryCluster'));
   });
-});
+
+  test('url visit requests image', async () => {
+    const clustersElement = await setupClustersElement();
+
+    callbackRouterRemote.onClustersQueryResult(getTestResult());
+    await callbackRouterRemote.$.flushForTesting();
+    flushTasks();
+
+    // Set a result for the image handler to pass back to the favicon component,
+    // so it doesn't throw a console error.
+    imageServiceHandler.setResultFor('getPageImageUrl', Promise.resolve({
+      result: {imageUrl: {url: 'https://example.com/image.png'}},
+    }));
+
+    const urlVisit =
+        clustersElement.$.clusters.querySelector('history-cluster')!.$.container
+            .querySelector('url-visit');
+    assertTrue(!!urlVisit);
+    // Assign a copied visit object with `isKnownToSync` set to true.
+    urlVisit.visit = Object.assign({}, urlVisit.visit, {isKnownToSync: true});
+
+    const [clientId, pageUrl] =
+        await imageServiceHandler.whenCalled('getPageImageUrl');
+    assertEquals(ImageServiceClientId.Journeys, clientId);
+    assertEquals(urlVisit.visit.normalizedUrl, pageUrl);
+
+    // Verify the icon element received the handler's response.
+    const icon = urlVisit.shadowRoot!.querySelector('page-favicon');
+    assertTrue(!!icon);
+    const imageUrl = icon.getImageUrlForTesting();
+    assertTrue(!!imageUrl);
+    assertEquals('https://example.com/image.png', imageUrl.url);
+  });
+});
diff --git a/components/history_clusters/core/history_clusters_service.cc b/components/history_clusters/core/history_clusters_service.cc
@@ -118,6 +118,11 @@ bool HistoryClustersService::IsJourneysEnabled() const {
   return is_journeys_enabled_;
 }
 
+// static
+bool HistoryClustersService::IsJourneysImagesEnabled() {
+  return GetConfig().images;
+}
+
 void HistoryClustersService::AddObserver(Observer* obs) {
   observers_.AddObserver(obs);
 }

diff --git a/components/history_clusters/core/history_clusters_service.h b/components/history_clusters/core/history_clusters_service.h
@@ -114,6 +114,9 @@ class HistoryClustersService : public base::SupportsUserData,
   // that's already evaluated against the g_browser_process application locale.
   bool IsJourneysEnabled() const;
 
+  // Returns true if the Journeys use of Images is enabled.
+  static bool IsJourneysImagesEnabled();
+
   // Used to add and remove observers.
   void AddObserver(Observer* obs);
   void RemoveObserver(Observer* obs);

diff --git a/components/image_service/BUILD.gn b/components/image_service/BUILD.gn
@@ -11,8 +11,8 @@ source_set("image_service") {
     "image_service_handler.cc",
     "image_service_handler.h",
   ]
+  public_deps = [ "mojom:mojo_bindings" ]
   deps = [
-    "mojom:mojo_bindings",
     "//base",
     "//components/google/core/common",
     "//components/keyed_service/core",

diff --git a/tools/typescript/path_mappings.py b/tools/typescript/path_mappings.py
@@ -17,6 +17,7 @@ def GetDepToPathMappings(root_gen_dir):
       "cr_components/customize_themes",
       "cr_components/help_bubble",
       "cr_components/history_clusters",
+      "cr_components/image_service",
       "cr_components/localized_link",
       "cr_components/managed_dialog",
       "cr_components/managed_footnote",

diff --git a/ui/webui/DEPS b/ui/webui/DEPS
@@ -5,5 +5,6 @@ include_rules = [
   "+services/service_manager/public/cpp/binder_registry.h",
   "+ui/base",
   "+ui/gfx",
-  "+components/content_settings/core"
+  "+components/content_settings/core",
+  "+components/image_service"
 ]
diff --git a/ui/webui/resources/BUILD.gn b/ui/webui/resources/BUILD.gn
@@ -112,11 +112,13 @@ generate_grd("build_grd") {
   if (!is_android && !is_ios) {
     public_deps += [
       "cr_components/color_change_listener:build_grdp",
+      "cr_components/image_service:build_grdp",
       "//third_party/lottie:build_grdp",
     ]
     grdp_files += [
       "$root_gen_dir/third_party/lottie/resources.grdp",
       "$root_gen_dir/ui/webui/resources/cr_components/color_change_listener/resources.grdp",
+      "$root_gen_dir/ui/webui/resources/cr_components/image_service/resources.grdp",
     ]
   }
 }

diff --git a/ui/webui/resources/cr_components/history_clusters/BUILD.gn b/ui/webui/resources/cr_components/history_clusters/BUILD.gn
@@ -59,6 +59,7 @@ build_cr_component("build") {
       "$root_gen_dir/ui/webui/resources/tsc/cr_components/history_clusters"
   ts_definitions = [ "//tools/typescript/definitions/metrics_private.d.ts" ]
   ts_deps = [
+    "../image_service:build_ts",
     "//third_party/polymer/v3_0:library",
     "//ui/webui/resources/cr_elements:build_ts",
     "//ui/webui/resources/js:build_ts",

diff --git a/ui/webui/resources/cr_components/history_clusters/page_favicon.html b/ui/webui/resources/cr_components/history_clusters/page_favicon.html
@@ -24,6 +24,6 @@
   }
 </style>
 
-<template is="dom-if" if="[[imageUrl]]">
-  <img id="page-image" is="cr-auto-img" auto-src="[[imageUrl.url]]">
+<template is="dom-if" if="[[imageUrl_]]">
+  <img id="page-image" is="cr-auto-img" auto-src="[[imageUrl_.url]]">
 </template>