Bookmarks++: Add side panel images

Bug: 1365606
Change-Id: I2af5b24eb98000bf0e713ecae6e9f6bde8763267
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4303667
Reviewed-by: Chris Bookholt <bookholt@chromium.org>
Reviewed-by: Mihai Sardarescu <msarda@chromium.org>
Commit-Queue: Emily Shack <emshack@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1116561}

chrome/browser/chrome_browser_interface_binders.cc

@@ -1059,6 +1059,11 @@ void PopulateChromeWebUIFrameBinders(
       shopping_list::mojom::ShoppingListHandlerFactory, BookmarksSidePanelUI>(
       map);
 
+  if (base::FeatureList::IsEnabled(features::kPowerBookmarksSidePanel)) {
+    RegisterWebUIControllerInterfaceBinder<
+        image_service::mojom::ImageServiceHandler, BookmarksSidePanelUI>(map);
+  }
+
   if (base::FeatureList::IsEnabled(features::kSidePanelSearchCompanion)) {
     RegisterWebUIControllerInterfaceBinder<
         side_panel::mojom::SearchCompanionPageHandlerFactory,

chrome/browser/resources/side_panel/bookmarks/BUILD.gn

@@ -59,6 +59,7 @@ build_webui("build") {
     "../shared:build_ts",
     "//third_party/polymer/v3_0:library",
     "//ui/webui/resources/cr_components/color_change_listener:build_ts",
+    "//ui/webui/resources/cr_components/image_service:build_ts",
     "//ui/webui/resources/cr_elements:build_ts",
     "//ui/webui/resources/js:build_ts",
     "//ui/webui/resources/mojo:build_ts",

chrome/browser/resources/side_panel/bookmarks/power_bookmark_row.html

@@ -25,6 +25,7 @@
     role="listitem"
     size="[[getItemSize_(compact)]]"
     url="[[bookmark.url]]"
+    image-url="[[imageUrl]]"
     count="[[bookmark.children.length]]"
     title="[[bookmark.title]]"
     description="[[description]]"

chrome/browser/resources/side_panel/bookmarks/power_bookmark_row.ts

@@ -49,6 +49,10 @@ export class PowerBookmarkRowElement extends PolymerElement {
         reflectToAttribute: true,
         value: false,
       },
+      imageUrl: {
+        type: String,
+        value: '',
+      },
       rowAriaDescription: {
         type: String,
         value: '',
@@ -78,6 +82,7 @@ export class PowerBookmarkRowElement extends PolymerElement {
   rowAriaLabel: string;
   trailingIcon: string;
   trailingIconAriaLabel: string;
+  imageUrl: string;
 
   override connectedCallback() {
     super.connectedCallback();

chrome/browser/resources/side_panel/bookmarks/power_bookmarks_list.html

@@ -197,7 +197,8 @@ <h1 slot="heading">[[getActiveFolderLabel_(activeFolderPath_.*)]]</h1>
               on-input-change="onRename_"
               row-aria-label="[[getBookmarkAllyLabel_(item)]]"
               row-aria-description="[[getBookmarkA11yDescription_(item)]]"
-              tabindex$="[[tabIndex]]">
+              tabindex$="[[tabIndex]]"
+              image-url="[[getBookmarkImageUrl_(item, imageUrls_.*)]]">
             <template is="dom-if" restamp
                 if="[[isPriceTracked(item, trackedProductInfos_.*)]]">
               <sp-list-item-badge slot="badges"

chrome/browser/resources/side_panel/bookmarks/power_bookmarks_list.ts

@@ -180,6 +180,7 @@ export class PowerBookmarksListElement extends PolymerElement {
   private labels_: Label[];
   private compactDescriptions_ = new Map<string, string>();
   private expandedDescriptions_ = new Map<string, string>();
+  private imageUrls_ = new Map<string, string>();
   private activeSortIndex_: number;
   private sortTypes_: SortOption[];
   private searchQuery_: string|undefined;
@@ -257,6 +258,10 @@ export class PowerBookmarksListElement extends PolymerElement {
     this.set(`expandedDescriptions_.${bookmark.id}`, description);
   }
 
+  setImageUrl(bookmark: chrome.bookmarks.BookmarkTreeNode, url: string) {
+    this.set(`imageUrls_.${bookmark.id.toString()}`, url);
+  }
+
   onBookmarksLoaded() {
     this.updateShownBookmarks_();
   }
@@ -420,6 +425,11 @@ export class PowerBookmarksListElement extends PolymerElement {
     return description;
   }
 
+  private getBookmarkImageUrl_(bookmark: chrome.bookmarks.BookmarkTreeNode):
+      string {
+    return this.get(`imageUrls_.${bookmark.id.toString()}`);
+  }
+
   private getActiveFolderLabel_(): string {
     return this.getFolderLabel_(this.getActiveFolder_());
   }
@@ -458,6 +468,7 @@ export class PowerBookmarksListElement extends PolymerElement {
     this.shownBookmarks_ = this.bookmarksService_.filterBookmarks(
         this.getActiveFolder_(), this.activeSortIndex_, this.searchQuery_,
         this.labels_);
+    this.bookmarksService_.refreshDataForBookmarks(this.shownBookmarks_);
   }
 
   private canAddCurrentUrl_(): boolean {

chrome/browser/resources/side_panel/bookmarks/power_bookmarks_service.ts

@@ -4,8 +4,11 @@
 
 // This file contains business logic for power bookmarks side panel content.
 
+import {ImageServiceBrowserProxy} from '//resources/cr_components/image_service/browser_proxy.js';
+import {ClientId as ImageServiceClientId} from '//resources/cr_components/image_service/image_service.mojom-webui.js';
 import {loadTimeData} from '//resources/js/load_time_data.js';
 import {PluralStringProxyImpl} from '//resources/js/plural_string_proxy.js';
+import {Url} from '//resources/mojo/url/mojom/url.mojom-webui.js';
 
 import {BookmarksApiProxy, BookmarksApiProxyImpl} from './bookmarks_api_proxy.js';
 
@@ -21,6 +24,7 @@ interface PowerBookmarksDelegate {
       bookmark: chrome.bookmarks.BookmarkTreeNode, description: string): void;
   setExpandedDescription(
       bookmark: chrome.bookmarks.BookmarkTreeNode, description: string): void;
+  setImageUrl(bookmark: chrome.bookmarks.BookmarkTreeNode, url: string): void;
   onBookmarksLoaded(): void;
   onBookmarkChanged(id: string, changedInfo: chrome.bookmarks.ChangeInfo): void;
   onBookmarkCreated(
@@ -57,6 +61,7 @@ export class PowerBookmarksService {
       BookmarksApiProxyImpl.getInstance();
   private listeners_ = new Map<string, Function>();
   private folders_: chrome.bookmarks.BookmarkTreeNode[] = [];
+  private bookmarksWithCachedImages_ = new Set<string>();
 
   constructor(delegate: PowerBookmarksDelegate) {
     this.delegate_ = delegate;
@@ -202,6 +207,16 @@ export class PowerBookmarksService {
     return changedPosition;
   }
 
+  /**
+   * Checks bookmarks for any relevant data and updates delegate_ with the
+   * results. Used to batch data fetching in any cases where it is particularly
+   * expensive.
+   */
+  refreshDataForBookmarks(bookmarks: chrome.bookmarks.BookmarkTreeNode[]) {
+    bookmarks.forEach(
+        (bookmark) => this.findBookmarkImageUrls_(bookmark, false));
+  }
+
   /**
    * Returns the BookmarkTreeNode with the given id, or undefined if one does
    * not exist.
@@ -243,6 +258,7 @@ export class PowerBookmarksService {
     const bookmark = this.findBookmarkWithId(id)!;
     Object.assign(bookmark, changedInfo);
     this.findBookmarkDescriptions_(bookmark, false);
+    this.findBookmarkImageUrls_(bookmark, false);
     this.delegate_.onBookmarkChanged(id, changedInfo);
   }
 
@@ -257,6 +273,7 @@ export class PowerBookmarksService {
     this.delegate_.onBookmarkCreated(node, parent);
     this.findBookmarkDescriptions_(parent, false);
     this.findBookmarkDescriptions_(node, false);
+    this.findBookmarkImageUrls_(node, false);
   }
 
   private onMoved_(movedInfo: chrome.bookmarks.MoveInfo) {
@@ -353,6 +370,54 @@ export class PowerBookmarksService {
     }
   }
 
+  /**
+   * Assigns an image url for the given bookmark. Also assigns an image url to
+   * all descendants if recurse is true.
+   */
+  private async findBookmarkImageUrls_(
+      bookmark: chrome.bookmarks.BookmarkTreeNode, recurse: boolean) {
+    const hasImage =
+        this.bookmarksWithCachedImages_.has(bookmark.id.toString());
+    if (!hasImage) {
+      // Reset image url to ensure old images don't persist while the new image
+      // is being fetched.
+      this.delegate_.setImageUrl(bookmark, '');
+      if (bookmark.url) {
+        const imageUrl = await this.findBookmarkImageUrl_(bookmark.url);
+        if (imageUrl) {
+          this.delegate_.setImageUrl(bookmark, imageUrl);
+          this.bookmarksWithCachedImages_.add(bookmark.id.toString());
+        }
+      }
+    }
+    if (recurse && bookmark.children) {
+      bookmark.children.forEach(
+          child => this.findBookmarkImageUrls_(child, recurse));
+    }
+  }
+
+  private async findBookmarkImageUrl_(bookmarkUrl: string): Promise<string> {
+    const emptyUrl = '';
+
+    if (!bookmarkUrl || !loadTimeData.getBoolean('urlImagesEnabled')) {
+      return emptyUrl;
+    }
+
+    const url: Url = new Url();
+    url.url = bookmarkUrl;
+
+    // Fetch the representative image for this page, if possible.
+    const {result} =
+        await ImageServiceBrowserProxy.getInstance().handler.getPageImageUrl(
+            ImageServiceClientId.Bookmarks, url,
+            {suggestImages: true, optimizationGuideImages: true});
+    if (result) {
+      return result.imageUrl.url;
+    }
+
+    return emptyUrl;
+  }
+
   // Return an array that includes folder and all its descendants.
   private expandFolder_(folder: chrome.bookmarks.BookmarkTreeNode):
       chrome.bookmarks.BookmarkTreeNode[] {

chrome/browser/ui/webui/side_panel/bookmarks/bookmarks_side_panel_ui.cc

@@ -12,6 +12,7 @@
 #include "chrome/browser/browser_process.h"
 #include "chrome/browser/commerce/shopping_service_factory.h"
 #include "chrome/browser/feature_engagement/tracker_factory.h"
+#include "chrome/browser/image_service/image_service_factory.h"
 #include "chrome/browser/profiles/profile.h"
 #include "chrome/browser/profiles/profiles_state.h"
 #include "chrome/browser/ui/ui_features.h"
@@ -35,6 +36,9 @@
 #include "components/commerce/core/shopping_service.h"
 #include "components/commerce/core/webui/shopping_list_handler.h"
 #include "components/favicon_base/favicon_url_parser.h"
+#include "components/image_service/features.h"
+#include "components/image_service/image_service.h"
+#include "components/image_service/image_service_handler.h"
 #include "components/prefs/pref_service.h"
 #include "components/strings/grit/components_strings.h"
 #include "content/public/browser/web_contents.h"
@@ -144,6 +148,8 @@ BookmarksSidePanelUI::BookmarksSidePanelUI(content::WebUI* web_ui)
       !prefs->GetList(bookmarks::prefs::kManagedBookmarks).empty());
   source->AddBoolean("shoppingListEnabled",
                      commerce::IsShoppingListAllowedForEnterprise(prefs));
+  source->AddBoolean("urlImagesEnabled", base::FeatureList::IsEnabled(
+                                             image_service::kImageService));
 
   source->AddBoolean("guestMode", profile->IsGuestSession());
   source->AddBoolean("incognitoMode", profile->IsIncognitoProfile());
@@ -230,6 +236,19 @@ void BookmarksSidePanelUI::BindInterface(
       web_ui()->GetWebContents(), std::move(pending_receiver));
 }
 
+void BookmarksSidePanelUI::BindInterface(
+    mojo::PendingReceiver<image_service::mojom::ImageServiceHandler>
+        pending_image_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_image_handler), std::move(image_service_weak));
+}
+
 void BookmarksSidePanelUI::CreateBookmarksPageHandler(
     mojo::PendingReceiver<side_panel::mojom::BookmarksPageHandler> receiver) {
   bookmarks_page_handler_ =

chrome/browser/ui/webui/side_panel/bookmarks/bookmarks_side_panel_ui.h

@@ -10,6 +10,7 @@
 #include "chrome/browser/ui/webui/side_panel/bookmarks/bookmarks.mojom.h"
 #include "chrome/browser/ui/webui/webui_load_timer.h"
 #include "components/commerce/core/mojom/shopping_list.mojom.h"
+#include "components/image_service/mojom/image_service.mojom-forward.h"
 #include "mojo/public/cpp/bindings/pending_receiver.h"
 #include "mojo/public/cpp/bindings/pending_remote.h"
 #include "mojo/public/cpp/bindings/receiver.h"
@@ -26,6 +27,10 @@ namespace ui {
 class ColorChangeHandler;
 }
 
+namespace image_service {
+class ImageServiceHandler;
+}
+
 class BookmarksSidePanelUI
     : public ui::MojoBubbleWebUIController,
       public side_panel::mojom::BookmarksPageHandlerFactory,
@@ -50,6 +55,10 @@ class BookmarksSidePanelUI
       mojo::PendingReceiver<color_change_listener::mojom::PageHandler>
           pending_receiver);
 
+  void BindInterface(
+      mojo::PendingReceiver<image_service::mojom::ImageServiceHandler>
+          pending_image_handler);
+
  private:
   // side_panel::mojom::BookmarksPageHandlerFactory:
   void CreateBookmarksPageHandler(
@@ -69,6 +78,7 @@ class BookmarksSidePanelUI
   mojo::Receiver<shopping_list::mojom::ShoppingListHandlerFactory>
       shopping_list_factory_receiver_{this};
   std::unique_ptr<ui::ColorChangeHandler> color_provider_handler_;
+  std::unique_ptr<image_service::ImageServiceHandler> image_service_handler_;
 
   WEB_UI_CONTROLLER_TYPE_DECL();
 };

chrome/test/data/webui/side_panel/BUILD.gn

@@ -56,6 +56,7 @@ build_webui_tests("build") {
     "//chrome/browser/resources/side_panel/read_anything:build_ts",
     "//chrome/browser/resources/side_panel/reading_list:build_ts",
     "//chrome/browser/resources/side_panel/user_notes:build_ts",
+    "//ui/webui/resources/cr_components/image_service:build_ts",
     "//ui/webui/resources/cr_elements:build_ts",
     "//ui/webui/resources/js:build_ts",
     "//ui/webui/resources/mojo:build_ts",

chrome/test/data/webui/side_panel/bookmarks/power_bookmarks_list_test.ts

@@ -10,11 +10,14 @@ import {BookmarksApiProxyImpl} from 'chrome://bookmarks-side-panel.top-chrome/bo
 import {ShoppingListApiProxyImpl} from 'chrome://bookmarks-side-panel.top-chrome/commerce/shopping_list_api_proxy.js';
 import {PowerBookmarkRowElement} from 'chrome://bookmarks-side-panel.top-chrome/power_bookmark_row.js';
 import {PowerBookmarksListElement} from 'chrome://bookmarks-side-panel.top-chrome/power_bookmarks_list.js';
+import {ImageServiceBrowserProxy} from 'chrome://resources/cr_components/image_service/browser_proxy.js';
+import {ImageServiceHandlerRemote} from 'chrome://resources/cr_components/image_service/image_service.mojom-webui.js';
 import {loadTimeData} from 'chrome://resources/js/load_time_data.js';
 import {PluralStringProxyImpl} from 'chrome://resources/js/plural_string_proxy.js';
 import {flush} from 'chrome://resources/polymer/v3_0/polymer/polymer_bundled.min.js';
 import {assertEquals, assertNotEquals} 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';
 import {TestPluralStringProxy} from 'chrome://webui-test/test_plural_string_proxy.js';
 
 import {TestShoppingListApiProxy} from './commerce/test_shopping_list_api_proxy.js';
@@ -24,6 +27,8 @@ suite('SidePanelPowerBookmarksListTest', () => {
   let powerBookmarksList: PowerBookmarksListElement;
   let bookmarksApi: TestBookmarksApiProxy;
   let shoppingListApi: TestShoppingListApiProxy;
+  let imageServiceHandler: TestMock<ImageServiceHandlerRemote>&
+      ImageServiceHandlerRemote;
 
   const folders: chrome.bookmarks.BookmarkTreeNode[] = [
     {
@@ -81,6 +86,13 @@ suite('SidePanelPowerBookmarksListTest', () => {
     const pluralString = new TestPluralStringProxy();
     PluralStringProxyImpl.setInstance(pluralString);
 
+    imageServiceHandler = TestMock.fromClass(ImageServiceHandlerRemote);
+    ImageServiceBrowserProxy.setInstance(
+        new ImageServiceBrowserProxy(imageServiceHandler));
+    imageServiceHandler.setResultFor('getPageImageUrl', Promise.resolve({
+      result: {imageUrl: {url: 'https://example.com/image.png'}},
+    }));
+
     loadTimeData.overrideValues({
       sortOrder: SortOrder.kNewest,
       viewType: ViewType.kCompact,

chrome/test/data/webui/side_panel/bookmarks/power_bookmarks_service_test.ts

@@ -8,8 +8,11 @@ import 'chrome://bookmarks-side-panel.top-chrome/power_bookmarks_list.js';
 import {BookmarksApiProxyImpl} from 'chrome://bookmarks-side-panel.top-chrome/bookmarks_api_proxy.js';
 import {ShoppingListApiProxyImpl} from 'chrome://bookmarks-side-panel.top-chrome/commerce/shopping_list_api_proxy.js';
 import {PowerBookmarksService} from 'chrome://bookmarks-side-panel.top-chrome/power_bookmarks_service.js';
+import {ImageServiceBrowserProxy} from 'chrome://resources/cr_components/image_service/browser_proxy.js';
+import {ImageServiceHandlerRemote} from 'chrome://resources/cr_components/image_service/image_service.mojom-webui.js';
 import {PluralStringProxyImpl} from 'chrome://resources/js/plural_string_proxy.js';
 import {assertEquals} from 'chrome://webui-test/chai_assert.js';
+import {TestMock} from 'chrome://webui-test/test_mock.js';
 import {TestPluralStringProxy} from 'chrome://webui-test/test_plural_string_proxy.js';
 
 import {TestShoppingListApiProxy} from './commerce/test_shopping_list_api_proxy.js';
@@ -28,6 +31,8 @@ suite('SidePanelPowerBookmarksServiceTest', () => {
   let service: PowerBookmarksService;
   let bookmarksApi: TestBookmarksApiProxy;
   let shoppingListApi: TestShoppingListApiProxy;
+  let imageServiceHandler: TestMock<ImageServiceHandlerRemote>&
+      ImageServiceHandlerRemote;
 
   const folders: chrome.bookmarks.BookmarkTreeNode[] = [
     {
@@ -81,6 +86,13 @@ suite('SidePanelPowerBookmarksServiceTest', () => {
     const pluralString = new TestPluralStringProxy();
     PluralStringProxyImpl.setInstance(pluralString);
 
+    imageServiceHandler = TestMock.fromClass(ImageServiceHandlerRemote);
+    ImageServiceBrowserProxy.setInstance(
+        new ImageServiceBrowserProxy(imageServiceHandler));
+    imageServiceHandler.setResultFor('getPageImageUrl', Promise.resolve({
+      result: {imageUrl: {url: 'https://example.com/image.png'}},
+    }));
+
     delegate = new ServiceTestPowerBookmarksDelegate();
     service = new PowerBookmarksService(delegate);
     service.startListening();

chrome/test/data/webui/side_panel/bookmarks/test_power_bookmarks_delegate.ts

@@ -10,6 +10,7 @@ export class TestPowerBookmarksDelegate extends TestBrowserProxy {
       'setCurrentUrl',
       'setCompactDescription',
       'setExpandedDescription',
+      'setImageUrl',
       'onBookmarksLoaded',
       'onBookmarkChanged',
       'onBookmarkCreated',
@@ -33,6 +34,10 @@ export class TestPowerBookmarksDelegate extends TestBrowserProxy {
     this.methodCalled('setExpandedDescription', bookmark, description);
   }
 
+  setImageUrl(bookmark: chrome.bookmarks.BookmarkTreeNode, url: string) {
+    this.methodCalled('setImageUrl', bookmark, url);
+  }
+
   onBookmarksLoaded() {
     this.methodCalled('onBookmarksLoaded');
   }