From f803cb3cb1a2caa9e89b72bc4f879061c2fba644 Mon Sep 17 00:00:00 2001
From: Michel Weststrate <mweststrate@fb.com>
Date: Mon, 30 Mar 2020 07:57:03 -0700
Subject: [PATCH] Avoid dropping plugin messages if the connected device is not
 found

Summary:
Before we dropped all messages if the client connected before the device has been registered. Which happens typically on iOS (it can actually take very long, will investigate that as well, but not in this diff).
However, we don't need to wait for the device to process messages, we just need its serial, which we already know (given that we use that query id to find the matching device in the first place).

I think this will greatly improved perceived stability for iOS

Reviewed By: jknoxville

Differential Revision: D20734892

fbshipit-source-id: f98e8d31558ef606b9a8287e03fc41ab6c3a087d
---
 desktop/app/src/Client.tsx                    | 50 ++++++-------------
 .../createMockFlipperWithPlugin.tsx           |  1 -
 desktop/app/src/utils/pluginUtils.tsx         |  2 +-
 3 files changed, 16 insertions(+), 37 deletions(-)

diff --git a/desktop/app/src/Client.tsx b/desktop/app/src/Client.tsx
index 1b380886d..26be4d3bf 100644
--- a/desktop/app/src/Client.tsx
+++ b/desktop/app/src/Client.tsx
@@ -118,7 +118,6 @@ export default class Client extends EventEmitter {
   activePlugins: Set<string>;
   device: Promise<BaseDevice>;
   _deviceResolve: (device: BaseDevice) => void = (_) => {};
-  _deviceSet: false | BaseDevice = false;
   logger: Logger;
   lastSeenDeviceList: Array<BaseDevice>;
   broadcastCallbacks: Map<string, Map<string, Set<Function>>>;
@@ -162,9 +161,6 @@ export default class Client extends EventEmitter {
       : new Promise((resolve, _reject) => {
           this._deviceResolve = resolve;
         });
-    if (device) {
-      this._deviceSet = device;
-    }
 
     const client = this;
     if (conn) {
@@ -184,11 +180,8 @@ export default class Client extends EventEmitter {
   /* All clients should have a corresponding Device in the store.
      However, clients can connect before a device is registered, so wait a
      while for the device to be registered if it isn't already. */
-  setMatchingDevice(): void {
-    if (this._deviceSet) {
-      return;
-    }
-    reportPlatformFailures(
+  async setMatchingDevice(): Promise<void> {
+    return reportPlatformFailures(
       new Promise<BaseDevice>((resolve, reject) => {
         let unsubscribe: () => void = () => {};
 
@@ -230,7 +223,6 @@ export default class Client extends EventEmitter {
       }),
       'client-setMatchingDevice',
     ).then((device) => {
-      this._deviceSet = device;
       this._deviceResolve(device);
     });
   }
@@ -330,29 +322,21 @@ export default class Client extends EventEmitter {
         invariant(data.params, 'expected params');
         const params: Params = data.params;
 
-        const device = this.getDeviceSync();
-        if (device) {
-          const persistingPlugin:
-            | typeof FlipperPlugin
-            | typeof FlipperDevicePlugin
-            | undefined =
-            this.store.getState().plugins.clientPlugins.get(params.api) ||
-            this.store.getState().plugins.devicePlugins.get(params.api);
+        const persistingPlugin:
+          | typeof FlipperPlugin
+          | typeof FlipperDevicePlugin
+          | undefined =
+          this.store.getState().plugins.clientPlugins.get(params.api) ||
+          this.store.getState().plugins.devicePlugins.get(params.api);
 
-          if (persistingPlugin && persistingPlugin.persistedStateReducer) {
-            const pluginKey = getPluginKey(this.id, device, params.api);
-            flipperRecorderAddEvent(pluginKey, params.method, params.params);
-            processMessageLater(
-              this.store,
-              pluginKey,
-              persistingPlugin,
-              params,
-            );
-          }
-        } else {
-          console.warn(
-            `Received a message for plugin ${params.api}.${params.method}, which will be ignored because the device has not connected yet`,
+        if (persistingPlugin && persistingPlugin.persistedStateReducer) {
+          const pluginKey = getPluginKey(
+            this.id,
+            {serial: this.query.device_id},
+            params.api,
           );
+          flipperRecorderAddEvent(pluginKey, params.method, params.params);
+          processMessageLater(this.store, pluginKey, persistingPlugin, params);
         }
         const apiCallbacks = this.broadcastCallbacks.get(params.api);
         if (!apiCallbacks) {
@@ -493,10 +477,6 @@ export default class Client extends EventEmitter {
     });
   }
 
-  getDeviceSync(): BaseDevice | undefined {
-    return this._deviceSet || undefined;
-  }
-
   startTimingRequestResponse(data: RequestMetadata) {
     performance.mark(this.getPerformanceMark(data));
   }
diff --git a/desktop/app/src/test-utils/createMockFlipperWithPlugin.tsx b/desktop/app/src/test-utils/createMockFlipperWithPlugin.tsx
index 28bbc069e..f46631d84 100644
--- a/desktop/app/src/test-utils/createMockFlipperWithPlugin.tsx
+++ b/desktop/app/src/test-utils/createMockFlipperWithPlugin.tsx
@@ -87,7 +87,6 @@ export async function createMockFlipperWithPlugin(
     );
 
     // yikes
-    client._deviceSet = device;
     client.device = {
       then() {
         return device;
diff --git a/desktop/app/src/utils/pluginUtils.tsx b/desktop/app/src/utils/pluginUtils.tsx
index 78f810663..3704a913a 100644
--- a/desktop/app/src/utils/pluginUtils.tsx
+++ b/desktop/app/src/utils/pluginUtils.tsx
@@ -35,7 +35,7 @@ export function pluginsClassMap(
 
 export function getPluginKey(
   selectedAppId: string | null,
-  baseDevice: BaseDevice | null,
+  baseDevice: {serial: string} | null,
   pluginID: string,
 ): string {
   if (selectedAppId) {