From 476d99c927db659ffcf2ccde286552dbca7aeaef Mon Sep 17 00:00:00 2001 From: Ya-Chen Hsieh <1697814+ychsieh@users.noreply.github.com> Date: Thu, 25 Feb 2021 23:36:07 +0800 Subject: [PATCH] =?UTF-8?q?=E2=9C=A8=20=20Assist.js:=20Add=20a=20shared=20?= =?UTF-8?q?FrameService=20to=20assistjs=20extension=20(#32784)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * Create the skeleton of the new extension for assist.js: amp-google-assistant-assistjs. It consists of one shared service and multiple custom elements. * Remove unneeded regenerator- runtime dependency in package.json * - Add/modify copyright statement and header comment for each file - Add appropriate validator rules for the new extension - Add more detailed description about what this extension does * Add a shared service(AssistjsConfigService) and a new config element. Also includes some minor refactoring. The service provides config to other components and the element take prespecified config json. * Fix styling and remove commented code in AssistjsFrameService. * Improve config service: - Ensure config json retrieval - Move widget iframe url creation into one single place. - Add assertion to ensure there's only one config element on the page Minor fixes: - Replace await usage with then to avoid extra dependencies. - Update validator rules for the invisible config element * Fix prettify errors. * Unit test fix and some minor fixes. * Fix lint errors. * Update extensions/amp-google-assistant-assistjs/0.1/assistjs-frame-service.js Co-authored-by: Justin Ridgewell * Minor fixes. * Fix errors caused by not having this.win in a service class(non-AMP.BaseElement). Also add a config service mock to the unit test. * ampdoc.getWin() is forbidden. Replace it with "window" directly. * - Append iframe to the document after frame src is retrieved - Parse config in config element to save a DOM query - Delete assistjs-frame-service.js. Add it in next PR with real usage. * Fix FrameService reference errors and lint errors. * Add a shared FrameService for all custom elements to handle requests from underlying iframes. Actual implementation of two endpoints: OpenMic and SendTextQuery is empty now and will be implemented in upcoming PRs. * Use ampdoc.whenready instead to fix the loading error. Add sandbox attributes to all iframes. * Update extensions/amp-google-assistant-assistjs/0.1/assistjs-frame-service.js Co-authored-by: Justin Ridgewell * Update extensions/amp-google-assistant-assistjs/0.1/assistjs-frame-service.js Co-authored-by: Justin Ridgewell Co-authored-by: Justin Ridgewell --- .../0.1/amp-google-assistant-assistjs.js | 2 + ...-google-assistant-inline-suggestion-bar.js | 10 +++ .../0.1/amp-google-assistant-voice-bar.js | 11 +++ .../0.1/amp-google-assistant-voice-button.js | 10 +++ .../0.1/assistjs-frame-service.js | 71 +++++++++++++++++++ .../test-amp-google-assistant-assistjs.js | 5 ++ 6 files changed, 109 insertions(+) create mode 100644 extensions/amp-google-assistant-assistjs/0.1/assistjs-frame-service.js diff --git a/extensions/amp-google-assistant-assistjs/0.1/amp-google-assistant-assistjs.js b/extensions/amp-google-assistant-assistjs/0.1/amp-google-assistant-assistjs.js index ae6363450dad..2bfcfe2395b3 100644 --- a/extensions/amp-google-assistant-assistjs/0.1/amp-google-assistant-assistjs.js +++ b/extensions/amp-google-assistant-assistjs/0.1/amp-google-assistant-assistjs.js @@ -23,6 +23,7 @@ import {AmpGoogleAssistantInlineSuggestionBar} from './amp-google-assistant-inli import {AmpGoogleAssistantVoiceBar} from './amp-google-assistant-voice-bar'; import {AmpGoogleAssistantVoiceButton} from './amp-google-assistant-voice-button'; import {AssistjsConfigService} from './assistjs-config-service'; +import {AssistjsFrameService} from './assistjs-frame-service'; import {CSS} from '../../../build/amp-google-assistant-assistjs-0.1.css'; import {Services} from '../../../src/services'; @@ -38,6 +39,7 @@ export class AmpGoogleAssistantAssistjsConfig extends AMP.BaseElement { AMP.extension('amp-google-assistant-assistjs', '0.1', (AMP) => { AMP.registerServiceForDoc('assistjs-config-service', AssistjsConfigService); + AMP.registerServiceForDoc('assistjs-frame-service', AssistjsFrameService); AMP.registerElement( 'amp-google-assistant-assistjs-config', AmpGoogleAssistantAssistjsConfig, diff --git a/extensions/amp-google-assistant-assistjs/0.1/amp-google-assistant-inline-suggestion-bar.js b/extensions/amp-google-assistant-assistjs/0.1/amp-google-assistant-inline-suggestion-bar.js index 41a6b54caf1d..1e29273edd13 100644 --- a/extensions/amp-google-assistant-assistjs/0.1/amp-google-assistant-inline-suggestion-bar.js +++ b/extensions/amp-google-assistant-assistjs/0.1/amp-google-assistant-inline-suggestion-bar.js @@ -30,11 +30,15 @@ export class AmpGoogleAssistantInlineSuggestionBar extends AMP.BaseElement { /** @private {?AssistjsConfigService} */ this.configService_ = null; + + /** @private {?AssistjsFrameService} */ + this.frameService_ = null; } /** @override */ buildCallback() { this.configService_ = Services.assistjsConfigServiceForDoc(this.element); + this.frameService_ = Services.assistjsFrameServiceForDoc(this.element); } /** @override */ @@ -51,6 +55,7 @@ export class AmpGoogleAssistantInlineSuggestionBar extends AMP.BaseElement { .then((iframeUrl) => { addAttributesToElement(iframe, { src: iframeUrl, + sandbox: 'allow-scripts', }); // applyFillContent so that frame covers the entire component. @@ -59,6 +64,11 @@ export class AmpGoogleAssistantInlineSuggestionBar extends AMP.BaseElement { this.element.appendChild(iframe); }); + iframe.addEventListener('load', () => { + // TODO: create a channel to receive requests from underlying assist.js iframe. + this.frameService_.sendTextQuery(); + }); + // Return a load promise for the frame so the runtime knows when the // component is ready. return this.loadPromise(iframe); diff --git a/extensions/amp-google-assistant-assistjs/0.1/amp-google-assistant-voice-bar.js b/extensions/amp-google-assistant-assistjs/0.1/amp-google-assistant-voice-bar.js index f7c85a9785f0..75992d9ab111 100644 --- a/extensions/amp-google-assistant-assistjs/0.1/amp-google-assistant-voice-bar.js +++ b/extensions/amp-google-assistant-assistjs/0.1/amp-google-assistant-voice-bar.js @@ -30,11 +30,15 @@ export class AmpGoogleAssistantVoiceBar extends AMP.BaseElement { /** @private {?AssistjsConfigService} */ this.configService_ = null; + + /** @private {?AssistjsFrameService} */ + this.frameService_ = null; } /** @override */ buildCallback() { this.configService_ = Services.assistjsConfigServiceForDoc(this.element); + this.frameService_ = Services.assistjsFrameServiceForDoc(this.element); } /** @override */ @@ -49,6 +53,7 @@ export class AmpGoogleAssistantVoiceBar extends AMP.BaseElement { this.configService_.getWidgetIframeUrl('voicebar').then((iframeUrl) => { addAttributesToElement(iframe, { src: iframeUrl, + sandbox: 'allow-scripts', }); // applyFillContent so that frame covers the entire component. @@ -57,6 +62,12 @@ export class AmpGoogleAssistantVoiceBar extends AMP.BaseElement { this.element.appendChild(iframe); }); + iframe.addEventListener('load', () => { + // TODO: create a channel to receive requests from underlying assist.js iframe. + this.frameService_.openMic(); + this.frameService_.sendTextQuery(); + }); + // Return a load promise for the frame so the runtime knows when the // component is ready. return this.loadPromise(iframe); diff --git a/extensions/amp-google-assistant-assistjs/0.1/amp-google-assistant-voice-button.js b/extensions/amp-google-assistant-assistjs/0.1/amp-google-assistant-voice-button.js index b8318d48b140..e919858c9461 100644 --- a/extensions/amp-google-assistant-assistjs/0.1/amp-google-assistant-voice-button.js +++ b/extensions/amp-google-assistant-assistjs/0.1/amp-google-assistant-voice-button.js @@ -30,11 +30,15 @@ export class AmpGoogleAssistantVoiceButton extends AMP.BaseElement { /** @private {?AssistjsConfigService} */ this.configService_ = null; + + /** @private {?AssistjsFrameService} */ + this.frameService_ = null; } /** @override */ buildCallback() { this.configService_ = Services.assistjsConfigServiceForDoc(this.element); + this.frameService_ = Services.assistjsFrameServiceForDoc(this.element); } /** @override */ @@ -49,6 +53,7 @@ export class AmpGoogleAssistantVoiceButton extends AMP.BaseElement { this.configService_.getWidgetIframeUrl('voicebutton').then((iframeUrl) => { addAttributesToElement(iframe, { src: iframeUrl, + sandbox: 'allow-scripts', }); // applyFillContent so that frame covers the entire component. @@ -57,6 +62,11 @@ export class AmpGoogleAssistantVoiceButton extends AMP.BaseElement { this.element.appendChild(iframe); }); + iframe.addEventListener('load', () => { + // TODO: create a channel to receive requests from underlying assist.js iframe. + this.frameService_.openMic(); + }); + // Return a load promise for the frame so the runtime knows when the // component is ready. return this.loadPromise(iframe); diff --git a/extensions/amp-google-assistant-assistjs/0.1/assistjs-frame-service.js b/extensions/amp-google-assistant-assistjs/0.1/assistjs-frame-service.js new file mode 100644 index 000000000000..b76a04e3378d --- /dev/null +++ b/extensions/amp-google-assistant-assistjs/0.1/assistjs-frame-service.js @@ -0,0 +1,71 @@ +/** + * Copyright 2021 The AMP HTML Authors. All Rights Reserved. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS-IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +/** + * @fileoverview The shared service used by all custom elements to talk to Assistant services. It loads an iframe that exports + * endpoints to handle requests from custom elements. + */ + +import {Services} from '../../../src/services'; +import {addAttributesToElement} from '../../../src/dom'; +import {toggle} from '../../../src/style'; + +export class AssistjsFrameService { + /** + * @param {!../../../src/service/ampdoc-impl.AmpDoc} ampdoc + */ + constructor(ampdoc) { + /** @private @const {!../../../src/service/ampdoc-impl.AmpDoc} */ + this.ampDoc_ = ampdoc; + + /** @private {?AssistjsConfigService} */ + this.configService_ = null; + + /** Create Assistant iframe and append it to the main AMP document. */ + this.createAssistantIframe_(); + } + + /** @private */ + createAssistantIframe_() { + this.ampDoc_.whenFirstVisible().then(() => { + this.configService_ = Services.assistjsConfigServiceForDoc(this.ampDoc_); + const iframe = this.ampDoc_.win.document.createElement('iframe'); + this.configService_.getWidgetIframeUrl('frame').then((iframeUrl) => { + addAttributesToElement(iframe, { + src: iframeUrl, + allow: 'microphone', + sandbox: 'allow-scripts', + }); + toggle(iframe, false); + document.body.appendChild(iframe); + }); + }); + } + + /** + * Activates Assistant microphone on 3P page. + */ + openMic() { + // TODO: add implementation once the channels for iframes are implemented. + } + + /** + * Sends text query to Assistant server. + */ + sendTextQuery() { + // TODO: add implementation once the channels for iframes are implemented. + } +} diff --git a/extensions/amp-google-assistant-assistjs/0.1/test/test-amp-google-assistant-assistjs.js b/extensions/amp-google-assistant-assistjs/0.1/test/test-amp-google-assistant-assistjs.js index c78931b858d7..0c6f6ad92518 100644 --- a/extensions/amp-google-assistant-assistjs/0.1/test/test-amp-google-assistant-assistjs.js +++ b/extensions/amp-google-assistant-assistjs/0.1/test/test-amp-google-assistant-assistjs.js @@ -41,6 +41,11 @@ describes.realWin( .withExactArgs('voicebutton') .once(); + const frameServiceMock = env.sandbox.mock( + Services.assistjsFrameServiceForDoc(document) + ); + frameServiceMock.expects('openMic').once(); + configElement = document.createElement( 'amp-google-assistant-assistjs-config' );