Highlight filename in filepaths detected by Chrome Cleanup Tool.

Highlight the file name with strong tag, so that users can scan the list easily.

Bug: crbug.com/883737
Change-Id: Iaccf05983efe0414ce39fe35c1c89be4b4102d12
Reviewed-on: https://chromium-review.googlesource.com/1231835
Commit-Queue: Che-yu Wu <cheyuw@google.com>
Reviewed-by: Demetrios Papadopoulos <dpapad@chromium.org>
Reviewed-by: proberge <proberge@chromium.org>
Cr-Commit-Position: refs/heads/master@{#592912}

chrome/browser/resources/settings/chrome_cleanup_page/chrome_cleanup_page.html

@@ -90,19 +90,22 @@
           id="files-to-remove-list"
           hidden="[[!hasFilesToShow_]]"
           title="$i18n{chromeCleanupDetailsFilesAndPrograms}"
-          items-to-show="[[scannerResults_.files]]">
+          items-to-show="[[
+              getListEntriesFromFilePaths_(scannerResults_.files)]]">
       </items-to-remove-list>
       <items-to-remove-list
           id="registry-keys-list"
           hidden="[[!hasRegistryKeysToShow_]]"
           title="$i18n{chromeCleanupDetailsRegistryEntries}"
-          items-to-show="[[scannerResults_.registryKeys]]">
+          items-to-show="[[
+              getListEntriesFromStrings_(scannerResults_.registryKeys)]]">
       </items-to-remove-list>
       <items-to-remove-list
           id="extensions-list"
           hidden="[[!hasExtensionsToShow_]]"
           title="$i18n{chromeCleanupDetailsExtensions}"
-          items-to-show="[[scannerResults_.extensions]]">
+          items-to-show="[[
+              getListEntriesFromStrings_(scannerResults_.extensions)]]">
       </items-to-remove-list>
       <div class="settings-box continuation">
         <div class="secondary">

chrome/browser/resources/settings/chrome_cleanup_page/chrome_cleanup_page.js

@@ -77,8 +77,18 @@ settings.ChromeCleanupCardActionButton;
 settings.ChromeCleanupCardComponents;
 
 /**
+ * Represents the file path structure of a base::FilePath.
+ * dirname ends with a separator.
  * @typedef {{
- *   files: Array<string>,
+ *   dirname: string,
+ *   basename: string,
+ * }}
+ */
+settings.ChromeCleanupFilePath;
+
+/**
+ * @typedef {{
+ *   files: Array<settings.ChromeCleanupFilePath>,
  *   registryKeys: Array<string>,
  *   extensions: Array<string>,
  * }}
@@ -741,4 +751,23 @@ Polymer({
       ],
     ]);
   },
+
+  /**
+   * @param {!Array<string>} list
+   * @return {!Array<settings.ChromeCleanupRemovalListItem>}
+   * @private
+   */
+  getListEntriesFromStrings_: function(list) {
+    return list.map(entry => ({text: entry, highlightSuffix: null}));
+  },
+
+  /**
+   * @param {!Array<settings.ChromeCleanupFilePath>} paths
+   * @return {!Array<settings.ChromeCleanupRemovalListItem>}
+   * @private
+   */
+  getListEntriesFromFilePaths_: function(paths) {
+    return paths.map(
+        path => ({text: path.dirname, highlightSuffix: path.basename}));
+  },
 });

chrome/browser/resources/settings/chrome_cleanup_page/items_to_remove_list.html

@@ -25,11 +25,21 @@
         margin-top: -1em;
       }
 
+      .highlight-suffix {
+        font-weight: bold;
+      }
+
     </style>
     <div id="title" class="secondary">[[title]]</div>
     <ul class="secondary">
       <template is="dom-repeat" items="[[initialItems_]]">
