Commit 25be7d90 authored by Callistus's avatar Callistus Committed by Commit Bot

Implement logic in help app delegate to use the Local Search Service.

- Added |IsLSSEnabled| to help_app_ui.mojom.
- LSS methods in receiver forward requests via message pipe.
- Implement message handlers in browser_proxy.
- Added browsertest to ensure LSS methods used by delegate.

Bug: b/166047521
Change-Id: Iadc5fba94e79631d7b293be0e5b917f72bdc8fc8
Cq-Include-Trybots: chrome/try:linux-chromeos-chrome
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2437560Reviewed-by: default avatarTrent Apted <tapted@chromium.org>
Reviewed-by: default avatardstockwell <dstockwell@google.com>
Reviewed-by: default avatarDominick Ng <dominickn@chromium.org>
Commit-Queue: Callistus Tan <callistus@google.com>
Cr-Commit-Position: refs/heads/master@{#813011}
parent f2d543fb
...@@ -47,6 +47,7 @@ class HelpAppIntegrationTest : public SystemWebAppIntegrationTest { ...@@ -47,6 +47,7 @@ class HelpAppIntegrationTest : public SystemWebAppIntegrationTest {
HelpAppIntegrationTest() { HelpAppIntegrationTest() {
scoped_feature_list_.InitWithFeatures( scoped_feature_list_.InitWithFeatures(
{chromeos::features::kHelpAppReleaseNotes, {chromeos::features::kHelpAppReleaseNotes,
chromeos::features::kHelpAppSearchServiceIntegration,
chromeos::features::kReleaseNotesNotificationAllChannels}, chromeos::features::kReleaseNotesNotificationAllChannels},
{}); {});
} }
...@@ -354,6 +355,45 @@ IN_PROC_BROWSER_TEST_P(HelpAppIntegrationTest, HelpAppV2ShowParentalControls) { ...@@ -354,6 +355,45 @@ IN_PROC_BROWSER_TEST_P(HelpAppIntegrationTest, HelpAppV2ShowParentalControls) {
EXPECT_EQ(expected_url, GetActiveWebContents()->GetVisibleURL()); EXPECT_EQ(expected_url, GetActiveWebContents()->GetVisibleURL());
} }
// Test that the Help App delegate uses the local search service methods.
IN_PROC_BROWSER_TEST_P(HelpAppIntegrationTest,
HelpAppV2UsesLocalSearchServiceMethods) {
WaitForTestSystemAppInstall();
content::WebContents* web_contents = LaunchApp(web_app::SystemAppType::HELP);
// Script that adds a data item to the search index, then tries to find that
// data item.
constexpr char kScript[] = R"(
(async () => {
const delegate = document.querySelector('showoff-app').getDelegate();
await delegate.addOrUpdateSearchIndex([{
id: 'test-id',
title: 'foobar',
body: 'foo bar baz',
mainCategoryName: 'Help',
locale: 'en-US',
}]);
// Polling is required as addOrUpdateSearchIndex resolves before the
// search index is actually updated.
setInterval(async () => {
// Note that the LSS will fuzzy match into foobar.
const {results} = await delegate.findInSearchIndex('foober');
if (results && results.length === 1) {
window.domAutomationController.send(results[0].id);
}
}, 10);
})();
)";
std::string result;
// Use ExecuteScript instead of EvalJsInAppFrame because the script needs to
// run in the same world as the page's code.
EXPECT_TRUE(content::ExecuteScriptAndExtractString(
SandboxedWebUiAppTestBase::GetAppFrame(web_contents), kScript, &result));
EXPECT_EQ(result, "test-id");
}
// Test that the Help App opens when Gesture help requested. // Test that the Help App opens when Gesture help requested.
IN_PROC_BROWSER_TEST_P(HelpAppAllProfilesIntegrationTest, HelpAppOpenGestures) { IN_PROC_BROWSER_TEST_P(HelpAppAllProfilesIntegrationTest, HelpAppOpenGestures) {
WaitForTestSystemAppInstall(); WaitForTestSystemAppInstall();
......
...@@ -6,13 +6,18 @@ ...@@ -6,13 +6,18 @@
#include <utility> #include <utility>
#include "base/feature_list.h"
#include "chromeos/components/help_app_ui/help_app_ui.h" #include "chromeos/components/help_app_ui/help_app_ui.h"
#include "chromeos/components/help_app_ui/help_app_ui_delegate.h" #include "chromeos/components/help_app_ui/help_app_ui_delegate.h"
#include "chromeos/constants/chromeos_features.h"
HelpAppPageHandler::HelpAppPageHandler( HelpAppPageHandler::HelpAppPageHandler(
chromeos::HelpAppUI* help_app_ui, chromeos::HelpAppUI* help_app_ui,
mojo::PendingReceiver<help_app_ui::mojom::PageHandler> receiver) mojo::PendingReceiver<help_app_ui::mojom::PageHandler> receiver)
: receiver_(this, std::move(receiver)), help_app_ui_(help_app_ui) {} : receiver_(this, std::move(receiver)),
help_app_ui_(help_app_ui),
is_lss_enabled_(base::FeatureList::IsEnabled(
chromeos::features::kHelpAppSearchServiceIntegration)) {}
HelpAppPageHandler::~HelpAppPageHandler() = default; HelpAppPageHandler::~HelpAppPageHandler() = default;
...@@ -25,3 +30,7 @@ void HelpAppPageHandler::OpenFeedbackDialog( ...@@ -25,3 +30,7 @@ void HelpAppPageHandler::OpenFeedbackDialog(
void HelpAppPageHandler::ShowParentalControls() { void HelpAppPageHandler::ShowParentalControls() {
help_app_ui_->delegate()->ShowParentalControls(); help_app_ui_->delegate()->ShowParentalControls();
} }
void HelpAppPageHandler::IsLssEnabled(IsLssEnabledCallback callback) {
std::move(callback).Run(is_lss_enabled_);
}
...@@ -28,10 +28,12 @@ class HelpAppPageHandler : public help_app_ui::mojom::PageHandler { ...@@ -28,10 +28,12 @@ class HelpAppPageHandler : public help_app_ui::mojom::PageHandler {
// help_app_ui::mojom::PageHandler: // help_app_ui::mojom::PageHandler:
void OpenFeedbackDialog(OpenFeedbackDialogCallback callback) override; void OpenFeedbackDialog(OpenFeedbackDialogCallback callback) override;
void ShowParentalControls() override; void ShowParentalControls() override;
void IsLssEnabled(IsLssEnabledCallback callback) override;
private: private:
mojo::Receiver<help_app_ui::mojom::PageHandler> receiver_; mojo::Receiver<help_app_ui::mojom::PageHandler> receiver_;
chromeos::HelpAppUI* help_app_ui_; // Owns |this|. chromeos::HelpAppUI* help_app_ui_; // Owns |this|.
bool is_lss_enabled_;
}; };
#endif // CHROMEOS_COMPONENTS_HELP_APP_UI_HELP_APP_PAGE_HANDLER_H_ #endif // CHROMEOS_COMPONENTS_HELP_APP_UI_HELP_APP_PAGE_HANDLER_H_
...@@ -19,4 +19,7 @@ interface PageHandler { ...@@ -19,4 +19,7 @@ interface PageHandler {
// Opens the parental controls part of OS settings. // Opens the parental controls part of OS settings.
ShowParentalControls(); ShowParentalControls();
// Returns true if Local Search Service integration is enabled.
IsLssEnabled() => (bool enabled);
}; };
...@@ -11,16 +11,32 @@ helpAppUi.mojom.PageHandlerFactory.getRemote().createPageHandler( ...@@ -11,16 +11,32 @@ helpAppUi.mojom.PageHandlerFactory.getRemote().createPageHandler(
help_app.handler.$.bindNewPipeAndPassReceiver()); help_app.handler.$.bindNewPipeAndPassReceiver());
// Set up an index remote to talk to Local Search Service. // Set up an index remote to talk to Local Search Service.
// TODO(b/166047521): Define and use API to make calls to this from untrusted /** @type {!chromeos.localSearchService.mojom.IndexProxyRemote} */
// context.
const indexRemote = chromeos.localSearchService.mojom.IndexProxy.getRemote(); const indexRemote = chromeos.localSearchService.mojom.IndexProxy.getRemote();
const GUEST_ORIGIN = 'chrome-untrusted://help-app'; const GUEST_ORIGIN = 'chrome-untrusted://help-app';
const guestFrame = /** @type {!HTMLIFrameElement} */ ( const guestFrame =
document.createElement('iframe')); /** @type {!HTMLIFrameElement} */ (document.createElement('iframe'));
guestFrame.src = `${GUEST_ORIGIN}${location.pathname}`; guestFrame.src = `${GUEST_ORIGIN}${location.pathname}`;
document.body.appendChild(guestFrame); document.body.appendChild(guestFrame);
// Cached result whether Local Search Service is enabled.
/** @type {Promise<boolean>} */
const isLssEnabled =
help_app.handler.isLssEnabled().then(result => result.enabled);
/**
* @param {string} s
* @return {!mojoBase.mojom.String16Spec}
*/
function toString16(s) {
return /** @type {!mojoBase.mojom.String16Spec} */ (
{data: Array.from(s, (/** @type {string} */ c) => c.charCodeAt())});
}
const TITLE_ID = 'title';
const BODY_ID = 'body';
const CATEGORY_ID = 'main-category';
/** /**
* A pipe through which we can send messages to the guest frame. * A pipe through which we can send messages to the guest frame.
* Use an undefined `target` to find the <iframe> automatically. * Use an undefined `target` to find the <iframe> automatically.
...@@ -28,9 +44,9 @@ document.body.appendChild(guestFrame); ...@@ -28,9 +44,9 @@ document.body.appendChild(guestFrame);
* throw exceptions that are handled on the other side of the pipe (in the guest * throw exceptions that are handled on the other side of the pipe (in the guest
* frame), not on this side. * frame), not on this side.
*/ */
const guestMessagePipe = const guestMessagePipe = new MessagePipe(
new MessagePipe('chrome-untrusted://help-app', /*target=*/ undefined, 'chrome-untrusted://help-app', /*target=*/ undefined,
/*rethrowErrors=*/ false); /*rethrowErrors=*/ false);
guestMessagePipe.registerHandler(Message.OPEN_FEEDBACK_DIALOG, () => { guestMessagePipe.registerHandler(Message.OPEN_FEEDBACK_DIALOG, () => {
return help_app.handler.openFeedbackDialog(); return help_app.handler.openFeedbackDialog();
...@@ -39,3 +55,89 @@ guestMessagePipe.registerHandler(Message.OPEN_FEEDBACK_DIALOG, () => { ...@@ -39,3 +55,89 @@ guestMessagePipe.registerHandler(Message.OPEN_FEEDBACK_DIALOG, () => {
guestMessagePipe.registerHandler(Message.SHOW_PARENTAL_CONTROLS, () => { guestMessagePipe.registerHandler(Message.SHOW_PARENTAL_CONTROLS, () => {
help_app.handler.showParentalControls(); help_app.handler.showParentalControls();
}); });
guestMessagePipe.registerHandler(
Message.ADD_OR_UPDATE_SEARCH_INDEX, async (message) => {
if (!(await isLssEnabled)) {
return;
}
const data_from_app =
/** @type {!Array<!helpApp.SearchableItem>} */ (message);
const data_to_send = data_from_app.map(searchable_item => {
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,
},
];
return {
id: searchable_item.id,
contents,
locale: searchable_item.locale,
};
});
indexRemote.addOrUpdate(data_to_send);
});
guestMessagePipe.registerHandler(Message.CLEAR_SEARCH_INDEX, async () => {
if (!(await isLssEnabled)) {
return;
}
// TODO(b/166047521): Clear the index when that method is available.
});
guestMessagePipe.registerHandler(
Message.FIND_IN_SEARCH_INDEX, async (message) => {
if (!(await isLssEnabled)) {
return {results: null};
}
const response = await indexRemote.find(
toString16((/** @type {{query: string}} */ (message)).query),
/*max_results=*/ 100);
if (response.status !==
chromeos.localSearchService.mojom.ResponseStatus.kSuccess ||
!response.results) {
return {results: null};
}
const search_results =
/** @type {!Array<!chromeos.localSearchService.mojom.Result>} */ (
response.results);
// Sort results by decreasing score.
search_results.sort((a, b) => b.score - a.score);
/** @type {!Array<!helpApp.SearchResult>} */
const results = search_results.map(result => {
/** @type {!Array<!helpApp.Position>} */
const titlePositions = [];
/** @type {!Array<!helpApp.Position>} */
const bodyPositions = [];
for (const position of result.positions) {
if (position.contentId === TITLE_ID) {
titlePositions.push(
{length: position.length, start: position.start});
} else if (position.contentId === BODY_ID) {
bodyPositions.push(
{length: position.length, start: position.start});
}
}
// Sort positions by start index.
titlePositions.sort((a, b) => a.start - b.start);
bodyPositions.sort((a, b) => a.start - b.start);
return {
id: result.id,
titlePositions,
bodyPositions,
};
});
return {results};
});
...@@ -77,12 +77,14 @@ helpApp.SearchResult = function() {}; ...@@ -77,12 +77,14 @@ helpApp.SearchResult = function() {};
/** @type {string} */ /** @type {string} */
helpApp.SearchResult.prototype.id; helpApp.SearchResult.prototype.id;
/** /**
* List of positions corresponding to the title. Used in snippet. * List of positions corresponding to the title, sorted by start index. Used in
* snippet.
* @type {?Array<!helpApp.Position>} * @type {?Array<!helpApp.Position>}
*/ */
helpApp.SearchResult.prototype.titlePositions; helpApp.SearchResult.prototype.titlePositions;
/** /**
* List of positions corresponding to the body. Used in snippet. * List of positions corresponding to the body sorted by start index. Used in
* snippet.
* @type {?Array<!helpApp.Position>} * @type {?Array<!helpApp.Position>}
*/ */
helpApp.SearchResult.prototype.bodyPositions; helpApp.SearchResult.prototype.bodyPositions;
......
...@@ -13,5 +13,8 @@ ...@@ -13,5 +13,8 @@
*/ */
const Message = { const Message = {
OPEN_FEEDBACK_DIALOG: 'open-feedback-dialog', OPEN_FEEDBACK_DIALOG: 'open-feedback-dialog',
SHOW_PARENTAL_CONTROLS: 'show-parental-controls' SHOW_PARENTAL_CONTROLS: 'show-parental-controls',
ADD_OR_UPDATE_SEARCH_INDEX: 'add-or-update-search-index',
CLEAR_SEARCH_INDEX: 'clear-search-index',
FIND_IN_SEARCH_INDEX: 'find-in-search-index'
}; };
...@@ -24,15 +24,27 @@ const DELEGATE = { ...@@ -24,15 +24,27 @@ const DELEGATE = {
async showParentalControls() { async showParentalControls() {
await parentMessagePipe.sendMessage(Message.SHOW_PARENTAL_CONTROLS); await parentMessagePipe.sendMessage(Message.SHOW_PARENTAL_CONTROLS);
}, },
// TODO(b/166047521): Complete the implementation of these. /**
async addOrUpdateSearchIndex() { * @override
return; * @param {!Array<!helpApp.SearchableItem>} data
*/
async addOrUpdateSearchIndex(data) {
await parentMessagePipe.sendMessage(
Message.ADD_OR_UPDATE_SEARCH_INDEX, data);
}, },
async clearSearchIndex() { async clearSearchIndex() {
// TODO(b/166047521): Send the message when clear search index has been
// implemented. the index when that method is available.
return; return;
}, },
async findInSearchIndex() { /**
return {results: null}; * @override
* @param {string} query
* @return {!Promise<!helpApp.FindResponse>}
*/
findInSearchIndex(query) {
return /** @type {!Promise<!helpApp.FindResponse>} */ (
parentMessagePipe.sendMessage(Message.FIND_IN_SEARCH_INDEX, {query}));
}, },
}; };
......
...@@ -44,8 +44,6 @@ var HelpAppUIBrowserTest = class extends testing.Test { ...@@ -44,8 +44,6 @@ var HelpAppUIBrowserTest = class extends testing.Test {
} }
}; };
const toString16 = s => ({data: Array.from(s, c => c.charCodeAt())});
// Tests that chrome://help-app goes somewhere instead of 404ing or crashing. // Tests that chrome://help-app goes somewhere instead of 404ing or crashing.
TEST_F('HelpAppUIBrowserTest', 'HasChromeSchemeURL', () => { TEST_F('HelpAppUIBrowserTest', 'HasChromeSchemeURL', () => {
const guest = document.querySelector('iframe'); const guest = document.querySelector('iframe');
...@@ -64,6 +62,7 @@ TEST_F('HelpAppUIBrowserTest', 'HasTitleAndLang', () => { ...@@ -64,6 +62,7 @@ TEST_F('HelpAppUIBrowserTest', 'HasTitleAndLang', () => {
// Tests that we can make calls to the LSS to search. // Tests that we can make calls to the LSS to search.
TEST_F('HelpAppUIBrowserTest', 'CanSearchViaLSSIndex', async () => { TEST_F('HelpAppUIBrowserTest', 'CanSearchViaLSSIndex', async () => {
const toString16 = s => ({data: Array.from(s, c => c.charCodeAt())});
const result = await indexRemote.find(toString16('search string!'), 100); const result = await indexRemote.find(toString16('search string!'), 100);
// Status 3 corresponds to kEmptyIndex. // Status 3 corresponds to kEmptyIndex.
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment