[memories] Allows deleting visits from the Memories landing page.

This CL introduces the ability to delete one or more visit from the Memories landing page. Given that we cannot guarantee that the output of the clustering model will not contain duplicate visits to the same URL, a visit in the Memories UI may represent the latest visit in a set of visits to the same URL in a given Memory. This is similar to the history UI itself which contains duplicate visits collapsed per day. This allows the Memories UI to take advantage of existing HistoryService API for deleting a set of visits to a given set of URLs in a given timespan. Given that we also cannot guarantee that a visit will only appear in one cluster, all Memories currently present in the UI are notified of the receipt of the deletion confirmation from the browser in order to get an opportunity to delete their matching visits. screenshot/4gfQ2BeAbaHu77r screenshot/6XGUpjLdcVkxpeQ Bug: 1184884 Change-Id: I726285b39ed1f48a50a4e610504a31504d475388 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2855789 Reviewed-by: Tommy Li <tommycli@chromium.org> Reviewed-by: Alex Gough <ajgo@chromium.org> Commit-Queue: Moe Ahmadi <mahmadi@chromium.org> Cr-Commit-Position: refs/heads/master@{#878006}
chromium · Apr 30, 2021 · 10cfc21 · 10cfc21
1 parent 43aa5d8
commit 10cfc21
Show file tree

Hide file tree

Showing 16 changed files with 370 additions and 33 deletions.
diff --git a/chrome/browser/resources/memories/BUILD.gn b/chrome/browser/resources/memories/BUILD.gn
@@ -25,6 +25,9 @@ js_library("app") {
     "//third_party/polymer/v3_0/components-chromium/iron-list",
     "//third_party/polymer/v3_0/components-chromium/iron-scroll-threshold",
     "//third_party/polymer/v3_0/components-chromium/polymer:polymer_bundled",
+    "//ui/webui/resources/cr_elements/cr_button:cr_button.m",
+    "//ui/webui/resources/cr_elements/cr_dialog:cr_dialog.m",
+    "//ui/webui/resources/cr_elements/cr_lazy_render:cr_lazy_render.m",
     "//ui/webui/resources/cr_elements/cr_toolbar:cr_toolbar",
     "//ui/webui/resources/cr_elements/cr_toolbar:cr_toolbar_search_field",
     "//ui/webui/resources/js:assert.m",
@@ -113,6 +116,9 @@ js_library("visit_row") {
     ":utils",
     "//components/history_clusters/core:mojo_bindings_webui_js",
     "//third_party/polymer/v3_0/components-chromium/polymer:polymer_bundled",
+    "//ui/webui/resources/cr_elements/cr_action_menu:cr_action_menu.m",
+    "//ui/webui/resources/cr_elements/cr_lazy_render:cr_lazy_render.m",
+    "//ui/webui/resources/js:load_time_data.m",
     "//url/mojom:url_mojom_origin_webui_js",
   ]
 }

diff --git a/chrome/browser/resources/memories/app.html b/chrome/browser/resources/memories/app.html
@@ -67,7 +67,8 @@
       <span id="text">[[result_.title]]</span>
     </div>
   </div>
-  <div id="memories">
+  <div id="memories" on-remove-visits="onRemoveVisits_"
+      on-remove-empty-memory-element="onRemoveEmptyMemoryEement_">
     <template is="dom-repeat" items="[[result_.memories]]">
       <memory-card memory="[[item]]"></memory-card>
     </template>
@@ -76,3 +77,20 @@
 <iron-scroll-threshold id="scroll-threshold" scroll-target="container"
     on-lower-threshold="onScrolledToBottom_">
 </iron-scroll-threshold>
+
+<cr-lazy-render id="confirmationDialog">
+  <template>
+    <cr-dialog consume-keydown-event on-cancel="onConfirmationDialogCancel_">
+      <div slot="title">$i18n{removeSelected}</div>
+      <div slot="body">$i18n{removeWarning}</div>
+      <div slot="button-container">
+        <cr-button class="cancel-button" on-click="onCancelButtonTap_">
+          $i18n{cancel}
+        </cr-button>
+        <cr-button class="action-button" on-click="onRemoveButtonTap_">
+          $i18n{remove}
+        </cr-button>
+      </div>
+    </cr-dialog>
+  </template>
+</cr-lazy-render>
diff --git a/chrome/browser/resources/memories/app.js b/chrome/browser/resources/memories/app.js
@@ -6,13 +6,18 @@ import './memory_card.js';
 import './page_thumbnail.js';
 import './router.js';
 import './shared_vars.js';
+import 'chrome://resources/cr_elements/cr_button/cr_button.m.js';
+import 'chrome://resources/cr_elements/cr_dialog/cr_dialog.m.js';
+import 'chrome://resources/cr_elements/cr_lazy_render/cr_lazy_render.m.js';
 import 'chrome://resources/cr_elements/cr_toolbar/cr_toolbar.js';
 import 'chrome://resources/cr_elements/shared_style_css.m.js';
 import 'chrome://resources/polymer/v3_0/iron-list/iron-list.js';
 import 'chrome://resources/polymer/v3_0/iron-scroll-threshold/iron-scroll-threshold.js';
 
 import {MemoriesResult, PageCallbackRouter, PageHandlerRemote} from '/chrome/browser/ui/webui/memories/memories.mojom-webui.js';
+import {Visit} from '/components/history_clusters/core/memories.mojom-webui.js';
 import {assert} from 'chrome://resources/js/assert.m.js';
+import {UnguessableToken} from 'chrome://resources/mojo/mojo/public/mojom/base/unguessable_token.mojom-webui.js';
 import {Url} from 'chrome://resources/mojo/url/mojom/url.mojom-webui.js';
 import {html, PolymerElement} from 'chrome://resources/polymer/v3_0/polymer/polymer_bundled.min.js';
 
@@ -41,6 +46,15 @@ class MemoriesAppElement extends PolymerElement {
       // Private properties
       //========================================================================
 
+      /**
+       * The current query for which related Memories are requested and shown.
+       * @private {string}
+       */
+      query_: {
+        type: String,
+        observer: 'onQueryChanged_',
+      },
+
       /**
        * Contains 1) the Memories returned by the browser in response to a
        * request for the freshest Memories related to a given query until a
@@ -52,12 +66,13 @@ class MemoriesAppElement extends PolymerElement {
       result_: Object,
 
       /**
-       * The current query for which related Memories are requested and shown.
-       * @private {string}
+       * The list of visits to be removed. A non-empty array indicates a pending
+       * remove request to the browser.
+       * @private {!Array<!Visit>}
        */
-      query_: {
-        type: String,
-        observer: 'onQueryChanged_',
+      visitsToBeRemoved_: {
+        type: Object,
+        value: [],
       },
     };
   }
@@ -70,6 +85,8 @@ class MemoriesAppElement extends PolymerElement {
     this.callbackRouter_ = BrowserProxy.getInstance().callbackRouter;
     /** @private {?number} */
     this.onMemoriesQueryResultListenerId_ = null;
+    /** @private {?number} */
+    this.onVisitsRemovedListenerId_ = null;
   }
 
   /** @override */
@@ -78,6 +95,9 @@ class MemoriesAppElement extends PolymerElement {
     this.onMemoriesQueryResultListenerId_ =
         this.callbackRouter_.onMemoriesQueryResult.addListener(
             this.onMemoriesQueryResult_.bind(this));
+    this.onVisitsRemovedListenerId_ =
+        this.callbackRouter_.onVisitsRemoved.addListener(
+            this.onVisitsRemoved_.bind(this));
   }
 
   /** @override */
@@ -86,12 +106,74 @@ class MemoriesAppElement extends PolymerElement {
     this.callbackRouter_.removeListener(
         assert(this.onMemoriesQueryResultListenerId_));
     this.onMemoriesQueryResultListenerId_ = null;
+    this.callbackRouter_.removeListener(
+        assert(this.onVisitsRemovedListenerId_));
+    this.onVisitsRemovedListenerId_ = null;
   }
 
   //============================================================================
   // Event handlers
   //============================================================================
 
+  /**
+   * @private
+   */
+  onCancelButtonTap_() {
+    this.visitsToBeRemoved_ = [];
+    this.$.confirmationDialog.get().close();
+  }
+
+  /**
+   * @private
+   */
+  onConfirmationDialogCancel_() {
+    this.visitsToBeRemoved_ = [];
+  }
+
+  /**
+   * @private
+   */
+  onRemoveButtonTap_() {
+    this.pageHandler_.removeVisits(this.visitsToBeRemoved_)
+        .then(({accepted}) => {
+          if (!accepted) {
+            this.visitsToBeRemoved_ = [];
+          }
+        });
+    this.$.confirmationDialog.get().close();
+  }
+
+  /**
+   * @param {CustomEvent<!UnguessableToken>} event Event received from an empty
+   *     Memory whose visits have been removed entirely and it should also be
+   *     removed from the page. Contains the id of the Memory to be removed.
+   * @private
+   */
+  onRemoveEmptyMemoryEement_(event) {
+    const index = this.result_.memories.findIndex((memory) => {
+      return memory.id === event.detail;
+    });
+    if (index > -1) {
+      this.splice('result_.memories', index, 1);
+    }
+  }
+
+  /**
+   * @param {CustomEvent<!Array<!Visit>>} event Event received from a visit
+   *     requesting to be removed. The array may contain the related visits of
+   *     the said visit, if applicable.
+   * @private
+   */
+  onRemoveVisits_(event) {
+    // Return early if there is a pending remove request.
+    if (this.visitsToBeRemoved_.length) {
+      return;
+    }
+
+    this.visitsToBeRemoved_ = event.detail;
+    this.$.confirmationDialog.get().showModal();
+  }
+
   /**
    * Called when the value of the search field changes.
    * @param {!CustomEvent<string>} e
@@ -189,6 +271,14 @@ class MemoriesAppElement extends PolymerElement {
       }
     });
   }
+
+  /**
+   * Called when the last accepted request to browser to remove visits succeeds.
+   * @private
+   */
+  onVisitsRemoved_() {
+    this.visitsToBeRemoved_ = [];
+  }
 }
 
 customElements.define(MemoriesAppElement.is, MemoriesAppElement);
