From 43c68c0e7c94259f4991c4fbb210248a91f7a17a Mon Sep 17 00:00:00 2001
From: Michel Weststrate <mweststrate@fb.com>
Date: Tue, 9 Feb 2021 04:12:09 -0800
Subject: [PATCH] Separate the concepts of `archived` and `disconnected`
 devices

Summary:
Minor code cleanup to avoid future confusion:

- archived: a device that was imported from a Flipper trace, and only has persisted state
- (dis)connected: a real stateful device that might or might not have an active connection

Reviewed By: nikoant

Differential Revision: D26275459

fbshipit-source-id: eba554b37c39711e367c3795ff4456329a303c22
---
 desktop/app/src/PluginContainer.tsx           |  2 +-
 desktop/app/src/__tests__/disconnect.node.tsx | 11 +++++--
 desktop/app/src/chrome/MetroButton.tsx        |  6 ++--
 desktop/app/src/devices/ArchivedDevice.tsx    |  4 ++-
 desktop/app/src/devices/BaseDevice.tsx        | 11 +++----
 desktop/app/src/devices/IOSDevice.tsx         |  4 +--
 desktop/app/src/devices/MetroDevice.tsx       | 11 -------
 desktop/app/src/dispatcher/androidDevice.tsx  |  2 +-
 desktop/app/src/dispatcher/iOSDevice.tsx      |  2 +-
 .../reducers/__tests__/connections.node.tsx   | 12 ++-----
 .../sandy-chrome/appinspect/AppInspect.tsx    | 32 +++++++++++++------
 .../sandy-chrome/appinspect/AppSelector.tsx   |  6 ++--
 .../sandy-chrome/appinspect/PluginList.tsx    | 12 +++----
 desktop/app/src/utils/exportData.tsx          |  8 ++---
 desktop/app/src/utils/pluginUtils.tsx         |  2 +-
 desktop/app/src/utils/screenshot.tsx          |  4 +--
 .../src/plugin/DevicePlugin.tsx               |  2 ++
 .../flipper-plugin/src/plugin/PluginBase.tsx  |  3 +-
 .../src/test-utils/test-utils.tsx             |  1 +
 19 files changed, 69 insertions(+), 66 deletions(-)

diff --git a/desktop/app/src/PluginContainer.tsx b/desktop/app/src/PluginContainer.tsx
index 9bf380dbb..8c7502c51 100644
--- a/desktop/app/src/PluginContainer.tsx
+++ b/desktop/app/src/PluginContainer.tsx
@@ -588,7 +588,7 @@ export default connect<StateFromProps, DispatchFromProps, OwnProps, Store>(
     }
     const isArchivedDevice = !selectedDevice
       ? false
-      : selectedDevice instanceof ArchivedDevice;
+      : selectedDevice.isArchived;
     if (isArchivedDevice) {
       pluginIsEnabled = true;
     }
