[Extensions] Group activity log entries by api call

Occasionally, 2 activity log entries with the same api call exist in the
database. When presented to the UI, this can confuse some users. This CL
groups activities by their api call and sorts them based on the total
number of calls. This achieves feature parity with the Chrome Apps and
Extensions Developer Tool.

Bug: 832354
Change-Id: I566a3620b6a5fd3b35825e49d3db5fb9fbb7f416
Reviewed-on: https://chromium-review.googlesource.com/c/1350924
Commit-Queue: Kelvin Jiang <kelvinjiang@chromium.org>
Reviewed-by: Demetrios Papadopoulos <dpapad@chromium.org>
Reviewed-by: Devlin <rdevlin.cronin@chromium.org>
Cr-Commit-Position: refs/heads/master@{#613311}

chrome/browser/resources/md_extensions/activity_log.html

@@ -45,8 +45,8 @@
         </div>
         <div id="activity-list"
             hidden$="[[!shouldShowActivities_(pageState_, activityData_)]]">
-          <template is="dom-repeat" items="[[activityData_.activities]]">
-            <activity-log-item id="[[item.activityId]]" data="[[item]]">
+          <template is="dom-repeat" items="[[activityData_]]">
+            <activity-log-item id="[[item.apiCall]]" data="[[item]]">
             </activity-log-item>
           </template>
         </div>

chrome/browser/resources/md_extensions/activity_log.js

@@ -28,6 +28,61 @@ cr.define('extensions', function() {
     getExtensionActivityLog(extensionId) {}
   }
 
+  /**
+   * Group activity log entries by the API call and merge their counts.
+   * We currently assume that every API call matches to one activity type.
+   * @param {!Array<!chrome.activityLogPrivate.ExtensionActivity>}
+   *     activityData
+   * @return {!Map<string, !extensions.ApiGroup>}
+   */
+  function groupActivitiesByApiCall(activityData) {
+    const activitiesByApiCall = new Map();
+
+    for (const activity of activityData) {
+      const apiCall = activity.apiCall;
+      const count = activity.count;
+      const pageUrl = activity.pageUrl;
+
+      if (!activitiesByApiCall.has(apiCall)) {
+        const apiGroup = {
+          apiCall,
+          count,
+          activityType: activity.activityType,
+          countsByUrl: pageUrl ? new Map([[pageUrl, count]]) : new Map()
+        };
+        activitiesByApiCall.set(apiCall, apiGroup);
+      } else {
+        const apiGroup = activitiesByApiCall.get(apiCall);
+        apiGroup.count += count;
+
+        if (pageUrl) {
+          const currentCount = apiGroup.countsByUrl.get(pageUrl) || 0;
+          apiGroup.countsByUrl.set(pageUrl, currentCount + count);
+        }
+      }
+    }
+
+    return activitiesByApiCall;
+  }
+
+  /**
+   * Sort activities by the total count for each API call. Resolve ties by the
+   * alphabetical order of the API call name.
+   * @param {!Map<string, !extensions.ApiGroup>} activitiesByApiCall
+   * @return {!Array<!extensions.ApiGroup>}
+   */
+  function sortActivitiesByCallCount(activitiesByApiCall) {
+    return Array.from(activitiesByApiCall.values()).sort(function(a, b) {
+      if (a.count != b.count)
+        return b.count - a.count;
+      if (a.apiCall < b.apiCall)
+        return -1;
+      if (a.apiCall > b.apiCall)
+        return 1;
+      return 0;
+    });
+  }
+
   const ActivityLog = Polymer({
     is: 'extensions-activity-log',
 
@@ -43,10 +98,15 @@ cr.define('extensions', function() {
       delegate: Object,
 
       /**
+       * An array representing the activity log. Stores activities grouped by
+       * API call sorted in descending order of the call count.
        * @private
-       * @type {!chrome.activityLogPrivate.ActivityResultSet|undefined}
+       * @type {!Array<!extensions.ApiGroup>}
        */
-      activityData_: Object,
+      activityData_: {
+        type: Array,
+        value: () => [],
+      },
 
       /**
        * @private
@@ -99,7 +159,7 @@ cr.define('extensions', function() {
      */
     shouldShowEmptyActivityLogMessage_: function() {
       return this.pageState_ === ActivityLogPageState.LOADED &&
-          (!this.activityData_ || this.activityData_.activities.length === 0);
+          this.activityData_.length === 0;
     },
 
     /**
@@ -116,7 +176,7 @@ cr.define('extensions', function() {
      */
     shouldShowActivities_: function() {
       return this.pageState_ === ActivityLogPageState.LOADED &&
-          !!this.activityData_ && this.activityData_.activities.length > 0;
+          this.activityData_.length > 0;
     },
 
     /** @private */
@@ -130,7 +190,8 @@ cr.define('extensions', function() {
       this.pageState_ = ActivityLogPageState.LOADING;
       this.delegate.getExtensionActivityLog(this.extensionId).then(result => {
         this.pageState_ = ActivityLogPageState.LOADED;
-        this.activityData_ = result;
+        this.activityData_ = sortActivitiesByCallCount(
+            groupActivitiesByApiCall(result.activities));
         this.onDataFetched.resolve();
       });
     },

chrome/browser/resources/md_extensions/activity_log_item.html

@@ -19,7 +19,8 @@
         margin-inline-start: 10px;
       }
 
-      #page-url-link {
+      .page-url-link {
+        flex-grow: 1;
         margin-inline-start: 16px;
       }
     </style>
@@ -28,8 +29,19 @@
       <span id="api-call">[[data.apiCall]]</span>
       <span id="activity-count">[[data.count]]</span>
     </div>
-    <div id="page-url" hidden$="[[!data.pageUrl]]">
-      <a id="page-url-link" href="[[data.pageUrl]]">[[data.pageUrl]]</a>
+    <div id="page-url-list"
+        hidden$="[[!shouldShowPageUrls_(data)]]">
+      <template is="dom-repeat" items="[[getPageUrls_(data)]]">
+        <div class="page-url layout horizontal">
+          <a class="page-url-link" href="[[item.page]]">
+            [[item.page]]
+          </a>
+          <span class="page-url-count"
+              hidden$="[[!shouldShowPageUrlCount_(data)]]">
+            [[item.count]]
+          </span>
+        </div>
+      </template>
     </div>
   </template>
   <script src="activity_log_item.js"></script>

chrome/browser/resources/md_extensions/activity_log_item.js

@@ -5,18 +5,74 @@
 cr.define('extensions', function() {
   'use strict';
 
+  /**
+   * @typedef {{
+   *   apiCall: string,
+   *   count: number,
+   *   activityType: !chrome.activityLogPrivate.ExtensionActivityFilter,
+   *   countsByUrl: !Map<string, number>
+   * }}
+   */
+  let ApiGroup;
+
+  /**
+   * @typedef {{
+   *   page: string,
+   *   count: number
+   * }}
+   */
+  let PageUrlItem;
+
   const ActivityLogItem = Polymer({
     is: 'activity-log-item',
 
     properties: {
       /**
-       * The underlying ExtensionActivity that provides data for the
+       * The underlying ApiGroup that provides data for the
        * ActivityLogItem displayed.
-       * @type {chrome.activityLogPrivate.ExtensionActivity}
+       * @type {!extensions.ApiGroup}
        */
       data: Object,
     },
+
+    /**
+     * Show page URLs if there is at least one non-empty page URL. We are
+     * guaranteed to have at least one based on how the data in this component
+     * is generated from activity_log.js.
+     * @private
+     * @return {boolean}
+     */
+    shouldShowPageUrls_: function() {
+      return this.data.countsByUrl.size > 0;
+    },
+
+    /**
+     * Show the API call count for a particular page URL if more than one page
+     * URL is associated with this API call.
+     * @private
+     * @return {boolean}
+     */
+    shouldShowPageUrlCount_: function() {
+      return this.data.countsByUrl.size > 1;
+    },
+
+    /**
+     * Sort the page URLs by the number of times the API call was made for that
+     * page URL. Resolve ties by the alphabetical order of the page URL.
+     * @private
+     * @return {!Array<PageUrlItem>}
+     */
+    getPageUrls_: function() {
+      return Array.from(this.data.countsByUrl.entries())
+          .map(e => ({page: e[0], count: e[1]}))
+          .sort(function(a, b) {
+            return a.page < b.page ? -1 : (a.page > b.page ? 1 : 0);
+          });
+    },
   });
 
-  return {ActivityLogItem: ActivityLogItem};
+  return {
+    ActivityLogItem: ActivityLogItem,
+    ApiGroup: ApiGroup,
+  };
 });

chrome/test/data/webui/extensions/activity_log_item_test.js

+// Copyright 2018 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.
+
+/** @fileoverview Suite of tests for activity-log-item. */
+suite('ExtensionsActivityLogItemTest', function() {
+  /**
+   * Extension activityLogItem created before each test.
+   * @type {extensions.ActivityLogItem}
+   */
+  let activityLogItem;
+  let testVisible;
+
+  /**
+   * ApiGroup data for the activityLogItem
+   * @type {extensions.ApiGroup}
+   */
+  let testApiGroup;
+
+  // Initialize an extension activity log item before each test.
+  setup(function() {
+    PolymerTest.clearBody();
+    testApiGroup = {
+      apiCall: 'i18n.getUILanguage',
+      count: 1,
+      activityType: chrome.activityLogPrivate.ExtensionActivityFilter.API_CALL,
+      countsByUrl: new Map()
+    };
+
+    activityLogItem = new extensions.ActivityLogItem();
+    activityLogItem.data = testApiGroup;
+    testVisible = extension_test_util.testVisible.bind(null, activityLogItem);
+
+    document.body.appendChild(activityLogItem);
+  });
+
+  teardown(function() {
+    activityLogItem.remove();
+  });
+
+  test('no page urls shown when activity has no associated page', function() {
+    Polymer.dom.flush();
+
+    testVisible('#activity-call-and-count', true);
+    testVisible('#page-url-list', false);
+  });
+
+  test('count not shown when there is only 1 page url', function() {
+    const countsByUrl = new Map([['google.com', 1]]);
+
+    testApiGroup = {
+      apiCall: 'Storage.getItem',
+      count: 3,
+      activityType:
+          chrome.activityLogPrivate.ExtensionActivityFilter.DOM_ACCESS,
+      countsByUrl
+    };
+    activityLogItem.set('data', testApiGroup);
+
+    Polymer.dom.flush();
+
+    testVisible('#activity-call-and-count', true);
+    testVisible('#page-url-list', true);
+    testVisible('.page-url-count', false);
+  });
+
+  test('count shown in descending order for multiple page urls', function() {
+    const countsByUrl =
+        new Map([['google.com', 5], ['chrome://extensions', 10]]);
+
+    testApiGroup = {
+      apiCall: 'Storage.getItem',
+      count: 15,
+      activityType:
+          chrome.activityLogPrivate.ExtensionActivityFilter.DOM_ACCESS,
+      countsByUrl
+    };
+    activityLogItem.set('data', testApiGroup);
+
+    Polymer.dom.flush();
+
+    testVisible('#activity-call-and-count', true);
+    testVisible('#page-url-list', true);
+    testVisible('.page-url-count', true);
+
+    const pageUrls = activityLogItem.shadowRoot.querySelectorAll('.page-url');
+    expectEquals(pageUrls.length, 2);
+
+    // Test the order of the page urls and activity count for the activity
+    // log item here. Apparently a space is added at the end of the innerText,
+    // hence the use of .includes.
+    expectTrue(pageUrls[0]
+                   .querySelector('.page-url-link')
+                   .innerText.includes('chrome://extensions'));
+    expectEquals(pageUrls[0].querySelector('.page-url-count').innerText, '10');
+
+    expectTrue(pageUrls[1]
+                   .querySelector('.page-url-link')
+                   .innerText.includes('google.com'));
+    expectEquals(pageUrls[1].querySelector('.page-url-count').innerText, '5');
+  });
+});

chrome/test/data/webui/extensions/activity_log_test.js

@@ -35,13 +35,22 @@ suite('ExtensionsActivityLogTest', function() {
         activityType: 'dom_access',
         apiCall: 'Storage.getItem',
         args: 'null',
-        count: 9,
+        count: 35,
         extensionId: EXTENSION_ID,
         other: {domVerb: 'method'},
         pageTitle: 'Test Extension',
         pageUrl: `chrome-extension://${EXTENSION_ID}/index.html`,
         time: 1541203131994.837
       },
+      {
+        activityId: '301',
+        activityType: 'api_call',
+        apiCall: 'i18n.getUILanguage',
+        args: 'null',
+        count: 30,
+        extensionId: EXTENSION_ID,
+        time: 1541203172002.664
+      },
     ]
   };
 
@@ -72,15 +81,26 @@ suite('ExtensionsActivityLogTest', function() {
     testVisible('#no-activities', false);
     testVisible('#loading-activities', false);
     testVisible('#activity-list', true);
+
+    const activityLogItems =
+        activityLog.shadowRoot.querySelectorAll('activity-log-item');
+    expectEquals(activityLogItems.length, 2);
+
+    // Test the order of the activity log items here. This test is in this
+    // file because the logic to group activity log items by their API call
+    // is in activity_log.js.
+    expectEquals(
+        activityLogItems[0].$$('#api-call').innerText, 'i18n.getUILanguage');
+    expectEquals(activityLogItems[0].$$('#activity-count').innerText, '40');
+
     expectEquals(
-        activityLog.shadowRoot.querySelectorAll('activity-log-item').length, 2);
+        activityLogItems[1].$$('#api-call').innerText, 'Storage.getItem');
+    expectEquals(activityLogItems[1].$$('#activity-count').innerText, '35');
   });
 
   test('message shown when no activities present for extension', function() {
     // Spoof an API call and pretend that the extension has no activities.
-    activityLog.activityData_ = {
-      activities: [],
-    };
+    activityLog.activityData_ = [];
 
     Polymer.dom.flush();
 

chrome/test/data/webui/extensions/cr_extensions_browsertest.js

@@ -216,6 +216,26 @@ TEST_F('CrExtensionsActivityLogTest', 'All', () => {
   mocha.run();
 });
 
+////////////////////////////////////////////////////////////////////////////////
+// Extension Activity Log Item Tests
+
+CrExtensionsActivityLogItemTest = class extends CrExtensionsBrowserTest {
+  /** @override */
+  get browsePreload() {
+    return 'chrome://extensions/activity_log_item.html';
+  }
+
+  get extraLibraries() {
+    return super.extraLibraries.concat([
+      'activity_log_item_test.js',
+    ]);
+  }
+};
+
+TEST_F('CrExtensionsActivityLogItemTest', 'All', () => {
+  mocha.run();
+});
+
 ////////////////////////////////////////////////////////////////////////////////
 // Extension Detail View Tests
 

testing/buildbot/filters/webui_polymer2_browser_tests.filter

@@ -71,6 +71,7 @@ CrElementsToastTest.*
 CrElementsToggleTest.*
 CrElementsToolbarSearchFieldTest.*
 CrExtensionsActivityLogTest.*
+CrExtensionsActivityLogItemTest.*
 CrExtensionsA11yTest.*
 CrExtensionsA11yTestWithMultipleExensions.*
 CrExtensionsCodeSectionTest.*