Make help app search using subheadings

If a searchable item provides subheadings, use those instead of the
body. The subheadings should have less text than the body, so index
update should happen in less time.

If a search result has multiple subheadings, pick the one with the
most positions.

Also add js type check for the browsertests. The build rules are based
on the ones used in media_app_ui/BUILD.gn

Bug: b/170162418
Change-Id: Ic9682ac13b3d5e674c5d30e7042d199e98f42206
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2466011
Commit-Queue: Zufeng Wang <zufeng@google.com>
Reviewed-by: Rachel Carpenter <carpenterr@chromium.org>
Cr-Commit-Position: refs/heads/master@{#816888}

chromeos/components/help_app_ui/BUILD.gn

@@ -50,7 +50,8 @@ mojom("mojo_bindings") {
 group("closure_compile") {
   testonly = true
   deps = [
-    ":closure_compile_tests",
+    ":closure_compile_browsertests",
+    ":closure_compile_test_lib",
     "resources:closure_compile",
     "resources/mock:closure_compile",
   ]
@@ -92,7 +93,7 @@ source_set("browser_test_support") {
   ]
 }
 
-js_type_check("closure_compile_tests") {
+js_type_check("closure_compile_test_lib") {
   testonly = true
   closure_flags = system_app_closure_flags_strict
   deps = [
@@ -102,6 +103,17 @@ js_type_check("closure_compile_tests") {
   ]
 }
 
+# Use relaxed flags for the browsertest files themselves. This removes null
+# checks and "could not determine type" errors which don't add a lot of value.
+js_type_check("closure_compile_browsertests") {
+  testonly = true
+  closure_flags = system_app_closure_flags
+  deps = [
+    ":test_help_app_guest_ui_browsertest_js",
+    ":test_help_app_ui_browsertest_js",
+  ]
+}
+
 js_library("test_driver_api_js") {
   testonly = true
   sources = [ "test/driver_api.js" ]
@@ -126,3 +138,27 @@ js_library("test_driver_js") {
     "//chromeos/components/system_apps/public/js:message_pipe",
   ]
 }
+
+js_library("test_help_app_ui_browsertest_js") {
+  testonly = true
+  sources = [ "test/help_app_ui_browsertest.js" ]
+  externs_list =
+      [ "//chromeos/components/web_applications/js2gtest_support.externs.js" ]
+  deps = [
+    ":test_driver_js",
+    "//chromeos/components/help_app_ui/resources:browser_proxy",
+    "//chromeos/components/system_apps/public/js:message_pipe",
+  ]
+}
+
+js_library("test_help_app_guest_ui_browsertest_js") {
+  testonly = true
+  sources = [ "test/help_app_guest_ui_browsertest.js" ]
+  externs_list =
+      [ "//chromeos/components/web_applications/js2gtest_support.externs.js" ]
+  deps = [
+    ":test_driver_js",
+    "//chromeos/components/help_app_ui/resources:receiver",
+    "//chromeos/components/system_apps/public/js:message_pipe",
+  ]
+}

chromeos/components/help_app_ui/resources/browser_proxy.js

@@ -36,6 +36,7 @@ function toString16(s) {
 const TITLE_ID = 'title';
 const BODY_ID = 'body';
 const CATEGORY_ID = 'main-category';
+const SUBHEADING_ID = 'subheading';
 
 /**
  * A pipe through which we can send messages to the guest frame.
@@ -64,23 +65,36 @@ guestMessagePipe.registerHandler(
       const data_from_app =
           /** @type {!Array<!helpApp.SearchableItem>} */ (message);
       const data_to_send = data_from_app.map(searchable_item => {
+        /** @type {!Array<!chromeos.localSearchService.mojom.Content>} */
         const contents = [
           {
             id: TITLE_ID,
             content: toString16(searchable_item.title),
             weight: 1.0,
           },
-          {
-            id: BODY_ID,
-            content: toString16(searchable_item.body),
-            weight: 0.2,
-          },
           {
             id: CATEGORY_ID,
             content: toString16(searchable_item.mainCategoryName),
             weight: 0.1,
           },
         ];
+        // If there are subheadings, use those instead of the body.
+        if (searchable_item.subheadings
+            && searchable_item.subheadings.length > 0) {
+          for (let i = 0; i < searchable_item.subheadings.length; ++i) {
+            contents.push({
+              id: SUBHEADING_ID + i,
+              content: toString16(searchable_item.subheadings[i]),
+              weight: 0.4,
+            });
+          }
+        } else if (searchable_item.body) {
+          contents.push({
+            id: BODY_ID,
+            content: toString16(searchable_item.body),
+            weight: 0.2,
+          });
+        }
         return {
           id: searchable_item.id,
           contents,
@@ -121,6 +135,16 @@ guestMessagePipe.registerHandler(
         const titlePositions = [];
         /** @type {!Array<!helpApp.Position>} */
         const bodyPositions = [];
+        // Id of the best subheading that appears in positions. We consider
+        // the subheading containing the most match positions to be the best.
+        // "" means no subheading positions found.
+        let bestSubheadingId = "";
+        /**
+         * Counts how many positions there are for each subheading id.
+         * @type {!Object<string, number>}
+         */
+        const subheadingPosCounts = {};
+        // Note: result.positions is not sorted.
         for (const position of result.positions) {
           if (position.contentId === TITLE_ID) {
             titlePositions.push(
@@ -128,16 +152,56 @@ guestMessagePipe.registerHandler(
           } else if (position.contentId === BODY_ID) {
             bodyPositions.push(
                 {length: position.length, start: position.start});
+          } else if (position.contentId.startsWith(SUBHEADING_ID)) {
+            // Update the subheadings's position count and check if it's the new
+            // best subheading.
+            const newCount = (subheadingPosCounts[position.contentId] || 0) + 1;
+            subheadingPosCounts[position.contentId] = newCount;
+            if (!bestSubheadingId
+                || newCount > subheadingPosCounts[bestSubheadingId]) {
+              bestSubheadingId = position.contentId;
+            }
           }
         }
+        // Use only the positions of the best subheading.
+        /** @type {!Array<!helpApp.Position>} */
+        const subheadingPositions = [];
+        if (bestSubheadingId) {
+          for (const position of result.positions) {
+            if (position.contentId === bestSubheadingId) {
+              subheadingPositions.push({
+                start: position.start,
+                length: position.length,
+              });
+            }
+          }
+          subheadingPositions.sort(compareByStart);
+        }
+
         // Sort positions by start index.
-        titlePositions.sort((a, b) => a.start - b.start);
-        bodyPositions.sort((a, b) => a.start - b.start);
+        titlePositions.sort(compareByStart);
+        bodyPositions.sort(compareByStart);
         return {
           id: result.id,
           titlePositions,
           bodyPositions,
+          subheadingIndex: bestSubheadingId
+              ? Number(bestSubheadingId.substring(SUBHEADING_ID.length))
+              : null,
+          subheadingPositions: bestSubheadingId
+              ? subheadingPositions
+              : null,
         };
       });
       return {results};
     });
+
+/**
+ * Compare two positions by their start index. Use for sorting.
+ *
+ * @param {!helpApp.Position} a
+ * @param {!helpApp.Position} b
+ */
+function compareByStart(a, b) {
+  return a.start - b.start;
+}

chromeos/components/help_app_ui/test/help_app_guest_ui_browsertest.js

@@ -8,3 +8,69 @@
 GUEST_TEST('GuestHasLang', () => {
   assertEquals(document.documentElement.lang, 'en-US');
 });
+
+// Test that search works when called from the guest frame, and it works for
+// searchable items with and without subheadings.
+GUEST_TEST('GuestCanSearchWithHeadings', async () => {
+  const delegate = /** @type {!helpApp.ClientApi} */ (
+      document.querySelector('showoff-app')).getDelegate();
+  await delegate.addOrUpdateSearchIndex([{
+    // Title match. No subheadings.
+    id: 'test-id-1',
+    title: 'Title with verycomplicatedsearchtoken',
+    body: 'Body text',
+    mainCategoryName: 'Help',
+    locale: 'en-US',
+  },{
+    // Subheading match.
+    id: 'test-id-2',
+    title: 'Title 2',
+    subheadings: [
+      'Subheading 1',
+      'verycomplicatedsearchtoken in subheading. Verycomplicatedsearchtoken',
+      'Another subheading with verycomplicatedsearchtoken',
+    ],
+    body: 'Body text',
+    mainCategoryName: 'Help',
+    locale: 'en-US',
+  },{
+    // Should not appear in the results.
+    id: 'test-id-3',
+    title: 'Title of irrelevant article',
+    body: 'Body text',
+    mainCategoryName: 'Help',
+    locale: 'en-US',
+  }]);
+
+  // Keep polling until the index finishes updating or too much time has passed.
+  /** @type {?helpApp.FindResponse} */
+  let response = null;
+  for (let numTries = 0; numTries < 100; numTries++) {
+    response = await delegate.findInSearchIndex('verycomplicatedsearchtoken');
+    if (response && response.results) break;
+    await new Promise(resolve => {setTimeout(resolve, 50)});
+  }
+
+  assertDeepEquals(response.results, [
+    // The first result only matches on the title.
+    {
+      id: 'test-id-1',
+      titlePositions: [{start: 11, length: 26}],
+      subheadingIndex: null,
+      subheadingPositions: null,
+      bodyPositions: [],
+    },
+    // The second result only matches on the second and third subheadings, and
+    // it uses the subheading with the most matches in the snippet.
+    {
+      id: 'test-id-2',
+      titlePositions: [],
+      subheadingIndex: 1,
+      subheadingPositions: [
+        {start: 0, length: 26},
+        {start: 42, length: 26},
+      ],
+      bodyPositions: [],
+    },
+  ]);
+});

chromeos/components/help_app_ui/test/help_app_ui_browsertest.js

@@ -33,6 +33,15 @@ var HelpAppUIBrowserTest = class extends testing.Test {
     return true;
   }
 
+  /** @override */
+  get featureList() {
+    return {
+      enabled: [
+        'chromeos::features::kHelpAppSearchServiceIntegration',
+      ]
+    };
+  }
+
   /** @override */
   get typedefCppFixture() {
     return 'HelpAppUiBrowserTest';
@@ -46,7 +55,8 @@ var HelpAppUIBrowserTest = class extends testing.Test {
 
 // Tests that chrome://help-app goes somewhere instead of 404ing or crashing.
 TEST_F('HelpAppUIBrowserTest', 'HasChromeSchemeURL', () => {
-  const guest = document.querySelector('iframe');
+  const guest = /** @type {!HTMLIFrameElement} */ (
+      document.querySelector('iframe'));
 
   assertEquals(document.location.origin, HOST_ORIGIN);
   assertEquals(guest.src, GUEST_ORIGIN + '/');
@@ -79,3 +89,8 @@ TEST_F('HelpAppUIBrowserTest', 'GuestHasLang', async () => {
   await runTestInGuest('GuestHasLang');
   testDone();
 });
+
+TEST_F('HelpAppUIBrowserTest', 'GuestCanSearchWithSubheadings', async () => {
+  await runTestInGuest('GuestCanSearchWithHeadings');
+  testDone();
+});