diff --git a/desktop/app/src/__tests__/disconnect.node.tsx b/desktop/app/src/__tests__/disconnect.node.tsx
index 918252342..1a3bc94ff 100644
--- a/desktop/app/src/__tests__/disconnect.node.tsx
+++ b/desktop/app/src/__tests__/disconnect.node.tsx
@@ -50,10 +50,12 @@ test('Devices can disconnect', async () => {
   ).toBe(true);
 
   expect(device.isArchived).toBe(false);
+  expect(device.connected.get()).toBe(true);
 
   device.disconnect();
 
-  expect(device.isArchived).toBe(true);
+  expect(device.isArchived).toBe(false);
+  expect(device.connected.get()).toBe(false);
   const instance = device.sandyPluginStates.get(deviceplugin.id)!;
   expect(instance.instanceApi.isConnected).toBe(false);
   expect(instance).toBeTruthy();
@@ -61,7 +63,8 @@ test('Devices can disconnect', async () => {
   expect(instance.instanceApi.destroy).toBeCalledTimes(0);
 
   device.destroy();
-  expect(device.isArchived).toBe(true);
+  expect(device.isArchived).toBe(false);
+  expect(device.connected.get()).toBe(false);
   expect(instance.instanceApi.destroy).toBeCalledTimes(1);
 
   expect(device.sandyPluginStates.get(deviceplugin.id)).toBeUndefined();
@@ -91,6 +94,7 @@ test('New device with same serial removes & cleans the old one', async () => {
   const instance = device.sandyPluginStates.get(deviceplugin.id)!;
 
   expect(device.isArchived).toBe(false);
+  expect(device.connected.get()).toBe(true);
   expect(instance.instanceApi.destroy).toBeCalledTimes(0);
   expect(store.getState().connections.devices).toEqual([device]);
 
@@ -107,7 +111,8 @@ test('New device with same serial removes & cleans the old one', async () => {
   });
   device2.loadDevicePlugins(store.getState().plugins.devicePlugins);
 
-  expect(device.isArchived).toBe(true);
+  expect(device.isArchived).toBe(false);
+  expect(device.connected.get()).toBe(false);
   expect(instance.instanceApi.destroy).toBeCalledTimes(1);
   expect(
     device2.sandyPluginStates.get(deviceplugin.id)!.instanceApi.destroy,
diff --git a/desktop/app/src/chrome/MetroButton.tsx b/desktop/app/src/chrome/MetroButton.tsx
index d54f44c01..cfa1fdfea 100644
--- a/desktop/app/src/chrome/MetroButton.tsx
+++ b/desktop/app/src/chrome/MetroButton.tsx
@@ -12,13 +12,12 @@ import MetroDevice, {MetroReportableEvent} from '../devices/MetroDevice';
 import {useStore} from '../utils/useStore';
 import {Button as AntButton} from 'antd';
 import {MenuOutlined, ReloadOutlined} from '@ant-design/icons';
-
-type LogEntry = {};
+import {theme} from 'flipper-plugin';
 
 export default function MetroButton() {
   const device = useStore((state) =>
     state.connections.devices.find(
-      (device) => device.os === 'Metro' && !device.isArchived,
+      (device) => device.os === 'Metro' && device.connected.get(),
     ),
   ) as MetroDevice | undefined;
 
@@ -69,6 +68,7 @@ export default function MetroButton() {
           sendCommand('reload');
         }}
         loading={progress < 1}
+        style={{color: _hasBuildError ? theme.errorColor : undefined}}
       />
       <AntButton
         icon={<MenuOutlined />}
diff --git a/desktop/app/src/devices/ArchivedDevice.tsx b/desktop/app/src/devices/ArchivedDevice.tsx
index ef1e13fba..f980eefb9 100644
--- a/desktop/app/src/devices/ArchivedDevice.tsx
+++ b/desktop/app/src/devices/ArchivedDevice.tsx
@@ -13,6 +13,8 @@ import {OS, DeviceShell} from './BaseDevice';
 import {SupportFormRequestDetailsState} from '../reducers/supportForm';
 
 export default class ArchivedDevice extends BaseDevice {
+  isArchived = true;
+
   constructor(options: {
     serial: string;
     deviceType: DeviceType;
@@ -23,7 +25,7 @@ export default class ArchivedDevice extends BaseDevice {
     supportRequestDetails?: SupportFormRequestDetailsState;
   }) {
     super(options.serial, options.deviceType, options.title, options.os);
-    this.archivedState.set(true);
+    this.connected.set(false);
     this.source = options.source || '';
     this.supportRequestDetails = options.supportRequestDetails;
     this.archivedScreenshotHandle = options.screenshotHandle;
diff --git a/desktop/app/src/devices/BaseDevice.tsx b/desktop/app/src/devices/BaseDevice.tsx
index 2803d67d5..b55ec88b3 100644
--- a/desktop/app/src/devices/BaseDevice.tsx
+++ b/desktop/app/src/devices/BaseDevice.tsx
@@ -38,6 +38,8 @@ export type DeviceExport = {
 };
 
 export default class BaseDevice {
+  isArchived = false;
+
   constructor(
     serial: string,
     deviceType: DeviceType,
@@ -72,10 +74,7 @@ export default class BaseDevice {
 
   logListeners: Map<Symbol, DeviceLogListener> = new Map();
 
-  archivedState = createState(false);
-  get isArchived() {
-    return this.archivedState.get();
-  }
+  readonly connected = createState(true);
 
   // if imported, stores the original source location
   source = '';
@@ -93,7 +92,7 @@ export default class BaseDevice {
   }
 
   displayTitle(): string {
-    return this.title;
+    return this.connected.get() ? this.title : `${this.title} (Offline)`;
   }
 
   async exportState(
@@ -226,7 +225,7 @@ export default class BaseDevice {
   }
 
   disconnect() {
-    this.archivedState.set(true);
+    this.connected.set(false);
   }
 
   destroy() {
diff --git a/desktop/app/src/devices/IOSDevice.tsx b/desktop/app/src/devices/IOSDevice.tsx
index 31584f65d..e658de465 100644
--- a/desktop/app/src/devices/IOSDevice.tsx
+++ b/desktop/app/src/devices/IOSDevice.tsx
@@ -53,7 +53,7 @@ export default class IOSDevice extends BaseDevice {
   }
 
   async screenshot(): Promise<Buffer> {
-    if (this.isArchived) {
+    if (!this.connected.get()) {
       return Buffer.from([]);
     }
     const tmpImageName = uuid() + '.png';
@@ -192,7 +192,7 @@ export default class IOSDevice extends BaseDevice {
   }
 
   async screenCaptureAvailable() {
-    return this.deviceType === 'emulator' && !this.isArchived;
+    return this.deviceType === 'emulator' && this.connected.get();
   }
 
   async startScreenCapture(destination: string) {
diff --git a/desktop/app/src/devices/MetroDevice.tsx b/desktop/app/src/devices/MetroDevice.tsx
index 8d0fc8cb2..c9e1ead48 100644
--- a/desktop/app/src/devices/MetroDevice.tsx
+++ b/desktop/app/src/devices/MetroDevice.tsx
@@ -9,7 +9,6 @@
 
 import {LogLevel} from 'flipper-plugin';
 import BaseDevice from './BaseDevice';
-import ArchivedDevice from './ArchivedDevice';
 import {EventEmitter} from 'events';
 
 // From xplat/js/metro/packages/metro/src/lib/reporting.js
@@ -198,14 +197,4 @@ export default class MetroDevice extends BaseDevice {
       console.warn('Cannot send command, no connection', command);
     }
   }
-
-  archive() {
-    return new ArchivedDevice({
-      serial: this.serial,
-      deviceType: this.deviceType,
-      title: this.title,
-      os: this.os,
-      screenshotHandle: null,
-    });
-  }
 }
diff --git a/desktop/app/src/dispatcher/androidDevice.tsx b/desktop/app/src/dispatcher/androidDevice.tsx
index 434bcf8fc..cbe4ac13e 100644
--- a/desktop/app/src/dispatcher/androidDevice.tsx
+++ b/desktop/app/src/dispatcher/androidDevice.tsx
@@ -231,7 +231,7 @@ export default (store: Store, logger: Logger) => {
       .getState()
       .connections.devices.filter(
         (device: BaseDevice) =>
-          device.serial === androidDevice.serial && device.isArchived,
+          device.serial === androidDevice.serial && !device.connected.get(),
       )
       .map((device) => device.serial);
 
diff --git a/desktop/app/src/dispatcher/iOSDevice.tsx b/desktop/app/src/dispatcher/iOSDevice.tsx
index 1fd4cb197..3571b947e 100644
--- a/desktop/app/src/dispatcher/iOSDevice.tsx
+++ b/desktop/app/src/dispatcher/iOSDevice.tsx
@@ -118,7 +118,7 @@ function processDevices(
         (device) =>
           device instanceof IOSDevice &&
           device.deviceType === type &&
-          !device.isArchived,
+          device.connected.get(),
       )
       .map((device) => device.serial),
   );
diff --git a/desktop/app/src/reducers/__tests__/connections.node.tsx b/desktop/app/src/reducers/__tests__/connections.node.tsx
index b82f5ed2e..427994c0f 100644
--- a/desktop/app/src/reducers/__tests__/connections.node.tsx
+++ b/desktop/app/src/reducers/__tests__/connections.node.tsx
@@ -41,17 +41,8 @@ test('register, remove, re-register a metro device works correctly', () => {
   expect(state.devices.length).toBe(1);
   expect(state.devices[0].displayTitle()).toBe('React Native');
 
-  const archived = device1.archive();
-  state = reducer(state, {
-    type: 'UNREGISTER_DEVICES',
-    payload: new Set([device1.serial]),
-  });
-  expect(state.devices.length).toBe(0);
+  device1.disconnect();
 
-  state = reducer(state, {
-    type: 'REGISTER_DEVICE',
-    payload: archived,
-  });
   expect(state.devices.length).toBe(1);
   expect(state.devices[0].displayTitle()).toBe('React Native (Offline)');
 
@@ -61,6 +52,7 @@ test('register, remove, re-register a metro device works correctly', () => {
   });
   expect(state.devices.length).toBe(1);
   expect(state.devices[0].displayTitle()).toBe('React Native');
+  expect(state.devices[0]).not.toBe(device1);
 });
 
 test('triggering REGISTER_DEVICE before REGISTER_PLUGINS still registers device plugins', () => {
diff --git a/desktop/app/src/sandy-chrome/appinspect/AppInspect.tsx b/desktop/app/src/sandy-chrome/appinspect/AppInspect.tsx
index 52007dfee..b98d2a8d4 100644
--- a/desktop/app/src/sandy-chrome/appinspect/AppInspect.tsx
+++ b/desktop/app/src/sandy-chrome/appinspect/AppInspect.tsx
@@ -23,7 +23,6 @@ import Client from '../../Client';
 import {State} from '../../reducers';
 import BaseDevice from '../../devices/BaseDevice';
 import MetroDevice from '../../devices/MetroDevice';
-import ArchivedDevice from '../../devices/ArchivedDevice';
 import {ExclamationCircleOutlined, FieldTimeOutlined} from '@ant-design/icons';
 
 const {Text} = Typography;
@@ -56,7 +55,7 @@ export function AppInspect() {
     metroDevice,
     connections.userPreferredDevice,
   ]);
-  const isDeviceArchived = useValue(activeDevice?.archivedState, false);
+  const isDeviceConnected = useValue(activeDevice?.connected, false);
   const isAppConnected = useValue(client?.connected, false);
 
   return (
@@ -69,13 +68,13 @@ export function AppInspect() {
           <Layout.Container padv="small" padh="medium" gap={theme.space.large}>
             <AppSelector />
             {renderStatusMessage(
-              isDeviceArchived,
+              isDeviceConnected,
               activeDevice,
               client,
               isAppConnected,
             )}
-            {!isDeviceArchived && isAppConnected && <BookmarkSection />}
-            {!isDeviceArchived && activeDevice && (
+            {isDeviceConnected && isAppConnected && <BookmarkSection />}
+            {isDeviceConnected && activeDevice && (
               <Toolbar gap>
                 <MetroButton />
                 <ScreenCaptureButtons />
@@ -145,13 +144,28 @@ export function findBestDevice(
 }
 
 function renderStatusMessage(
-  isDeviceArchived: boolean,
+  isDeviceConnected: boolean,
   activeDevice: BaseDevice | undefined,
   client: Client | undefined,
   isAppConnected: boolean,
 ): React.ReactNode {
-  return isDeviceArchived ? (
-    activeDevice instanceof ArchivedDevice ? (
+  if (!activeDevice) {
+    return (
+      <Layout.Horizontal gap center>
+        <ExclamationCircleOutlined style={{color: theme.warningColor}} />
+        <Text
+          type="secondary"
+          style={{
+            textTransform: 'uppercase',
+            fontSize: '0.8em',
+          }}>
+          Device disconnected
+        </Text>
+      </Layout.Horizontal>
+    );
+  }
+  return !isDeviceConnected ? (
+    activeDevice.isArchived ? (
       <Layout.Horizontal gap center>
         <FieldTimeOutlined style={{color: theme.primaryColor}} />
         <Text
@@ -160,7 +174,7 @@ function renderStatusMessage(
             textTransform: 'uppercase',
             fontSize: '0.8em',
           }}>
-          Device loaded from file
+          No device selected
         </Text>
       </Layout.Horizontal>
     ) : (
diff --git a/desktop/app/src/sandy-chrome/appinspect/AppSelector.tsx b/desktop/app/src/sandy-chrome/appinspect/AppSelector.tsx
index 839fa24d0..099c7235f 100644
--- a/desktop/app/src/sandy-chrome/appinspect/AppSelector.tsx
+++ b/desktop/app/src/sandy-chrome/appinspect/AppSelector.tsx
@@ -57,7 +57,7 @@ export function AppSelector() {
     uninitializedClients,
     selectedApp,
   } = useStore((state) => state.connections);
-  useValue(selectedDevice?.archivedState, false); // subscribe to future archived state changes
+  useValue(selectedDevice?.connected, false); // subscribe to future archived state changes
 
   const onSelectDevice = useTrackedCallback(
     'select-device',
@@ -223,8 +223,8 @@ function computeEntries(
 }
 
 function DeviceTitle({device}: {device: BaseDevice}) {
-  const connected = !useValue(device.archivedState);
-  const isImported = device instanceof ArchivedDevice;
+  const connected = useValue(device.connected);
+  const isImported = device.isArchived;
   return (
     <span>
       <>{device.title} </>
diff --git a/desktop/app/src/sandy-chrome/appinspect/PluginList.tsx b/desktop/app/src/sandy-chrome/appinspect/PluginList.tsx
index 1ae0c5e1f..f9edf59a9 100644
--- a/desktop/app/src/sandy-chrome/appinspect/PluginList.tsx
+++ b/desktop/app/src/sandy-chrome/appinspect/PluginList.tsx
@@ -85,7 +85,7 @@ export const PluginList = memo(function PluginList({
     connections.userStarredPlugins,
     pluginsChanged,
   ]);
-  const isArchived = useValue(activeDevice?.archivedState, false);
+  const isConnected = useValue(activeDevice?.connected, false);
 
   const annotatedDownloadablePlugins = useMemoize<
     [
@@ -198,7 +198,7 @@ export const PluginList = memo(function PluginList({
             ))}
           </PluginGroup>
 
-          {!isArchived && (
+          {isConnected && (
             <PluginGroup
               key="metro"
               title="React Native"
@@ -226,7 +226,7 @@ export const PluginList = memo(function PluginList({
                 onClick={handleAppPluginClick}
                 tooltip={getPluginTooltip(plugin.details)}
                 actions={
-                  isArchived ? null : (
+                  isConnected ? (
                     <ActionButton
                       id={plugin.id}
                       onClick={handleStarPlugin}
@@ -235,12 +235,12 @@ export const PluginList = memo(function PluginList({
                         <MinusOutlined size={16} style={{marginRight: 0}} />
                       }
                     />
-                  )
+                  ) : null
                 }
               />
             ))}
           </PluginGroup>
-          {!isArchived && (
+          {isConnected && (
             <PluginGroup
               key="disabled"
               title="Disabled"
@@ -305,7 +305,7 @@ export const PluginList = memo(function PluginList({
               />
             ))}
           </PluginGroup>
-          {!isArchived && (
+          {isConnected && (
             <PluginGroup
               key="unavailable"
               title="Unavailable plugins"
diff --git a/desktop/app/src/utils/exportData.tsx b/desktop/app/src/utils/exportData.tsx
index 5254b1ef4..f70db9630 100644
--- a/desktop/app/src/utils/exportData.tsx
+++ b/desktop/app/src/utils/exportData.tsx
@@ -431,14 +431,14 @@ export async function processStore(
       statusUpdate = () => {};
     }
     statusUpdate('Capturing screenshot...');
-    const deviceScreenshot = device.isArchived
-      ? null
-      : await capture(device).catch((e) => {
+    const deviceScreenshot = device.connected.get()
+      ? await capture(device).catch((e) => {
           console.warn(
             'Failed to capture device screenshot when exporting. ' + e,
           );
           return null;
-        });
+        })
+      : null;
     const processedClients = processClients(clients, serial, statusUpdate);
     const processedPluginStates = processPluginStates({
       clients: processedClients,
diff --git a/desktop/app/src/utils/pluginUtils.tsx b/desktop/app/src/utils/pluginUtils.tsx
index 11ec3b4a6..8e39d9e91 100644
--- a/desktop/app/src/utils/pluginUtils.tsx
+++ b/desktop/app/src/utils/pluginUtils.tsx
@@ -301,7 +301,7 @@ function getFavoritePlugins(
   starredPlugins: undefined | string[],
   returnFavoredPlugins: boolean, // if false, unfavoried plugins are returned
 ): PluginDefinition[] {
-  if (device instanceof ArchivedDevice) {
+  if (device.isArchived) {
     if (!returnFavoredPlugins) {
       return [];
     }
diff --git a/desktop/app/src/utils/screenshot.tsx b/desktop/app/src/utils/screenshot.tsx
index f74ac014a..0b07b8297 100644
--- a/desktop/app/src/utils/screenshot.tsx
+++ b/desktop/app/src/utils/screenshot.tsx
@@ -27,8 +27,8 @@ export function getFileName(extension: 'png' | 'mp4'): string {
 }
 
 export async function capture(device: BaseDevice): Promise<string> {
-  if (device.isArchived) {
-    console.log('Skipping screenshot for archived device');
+  if (!device.connected.get()) {
+    console.log('Skipping screenshot for disconnected device');
     return '';
   }
   const pngPath = path.join(CAPTURE_LOCATION, getFileName('png'));
diff --git a/desktop/flipper-plugin/src/plugin/DevicePlugin.tsx b/desktop/flipper-plugin/src/plugin/DevicePlugin.tsx
index 1a4cd23b1..d1072a93d 100644
--- a/desktop/flipper-plugin/src/plugin/DevicePlugin.tsx
+++ b/desktop/flipper-plugin/src/plugin/DevicePlugin.tsx
@@ -11,6 +11,7 @@ import {SandyPluginDefinition} from './SandyPluginDefinition';
 import {BasePluginInstance, BasePluginClient} from './PluginBase';
 import {FlipperLib} from './FlipperLib';
 import {DeviceType as PluginDeviceType} from 'flipper-plugin-lib';
+import {Atom} from '../state/atom';
 
 export type DeviceLogListener = (entry: DeviceLogEntry) => void;
 
@@ -67,6 +68,7 @@ export interface RealFlipperDevice {
   os: string;
   serial: string;
   isArchived: boolean;
+  connected: Atom<boolean>;
   deviceType: DeviceType;
   addLogListener(callback: DeviceLogListener): Symbol;
   removeLogListener(id: Symbol): void;
diff --git a/desktop/flipper-plugin/src/plugin/PluginBase.tsx b/desktop/flipper-plugin/src/plugin/PluginBase.tsx
index e0cdc9990..9fbb1c76d 100644
--- a/desktop/flipper-plugin/src/plugin/PluginBase.tsx
+++ b/desktop/flipper-plugin/src/plugin/PluginBase.tsx
@@ -136,8 +136,7 @@ export abstract class BasePluginInstance {
         return realDevice.isArchived;
       },
       get isConnected() {
-        // for now same as isArchived, in the future we might distinguish between archived/imported and disconnected/offline devices
-        return !realDevice.isArchived;
+        return realDevice.connected.get();
       },
       deviceType: realDevice.deviceType,
 
diff --git a/desktop/flipper-plugin/src/test-utils/test-utils.tsx b/desktop/flipper-plugin/src/test-utils/test-utils.tsx
index 54adef10c..15f76ab44 100644
--- a/desktop/flipper-plugin/src/test-utils/test-utils.tsx
+++ b/desktop/flipper-plugin/src/test-utils/test-utils.tsx
@@ -421,6 +421,7 @@ function createMockDevice(options?: StartPluginOptions): RealFlipperDevice {
     deviceType: 'emulator',
     serial: 'serial-000',
     isArchived: !!options?.isArchived,
+    connected: createState(true),
     devicePlugins: [],
     addLogListener(cb) {
       logListeners.push(cb);