-        <li class="visible-item">[[item]]</li>
+        <li class="visible-item">
+          <span>[[item.text]]</span><!--
+       --><span class="highlight-suffix"
+                hidden="[[!hasHighlightSuffix_(item)]]"><!--
+         -->[[item.highlightSuffix]]
+          </span>
+        </li>
       </template>
       <li id="more-items-link" hidden="[[expanded_]]" on-click="expandList_">
         [[moreItemsLinkText_]]
@@ -44,7 +54,13 @@
          expected action. -->
     <ul id="remaining-list" hidden="[[!expanded_]]" class="secondary">
       <template is="dom-repeat" items="[[remainingItems_]]">
-        <li class$="[[remainingItemsClass_(expanded_)]]">[[item]]</li>
+        <li class$="[[remainingItemsClass_(expanded_)]]">
+          <span>[[item.text]]</span><!--
+       --><span class="highlight-suffix"
+                hidden="[[!hasHighlightSuffix_(item)]]"><!--
+         -->[[item.highlightSuffix]]
+          </span>
+        </li>
       </template>
     </ul>
   </template>

chrome/browser/resources/settings/chrome_cleanup_page/items_to_remove_list.js

@@ -4,6 +4,17 @@
 
 cr.exportPath('settings');
 
+/**
+ * For each line in the item list, the text field will be shown in normal style
+ * at front of the line. The highlightSuffix will be appended to the end of line
+ * and emphasized with bold font.
+ * @typedef {{
+ *   text: string,
+ *   highlightSuffix: ?string,
+ * }}
+ */
+settings.ChromeCleanupRemovalListItem;
+
 /**
  * The default number of items to show for files, registry keys and extensions
  * on the detailed view when user-initiated cleanups are enabled.
@@ -37,7 +48,7 @@ Polymer({
       value: '',
     },
 
-    /** @type {!Array<string>} */
+    /** @type {!Array<settings.ChromeCleanupRemovalListItem>} */
     itemsToShow: {
       type: Array,
       observer: 'updateVisibleState_',
@@ -55,7 +66,7 @@ Polymer({
     /**
      * The first |CHROME_CLEANUP_DEFAULT_ITEMS_TO_SHOW| items of |itemsToShow|
      * if the list is longer than |CHROME_CLEANUP_DEFAULT_ITEMS_TO_SHOW|.
-     * @private {?Array<string>}
+     * @private {?Array<settings.ChromeCleanupRemovalListItem>}
      */
     initialItems_: Array,
 
@@ -63,7 +74,7 @@ Polymer({
      * The remaining items to be presented that are not included in
      * |initialItems_|. Items in this list are only shown to the user if
      * |expanded_| is true.
-     * @private {?Array<string>}
+     * @private {?Array<settings.ChromeCleanupRemovalListItem>}
      */
     remainingItems_: Array,
 
@@ -95,7 +106,7 @@ Polymer({
    *    the user will contain exactly |CHROME_CLEANUP_DEFAULT_ITEMS_TO_SHOW|
    *    elements, and the last one will be the "show more" link.
    *
-   * @param {!Array<string>} itemsToShow
+   * @param {!Array<settings.ChromeCleanupRemovalListItem>} itemsToShow
    */
   updateVisibleState_: function(itemsToShow) {
     // Start expanded if there are less than
@@ -130,4 +141,13 @@ Polymer({
   remainingItemsClass_: function(expanded) {
     return expanded ? 'visible-item' : 'hidden-item';
   },
+
+  /**
+   * @param {settings.ChromeCleanupRemovalListItem} item
+   * @return {boolean} Whether a highlight suffix exists.
+   * @private
+   */
+  hasHighlightSuffix_: function(item) {
+    return item.highlightSuffix !== null;
+  },
 });

chrome/browser/ui/webui/settings/chrome_cleanup_handler.cc

@@ -6,6 +6,7 @@
 
 #include <memory>
 #include <string>
+#include <utility>
 
 #include "base/command_line.h"
 #include "base/feature_list.h"
@@ -38,9 +39,12 @@ namespace {
 std::unique_ptr<base::ListValue> GetFilesAsListStorage(
     const std::set<base::FilePath>& files) {
   auto value = std::make_unique<base::ListValue>();
-  for (const base::FilePath& path : files)
-    value->AppendString(path.value());
-
+  for (const base::FilePath& path : files) {
+    auto item = std::make_unique<base::DictionaryValue>();
+    item->SetString("dirname", path.DirName().AsEndingWithSeparator().value());
+    item->SetString("basename", path.BaseName().value());
+    value->Append(std::move(item));
+  }
   return value;
 }
 

chrome/test/data/webui/settings/chrome_cleanup_page_test.js

@@ -71,9 +71,15 @@ let chromeCleanupPage = null;
 /** @type {?TestDownloadsBrowserProxy} */
 let chromeCleanupProxy = null;
 
-const shortFileList = ['file 1', 'file 2', 'file 3'];
-const exactSizeFileList = ['file 1', 'file 2', 'file 3', 'file 4'];
-const longFileList = ['file 1', 'file 2', 'file 3', 'file 4', 'file 5'];
+const shortFileList = [
+  {'dirname': 'C:\\', 'basename': 'file 1'},
+  {'dirname': 'C:\\', 'basename': 'file 2'},
+  {'dirname': 'C:\\', 'basename': 'file 3'},
+];
+const exactSizeFileList =
+    shortFileList.concat([{'dirname': 'C:\\', 'basename': 'file 4'}]);
+const longFileList =
+    exactSizeFileList.concat([{'dirname': 'C:\\', 'basename': 'file 5'}]);
 const shortRegistryKeysList = ['key 1', 'key 2'];
 const exactSizeRegistryKeysList = ['key 1', 'key 2', 'key 3', 'key 4'];
 const longRegistryKeysList =
@@ -126,6 +132,22 @@ function validateVisibleItemsList(originalItems, visibleItems) {
   }
 }
 
+/**
+ * @param {!Array} originalItems
+ * @param {!Element} container
+ * @param {boolean} expectSuffix Whether a highlight suffix should exist.
+ */
+function validateHighlightSuffix(originalItems, container, expectSuffix) {
+  let itemList =
+      container.shadowRoot.querySelectorAll('li:not(#more-items-link)');
+  assertEquals(originalItems.length, itemList.length);
+  for (let item of itemList) {
+    let suffixes = item.querySelectorAll('.highlight-suffix');
+    assertEquals(suffixes.length, 1);
+    assertEquals(expectSuffix, !suffixes[0].hidden);
+  }
+}
+
 /**
  * @param {!Array} files The list of files to be cleaned.
  * @param {!Array} registryKeys The list of registry entries to be cleaned.
@@ -151,6 +173,7 @@ function startCleanupFromInfected(files, registryKeys, extensions) {
   const filesToRemoveList = chromeCleanupPage.$$('#files-to-remove-list');
   assertTrue(!!filesToRemoveList);
   validateVisibleItemsList(files, filesToRemoveList);
+  validateHighlightSuffix(files, filesToRemoveList, true /* expectSuffix */);
 
   const registryKeysListContainer = chromeCleanupPage.$$('#registry-keys-list');
   assertTrue(!!registryKeysListContainer);
@@ -158,6 +181,8 @@ function startCleanupFromInfected(files, registryKeys, extensions) {
     assertFalse(registryKeysListContainer.hidden);
     assertTrue(!!registryKeysListContainer);
     validateVisibleItemsList(registryKeys, registryKeysListContainer);
+    validateHighlightSuffix(
+        registryKeys, registryKeysListContainer, false /* expectSuffix */);
   } else {
     assertTrue(registryKeysListContainer.hidden);
   }
@@ -168,6 +193,8 @@ function startCleanupFromInfected(files, registryKeys, extensions) {
     assertFalse(extensionsListContainer.hidden);
     assertTrue(!!extensionsListContainer);
     validateVisibleItemsList(extensions, extensionsListContainer);
+    validateHighlightSuffix(
+        extensions, extensionsListContainer, false /* expectSuffix */);
   } else {
     assertTrue(extensionsListContainer.hidden);
   }