Commit 564e8cfe authored by Robert Knight's avatar Robert Knight

Use separate channels for guest / host messages from sidebar

Use separate channels for sending messages to guests vs the host in the
sidebar, even for the common case when there is only one guest and it is
the same frame as the host.

This change makes it clear for readers which part of the annotator code
is intended to handle a particular message from the sidebar. It is also
a step towards supporting host frames that are not guests. This will be
needed in ebook readers where the host frame provides the navigation UI
and contains the frame displaying the book content, but should not be
annotatble itself.

 - Remove the `bridge` service in the sidebar. The `frameSync` service
   now provides the entry point for other services/components to make RPC calls
   to the host or guest frames. Currently the only use case is
   sending notifications to the host via `FrameSyncService.notifyHost`.

 - Create separate `Bridge` instances in `FrameSyncService` for sidebar
   <-> guest and sidebar <-> host communication. The sidebar <-> guest
   bridge works the same as before. The sidebar <-> host bridge
   is established by having `FrameSyncService` create a MessageChannel
   when sending the `hypothesisSidebarReady` notification to the host.

   The sidebar / host frames then add respective ports of this channel
   to a Bridge instance.

 - Change the various existing RPC calls between frames to use either
   the guest <-> sidebar or host <-> sidebar communication channels as
   appropriate
