Stops empty suggestion headers from showing up in the NTP realbox

The server provided suggestion header map may come with entries
that do not correspond to any of the matches. Using that map to display
suggestion headers can result in empty headers. This CL fixes that by
using the header information in the matches themselves as the source of
truth about what headers actually exist in the server response. It also
modifies the type annotations to consistently use the "number" type for
the suggestion IDs.

Bug: 1113447
Change-Id: I5f685595cfe5a295d483447e6965cb77b7c581fe
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2347047
Commit-Queue: Moe Ahmadi <mahmadi@chromium.org>
Reviewed-by: Tommy Li <tommycli@chromium.org>
Cr-Commit-Position: refs/heads/master@{#796560}

chrome/browser/resources/new_tab_page/new_tab_page.js

@@ -18,5 +18,4 @@ export {BackgroundSelectionType} from './customize_dialog.js';
 export {dummyDescriptor} from './modules/dummy/module.js';
 export {ModuleRegistry} from './modules/module_registry.js';
 export {PromoBrowserCommandProxy} from './promo_browser_command_proxy.js';
-export {NO_SUGGESTION_GROUP_ID} from './realbox_dropdown.js';
 export {$$, createScrollBorders, decodeString16, hexColorToSkColor, mojoString16, skColorToRgba} from './utils.js';

chrome/browser/resources/new_tab_page/realbox_dropdown.js

@@ -13,13 +13,6 @@ import {html, PolymerElement} from 'chrome://resources/polymer/v3_0/polymer/poly
 import {BrowserProxy} from './browser_proxy.js';
 import {decodeString16, skColorToRgba} from './utils.js';
 
-/**
- * Indicates a missing suggestion group Id. Based on
- * SearchSuggestionParser::kNoSuggestionGroupId.
- * @type {string}
- */
-export const NO_SUGGESTION_GROUP_ID = '-1';
-
 // A dropdown element that contains autocomplete matches. Provides an API for
 // the embedder (i.e., <ntp-realbox>) to change the selection.
 class RealboxDropdownElement extends PolymerElement {
@@ -68,7 +61,7 @@ class RealboxDropdownElement extends PolymerElement {
 
       /**
        * The list of suggestion group IDs matches belong to.
-       * @type {!Array<string>}
+       * @type {!Array<number>}
        * @private
        */
       groupIds_: {
@@ -78,7 +71,7 @@ class RealboxDropdownElement extends PolymerElement {
 
       /**
        * The list of suggestion group IDs whose matches should be hidden.
-       * @type {!Array<string>}
+       * @type {!Array<number>}
        * @private
        */
       hiddenGroupIds_: {
@@ -286,7 +279,7 @@ class RealboxDropdownElement extends PolymerElement {
    * @private
    */
   onHeaderClick_(e) {
-    const groupId = e.currentTarget.dataset.id;
+    const groupId = Number(e.currentTarget.dataset.id);
 
     // Tell the backend to toggle visibility of the given suggestion group ID.
     this.pageHandler_.toggleSuggestionGroupIdVisibility(groupId);
@@ -332,21 +325,23 @@ class RealboxDropdownElement extends PolymerElement {
   }
 
   /**
-   * @returns {!Array<string>}
+   * @returns {!Array<number>}
    * @private
    */
   computeGroupIds_() {
-    if (!this.result) {
+    if (!this.result || !this.result.matches) {
       return [];
     }
 
-    // Add |NO_SUGGESTION_GROUP_ID| to the list of suggestion group IDs.
-    return [NO_SUGGESTION_GROUP_ID].concat(
-        Object.keys(this.result.suggestionGroupsMap));
+    // Extract the suggestion group IDs from autocomplete matches and return the
+    // unique IDs while preserving the order. Autocomplete matches are the
+    // ultimate source of truth for suggestion groups IDs matches belong to.
+    return [...new Set(
+        this.result.matches.map(match => match.suggestionGroupId))];
   }
 
   /**
-   * @returns {!Array<string>}
+   * @returns {!Array<number>}
    * @private
    */
   computeHiddenGroupIds_() {
@@ -355,13 +350,14 @@ class RealboxDropdownElement extends PolymerElement {
     }
 
     return Object.keys(this.result.suggestionGroupsMap)
+        .map(groupId => Number(groupId))
         .filter((groupId => {
                   return this.result.suggestionGroupsMap[groupId].hidden;
                 }).bind(this));
   }
 
   /**
-   * @param {string} groupId
+   * @param {number} groupId
    * @returns {!function(!search.mojom.AutocompleteMatch):boolean} The filter
    *     function to filter matches that belong to the given suggestion group
    *     ID.
@@ -369,21 +365,21 @@ class RealboxDropdownElement extends PolymerElement {
    */
   computeMatchBelongsToGroup_(groupId) {
     return (match) => {
-      return match.suggestionGroupId === Number(groupId);
+      return match.suggestionGroupId === groupId;
     };
   }
 
   /**
-   * @param {string} groupId
+   * @param {number} groupId
    * @returns {boolean} Whether the given suggestion group ID has a header.
    * @private
    */
   groupHasHeader_(groupId) {
-    return groupId !== NO_SUGGESTION_GROUP_ID;
+    return !!this.headerForGroup_(groupId);
   }
 
   /**
-   * @param {string} groupId
+   * @param {number} groupId
    * @returns {boolean} Whether matches with the given suggestion group ID
    *     should be hidden.
    * @private
@@ -393,15 +389,12 @@ class RealboxDropdownElement extends PolymerElement {
   }
 
   /**
-   * @param {string} groupId
+   * @param {number} groupId
    * @returns {string} The header for the given suggestion group ID.
    * @private
    * @suppress {checkTypes}
    */
   headerForGroup_(groupId) {
-    if (!this.groupHasHeader_(groupId)) {
-      return '';
-    }
     return (this.result && this.result.suggestionGroupsMap &&
             this.result.suggestionGroupsMap[groupId]) ?
         decodeString16(this.result.suggestionGroupsMap[groupId].header) :
@@ -409,7 +402,7 @@ class RealboxDropdownElement extends PolymerElement {
   }
 
   /**
-   * @param {string} groupId
+   * @param {number} groupId
    * @returns {string} Tooltip for suggestion group show/hide toggle button.
    * @private
    */
@@ -422,7 +415,7 @@ class RealboxDropdownElement extends PolymerElement {
   }
 
   /**
-   * @param {string} groupId
+   * @param {number} groupId
    * @returns {string} A11y label for suggestion group show/hide toggle button.
    * @private
    */

chrome/test/data/webui/new_tab_page/realbox_test.js

@@ -2,7 +2,7 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
-import {BrowserProxy, decodeString16, mojoString16, NO_SUGGESTION_GROUP_ID} from 'chrome://new-tab-page/new_tab_page.js';
+import {BrowserProxy, decodeString16, mojoString16} from 'chrome://new-tab-page/new_tab_page.js';
 import {loadTimeData} from 'chrome://resources/js/load_time_data.m.js';
 import {getDeepActiveElement} from 'chrome://resources/js/util.m.js';
 import {assertStyle, createTestProxy, createTheme} from 'chrome://test/new_tab_page/test_support.js';
@@ -99,7 +99,7 @@ function createAutocompleteMatch() {
     isSearchType: false,
     swapContentsAndDescription: false,
     supportsDeletion: false,
-    suggestionGroupId: NO_SUGGESTION_GROUP_ID,
+    suggestionGroupId: -1,  // Indicates a missing suggestion group Id.
     contents: mojoString16(''),
     contentsClass: [{offset: 0, style: 0}],
     description: mojoString16(''),
@@ -2006,7 +2006,7 @@ suite('NewTabPageRealboxTest', () => {
 
     await testProxy.handler.whenCalled('toggleSuggestionGroupIdVisibility')
         .then((args) => {
-          assertEquals('100', args.suggestionGroupId);
+          assertEquals(100, args.suggestionGroupId);
         });
     assertEquals(
         1, testProxy.handler.getCallCount('toggleSuggestionGroupIdVisibility'));
@@ -2023,7 +2023,7 @@ suite('NewTabPageRealboxTest', () => {
 
     await testProxy.handler.whenCalled('toggleSuggestionGroupIdVisibility')
         .then((args) => {
-          assertEquals('100', args.suggestionGroupId);
+          assertEquals(100, args.suggestionGroupId);
         });
     assertEquals(
         1, testProxy.handler.getCallCount('toggleSuggestionGroupIdVisibility'));
@@ -2039,7 +2039,7 @@ suite('NewTabPageRealboxTest', () => {
     headerEl.click();
     await testProxy.handler.whenCalled('toggleSuggestionGroupIdVisibility')
         .then((args) => {
-          assertEquals('100', args.suggestionGroupId);
+          assertEquals(100, args.suggestionGroupId);
         });
     assertEquals(
         1, testProxy.handler.getCallCount('toggleSuggestionGroupIdVisibility'));