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
This commit is contained in:
Michel Weststrate
2020-03-30 07:57:03 -07:00
committed by Facebook GitHub Bot
parent 8b9c3322f1
commit f803cb3cb1
3 changed files with 16 additions and 37 deletions

View File

@@ -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,8 +322,6 @@ 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
@@ -340,19 +330,13 @@ export default class Client extends EventEmitter {
this.store.getState().plugins.devicePlugins.get(params.api);
if (persistingPlugin && persistingPlugin.persistedStateReducer) {
const pluginKey = getPluginKey(this.id, device, params.api);
const pluginKey = getPluginKey(
this.id,
{serial: this.query.device_id},
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`,
);
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));
}

View File

@@ -87,7 +87,6 @@ export async function createMockFlipperWithPlugin(
);
// yikes
client._deviceSet = device;
client.device = {
then() {
return device;

View File

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