diff --git a/chrome/browser/resources/memories/memory_card.js b/chrome/browser/resources/memories/memory_card.js
@@ -10,10 +10,13 @@ import './shared_vars.js';
 import './top_visit.js';
 import 'chrome://resources/cr_elements/shared_style_css.m.js';
 
-import {Memory} from '/components/history_clusters/core/memories.mojom-webui.js';
+import {PageCallbackRouter, PageHandlerRemote} from '/chrome/browser/ui/webui/memories/memories.mojom-webui.js';
+import {Memory, Visit} from '/components/history_clusters/core/memories.mojom-webui.js';
+import {assert} from 'chrome://resources/js/assert.m.js';
 import {Url} from 'chrome://resources/mojo/url/mojom/url.mojom-webui.js';
 import {html, PolymerElement} from 'chrome://resources/polymer/v3_0/polymer/polymer_bundled.min.js';
 
+import {BrowserProxy} from './browser_proxy.js';
 import {getHostnameFromUrl} from './utils.js';
 
 /**
@@ -56,10 +59,44 @@ class MemoryCardElement extends PolymerElement {
     };
   }
 
+  constructor() {
+    super();
+    /** @private {!PageCallbackRouter} */
+    this.callbackRouter_ = BrowserProxy.getInstance().callbackRouter;
+    /** @private {?number} */
+    this.onVisitsRemovedListenerId_ = null;
+  }
+
+  /** @override */
+  connectedCallback() {
+    super.connectedCallback();
+    this.onVisitsRemovedListenerId_ =
+        this.callbackRouter_.onVisitsRemoved.addListener(
+            this.onVisitsRemoved_.bind(this));
+  }
+
+  /** @override */
+  disconnectedCallback() {
+    super.disconnectedCallback();
+    this.callbackRouter_.removeListener(
+        assert(this.onVisitsRemovedListenerId_));
+    this.onVisitsRemovedListenerId_ = null;
+  }
+
   //============================================================================
   // Helper methods
   //============================================================================
 
