History WebUI: Replace CallJavascriptFunctionUnsafe for deleteHistory

- Replace CallJavascriptFunctionUnsafe('deleteHistory') with a
  FireWebUIListener() call.
- Remove global deleteHistory function in history.js and forwarding
  in app.js, and directly listen for the event in history_list.js.
- Add a test verifying that the event is correctly handled.

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

chrome/browser/resources/history/BUILD.gn

@@ -119,6 +119,7 @@ js_library("history_list") {
     "//ui/webui/resources/js:i18n_behavior",
     "//ui/webui/resources/js:load_time_data",
     "//ui/webui/resources/js:util",
+    "//ui/webui/resources/js:web_ui_listener_behavior",
   ]
 }
 

chrome/browser/resources/history/app.js

@@ -327,13 +327,6 @@ Polymer({
     this.set('queryResult_.sessionList', sessionList);
   },
 
-  /**
-   * Called when browsing data is cleared.
-   */
-  historyDeleted: function() {
-    this.$.history.historyDeleted();
-  },
-
   /**
    * Update sign in state of synced device manager after user logs in or out.
    * @param {boolean} isUserSignedIn

chrome/browser/resources/history/history.js

@@ -61,12 +61,3 @@ function setForeignSessions(sessionList) {
         .setForeignSessions(sessionList);
   });
 }
-
-/**
- * Called when the history is deleted by someone else.
- */
-function historyDeleted() {
-  waitForAppUpgrade().then(function() {
-    /** @type {HistoryAppElement} */ ($('history-app')).historyDeleted();
-  });
-}

chrome/browser/resources/history/history_list.html

@@ -3,6 +3,7 @@
 <link rel="import" href="chrome://resources/cr_elements/cr_lazy_render/cr_lazy_render.html">
 <link rel="import" href="chrome://resources/cr_elements/shared_style_css.html">
 <link rel="import" href="chrome://resources/html/i18n_behavior.html">
+<link rel="import" href="chrome://resources/html/web_ui_listener_behavior.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="chrome://resources/polymer/v1_0/iron-scroll-threshold/iron-scroll-threshold.html">

chrome/browser/resources/history/history_list.js

@@ -5,7 +5,7 @@
 Polymer({
   is: 'history-list',
 
-  behaviors: [I18nBehavior],
+  behaviors: [I18nBehavior, WebUIListenerBehavior],
 
   properties: {
     // The search term for the current query. Set when the query returns.
@@ -87,6 +87,8 @@ Polymer({
     this.$['infinite-list'].scrollTarget = this;
     this.$['scroll-threshold'].scrollTarget = this;
     this.setAttribute('aria-roledescription', this.i18n('ariaRoleDescription'));
+
+    this.addWebUIListener('history-deleted', () => this.onHistoryDeleted_());
   },
 
   /////////////////////////////////////////////////////////////////////////////
@@ -148,7 +150,8 @@ Polymer({
     this.resultLoadingDisabled_ = finished;
   },
 
-  historyDeleted: function() {
+  /** @private */
+  onHistoryDeleted_: function() {
     // Do not reload the list when there are items checked.
     if (this.getSelectedItemCount() > 0) {
       return;

chrome/browser/ui/webui/history/browsing_history_handler.cc

@@ -440,7 +440,12 @@ void BrowsingHistoryHandler::OnRemoveVisitsFailed() {
 }
 
 void BrowsingHistoryHandler::HistoryDeleted() {
-  web_ui()->CallJavascriptFunctionUnsafe("historyDeleted");
+  if (IsJavascriptAllowed()) {
+    FireWebUIListener("history-deleted", base::Value());
+  } else {
+    deferred_callbacks_.push_back(base::BindOnce(
+        &BrowsingHistoryHandler::HistoryDeleted, weak_factory_.GetWeakPtr()));
+  }
 }
 
 void BrowsingHistoryHandler::HandleHistoryLoaded(const base::ListValue* args) {

chrome/browser/ui/webui/history/browsing_history_handler_unittest.cc

@@ -99,6 +99,21 @@ class BrowsingHistoryHandlerTest : public ChromeRenderViewHostTestHarness {
     };
   }
 
+  void VerifyHistoryDeletedFired() {
+    const content::TestWebUI::CallData& data = *web_ui()->call_data().back();
+    EXPECT_EQ("cr.webUIListenerCallback", data.function_name());
+    std::string event_fired;
+    ASSERT_TRUE(data.arg1()->GetAsString(&event_fired));
+    EXPECT_EQ("history-deleted", event_fired);
+  }
+
+  void InitializeWebUI(BrowsingHistoryHandlerWithWebUIForTesting& handler) {
+    // Send historyLoaded so that JS will be allowed.
+    base::Value init_args(base::Value::Type::LIST);
+    init_args.Append("history-loaded-callback-id");
+    handler.HandleHistoryLoaded(&base::Value::AsListValue(init_args));
+  }
+
   syncer::TestSyncService* sync_service() { return sync_service_; }
   history::WebHistoryService* web_history_service() {
     return web_history_service_;
@@ -136,13 +151,14 @@ TEST_F(BrowsingHistoryHandlerTest, ObservingWebHistoryDeletions) {
         syncer::SyncService::TransportState::ACTIVE);
     BrowsingHistoryHandlerWithWebUIForTesting handler(web_ui());
     handler.RegisterMessages();
+    InitializeWebUI(handler);
 
     web_history_service()->ExpireHistoryBetween(
         std::set<GURL>(), base::Time(), base::Time::Max(), callback,
         PARTIAL_TRAFFIC_ANNOTATION_FOR_TESTS);
 
     EXPECT_EQ(1U, web_ui()->call_data().size());
-    EXPECT_EQ("historyDeleted", web_ui()->call_data().back()->function_name());
+    VerifyHistoryDeletedFired();
   }
 
   // BrowsingHistoryHandler will be informed about WebHistoryService deletions
@@ -160,8 +176,11 @@ TEST_F(BrowsingHistoryHandlerTest, ObservingWebHistoryDeletions) {
         std::set<GURL>(), base::Time(), base::Time::Max(), callback,
         PARTIAL_TRAFFIC_ANNOTATION_FOR_TESTS);
 
+    // Simulate initialization after history has been deleted.
+    InitializeWebUI(handler);
+
     EXPECT_EQ(2U, web_ui()->call_data().size());
-    EXPECT_EQ("historyDeleted", web_ui()->call_data().back()->function_name());
+    VerifyHistoryDeletedFired();
   }
 
   // BrowsingHistoryHandler does not fire historyDeleted while a web history
@@ -171,11 +190,7 @@ TEST_F(BrowsingHistoryHandlerTest, ObservingWebHistoryDeletions) {
         syncer::SyncService::TransportState::ACTIVE);
     BrowsingHistoryHandlerWithWebUIForTesting handler(web_ui());
     handler.RegisterMessages();
-
-    // First, send historyLoaded so that JS will be allowed.
-    base::Value init_args(base::Value::Type::LIST);
-    init_args.Append("history-loaded-callback-id");
-    handler.HandleHistoryLoaded(&base::Value::AsListValue(init_args));
+    InitializeWebUI(handler);
 
     // Simulate a delete request.
     base::Value args(base::Value::Type::LIST);

chrome/test/data/webui/history/history_list_test.js

@@ -638,6 +638,24 @@ suite('<history-list>', function() {
         });
   });
 
+  test('deleteHistory results in query-history event', function() {
+    app.historyResult(createHistoryInfo(), TEST_HISTORY_RESULTS);
+
+    cr.webUIListenerCallback('history-deleted');
+    return testService.whenCalled('queryHistory').then(() => {
+      Polymer.dom.flush();
+      const items = polymerSelectAll(element, 'history-item');
+
+      items[2].$.checkbox.click();
+      items[3].$.checkbox.click();
+
+      testService.resetResolver('queryHistory');
+      cr.webUIListenerCallback('history-deleted');
+      test_util.flushTasks();
+      assertEquals(0, testService.getCallCount('queryHistory'));
+    });
+  });
+
   teardown(function() {
     app.fire('change-query', {search: ''});
   });