From b66f452271345c1bf3ea190ca95b5e4dd0bb377a Mon Sep 17 00:00:00 2001 From: Michel Weststrate Date: Thu, 12 Nov 2020 04:13:16 -0800 Subject: [PATCH] Keep Navigation plugin alive even when disabled Summary: Navigation plugin is a special cause that will remain connected and process messages directly even when disabled, this is to make sure the bookmarks feature keeps working even when the plugin is not enabled. Changelog: [Sandy][Navigation] on Android, the currently active deeplink of the application will now be shown in the sidebar Reviewed By: jknoxville Differential Revision: D24890375 fbshipit-source-id: eb5e4141740e0436396cea5a7aae24337f2e903e --- desktop/app/src/Client.tsx | 9 ++- .../reducers/__tests__/sandyplugins.node.tsx | 64 +++++++++++++++++++ .../__tests__/messageQueueSandy.node.tsx | 50 +++++++++++++++ desktop/app/src/utils/messageQueue.tsx | 3 +- 4 files changed, 122 insertions(+), 4 deletions(-) diff --git a/desktop/app/src/Client.tsx b/desktop/app/src/Client.tsx index f90808d01..e3577c581 100644 --- a/desktop/app/src/Client.tsx +++ b/desktop/app/src/Client.tsx @@ -295,7 +295,7 @@ export default class Client extends EventEmitter { // start a plugin on start if it is a SandyPlugin, which is starred, and doesn't have persisted state yet if ( isSandyPlugin(plugin) && - isEnabled && + (isEnabled || defaultEnabledBackgroundPlugins.includes(plugin.id)) && !this.sandyPluginStates.has(plugin.id) ) { // TODO: needs to be wrapped in error tracking T68955280 @@ -306,7 +306,10 @@ export default class Client extends EventEmitter { } } - stopPluginIfNeeded(pluginId: string) { + stopPluginIfNeeded(pluginId: string, force = false) { + if (defaultEnabledBackgroundPlugins.includes(pluginId) && !force) { + return; + } const pluginKey = getPluginKey( this.id, {serial: this.query.device_id}, @@ -322,7 +325,7 @@ export default class Client extends EventEmitter { close() { this.emit('close'); - this.plugins.forEach((pluginId) => this.stopPluginIfNeeded(pluginId)); + this.plugins.forEach((pluginId) => this.stopPluginIfNeeded(pluginId, true)); } // gets a plugin by pluginId diff --git a/desktop/app/src/reducers/__tests__/sandyplugins.node.tsx b/desktop/app/src/reducers/__tests__/sandyplugins.node.tsx index 868872098..efcdf0a51 100644 --- a/desktop/app/src/reducers/__tests__/sandyplugins.node.tsx +++ b/desktop/app/src/reducers/__tests__/sandyplugins.node.tsx @@ -30,10 +30,14 @@ function plugin(client: PluginClient) { const connectStub = jest.fn(); const disconnectStub = jest.fn(); const destroyStub = jest.fn(); + const messages: any[] = []; client.onConnect(connectStub); client.onDisconnect(disconnectStub); client.onDestroy(destroyStub); + client.onMessage('message', (msg) => { + messages.push(msg); + }); initialized = true; @@ -42,6 +46,7 @@ function plugin(client: PluginClient) { disconnectStub, destroyStub, send: client.send, + messages, }; } const TestPlugin = new SandyPluginDefinition(pluginDetails, { @@ -229,3 +234,62 @@ test('it can send messages from sandy clients', async () => { } `); }); + +test('it should initialize "Navigation" plugin if not enabled', async () => { + const {client, store} = await createMockFlipperWithPlugin(TestPlugin); + + const Plugin2 = new SandyPluginDefinition( + TestUtils.createMockPluginDetails({ + name: 'Plugin2', + id: 'Navigation', + }), + { + plugin: jest.fn().mockImplementation(plugin), + Component() { + return null; + }, + }, + ); + + const pluginState1 = client.sandyPluginStates.get(TestPlugin.id); + expect(pluginState1).toBeInstanceOf(SandyPluginInstance); + store.dispatch(registerPlugins([Plugin2])); + await client.refreshPlugins(); + // not enabled, but Navigation is an exception, so we still get an instance + const origInstance = client.sandyPluginStates.get(Plugin2.id); + expect(origInstance).toBeDefined(); + expect(Plugin2.asPluginModule().plugin).toBeCalledTimes(1); + + store.dispatch( + starPlugin({ + plugin: Plugin2, + selectedApp: client.query.app, + }), + ); + + expect(client.sandyPluginStates.get(Plugin2.id)).toBe(origInstance); + const instance = client.sandyPluginStates.get(Plugin2.id)! + .instanceApi as PluginApi; + expect(Plugin2.asPluginModule().plugin).toBeCalledTimes(1); + expect(instance.destroyStub).toHaveBeenCalledTimes(0); + + // disable plugin again + store.dispatch( + starPlugin({ + plugin: Plugin2, + selectedApp: client.query.app, + }), + ); + + // stil enabled + expect(client.sandyPluginStates.get(Plugin2.id)).toBe(origInstance); + expect(instance.connectStub).toHaveBeenCalledTimes(0); + // disconnect wasn't called because connect was never called + expect(instance.disconnectStub).toHaveBeenCalledTimes(0); + expect(instance.destroyStub).toHaveBeenCalledTimes(0); + + // closing does stop the plugin! + client.close(); + expect(instance.destroyStub).toHaveBeenCalledTimes(1); + expect(client.sandyPluginStates.get(Plugin2.id)).toBeUndefined(); +}); diff --git a/desktop/app/src/utils/__tests__/messageQueueSandy.node.tsx b/desktop/app/src/utils/__tests__/messageQueueSandy.node.tsx index 9aa5a712d..68f6cd444 100644 --- a/desktop/app/src/utils/__tests__/messageQueueSandy.node.tsx +++ b/desktop/app/src/utils/__tests__/messageQueueSandy.node.tsx @@ -210,6 +210,56 @@ test('queue - events are NOT processed immediately if plugin is NOT selected (bu }); }); +test('queue - events ARE processed immediately if plugin is NOT selected / enabled BUT NAVIGATION', async () => { + const NavigationPlugin = new SandyPluginDefinition( + TestUtils.createMockPluginDetails({ + id: 'Navigation', + }), + { + plugin, + Component() { + return null; + }, + }, + ); + const {store, client, sendMessage} = await createMockFlipperWithPlugin( + NavigationPlugin, + ); + + // Pre setup, deselect AND disable + selectDeviceLogs(store); + expect(store.getState().connections.selectedPlugin).toBe('DeviceLogs'); + store.dispatch( + starPlugin({ + plugin: NavigationPlugin, + selectedApp: client.query.app, + }), + ); + expect(store.getState().connections.userStarredPlugins) + .toMatchInlineSnapshot(` + Object { + "TestApp": Array [], + } + `); + + // ...mesages are still going to arrive + const pluginState = () => + client.sandyPluginStates.get(NavigationPlugin.id)!.instanceApi.state; + + sendMessage('inc', {}); + sendMessage('inc', {delta: 2}); + sendMessage('inc', {delta: 3}); + // the first message is already visible cause of the leading debounce + expect(pluginState().count).toBe(1); + // message queue was never involved due to the bypass... + expect(store.getState().pluginMessageQueue).toMatchInlineSnapshot( + `Object {}`, + ); + // flush will make the others visible + client.flushMessageBuffer(); + expect(pluginState().count).toBe(6); +}); + test('queue - events are queued for plugins that are favorite when app is not selected', async () => { const { client, diff --git a/desktop/app/src/utils/messageQueue.tsx b/desktop/app/src/utils/messageQueue.tsx index b6d9c9d0a..c28d03b5d 100644 --- a/desktop/app/src/utils/messageQueue.tsx +++ b/desktop/app/src/utils/messageQueue.tsx @@ -124,7 +124,8 @@ export function processMessagesLater( const isSelected = pluginKey === getSelectedPluginKey(store.getState().connections); switch (true) { - case pluginId === 'Navigation': // Navigation events are always processed, to make sure the navbar stays up to date + // Navigation events are always processed immediately, to make sure the navbar stays up to date, see also T69991064 + case pluginId === 'Navigation': case isSelected && getPendingMessages(store, pluginKey).length === 0: processMessagesImmediately(store, pluginKey, plugin, messages); break;