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

@@ -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

desktop/app/src/reducers/__tests__/sandyplugins.node.tsx

@@ -30,10 +30,14 @@ function plugin(client: PluginClient<any, any>) {
   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<any, any>) {
     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();
+});

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,

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;