+  /**
+   * @param {!Array} array
+   * @param {number} num
+   * @return {!Array} Shallow copy of the first |num| items of the input array.
+   * @private
+   */
+  arrayItems_(array, num) {
+    return array.slice(0, num);
+  }
+
   /** @private */
   computeHasRelatedTabGroupsOrBookmarks_() {
     return this.memory.relatedTabGroups.length > 0 ||
@@ -76,13 +113,48 @@ class MemoryCardElement extends PolymerElement {
   }
 
   /**
-   * @param {!Array} array
-   * @param {number} num
-   * @return {!Array} Shallow copy of the first |num| items of the input array.
+   * Called with the original remove params when the last accepted request to
+   * browser to remove visits succeeds. Since the same visit may appear in
+   * multiple Memories, all memories receive this callback in order to get a
+   * chance to remove their matching visits.
+   * @param {!Array<!Visit>} removedVisits
    * @private
    */
-  arrayItems_(array, num) {
-    return array.slice(0, num);
+  onVisitsRemoved_(removedVisits) {
+    // A matching visit is a visit to the removed visit's URL whose timespan
+    // falls within that of the removed visit.
+    const matchingVisit = (visit) => {
+      return removedVisits.findIndex((removedVisit) => {
+        return visit.url.url === removedVisit.url.url &&
+            visit.time.internalValue <= removedVisit.time.internalValue &&
+            visit.firstVisitTime.internalValue >=
+            removedVisit.firstVisitTime.internalValue;
+      }) !== -1;
+    };
+    this.memory.topVisits.forEach((topVisit, topVisitIndex) => {
+      if (matchingVisit(topVisit)) {
+        this.splice('memory.topVisits', topVisitIndex, 1);
+        return;
+      }
+      topVisit.relatedVisits.forEach((relatedVisit, relatedVisitIndex) => {
+        if (matchingVisit(relatedVisit)) {
+          this.splice(
+              `memory.topVisits.${topVisitIndex}.relatedVisits`,
+              relatedVisitIndex, 1);
+          return;
+        }
+      });
+    });
+
+    // If no more visits are left in the Memory, notify the enclosing
+    // <memories-app> to remove this Memory element from the page.
+    if (this.memory.topVisits.length === 0) {
+      this.dispatchEvent(new CustomEvent('remove-empty-memory-element', {
+        bubbles: true,
+        composed: true,
+        detail: this.memory.id,
+      }));
+    }
   }
 }
 

diff --git a/chrome/browser/resources/memories/top_visit.html b/chrome/browser/resources/memories/top_visit.html
@@ -39,7 +39,7 @@
 <div id="vertical-line"></div>
 <template is="dom-if" if="[[visit.relatedVisits.length]]">
   <cr-expand-button expanded="{{expanded_}}">
-    <visit-row is-top-visit visit="[[visit]]" on-visit-click="onVisitClick_">
+    <visit-row is-top-visit visit="[[visit]]" on-visit-tap="onVisitTap_">
     </visit-row>
   </cr-expand-button>
   <iron-collapse opened="[[expanded_]]">

diff --git a/chrome/browser/resources/memories/top_visit.js b/chrome/browser/resources/memories/top_visit.js
@@ -57,7 +57,7 @@ class TopVisitElement extends PolymerElement {
    * @param {!CustomEvent<{event:!MouseEvent}>} e
    * @private
    */
-  onVisitClick_(e) {
+  onVisitTap_(e) {
     // Prevent the enclosing <cr-expand-button> from receiving this event.
     e.detail.event.stopImmediatePropagation();
   }

diff --git a/chrome/browser/resources/memories/visit_row.html b/chrome/browser/resources/memories/visit_row.html
@@ -52,18 +52,33 @@
   #timestamp {
     color: var(--cr-secondary-text-color);
   }
+
+  .dropdown-item {
+    height: 32px;
+  }
 </style>
 <div id="header">
-  <a id="start-justtified" href="[[visit.url.url]]" on-click="onClick_">
+  <a id="start-justtified" href="[[visit.url.url]]" on-click="onTap_">
     <page-favicon url="[[visit.url]]"></page-favicon>
     <span id="title">[[visit.pageTitle]]</span>
     <span id="separator">–</span>
     <span id="hostname">[[getHostnameFromUrl_(visit.url)]]</span>
   </a>
   <div id="end-justified">
     <div id="timestamp">[[getTimeOfVisit_(visit)]]</div>
-    <cr-icon-button id="action" class="icon-more-vert"
-        hidden="[[visit.relatedVisits.length]]">
+    <cr-icon-button id="actionMenuButton" class="icon-more-vert"
+        on-click="onActionMenuButtonTap_">
     </cr-icon-button>
   </div>
 </div>
+
+<cr-lazy-render id="actionMenu">
+  <template>
+    <cr-action-menu role-description="$i18n{actionMenuDescription}">
+      <button id="removeButton" class="dropdown-item"
+          on-click="onRemoveButtonTap_">
+        [[removeButtonTitle_]]
+      </button>
+    </cr-action-menu>
+  </template>
+</cr-lazy-render>