Bookmarks Web UI: Use browser proxy, update tests

Change the bookmarks Web UI to use a browser proxy for communication
with C++. Also update tests to use a test version of the proxy
instead of registering test message handlers in C++.

Bug: 1022213
Change-Id: I7a824d51e1fdaa02e57baa44c555c634f8e230b1
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1906959
Commit-Queue: Rebekah Potter <rbpotter@chromium.org>
Reviewed-by: Demetrios Papadopoulos <dpapad@chromium.org>
Cr-Commit-Position: refs/heads/master@{#714368}

chrome/browser/resources/bookmarks/BUILD.gn

@@ -55,6 +55,7 @@ js_type_check("closure_compile") {
     ":actions",
     ":api_listener",
     ":app",
+    ":browser_proxy",
     ":command_manager",
     ":constants",
     ":debouncer",
@@ -88,6 +89,7 @@ js_library("actions") {
 js_library("api_listener") {
   deps = [
     ":actions",
+    ":browser_proxy",
     ":debouncer",
     ":store",
     ":util",
@@ -112,9 +114,17 @@ js_library("app") {
   externs_list = [ "$externs_path/chrome_extensions.js" ]
 }
 
+js_library("browser_proxy") {
+  deps = [
+    ":constants",
+    "//ui/webui/resources/js:cr",
+  ]
+}
+
 js_library("command_manager") {
   deps = [
     ":api_listener",
+    ":browser_proxy",
     ":dialog_focus_manager",
     ":edit_dialog",
     ":store_client",
@@ -197,6 +207,7 @@ js_library("item") {
 js_library("list") {
   deps = [
     ":actions",
+    ":browser_proxy",
     ":command_manager",
     ":item",
     ":store_client",

chrome/browser/resources/bookmarks/api_listener.html

 <link rel="import" href="chrome://resources/html/cr.html">
 <link rel="import" href="actions.html">
+<link rel="import" href="browser_proxy.html">
 <link rel="import" href="debouncer.html">
 <link rel="import" href="store.html">
 <link rel="import" href="util.html">

chrome/browser/resources/bookmarks/api_listener.js

@@ -172,12 +172,13 @@ cr.define('bookmarks.ApiListener', function() {
   function init() {
     listeners.forEach((listener) => listener.api.addListener(listener.fn));
 
-    cr.sendWithPromise('getIncognitoAvailability')
-        .then(onIncognitoAvailabilityChanged);
+    const browserProxy = bookmarks.BrowserProxy.getInstance();
+    browserProxy.getIncognitoAvailability().then(
+        onIncognitoAvailabilityChanged);
     cr.addWebUIListener(
         'incognito-availability-changed', onIncognitoAvailabilityChanged);
 
-    cr.sendWithPromise('getCanEditBookmarks').then(onCanEditBookmarksChanged);
+    browserProxy.getCanEditBookmarks().then(onCanEditBookmarksChanged);
     cr.addWebUIListener(
         'can-edit-bookmarks-changed', onCanEditBookmarksChanged);
   }

chrome/browser/resources/bookmarks/bookmarks_resources.grd

@@ -34,6 +34,12 @@
       <structure name="IDR_BOOKMARKS_BOOKMARKS_HTML"
                  file="bookmarks.html"
                  type="chrome_html" />
+      <structure name="IDR_BOOKMARKS_BROWSER_PROXY_HTML"
+                 file="browser_proxy.html"
+                 type="chrome_html" />
+      <structure name="IDR_BOOKMARKS_BROWSER_PROXY_JS"
+                 file="browser_proxy.js"
+                 type="chrome_html" />
       <structure name="IDR_BOOKMARKS_COMMAND_MANAGER_HTML"
                  file="command_manager.html"
                  type="chrome_html" />

chrome/browser/resources/bookmarks/browser_proxy.html

+<link rel="import" href="chrome://resources/html/cr.html">
+<link rel="import" href="constants.html">
+
+<script src="browser_proxy.js"></script>

chrome/browser/resources/bookmarks/browser_proxy.js

+// Copyright 2019 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.
+
+cr.define('bookmarks', function() {
+  'use strict';
+
+  class BrowserProxy {
+    /**
+     * @return {!Promise<!IncognitoAvailability>} Promise resolved with the
+     *     current incognito mode preference.
+     */
+    getIncognitoAvailability() {
+      return cr.sendWithPromise('getIncognitoAvailability');
+    }
+
+    /**
+     * @return {!Promise<boolean>} Promise resolved with whether the bookmarks
+     *     can be edited.
+     */
+    getCanEditBookmarks() {
+      return cr.sendWithPromise('getCanEditBookmarks');
+    }
+
+    /**
+     * @return {!Promise<string>} Promise resolved with the appropriate plural
+     *     string for |messageName| with |itemCount| items.
+     */
+    getPluralString(messageName, itemCount) {
+      return cr.sendWithPromise('getPluralString', messageName, itemCount);
+    }
+
+    /**
+     * Notifies the metrics handler to record a histogram value.
+     * @param {string} histogram The name of the histogram to record
+     * @param {number} bucket The bucket to record
+     * @param {number} maxBucket The maximum bucket value in the histogram.
+     */
+    recordInHistogram(histogram, bucket, maxBucket) {
+      chrome.send(
+          'metricsHandler:recordInHistogram', [histogram, bucket, maxBucket]);
+    }
+  }
+
+  cr.addSingletonGetter(BrowserProxy);
+
+  return {
+    BrowserProxy: BrowserProxy,
+  };
+});

chrome/browser/resources/bookmarks/command_manager.html

@@ -6,6 +6,7 @@
 <link rel="import" href="chrome://resources/cr_elements/shared_vars_css.html">
 <link rel="import" href="chrome://resources/html/cr/ui/command.html">
 <link rel="import" href="chrome://resources/polymer/v1_0/iron-a11y-keys-behavior/iron-a11y-keys-behavior.html">
+<link rel="import" href="browser_proxy.html">
 <link rel="import" href="dialog_focus_manager.html">
 <link rel="import" href="edit_dialog.html">
 <link rel="import" href="shared_style.html">

chrome/browser/resources/bookmarks/command_manager.js

@@ -56,6 +56,9 @@ cr.define('bookmarks', function() {
       assert(CommandManager.instance_ == null);
       CommandManager.instance_ = this;
 
+      /** @private {!bookmarks.BrowserProxy} */
+      this.browserProxy_ = bookmarks.BrowserProxy.getInstance();
+
       this.watch('globalCanEdit_', state => state.prefs.canEdit);
       this.updateFromStore();
 
@@ -318,8 +321,8 @@ cr.define('bookmarks', function() {
               labelPromise =
                   Promise.resolve(loadTimeData.getString('toastItemCopied'));
             } else {
-              labelPromise = cr.sendWithPromise(
-                  'getPluralString', 'toastItemsCopied', idList.length);
+              labelPromise = this.browserProxy_.getPluralString(
+                  'toastItemsCopied', idList.length);
             }
 
             this.showTitleToast_(
@@ -344,8 +347,8 @@ cr.define('bookmarks', function() {
             labelPromise =
                 Promise.resolve(loadTimeData.getString('toastItemDeleted'));
           } else {
-            labelPromise = cr.sendWithPromise(
-                'getPluralString', 'toastItemsDeleted', idList.length);
+            labelPromise = this.browserProxy_.getPluralString(
+                'toastItemsDeleted', idList.length);
           }
 
           chrome.bookmarkManagerPrivate.removeTrees(idList, () => {
@@ -778,7 +781,8 @@ cr.define('bookmarks', function() {
                                             Command.OPEN_BOOKMARK;
       }
 
-      bookmarks.util.recordEnumHistogram(histogram, command, Command.MAX_VALUE);
+      this.browserProxy_.recordInHistogram(
+          histogram, command, Command.MAX_VALUE);
     },
 
     /**
@@ -829,7 +833,7 @@ cr.define('bookmarks', function() {
       } else {
         this.openCommandMenuAtPosition(e.detail.x, e.detail.y, e.detail.source);
       }
-      bookmarks.util.recordEnumHistogram(
+      this.browserProxy_.recordInHistogram(
           'BookmarkManager.CommandMenuOpened', e.detail.source,
           MenuSource.NUM_VALUES);
     },

chrome/browser/resources/bookmarks/list.html

@@ -3,6 +3,7 @@
 <link rel="import" href="chrome://resources/cr_elements/shared_vars_css.html">
 <link rel="import" href="chrome://resources/polymer/v1_0/iron-a11y-announcer/iron-a11y-announcer.html">
 <link rel="import" href="chrome://resources/polymer/v1_0/iron-list/iron-list.html">
+<link rel="import" href="browser_proxy.html">
 <link rel="import" href="command_manager.html">
 <link rel="import" href="item.html">
 <link rel="import" href="shared_style.html">

chrome/browser/resources/bookmarks/list.js

@@ -119,8 +119,8 @@ Polymer({
     // Trigger a layout of the iron list. Otherwise some elements may render
     // as blank entries. See https://crbug.com/848683
     this.$.list.fire('iron-resize');
-    const label = await cr.sendWithPromise(
-        'getPluralString', 'listChanged', this.displayedList_.length);
+    const label = await bookmarks.BrowserProxy.getInstance().getPluralString(
+        'listChanged', this.displayedList_.length);
     this.fire('iron-announce', {text: label});
 
     if (!skipFocus && selectIndex > -1) {

chrome/browser/resources/bookmarks/util.js

@@ -171,15 +171,6 @@ cr.define('bookmarks.util', function() {
     return descendants;
   }
 
-  /**
-   * @param {string} name
-   * @param {number} value
-   * @param {number} maxValue
-   */
-  function recordEnumHistogram(name, value, maxValue) {
-    chrome.send('metricsHandler:recordInHistogram', [name, value, maxValue]);
-  }
-
   /**
    * @param {!Object<string, T>} map
    * @param {!Set<string>} ids
@@ -232,7 +223,6 @@ cr.define('bookmarks.util', function() {
     isShowingSearch: isShowingSearch,
     normalizeNode: normalizeNode,
     normalizeNodes: normalizeNodes,
-    recordEnumHistogram: recordEnumHistogram,
     removeIdsFromMap: removeIdsFromMap,
     removeIdsFromObject: removeIdsFromObject,
     removeIdsFromSet: removeIdsFromSet,

chrome/browser/ui/webui/bookmarks/bookmarks_browsertest.cc

@@ -4,11 +4,12 @@
 
 #include "chrome/browser/ui/webui/bookmarks/bookmarks_browsertest.h"
 
-#include "base/bind.h"
+#include <memory>
+#include <utility>
+
 #include "base/strings/utf_string_conversions.h"
 #include "chrome/browser/bookmarks/bookmark_model_factory.h"
 #include "chrome/browser/bookmarks/managed_bookmark_service_factory.h"
-#include "chrome/browser/prefs/incognito_mode_prefs.h"
 #include "chrome/browser/profiles/profile.h"
 #include "chrome/browser/ui/browser.h"
 #include "chrome/common/pref_names.h"
@@ -23,29 +24,6 @@ BookmarksBrowserTest::BookmarksBrowserTest() {}
 
 BookmarksBrowserTest::~BookmarksBrowserTest() {}
 
-void BookmarksBrowserTest::RegisterMessages() {
-  web_ui()->RegisterMessageCallback(
-      "testSetIncognito",
-      base::BindRepeating(&BookmarksBrowserTest::HandleSetIncognitoAvailability,
-                          base::Unretained(this)));
-  web_ui()->RegisterMessageCallback(
-      "testSetCanEdit",
-      base::BindRepeating(&BookmarksBrowserTest::HandleSetCanEditBookmarks,
-                          base::Unretained(this)));
-}
-
-void BookmarksBrowserTest::SetIncognitoAvailability(int availability) {
-  ASSERT_TRUE(availability >= 0 &&
-              availability < IncognitoModePrefs::AVAILABILITY_NUM_TYPES);
-  browser()->profile()->GetPrefs()->SetInteger(
-      prefs::kIncognitoModeAvailability, availability);
-}
-
-void BookmarksBrowserTest::SetCanEditBookmarks(bool canEdit) {
-  browser()->profile()->GetPrefs()->SetBoolean(
-      bookmarks::prefs::kEditBookmarksEnabled, canEdit);
-}
-
 void BookmarksBrowserTest::SetupExtensionAPITest() {
   // Add managed bookmarks.
   Profile* profile = browser()->profile();
@@ -87,37 +65,3 @@ void BookmarksBrowserTest::SetupExtensionAPIEditDisabledTest() {
   PrefService* prefs = user_prefs::UserPrefs::Get(profile);
   prefs->SetBoolean(bookmarks::prefs::kEditBookmarksEnabled, false);
 }
-
-void BookmarksBrowserTest::HandleSetIncognitoAvailability(
-    const base::ListValue* args) {
-  AllowJavascript();
-
-  ASSERT_EQ(2U, args->GetSize());
-  const base::Value* callback_id;
-  ASSERT_TRUE(args->Get(0, &callback_id));
-  int pref_value;
-  ASSERT_TRUE(args->GetInteger(1, &pref_value));
-
-  SetIncognitoAvailability(pref_value);
-
-  ResolveJavascriptCallback(*callback_id, base::Value());
-}
-
-void BookmarksBrowserTest::HandleSetCanEditBookmarks(
-    const base::ListValue* args) {
-  AllowJavascript();
-
-  ASSERT_EQ(2U, args->GetSize());
-  const base::Value* callback_id;
-  ASSERT_TRUE(args->Get(0, &callback_id));
-  bool pref_value;
-  ASSERT_TRUE(args->GetBoolean(1, &pref_value));
-
-  SetCanEditBookmarks(pref_value);
-
-  ResolveJavascriptCallback(*callback_id, base::Value());
-}
-
-content::WebUIMessageHandler* BookmarksBrowserTest::GetMockMessageHandler() {
-  return this;
-}

chrome/browser/ui/webui/bookmarks/bookmarks_browsertest.h

@@ -6,30 +6,16 @@
 #define CHROME_BROWSER_UI_WEBUI_BOOKMARKS_BOOKMARKS_BROWSERTEST_H_
 
 #include "chrome/test/base/web_ui_browser_test.h"
-#include "content/public/browser/web_ui_message_handler.h"
 
-class BookmarksBrowserTest : public WebUIBrowserTest,
-                             public content::WebUIMessageHandler {
+class BookmarksBrowserTest : public WebUIBrowserTest {
  public:
   BookmarksBrowserTest();
   ~BookmarksBrowserTest() override;
 
-  void SetIncognitoAvailability(int availability);
-  void SetCanEditBookmarks(bool canEdit);
-
   void SetupExtensionAPITest();
   void SetupExtensionAPIEditDisabledTest();
 
  private:
-  void HandleSetIncognitoAvailability(const base::ListValue* args);
-  void HandleSetCanEditBookmarks(const base::ListValue* args);
-
-  // content::WebUIMessageHandler:
-  void RegisterMessages() override;
-
-  // WebUIBrowserTest:
-  content::WebUIMessageHandler* GetMockMessageHandler() override;
-
   DISALLOW_COPY_AND_ASSIGN(BookmarksBrowserTest);
 };
 

chrome/browser/ui/webui/bookmarks/bookmarks_ui.cc

@@ -129,6 +129,9 @@ content::WebUIDataSource* CreateBookmarksUIHTMLSource(Profile* profile) {
   source->AddResourcePath("api_listener.js", IDR_BOOKMARKS_API_LISTENER_JS);
   source->AddResourcePath("app.html", IDR_BOOKMARKS_APP_HTML);
   source->AddResourcePath("app.js", IDR_BOOKMARKS_APP_JS);
+  source->AddResourcePath("browser_proxy.html",
+                          IDR_BOOKMARKS_BROWSER_PROXY_HTML);
+  source->AddResourcePath("browser_proxy.js", IDR_BOOKMARKS_BROWSER_PROXY_JS);
   source->AddResourcePath("command_manager.html",
                           IDR_BOOKMARKS_COMMAND_MANAGER_HTML);
   source->AddResourcePath("command_manager.js",

chrome/test/data/webui/bookmarks/policy_test.js

@@ -5,6 +5,8 @@
 suite('Bookmarks policies', function() {
   let store;
   let app;
+  /** @type {?bookmarks.BrowserProxy} */
+  let testBrowserProxy;
 
   setup(function() {
     const nodes = testTree(createFolder('1', [
@@ -20,6 +22,8 @@ suite('Bookmarks policies', function() {
     store.expectAction('set-can-edit');
     store.replaceSingleton();
 
+    testBrowserProxy = new bookmarks.TestBookmarksBrowserProxy();
+    bookmarks.BrowserProxy.instance_ = testBrowserProxy;
     app = document.createElement('bookmarks-app');
     replaceBody(app);
   });
@@ -28,13 +32,18 @@ suite('Bookmarks policies', function() {
     const commandManager = bookmarks.CommandManager.getInstance();
     // Incognito is disabled during testGenPreamble(). Wait for the front-end to
     // load the config.
-    const action = await store.waitForAction('set-incognito-availability');
+    const whenIncognitoSet = await Promise.all([
+      testBrowserProxy.whenCalled('getIncognitoAvailability'),
+      store.waitForAction('set-incognito-availability')
+    ]);
+
     assertEquals(
         IncognitoAvailability.DISABLED, store.data.prefs.incognitoAvailability);
     assertFalse(
         commandManager.canExecute(Command.OPEN_INCOGNITO, new Set(['11'])));
 
-    await cr.sendWithPromise('testSetIncognito', IncognitoAvailability.ENABLED);
+    cr.webUIListenerCallback(
+        'incognito-availability-changed', IncognitoAvailability.ENABLED);
     assertEquals(
         IncognitoAvailability.ENABLED, store.data.prefs.incognitoAvailability);
     assertTrue(
@@ -43,11 +52,14 @@ suite('Bookmarks policies', function() {
 
   test('canEdit updates when changed', async function() {
     const commandManager = bookmarks.CommandManager.getInstance();
-    const action = await store.waitForAction('set-can-edit');
+    const whenCanEditSet = await Promise.all([
+      testBrowserProxy.whenCalled('getCanEditBookmarks'),
+      store.waitForAction('set-can-edit')
+    ]);
     assertFalse(store.data.prefs.canEdit);
     assertFalse(commandManager.canExecute(Command.DELETE, new Set(['11'])));
 
-    await cr.sendWithPromise('testSetCanEdit', true);
+    cr.webUIListenerCallback('can-edit-bookmarks-changed', true);
     assertTrue(store.data.prefs.canEdit);
     assertTrue(commandManager.canExecute(Command.DELETE, new Set(['11'])));
   });

chrome/test/data/webui/bookmarks/test_browser_proxy.js

+// Copyright 2019 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.
+
+cr.define('bookmarks', function() {
+  /**
+   * Test version of the bookmarks browser proxy.
+   */
+  class TestBookmarksBrowserProxy extends TestBrowserProxy {
+    constructor() {
+      super(['getIncognitoAvailability', 'getCanEditBookmarks']);
+    }
+
+    getIncognitoAvailability() {
+      this.methodCalled('getIncognitoAvailability');
+      return Promise.resolve(IncognitoAvailability.DISABLED);
+    }
+
+    getCanEditBookmarks() {
+      this.methodCalled('getCanEditBookmarks');
+      return Promise.resolve(false);
+    }
+
+    getPluralString(messageName, itemCount) {
+      return Promise.resolve('test');
+    }
+
+    recordInHistogram(histogram, bucket, maxBucket) {}
+  }
+
+  return {
+    TestBookmarksBrowserProxy: TestBookmarksBrowserProxy,
+  };
+});