parent 4d3f72e9
import Hammer from 'hammerjs'; import Hammer from 'hammerjs';
import { Bridge } from '../shared/bridge';
import events from '../shared/bridge-events'; import events from '../shared/bridge-events';
import { ListenerCollection } from '../shared/listener-collection'; import { ListenerCollection } from '../shared/listener-collection';
...@@ -63,6 +64,8 @@ export default class Sidebar { ...@@ -63,6 +64,8 @@ export default class Sidebar {
constructor(element, eventBus, guest, config = {}) { constructor(element, eventBus, guest, config = {}) {
this._emitter = eventBus.createEmitter(); this._emitter = eventBus.createEmitter();
this._sidebarRPC = new Bridge();
/** /**
* The `<iframe>` element containing the sidebar application. * The `<iframe>` element containing the sidebar application.
*/ */
...@@ -199,8 +202,9 @@ export default class Sidebar { ...@@ -199,8 +202,9 @@ export default class Sidebar {
*/ */
this.ready = new Promise(resolve => { this.ready = new Promise(resolve => {
this._listeners.add(window, 'message', event => { this._listeners.add(window, 'message', event => {
const data = /** @type {MessageEvent} */ (event).data; const messageEvent = /** @type {MessageEvent} */ (event);
if (data?.type === 'hypothesisSidebarReady') { if (messageEvent.data?.type === 'hypothesisSidebarReady') {
this._sidebarRPC.createChannel(messageEvent.ports[0]);
resolve(); resolve();
} }
}); });
...@@ -220,22 +224,19 @@ export default class Sidebar { ...@@ -220,22 +224,19 @@ export default class Sidebar {
} }
_setupSidebarEvents() { _setupSidebarEvents() {
annotationCounts(document.body, this.guest.crossframe); annotationCounts(document.body, this._sidebarRPC);
sidebarTrigger(document.body, () => this.open()); sidebarTrigger(document.body, () => this.open());
features.init(this.guest.crossframe); features.init(this._sidebarRPC);
this.guest.crossframe.on('openSidebar', () => this.open()); this._sidebarRPC.on('openSidebar', () => this.open());
this.guest.crossframe.on('closeSidebar', () => this.close()); this._sidebarRPC.on('closeSidebar', () => this.close());
// Sidebar listens to the `openNotebook` event coming from the sidebar's // Sidebar listens to the `openNotebook` event coming from the sidebar's
// iframe and re-publishes it via the emitter to the Notebook // iframe and re-publishes it via the emitter to the Notebook
this.guest.crossframe.on( this._sidebarRPC.on('openNotebook', (/** @type {string} */ groupId) => {
'openNotebook',
(/** @type {string} */ groupId) => {
this.hide(); this.hide();
this._emitter.publish('openNotebook', groupId); this._emitter.publish('openNotebook', groupId);
} });
);
this._emitter.subscribe('closeNotebook', () => { this._emitter.subscribe('closeNotebook', () => {
this.show(); this.show();
}); });
...@@ -249,7 +250,7 @@ export default class Sidebar { ...@@ -249,7 +250,7 @@ export default class Sidebar {
]; ];
eventHandlers.forEach(([event, handler]) => { eventHandlers.forEach(([event, handler]) => {
if (handler) { if (handler) {
this.guest.crossframe.on(event, () => handler()); this._sidebarRPC.on(event, () => handler());
} }
}); });
} }
...@@ -427,7 +428,7 @@ export default class Sidebar { ...@@ -427,7 +428,7 @@ export default class Sidebar {
} }
open() { open() {
this.guest.crossframe.call('sidebarOpened'); this._sidebarRPC.call('sidebarOpened');
this._emitter.publish('sidebarOpened'); this._emitter.publish('sidebarOpened');
if (this.iframeContainer) { if (this.iframeContainer) {
...@@ -466,7 +467,7 @@ export default class Sidebar { ...@@ -466,7 +467,7 @@ export default class Sidebar {
* @param {boolean} shouldShowHighlights * @param {boolean} shouldShowHighlights
*/ */
setAllVisibleHighlights(shouldShowHighlights) { setAllVisibleHighlights(shouldShowHighlights) {
this.guest.crossframe.call('setVisibleHighlights', shouldShowHighlights); this._sidebarRPC.call('setVisibleHighlights', shouldShowHighlights);
} }
/** /**
......
...@@ -10,7 +10,6 @@ const EXTERNAL_CONTAINER_SELECTOR = 'test-external-container'; ...@@ -10,7 +10,6 @@ const EXTERNAL_CONTAINER_SELECTOR = 'test-external-container';
describe('Sidebar', () => { describe('Sidebar', () => {
const sandbox = sinon.createSandbox(); const sandbox = sinon.createSandbox();
let fakeCrossFrame;
let fakeGuest; let fakeGuest;
// Containers and Sidebar instances created by current test. // Containers and Sidebar instances created by current test.
...@@ -23,6 +22,8 @@ describe('Sidebar', () => { ...@@ -23,6 +22,8 @@ describe('Sidebar', () => {
let FakeToolbarController; let FakeToolbarController;
let fakeToolbar; let fakeToolbar;
let fakeBridge;
before(() => { before(() => {
sinon.stub(window, 'requestAnimationFrame').yields(); sinon.stub(window, 'requestAnimationFrame').yields();
}); });
...@@ -65,17 +66,12 @@ describe('Sidebar', () => { ...@@ -65,17 +66,12 @@ describe('Sidebar', () => {
beforeEach(() => { beforeEach(() => {
sidebars = []; sidebars = [];
containers = []; containers = [];
fakeCrossFrame = {
on: sandbox.stub(),
call: sandbox.stub(),
};
class FakeGuest { class FakeGuest {
constructor() { constructor() {
this.element = document.createElement('div'); this.element = document.createElement('div');
this.contentContainer = sinon.stub().returns(document.body); this.contentContainer = sinon.stub().returns(document.body);
this.createAnnotation = sinon.stub(); this.createAnnotation = sinon.stub();
this.crossframe = fakeCrossFrame;
this.fitSideBySide = sinon.stub(); this.fitSideBySide = sinon.stub();
this.setVisibleHighlights = sinon.stub(); this.setVisibleHighlights = sinon.stub();
} }
...@@ -100,7 +96,14 @@ describe('Sidebar', () => { ...@@ -100,7 +96,14 @@ describe('Sidebar', () => {
sidebars = []; sidebars = [];
fakeBridge = {
call: sinon.stub(),
createChannel: sinon.stub(),
on: sinon.stub(),
};
$imports.$mock({ $imports.$mock({
'../shared/bridge': { Bridge: sinon.stub().returns(fakeBridge) },
'./toolbar': { './toolbar': {
ToolbarController: FakeToolbarController, ToolbarController: FakeToolbarController,
}, },
...@@ -155,6 +158,27 @@ describe('Sidebar', () => { ...@@ -155,6 +158,27 @@ describe('Sidebar', () => {
}); });
}); });
function sendSidebarReadyMessage() {
const channel = new MessageChannel();
const event = new MessageEvent(
'message',
{
data: { type: 'hypothesisSidebarReady' },
},
[channel.port1]
);
window.dispatchEvent(event);
return event;
}
it('creates Bridge for host <-> sidebar RPC calls when `hypothesisSidebarReady` message is received', () => {
createSidebar();
const event = sendSidebarReadyMessage();
assert.calledWith(fakeBridge.createChannel, event.ports[0]);
});
describe('#ready', () => { describe('#ready', () => {
it('returns a promise that resolves when `hypothesisSidebarReady` message is received', async () => { it('returns a promise that resolves when `hypothesisSidebarReady` message is received', async () => {
const sidebar = createSidebar(); const sidebar = createSidebar();
...@@ -166,11 +190,7 @@ describe('Sidebar', () => { ...@@ -166,11 +190,7 @@ describe('Sidebar', () => {
'not-ready' 'not-ready'
); );
window.dispatchEvent( sendSidebarReadyMessage();
new MessageEvent('message', {
data: { type: 'hypothesisSidebarReady' },
})
);
return sidebar.ready; return sidebar.ready;
}); });
...@@ -281,10 +301,10 @@ describe('Sidebar', () => { ...@@ -281,10 +301,10 @@ describe('Sidebar', () => {
}); });
}); });
describe('crossframe listeners', () => { describe('sidebar RPC call handlers', () => {
const emitEvent = (event, ...args) => { const emitEvent = (event, ...args) => {
const result = []; const result = [];
for (let [evt, fn] of fakeCrossFrame.on.args) { for (let [evt, fn] of fakeBridge.on.args) {
if (event === evt) { if (event === evt) {
result.push(fn(...args)); result.push(fn(...args));
} }
...@@ -308,7 +328,7 @@ describe('Sidebar', () => { ...@@ -308,7 +328,7 @@ describe('Sidebar', () => {
assert.called(target); assert.called(target);
})); }));
describe('on "openNotebook" crossframe event', () => { describe('on "openNotebook" event', () => {
it('hides the sidebar', () => { it('hides the sidebar', () => {
const sidebar = createSidebar(); const sidebar = createSidebar();
sinon.stub(sidebar, 'hide').callThrough(); sinon.stub(sidebar, 'hide').callThrough();
...@@ -605,10 +625,10 @@ describe('Sidebar', () => { ...@@ -605,10 +625,10 @@ describe('Sidebar', () => {
}); });
describe('#setAllVisibleHighlights', () => describe('#setAllVisibleHighlights', () =>
it('sets the state through crossframe and emits', () => { it('requests sidebar to set highlight visibility in guest frames', () => {
const sidebar = createSidebar(); const sidebar = createSidebar();
sidebar.setAllVisibleHighlights(true); sidebar.setAllVisibleHighlights(true);
assert.calledWith(fakeCrossFrame.call, 'setVisibleHighlights', true); assert.calledWith(fakeBridge.call, 'setVisibleHighlights', true);
})); }));
it('hides toolbar controls when using the "clean" theme', () => { it('hides toolbar controls when using the "clean" theme', () => {
......
...@@ -23,7 +23,6 @@ import TopBar from './TopBar'; ...@@ -23,7 +23,6 @@ import TopBar from './TopBar';
/** /**
* @typedef {import('../../types/api').Profile} Profile * @typedef {import('../../types/api').Profile} Profile
* @typedef {import('../../types/config').MergedConfig} MergedConfig * @typedef {import('../../types/config').MergedConfig} MergedConfig
* @typedef {import('../../shared/bridge').Bridge} Bridge
* @typedef {import('./UserMenu').AuthState} AuthState * @typedef {import('./UserMenu').AuthState} AuthState
*/ */
...@@ -54,7 +53,7 @@ function authStateFromProfile(profile) { ...@@ -54,7 +53,7 @@ function authStateFromProfile(profile) {
/** /**
* @typedef HypothesisAppProps * @typedef HypothesisAppProps
* @prop {import('../services/auth').AuthService} auth * @prop {import('../services/auth').AuthService} auth
* @prop {Bridge} bridge * @prop {import('../services/frame-sync').FrameSyncService} frameSync
* @prop {MergedConfig} settings * @prop {MergedConfig} settings
* @prop {import('../services/session').SessionService} session * @prop {import('../services/session').SessionService} session
* @prop {import('../services/toast-messenger').ToastMessengerService} toastMessenger * @prop {import('../services/toast-messenger').ToastMessengerService} toastMessenger
...@@ -68,7 +67,7 @@ function authStateFromProfile(profile) { ...@@ -68,7 +67,7 @@ function authStateFromProfile(profile) {
* *
* @param {HypothesisAppProps} props * @param {HypothesisAppProps} props
*/ */
function HypothesisApp({ auth, bridge, settings, session, toastMessenger }) { function HypothesisApp({ auth, frameSync, settings, session, toastMessenger }) {
const store = useStoreProxy(); const store = useStoreProxy();
const hasFetchedProfile = store.hasFetchedProfile(); const hasFetchedProfile = store.hasFetchedProfile();
const profile = store.profile(); const profile = store.profile();
...@@ -99,7 +98,7 @@ function HypothesisApp({ auth, bridge, settings, session, toastMessenger }) { ...@@ -99,7 +98,7 @@ function HypothesisApp({ auth, bridge, settings, session, toastMessenger }) {
const login = async () => { const login = async () => {
if (serviceConfig(settings)) { if (serviceConfig(settings)) {
// Let the host page handle the login request // Let the host page handle the login request
bridge.call(bridgeEvents.LOGIN_REQUESTED); frameSync.notifyHost(bridgeEvents.LOGIN_REQUESTED);
return; return;
} }
...@@ -117,7 +116,7 @@ function HypothesisApp({ auth, bridge, settings, session, toastMessenger }) { ...@@ -117,7 +116,7 @@ function HypothesisApp({ auth, bridge, settings, session, toastMessenger }) {
const signUp = () => { const signUp = () => {
if (serviceConfig(settings)) { if (serviceConfig(settings)) {
// Let the host page handle the signup request // Let the host page handle the signup request
bridge.call(bridgeEvents.SIGNUP_REQUESTED); frameSync.notifyHost(bridgeEvents.SIGNUP_REQUESTED);
return; return;
} }
window.open(store.getLink('signup')); window.open(store.getLink('signup'));
...@@ -157,7 +156,7 @@ function HypothesisApp({ auth, bridge, settings, session, toastMessenger }) { ...@@ -157,7 +156,7 @@ function HypothesisApp({ auth, bridge, settings, session, toastMessenger }) {
store.discardAllDrafts(); store.discardAllDrafts();
if (serviceConfig(settings)) { if (serviceConfig(settings)) {
bridge.call(bridgeEvents.LOGOUT_REQUESTED); frameSync.notifyHost(bridgeEvents.LOGOUT_REQUESTED);
return; return;
} }
...@@ -203,7 +202,7 @@ function HypothesisApp({ auth, bridge, settings, session, toastMessenger }) { ...@@ -203,7 +202,7 @@ function HypothesisApp({ auth, bridge, settings, session, toastMessenger }) {
export default withServices(HypothesisApp, [ export default withServices(HypothesisApp, [
'auth', 'auth',
'bridge', 'frameSync',
'session', 'session',
'settings', 'settings',
'toastMessenger', 'toastMessenger',
......
...@@ -16,13 +16,12 @@ import UserMenu from './UserMenu'; ...@@ -16,13 +16,12 @@ import UserMenu from './UserMenu';
/** /**
* @typedef {import('../../types/config').MergedConfig} MergedConfig * @typedef {import('../../types/config').MergedConfig} MergedConfig
* @typedef {import('../components/UserMenu').AuthState} AuthState * @typedef {import('../components/UserMenu').AuthState} AuthState
* @typedef {import('../../shared/bridge').Bridge} Bridge
*/ */
/** /**
* @typedef TopBarProps * @typedef TopBarProps
* @prop {AuthState} auth * @prop {AuthState} auth
* @prop {Bridge} bridge * @prop {import('../services/frame-sync').FrameSyncService} frameSync
* @prop {boolean} isSidebar - Flag indicating whether the app is the sidebar or a top-level page. * @prop {boolean} isSidebar - Flag indicating whether the app is the sidebar or a top-level page.
* @prop {() => any} onLogin - Callback invoked when user clicks "Login" button. * @prop {() => any} onLogin - Callback invoked when user clicks "Login" button.
* @prop {() => any} onLogout - Callback invoked when user clicks "Logout" action in account menu. * @prop {() => any} onLogout - Callback invoked when user clicks "Logout" action in account menu.
...@@ -39,7 +38,7 @@ import UserMenu from './UserMenu'; ...@@ -39,7 +38,7 @@ import UserMenu from './UserMenu';
*/ */
function TopBar({ function TopBar({
auth, auth,
bridge, frameSync,
isSidebar, isSidebar,
onLogin, onLogin,
onLogout, onLogout,
...@@ -72,7 +71,7 @@ function TopBar({ ...@@ -72,7 +71,7 @@ function TopBar({
const requestHelp = () => { const requestHelp = () => {
const service = serviceConfig(settings); const service = serviceConfig(settings);
if (service && service.onHelpRequestProvided) { if (service && service.onHelpRequestProvided) {
bridge.call(bridgeEvents.HELP_REQUESTED); frameSync.notifyHost(bridgeEvents.HELP_REQUESTED);
} else { } else {
store.toggleSidebarPanel('help'); store.toggleSidebarPanel('help');
} }
...@@ -171,4 +170,4 @@ function TopBar({ ...@@ -171,4 +170,4 @@ function TopBar({
); );
} }
export default withServices(TopBar, ['bridge', 'settings', 'streamer']); export default withServices(TopBar, ['frameSync', 'settings', 'streamer']);
...@@ -28,7 +28,7 @@ import MenuSection from './MenuSection'; ...@@ -28,7 +28,7 @@ import MenuSection from './MenuSection';
* @typedef UserMenuProps * @typedef UserMenuProps
* @prop {AuthStateLoggedIn} auth - object representing authenticated user and auth status * @prop {AuthStateLoggedIn} auth - object representing authenticated user and auth status
* @prop {() => any} onLogout - onClick callback for the "log out" button * @prop {() => any} onLogout - onClick callback for the "log out" button
* @prop {object} bridge * @prop {import('../services/frame-sync').FrameSyncService} frameSync
* @prop {MergedConfig} settings * @prop {MergedConfig} settings
*/ */
...@@ -40,7 +40,7 @@ import MenuSection from './MenuSection'; ...@@ -40,7 +40,7 @@ import MenuSection from './MenuSection';
* *
* @param {UserMenuProps} props * @param {UserMenuProps} props
*/ */
function UserMenu({ auth, bridge, onLogout, settings }) { function UserMenu({ auth, frameSync, onLogout, settings }) {
const store = useStoreProxy(); const store = useStoreProxy();
const defaultAuthority = store.defaultAuthority(); const defaultAuthority = store.defaultAuthority();
...@@ -57,7 +57,7 @@ function UserMenu({ auth, bridge, onLogout, settings }) { ...@@ -57,7 +57,7 @@ function UserMenu({ auth, bridge, onLogout, settings }) {
!isThirdParty || serviceSupports('onLogoutRequestProvided'); !isThirdParty || serviceSupports('onLogoutRequestProvided');
const onSelectNotebook = () => { const onSelectNotebook = () => {
bridge.call('openNotebook', store.focusedGroupId()); frameSync.notifyHost('openNotebook', store.focusedGroupId());
}; };
// Temporary access to the Notebook without feature flag: // Temporary access to the Notebook without feature flag:
...@@ -70,7 +70,7 @@ function UserMenu({ auth, bridge, onLogout, settings }) { ...@@ -70,7 +70,7 @@ function UserMenu({ auth, bridge, onLogout, settings }) {
}; };
const onProfileSelected = () => const onProfileSelected = () =>
isThirdParty && bridge.call(bridgeEvents.PROFILE_REQUESTED); isThirdParty && frameSync.notifyHost(bridgeEvents.PROFILE_REQUESTED);
// Generate dynamic props for the profile <MenuItem> component // Generate dynamic props for the profile <MenuItem> component
const profileItemProps = (() => { const profileItemProps = (() => {
...@@ -129,4 +129,4 @@ function UserMenu({ auth, bridge, onLogout, settings }) { ...@@ -129,4 +129,4 @@ function UserMenu({ auth, bridge, onLogout, settings }) {
); );
} }
export default withServices(UserMenu, ['bridge', 'settings']); export default withServices(UserMenu, ['frameSync', 'settings']);
...@@ -9,7 +9,7 @@ describe('HypothesisApp', () => { ...@@ -9,7 +9,7 @@ describe('HypothesisApp', () => {
let fakeApplyTheme; let fakeApplyTheme;
let fakeStore = null; let fakeStore = null;
let fakeAuth = null; let fakeAuth = null;
let fakeBridge = null; let fakeFrameSync;
let fakeConfirm; let fakeConfirm;
let fakeServiceConfig = null; let fakeServiceConfig = null;
let fakeSession = null; let fakeSession = null;
...@@ -21,7 +21,7 @@ describe('HypothesisApp', () => { ...@@ -21,7 +21,7 @@ describe('HypothesisApp', () => {
return mount( return mount(
<HypothesisApp <HypothesisApp
auth={fakeAuth} auth={fakeAuth}
bridge={fakeBridge} frameSync={fakeFrameSync}
settings={fakeSettings} settings={fakeSettings}
session={fakeSession} session={fakeSession}
toastMessenger={fakeToastMessenger} toastMessenger={fakeToastMessenger}
...@@ -67,8 +67,8 @@ describe('HypothesisApp', () => { ...@@ -67,8 +67,8 @@ describe('HypothesisApp', () => {
fakeSettings = {}; fakeSettings = {};
fakeBridge = { fakeFrameSync = {
call: sinon.stub(), notifyHost: sinon.stub(),
}; };
fakeToastMessenger = { fakeToastMessenger = {
...@@ -228,7 +228,10 @@ describe('HypothesisApp', () => { ...@@ -228,7 +228,10 @@ describe('HypothesisApp', () => {
it('sends SIGNUP_REQUESTED event', () => { it('sends SIGNUP_REQUESTED event', () => {
const wrapper = createComponent(); const wrapper = createComponent();
clickSignUp(wrapper); clickSignUp(wrapper);
assert.calledWith(fakeBridge.call, bridgeEvents.SIGNUP_REQUESTED); assert.calledWith(
fakeFrameSync.notifyHost,
bridgeEvents.SIGNUP_REQUESTED
);
}); });
it('does not open a URL directly', () => { it('does not open a URL directly', () => {
...@@ -299,9 +302,9 @@ describe('HypothesisApp', () => { ...@@ -299,9 +302,9 @@ describe('HypothesisApp', () => {
const wrapper = createComponent(); const wrapper = createComponent();
await clickLogIn(wrapper); await clickLogIn(wrapper);
assert.equal(fakeBridge.call.callCount, 1); assert.equal(fakeFrameSync.notifyHost.callCount, 1);
assert.isTrue( assert.isTrue(
fakeBridge.call.calledWithExactly(bridgeEvents.LOGIN_REQUESTED) fakeFrameSync.notifyHost.calledWithExactly(bridgeEvents.LOGIN_REQUESTED)
); );
}); });
}); });
...@@ -408,9 +411,9 @@ describe('HypothesisApp', () => { ...@@ -408,9 +411,9 @@ describe('HypothesisApp', () => {
const wrapper = createComponent(); const wrapper = createComponent();
await clickLogOut(wrapper); await clickLogOut(wrapper);
assert.calledOnce(fakeBridge.call); assert.calledOnce(fakeFrameSync.notifyHost);
assert.calledWithExactly( assert.calledWithExactly(
fakeBridge.call, fakeFrameSync.notifyHost,
bridgeEvents.LOGOUT_REQUESTED bridgeEvents.LOGOUT_REQUESTED
); );
}); });
...@@ -422,7 +425,7 @@ describe('HypothesisApp', () => { ...@@ -422,7 +425,7 @@ describe('HypothesisApp', () => {
const wrapper = createComponent(); const wrapper = createComponent();
await clickLogOut(wrapper); await clickLogOut(wrapper);
assert.notCalled(fakeBridge.call); assert.notCalled(fakeFrameSync.notifyHost);
}); });
it('does not call session.logout()', async () => { it('does not call session.logout()', async () => {
......
...@@ -9,7 +9,7 @@ import mockImportedComponents from '../../../test-util/mock-imported-components' ...@@ -9,7 +9,7 @@ import mockImportedComponents from '../../../test-util/mock-imported-components'
describe('TopBar', () => { describe('TopBar', () => {
const fakeSettings = {}; const fakeSettings = {};
let fakeBridge; let fakeFrameSync;
let fakeStore; let fakeStore;
let fakeStreamer; let fakeStreamer;
let fakeIsThirdPartyService; let fakeIsThirdPartyService;
...@@ -26,8 +26,8 @@ describe('TopBar', () => { ...@@ -26,8 +26,8 @@ describe('TopBar', () => {
toggleSidebarPanel: sinon.stub(), toggleSidebarPanel: sinon.stub(),
}; };
fakeBridge = { fakeFrameSync = {
call: sinon.stub(), notifyHost: sinon.stub(),
}; };
fakeServiceConfig = sinon.stub().returns({}); fakeServiceConfig = sinon.stub().returns({});
...@@ -60,7 +60,7 @@ describe('TopBar', () => { ...@@ -60,7 +60,7 @@ describe('TopBar', () => {
return mount( return mount(
<TopBar <TopBar
auth={auth} auth={auth}
bridge={fakeBridge} frameSync={fakeFrameSync}
isSidebar={true} isSidebar={true}
settings={fakeSettings} settings={fakeSettings}
streamer={fakeStreamer} streamer={fakeStreamer}
...@@ -123,7 +123,10 @@ describe('TopBar', () => { ...@@ -123,7 +123,10 @@ describe('TopBar', () => {
helpButton.props().onClick(); helpButton.props().onClick();
assert.equal(fakeStore.toggleSidebarPanel.callCount, 0); assert.equal(fakeStore.toggleSidebarPanel.callCount, 0);
assert.calledWith(fakeBridge.call, bridgeEvents.HELP_REQUESTED); assert.calledWith(
fakeFrameSync.notifyHost,
bridgeEvents.HELP_REQUESTED
);
}); });
}); });
}); });
......
...@@ -9,7 +9,7 @@ import mockImportedComponents from '../../../test-util/mock-imported-components' ...@@ -9,7 +9,7 @@ import mockImportedComponents from '../../../test-util/mock-imported-components'
describe('UserMenu', () => { describe('UserMenu', () => {
let fakeAuth; let fakeAuth;
let fakeBridge; let fakeFrameSync;
let fakeIsThirdPartyUser; let fakeIsThirdPartyUser;
let fakeOnLogout; let fakeOnLogout;
let fakeServiceConfig; let fakeServiceConfig;
...@@ -20,7 +20,7 @@ describe('UserMenu', () => { ...@@ -20,7 +20,7 @@ describe('UserMenu', () => {
return mount( return mount(
<UserMenu <UserMenu
auth={fakeAuth} auth={fakeAuth}
bridge={fakeBridge} frameSync={fakeFrameSync}
onLogout={fakeOnLogout} onLogout={fakeOnLogout}
settings={fakeSettings} settings={fakeSettings}
/> />
...@@ -40,7 +40,7 @@ describe('UserMenu', () => { ...@@ -40,7 +40,7 @@ describe('UserMenu', () => {
userid: 'acct:eleanorFishtail@hypothes.is', userid: 'acct:eleanorFishtail@hypothes.is',
username: 'eleanorFishy', username: 'eleanorFishy',
}; };
fakeBridge = { call: sinon.stub() }; fakeFrameSync = { notifyHost: sinon.stub() };
fakeIsThirdPartyUser = sinon.stub(); fakeIsThirdPartyUser = sinon.stub();
fakeOnLogout = sinon.stub(); fakeOnLogout = sinon.stub();
fakeServiceConfig = sinon.stub(); fakeServiceConfig = sinon.stub();
...@@ -147,8 +147,11 @@ describe('UserMenu', () => { ...@@ -147,8 +147,11 @@ describe('UserMenu', () => {
onProfileSelected(); onProfileSelected();
assert.equal(fakeBridge.call.callCount, 1); assert.equal(fakeFrameSync.notifyHost.callCount, 1);
assert.calledWith(fakeBridge.call, bridgeEvents.PROFILE_REQUESTED); assert.calledWith(
fakeFrameSync.notifyHost,
bridgeEvents.PROFILE_REQUESTED
);
}); });
it('should not fire profile event for first-party user', () => { it('should not fire profile event for first-party user', () => {
...@@ -159,7 +162,7 @@ describe('UserMenu', () => { ...@@ -159,7 +162,7 @@ describe('UserMenu', () => {
onProfileSelected(); onProfileSelected();
assert.equal(fakeBridge.call.callCount, 0); assert.equal(fakeFrameSync.notifyHost.callCount, 0);
}); });
}); });
}); });
...@@ -201,8 +204,8 @@ describe('UserMenu', () => { ...@@ -201,8 +204,8 @@ describe('UserMenu', () => {
const openNotebookItem = findMenuItem(wrapper, 'Open notebook'); const openNotebookItem = findMenuItem(wrapper, 'Open notebook');
openNotebookItem.props().onClick(); openNotebookItem.props().onClick();
assert.calledOnce(fakeBridge.call); assert.calledOnce(fakeFrameSync.notifyHost);
assert.calledWith(fakeBridge.call, 'openNotebook', 'mygroup'); assert.calledWith(fakeFrameSync.notifyHost, 'openNotebook', 'mygroup');
}); });
it('opens the notebook and closes itself when `n` is typed', () => { it('opens the notebook and closes itself when `n` is typed', () => {
...@@ -215,8 +218,8 @@ describe('UserMenu', () => { ...@@ -215,8 +218,8 @@ describe('UserMenu', () => {
assert.isTrue(wrapper.find('Menu').props().open); assert.isTrue(wrapper.find('Menu').props().open);
wrapper.find('.UserMenu').simulate('keydown', { key: 'n' }); wrapper.find('.UserMenu').simulate('keydown', { key: 'n' });
assert.calledOnce(fakeBridge.call); assert.calledOnce(fakeFrameSync.notifyHost);
assert.calledWith(fakeBridge.call, 'openNotebook', 'mygroup'); assert.calledWith(fakeFrameSync.notifyHost, 'openNotebook', 'mygroup');
// Now the menu is "closed" again // Now the menu is "closed" again
assert.isFalse(wrapper.find('Menu').props().open); assert.isFalse(wrapper.find('Menu').props().open);
}); });
......
...@@ -108,8 +108,6 @@ import LaunchErrorPanel from './components/LaunchErrorPanel'; ...@@ -108,8 +108,6 @@ import LaunchErrorPanel from './components/LaunchErrorPanel';
import { ServiceContext } from './service-context'; import { ServiceContext } from './service-context';
// Services. // Services.
import { Bridge } from '../shared/bridge';
import { AnnotationsService } from './services/annotations'; import { AnnotationsService } from './services/annotations';
import { APIService } from './services/api'; import { APIService } from './services/api';
import { APIRoutesService } from './services/api-routes'; import { APIRoutesService } from './services/api-routes';
...@@ -152,7 +150,6 @@ function startApp(config, appEl) { ...@@ -152,7 +150,6 @@ function startApp(config, appEl) {
.register('apiRoutes', APIRoutesService) .register('apiRoutes', APIRoutesService)
.register('auth', AuthService) .register('auth', AuthService)
.register('autosaveService', AutosaveService) .register('autosaveService', AutosaveService)
.register('bridge', Bridge)
.register('features', FeaturesService) .register('features', FeaturesService)
.register('frameSync', FrameSyncService) .register('frameSync', FrameSyncService)
.register('groups', GroupsService) .register('groups', GroupsService)
......
...@@ -15,18 +15,18 @@ import { watch } from '../util/watch'; ...@@ -15,18 +15,18 @@ import { watch } from '../util/watch';
*/ */
export class FeaturesService { export class FeaturesService {
/** /**
* @param {import('../../shared/bridge').Bridge} bridge * @param {import('../services/frame-sync').FrameSyncService} frameSync
* @param {import('../store').SidebarStore} store * @param {import('../store').SidebarStore} store
*/ */
constructor(bridge, store) { constructor(frameSync, store) {
this._bridge = bridge; this._frameSync = frameSync;
this._store = store; this._store = store;
} }
init() { init() {
const currentFlags = () => this._store.profile().features; const currentFlags = () => this._store.profile().features;
const sendFeatureFlags = () => { const sendFeatureFlags = () => {
this._bridge.call( this._frameSync.notifyHost(
bridgeEvents.FEATURE_FLAGS_UPDATED, bridgeEvents.FEATURE_FLAGS_UPDATED,
currentFlags() || {} currentFlags() || {}
); );
......
import debounce from 'lodash.debounce'; import debounce from 'lodash.debounce';
import bridgeEvents from '../../shared/bridge-events'; import bridgeEvents from '../../shared/bridge-events';
import { Bridge } from '../../shared/bridge';
import { isReply, isPublic } from '../helpers/annotation-metadata'; import { isReply, isPublic } from '../helpers/annotation-metadata';
import { watch } from '../util/watch'; import { watch } from '../util/watch';
/**
* @typedef {import('../../shared/port-rpc').PortRPC} PortRPC
*/
/** /**
* Return a minimal representation of an annotation that can be sent from the * Return a minimal representation of an annotation that can be sent from the
* sidebar app to a connected frame. * sidebar app to a connected frame.
...@@ -47,11 +52,15 @@ export class FrameSyncService { ...@@ -47,11 +52,15 @@ export class FrameSyncService {
/** /**
* @param {Window} $window - Test seam * @param {Window} $window - Test seam
* @param {import('./annotations').AnnotationsService} annotationsService * @param {import('./annotations').AnnotationsService} annotationsService
* @param {import('../../shared/bridge').Bridge} bridge
* @param {import('../store').SidebarStore} store * @param {import('../store').SidebarStore} store
*/ */
constructor($window, annotationsService, bridge, store) { constructor($window, annotationsService, store) {
this._bridge = bridge; /** Channel for sidebar <-> host communication. */
this._hostRPC = new Bridge();
/** Channel for sidebar <-> guest(s) communication. */
this._guestRPC = new Bridge();
this._store = store; this._store = store;
this._window = $window; this._window = $window;
...@@ -60,9 +69,9 @@ export class FrameSyncService { ...@@ -60,9 +69,9 @@ export class FrameSyncService {
/** /**
* Watch for changes to the set of annotations displayed in the sidebar and * Watch for changes to the set of annotations displayed in the sidebar and
* notify connected frames about new/updated/deleted annotations. * notify connected guests about new/updated/deleted annotations.
*/ */
this._setupSyncToFrame = () => { this._setupSyncToGuests = () => {
let prevPublicAnns = 0; let prevPublicAnns = 0;
watch( watch(
...@@ -96,20 +105,20 @@ export class FrameSyncService { ...@@ -96,20 +105,20 @@ export class FrameSyncService {
// when they are added or removed in the sidebar, but not re-anchoring // when they are added or removed in the sidebar, but not re-anchoring
// annotations if their selectors are updated. // annotations if their selectors are updated.
if (added.length > 0) { if (added.length > 0) {
bridge.call('loadAnnotations', added.map(formatAnnot)); this._guestRPC.call('loadAnnotations', added.map(formatAnnot));
added.forEach(annot => { added.forEach(annot => {
inFrame.add(annot.$tag); inFrame.add(annot.$tag);
}); });
} }
deleted.forEach(annot => { deleted.forEach(annot => {
bridge.call('deleteAnnotation', formatAnnot(annot)); this._guestRPC.call('deleteAnnotation', formatAnnot(annot));
inFrame.delete(annot.$tag); inFrame.delete(annot.$tag);
}); });
if (frames.length > 0) { if (frames.length > 0) {
if (frames.every(frame => frame.isAnnotationFetchComplete)) { if (frames.every(frame => frame.isAnnotationFetchComplete)) {
if (publicAnns === 0 || publicAnns !== prevPublicAnns) { if (publicAnns === 0 || publicAnns !== prevPublicAnns) {
bridge.call( this._hostRPC.call(
bridgeEvents.PUBLIC_ANNOTATION_COUNT_CHANGED, bridgeEvents.PUBLIC_ANNOTATION_COUNT_CHANGED,
publicAnns publicAnns
); );
...@@ -131,21 +140,21 @@ export class FrameSyncService { ...@@ -131,21 +140,21 @@ export class FrameSyncService {
}; };
/** /**
* Listen for messages coming in from connected frames and add new annotations * Listen for messages coming in from connected guest frames and add new annotations
* to the sidebar. * to the sidebar.
*/ */
this._setupSyncFromFrame = () => { this._setupSyncFromGuests = () => {
// A new annotation, note or highlight was created in the frame // A new annotation, note or highlight was created in the frame
bridge.on('beforeCreateAnnotation', event => { this._guestRPC.on('beforeCreateAnnotation', event => {
const annot = Object.assign({}, event.msg, { $tag: event.tag }); const annot = Object.assign({}, event.msg, { $tag: event.tag });
// If user is not logged in, we can't really create a meaningful highlight // If user is not logged in, we can't really create a meaningful highlight
// or annotation. Instead, we need to open the sidebar, show an error, // or annotation. Instead, we need to open the sidebar, show an error,
// and delete the (unsaved) annotation so it gets un-selected in the // and delete the (unsaved) annotation so it gets un-selected in the
// target document // target document
if (!store.isLoggedIn()) { if (!store.isLoggedIn()) {
bridge.call('openSidebar'); this._hostRPC.call('openSidebar');
store.openSidebarPanel('loginPrompt'); store.openSidebarPanel('loginPrompt');
bridge.call('deleteAnnotation', formatAnnot(annot)); this._guestRPC.call('deleteAnnotation', formatAnnot(annot));
return; return;
} }
inFrame.add(event.tag); inFrame.add(event.tag);
...@@ -154,7 +163,10 @@ export class FrameSyncService { ...@@ -154,7 +163,10 @@ export class FrameSyncService {
annotationsService.create(annot); annotationsService.create(annot);
}); });
bridge.on('destroyFrame', frameIdentifier => // The `destroyFrame` message currently comes from the guests, but we'll
// likely need to route it via the host <-> sidebar channel to work around
// a Safari bug (https://bugs.webkit.org/show_bug.cgi?id=231167).
this._guestRPC.on('destroyFrame', frameIdentifier =>
destroyFrame(frameIdentifier) destroyFrame(frameIdentifier)
); );
...@@ -172,7 +184,7 @@ export class FrameSyncService { ...@@ -172,7 +184,7 @@ export class FrameSyncService {
}, 10); }, 10);
// Anchoring an annotation in the frame completed // Anchoring an annotation in the frame completed
bridge.on('sync', events_ => { this._guestRPC.on('sync', events_ => {
events_.forEach(event => { events_.forEach(event => {
inFrame.add(event.tag); inFrame.add(event.tag);
anchoringStatusUpdates[event.tag] = event.msg.$orphan anchoringStatusUpdates[event.tag] = event.msg.$orphan
...@@ -182,44 +194,38 @@ export class FrameSyncService { ...@@ -182,44 +194,38 @@ export class FrameSyncService {
}); });
}); });
bridge.on('showAnnotations', tags => { this._guestRPC.on('showAnnotations', tags => {
store.selectAnnotations(store.findIDsForTags(tags)); store.selectAnnotations(store.findIDsForTags(tags));
store.selectTab('annotation'); store.selectTab('annotation');
}); });
bridge.on('focusAnnotations', tags => { this._guestRPC.on('focusAnnotations', tags => {
store.focusAnnotations(tags || []); store.focusAnnotations(tags || []);
}); });
bridge.on('toggleAnnotationSelection', tags => { this._guestRPC.on('toggleAnnotationSelection', tags => {
store.toggleSelectedAnnotations(store.findIDsForTags(tags)); store.toggleSelectedAnnotations(store.findIDsForTags(tags));
}); });
bridge.on('sidebarOpened', () => { this._guestRPC.on('openSidebar', () => {
store.setSidebarOpened(true); this._hostRPC.call('openSidebar');
}); });
// These invoke the matching methods by name on the Guests this._guestRPC.on('closeSidebar', () => {
bridge.on('openSidebar', () => { this._hostRPC.call('closeSidebar');
bridge.call('openSidebar');
});
bridge.on('closeSidebar', () => {
bridge.call('closeSidebar');
});
bridge.on('setVisibleHighlights', state => {
bridge.call('setVisibleHighlights', state);
}); });
}; };
} }
/** /**
* Find and connect to Hypothesis clients in the current window. * Connect to the host frame and guest frame(s) in the current browser tab.
*/ */
connect() { connect() {
/** /**
* Query the Hypothesis annotation client in a frame for the URL and metadata * Query the guest in a frame for the URL and metadata of the document that
* of the document that is currently loaded and add the result to the set of * is currently loaded and add the result to the set of connected frames.
* connected frames. *
* @param {PortRPC} channel
*/ */
const addFrame = channel => { const addFrame = channel => {
channel.call('getDocumentInfo', (err, info) => { channel.call('getDocumentInfo', (err, info) => {
...@@ -236,13 +242,12 @@ export class FrameSyncService { ...@@ -236,13 +242,12 @@ export class FrameSyncService {
}); });
}; };
this._bridge.onConnect(addFrame); this._guestRPC.onConnect(addFrame);
// Listen for messages from new guest frames that want to connect. // Listen for messages from new guest frames that want to connect.
// //
// The message will include a `MessagePort` to use for communication with // The message will include a `MessagePort` to use for communication with
// the guest. Communication with the host currently relies on the host // the guest.
// frame also always being a guest frame.
this._window.addEventListener('message', e => { this._window.addEventListener('message', e => {
if (e.data?.type !== 'hypothesisGuestReady') { if (e.data?.type !== 'hypothesisGuestReady') {
return; return;
...@@ -254,14 +259,40 @@ export class FrameSyncService { ...@@ -254,14 +259,40 @@ export class FrameSyncService {
return; return;
} }
const port = e.ports[0]; const port = e.ports[0];
this._bridge.createChannel(port); this._guestRPC.createChannel(port);
});
this._setupSyncToGuests();
this._setupSyncFromGuests();
this._hostRPC.on('sidebarOpened', () => {
this._store.setSidebarOpened(true);
}); });
// Notify host frame that it is ready for guests to connect to it. // When user toggles the highlight visibility control in the sidebar container,
this._window.parent.postMessage({ type: 'hypothesisSidebarReady' }, '*'); // update the visibility in all the guest frames.
this._hostRPC.on('setVisibleHighlights', state => {
this._guestRPC.call('setVisibleHighlights', state);
});
this._setupSyncToFrame(); // Create channel for sidebar <-> host communication and send port to host.
this._setupSyncFromFrame(); //
// This also serves to notify the host that the sidebar application is ready.
const hostChannel = new MessageChannel();
this._hostRPC.createChannel(hostChannel.port1);
this._window.parent.postMessage({ type: 'hypothesisSidebarReady' }, '*', [
hostChannel.port2,
]);
}
/**
* Send an RPC message to the host frame.
*
* @param {string} method
* @param {any[]} args
*/
notifyHost(method, ...args) {
this._hostRPC.call(method, ...args);
} }
/** /**
...@@ -274,7 +305,7 @@ export class FrameSyncService { ...@@ -274,7 +305,7 @@ export class FrameSyncService {
*/ */
focusAnnotations(tags) { focusAnnotations(tags) {
this._store.focusAnnotations(tags); this._store.focusAnnotations(tags);
this._bridge.call('focusAnnotations', tags); this._guestRPC.call('focusAnnotations', tags);
} }
/** /**
...@@ -283,6 +314,6 @@ export class FrameSyncService { ...@@ -283,6 +314,6 @@ export class FrameSyncService {
* @param {string} tag * @param {string} tag
*/ */
scrollToAnnotation(tag) { scrollToAnnotation(tag) {
this._bridge.call('scrollToAnnotation', tag); this._guestRPC.call('scrollToAnnotation', tag);
} }
} }
...@@ -2,12 +2,12 @@ import bridgeEvents from '../../../shared/bridge-events'; ...@@ -2,12 +2,12 @@ import bridgeEvents from '../../../shared/bridge-events';
import { FeaturesService } from '../features'; import { FeaturesService } from '../features';
describe('FeaturesService', () => { describe('FeaturesService', () => {
let fakeBridge; let fakeFrameSync;
let fakeStore; let fakeStore;
beforeEach(() => { beforeEach(() => {
fakeBridge = { fakeFrameSync = {
call: sinon.stub(), notifyHost: sinon.stub(),
}; };
fakeStore = { fakeStore = {
...@@ -23,7 +23,7 @@ describe('FeaturesService', () => { ...@@ -23,7 +23,7 @@ describe('FeaturesService', () => {
}); });
function createService() { function createService() {
const service = new FeaturesService(fakeBridge, fakeStore); const service = new FeaturesService(fakeFrameSync, fakeStore);
service.init(); service.init();
return service; return service;
} }
...@@ -38,7 +38,7 @@ describe('FeaturesService', () => { ...@@ -38,7 +38,7 @@ describe('FeaturesService', () => {
// First update, with no changes to feature flags. // First update, with no changes to feature flags.
notifyStoreSubscribers(); notifyStoreSubscribers();
assert.notCalled(fakeBridge.call); assert.notCalled(fakeFrameSync.notifyHost);
// Second update, with changes to feature flags. // Second update, with changes to feature flags.
fakeStore.profile.returns({ fakeStore.profile.returns({
...@@ -51,7 +51,7 @@ describe('FeaturesService', () => { ...@@ -51,7 +51,7 @@ describe('FeaturesService', () => {
notifyStoreSubscribers(); notifyStoreSubscribers();
assert.calledWith( assert.calledWith(
fakeBridge.call, fakeFrameSync.notifyHost,
bridgeEvents.FEATURE_FLAGS_UPDATED, bridgeEvents.FEATURE_FLAGS_UPDATED,
fakeStore.profile().features fakeStore.profile().features
); );
...@@ -62,7 +62,7 @@ describe('FeaturesService', () => { ...@@ -62,7 +62,7 @@ describe('FeaturesService', () => {
// First update, with no changes to frames. // First update, with no changes to frames.
notifyStoreSubscribers(); notifyStoreSubscribers();
assert.notCalled(fakeBridge.call); assert.notCalled(fakeFrameSync.notifyHost);
// Second update, with changes to frames. // Second update, with changes to frames.
fakeStore.frames.returns([{ uri: 'https://example.com' }]); fakeStore.frames.returns([{ uri: 'https://example.com' }]);
...@@ -70,7 +70,7 @@ describe('FeaturesService', () => { ...@@ -70,7 +70,7 @@ describe('FeaturesService', () => {
notifyStoreSubscribers(); notifyStoreSubscribers();
assert.calledWith( assert.calledWith(
fakeBridge.call, fakeFrameSync.notifyHost,
bridgeEvents.FEATURE_FLAGS_UPDATED, bridgeEvents.FEATURE_FLAGS_UPDATED,
fakeStore.profile().features fakeStore.profile().features
); );
......
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