From 6c74f2dd18047449c50193720ed575627e6bbf6e Mon Sep 17 00:00:00 2001
From: Andrey Goncharov <aigoncharov@fb.com>
Date: Fri, 4 Mar 2022 02:00:23 -0800
Subject: [PATCH] Set device features on device initialization

Summary:
1. Identify if device supports screenshots/screen recording when it is created.
2. Disable screen recording/screenshot buttons when they are not supported

Reviewed By: passy

Differential Revision: D34611133

fbshipit-source-id: 82ad2d67e4af482d9becf7995187667b5d99bc36
---
 desktop/flipper-common/src/server-types.tsx   |  6 +++--
 .../src/plugin/DevicePlugin.tsx               |  2 +-
 .../src/FlipperServerImpl.tsx                 |  8 ------
 .../src/devices/DummyDevice.tsx               |  4 +++
 .../src/devices/ServerDevice.tsx              | 10 ++------
 .../src/devices/android/AndroidDevice.tsx     |  6 ++++-
 .../src/devices/android/KaiOSDevice.tsx       |  6 -----
 .../devices/android/androidDeviceManager.tsx  | 12 +++++++++
 .../src/devices/desktop/MacDevice.tsx         |  4 +++
 .../src/devices/desktop/WindowsDevice.tsx     |  4 +++
 .../src/devices/ios/IOSDevice.tsx             |  8 +++---
 .../src/devices/metro/MetroDevice.tsx         |  4 +++
 .../src/chrome/ScreenCaptureButtons.tsx       | 25 +++++++------------
 .../src/chrome/VideoRecordingButton.tsx       |  8 +++---
 .../src/devices/ArchivedDevice.tsx            |  4 +++
 .../src/devices/BaseDevice.tsx                | 23 +++--------------
 .../src/devices/TestDevice.tsx                |  4 +++
 .../flipper-ui-core/src/utils/screenshot.tsx  | 16 +++++++++---
 desktop/plugins/public/navigation/plugin.tsx  |  2 +-
 19 files changed, 81 insertions(+), 75 deletions(-)

diff --git a/desktop/flipper-common/src/server-types.tsx b/desktop/flipper-common/src/server-types.tsx
index 9d81ee093..2cbafca3d 100644
--- a/desktop/flipper-common/src/server-types.tsx
+++ b/desktop/flipper-common/src/server-types.tsx
@@ -43,6 +43,10 @@ export type DeviceDescription = {
   readonly deviceType: DeviceType;
   readonly serial: string;
   readonly icon?: string;
+  readonly features: {
+    screenshotAvailable: boolean;
+    screenCaptureAvailable: boolean;
+  };
   // Android specific information
   readonly specs?: DeviceSpec[];
   readonly abiList?: string[];
@@ -205,8 +209,6 @@ export type FlipperServerCommands = {
   'get-config': () => Promise<FlipperServerConfig>;
   'get-changelog': () => Promise<string>;
   'device-list': () => Promise<DeviceDescription[]>;
-  'device-supports-screenshot': (serial: string) => Promise<boolean>;
-  'device-supports-screencapture': (serial: string) => Promise<boolean>;
   'device-take-screenshot': (serial: string) => Promise<string>; // base64 encoded buffer
   'device-start-screencapture': (
     serial: string,
diff --git a/desktop/flipper-plugin/src/plugin/DevicePlugin.tsx b/desktop/flipper-plugin/src/plugin/DevicePlugin.tsx
index df5d774a9..4a1fd2395 100644
--- a/desktop/flipper-plugin/src/plugin/DevicePlugin.tsx
+++ b/desktop/flipper-plugin/src/plugin/DevicePlugin.tsx
@@ -41,7 +41,7 @@ export interface Device {
   clearLogs(): Promise<void>;
   sendMetroCommand(command: string): Promise<void>;
   navigateToLocation(location: string): Promise<void>;
-  screenshot(): Promise<Uint8Array>;
+  screenshot(): Promise<Uint8Array | undefined>;
 }
 
 export type DevicePluginPredicate = (device: Device) => boolean;
diff --git a/desktop/flipper-server-core/src/FlipperServerImpl.tsx b/desktop/flipper-server-core/src/FlipperServerImpl.tsx
index 890bb95c0..428be48bf 100644
--- a/desktop/flipper-server-core/src/FlipperServerImpl.tsx
+++ b/desktop/flipper-server-core/src/FlipperServerImpl.tsx
@@ -333,14 +333,6 @@ export class FlipperServerImpl implements FlipperServer {
     'device-list': async () => {
       return Array.from(this.devices.values()).map((d) => d.info);
     },
-    'device-supports-screenshot': async (serial: string) =>
-      this.devices.has(serial)
-        ? this.getDevice(serial).screenshotAvailable()
-        : false,
-    'device-supports-screencapture': async (serial: string) =>
-      this.devices.has(serial)
-        ? this.getDevice(serial).screenCaptureAvailable()
-        : false,
     'device-take-screenshot': async (serial: string) =>
       Base64.fromUint8Array(await this.getDevice(serial).screenshot()),
     'device-start-screencapture': async (serial, destination) =>
diff --git a/desktop/flipper-server-core/src/devices/DummyDevice.tsx b/desktop/flipper-server-core/src/devices/DummyDevice.tsx
index 205bc35d5..69d6807f5 100644
--- a/desktop/flipper-server-core/src/devices/DummyDevice.tsx
+++ b/desktop/flipper-server-core/src/devices/DummyDevice.tsx
@@ -26,6 +26,10 @@ export default class DummyDevice extends ServerDevice {
       deviceType: 'dummy',
       title,
       os,
+      features: {
+        screenCaptureAvailable: false,
+        screenshotAvailable: false,
+      },
     });
   }
 }
diff --git a/desktop/flipper-server-core/src/devices/ServerDevice.tsx b/desktop/flipper-server-core/src/devices/ServerDevice.tsx
index 3681f35ee..2c9bcdc40 100644
--- a/desktop/flipper-server-core/src/devices/ServerDevice.tsx
+++ b/desktop/flipper-server-core/src/devices/ServerDevice.tsx
@@ -44,25 +44,19 @@ export abstract class ServerDevice {
    */
   disconnect(): void {
     this.connected = false;
+    this.info.features.screenCaptureAvailable = false;
+    this.info.features.screenshotAvailable = false;
     this.logListener.stop();
     this.crashWatcher.stop();
     this.flipperServer.pluginManager.stopAllServerAddOns(this.info.serial);
   }
 
-  async screenshotAvailable(): Promise<boolean> {
-    return false;
-  }
-
   screenshot(): Promise<Buffer> {
     return Promise.reject(
       new Error('No screenshot support for current device'),
     );
   }
 
-  async screenCaptureAvailable(): Promise<boolean> {
-    return false;
-  }
-
   async startScreenCapture(_destination: string): Promise<void> {
     throw new Error('startScreenCapture not implemented on BaseDevice ');
   }
diff --git a/desktop/flipper-server-core/src/devices/android/AndroidDevice.tsx b/desktop/flipper-server-core/src/devices/android/AndroidDevice.tsx
index 99cd4b95e..8bffe0e8c 100644
--- a/desktop/flipper-server-core/src/devices/android/AndroidDevice.tsx
+++ b/desktop/flipper-server-core/src/devices/android/AndroidDevice.tsx
@@ -49,6 +49,10 @@ export default class AndroidDevice extends ServerDevice {
       specs,
       abiList,
       sdkVersion,
+      features: {
+        screenCaptureAvailable: false,
+        screenshotAvailable: false,
+      },
     });
     this.adb = adb;
 
@@ -115,7 +119,7 @@ export default class AndroidDevice extends ServerDevice {
     });
   }
 
-  async screenCaptureAvailable(): Promise<boolean> {
+  async screenRecordAvailable(): Promise<boolean> {
     try {
       await this.executeShellOrDie(
         `[ ! -f /system/bin/screenrecord ] && echo "File does not exist"`,
diff --git a/desktop/flipper-server-core/src/devices/android/KaiOSDevice.tsx b/desktop/flipper-server-core/src/devices/android/KaiOSDevice.tsx
index a588e90b5..b74a03392 100644
--- a/desktop/flipper-server-core/src/devices/android/KaiOSDevice.tsx
+++ b/desktop/flipper-server-core/src/devices/android/KaiOSDevice.tsx
@@ -26,10 +26,4 @@ export default class KaiOSDevice extends AndroidDevice {
       'KaiOS',
     ]);
   }
-
-  async screenCaptureAvailable() {
-    // The default way of capturing screenshots through adb does not seem to work
-    // There is a way of getting a screenshot through KaiOS dev tools though
-    return false;
-  }
 }
diff --git a/desktop/flipper-server-core/src/devices/android/androidDeviceManager.tsx b/desktop/flipper-server-core/src/devices/android/androidDeviceManager.tsx
index 11853b8ec..2391fc5df 100644
--- a/desktop/flipper-server-core/src/devices/android/androidDeviceManager.tsx
+++ b/desktop/flipper-server-core/src/devices/android/androidDeviceManager.tsx
@@ -86,6 +86,18 @@ export class AndroidDeviceManager {
               );
             });
           }
+
+          // The default way of capturing screenshots through adb does not seem to work
+          // There is a way of getting a screenshot through KaiOS dev tools though
+          if (androidLikeDevice instanceof AndroidDevice) {
+            const screenRecordAvailable =
+              await androidLikeDevice.screenRecordAvailable();
+            androidLikeDevice.info.features.screenCaptureAvailable =
+              screenRecordAvailable;
+            androidLikeDevice.info.features.screenshotAvailable =
+              screenRecordAvailable;
+          }
+
           resolve(androidLikeDevice);
         } catch (e) {
           reject(e);
diff --git a/desktop/flipper-server-core/src/devices/desktop/MacDevice.tsx b/desktop/flipper-server-core/src/devices/desktop/MacDevice.tsx
index e7f683144..6ce87fd3d 100644
--- a/desktop/flipper-server-core/src/devices/desktop/MacDevice.tsx
+++ b/desktop/flipper-server-core/src/devices/desktop/MacDevice.tsx
@@ -18,6 +18,10 @@ export default class MacDevice extends ServerDevice {
       title: 'Mac',
       os: 'MacOS',
       icon: 'app-apple',
+      features: {
+        screenCaptureAvailable: false,
+        screenshotAvailable: false,
+      },
     });
   }
 }
diff --git a/desktop/flipper-server-core/src/devices/desktop/WindowsDevice.tsx b/desktop/flipper-server-core/src/devices/desktop/WindowsDevice.tsx
index a792b1bc5..c98acb144 100644
--- a/desktop/flipper-server-core/src/devices/desktop/WindowsDevice.tsx
+++ b/desktop/flipper-server-core/src/devices/desktop/WindowsDevice.tsx
@@ -18,6 +18,10 @@ export default class WindowsDevice extends ServerDevice {
       title: 'Windows',
       os: 'Windows',
       icon: 'app-microsoft-windows',
+      features: {
+        screenCaptureAvailable: false,
+        screenshotAvailable: false,
+      },
     });
   }
 }
diff --git a/desktop/flipper-server-core/src/devices/ios/IOSDevice.tsx b/desktop/flipper-server-core/src/devices/ios/IOSDevice.tsx
index 10badd573..24f31d9e6 100644
--- a/desktop/flipper-server-core/src/devices/ios/IOSDevice.tsx
+++ b/desktop/flipper-server-core/src/devices/ios/IOSDevice.tsx
@@ -36,6 +36,10 @@ export default class IOSDevice extends ServerDevice {
       title,
       os: 'iOS',
       icon: 'mobile',
+      features: {
+        screenCaptureAvailable: true,
+        screenshotAvailable: true,
+      },
     });
     this.buffer = '';
     this.iOSBridge = iOSBridge;
@@ -76,10 +80,6 @@ export default class IOSDevice extends ServerDevice {
     });
   }
 
-  async screenCaptureAvailable() {
-    return this.connected;
-  }
-
   async startScreenCapture(destination: string) {
     const recording = this.recording;
     if (recording) {
diff --git a/desktop/flipper-server-core/src/devices/metro/MetroDevice.tsx b/desktop/flipper-server-core/src/devices/metro/MetroDevice.tsx
index aacfb2d39..41ab8be42 100644
--- a/desktop/flipper-server-core/src/devices/metro/MetroDevice.tsx
+++ b/desktop/flipper-server-core/src/devices/metro/MetroDevice.tsx
@@ -64,6 +64,10 @@ export default class MetroDevice extends ServerDevice {
       title: 'React Native',
       os: 'Metro',
       icon: 'mobile',
+      features: {
+        screenCaptureAvailable: false,
+        screenshotAvailable: false,
+      },
     });
     if (ws) {
       this.ws = ws;
diff --git a/desktop/flipper-ui-core/src/chrome/ScreenCaptureButtons.tsx b/desktop/flipper-ui-core/src/chrome/ScreenCaptureButtons.tsx
index 04130c5df..0754349d4 100644
--- a/desktop/flipper-ui-core/src/chrome/ScreenCaptureButtons.tsx
+++ b/desktop/flipper-ui-core/src/chrome/ScreenCaptureButtons.tsx
@@ -8,7 +8,7 @@
  */
 
 import {Button as AntButton, message} from 'antd';
-import React, {useState, useEffect, useCallback} from 'react';
+import React, {useState, useCallback} from 'react';
 import {capture, getCaptureLocation, getFileName} from '../utils/screenshot';
 import {CameraOutlined, VideoCameraOutlined} from '@ant-design/icons';
 import {useStore} from '../utils/useStore';
@@ -22,21 +22,8 @@ async function openFile(path: string) {
 export default function ScreenCaptureButtons() {
   const selectedDevice = useStore((state) => state.connections.selectedDevice);
   const [isTakingScreenshot, setIsTakingScreenshot] = useState(false);
-  const [isRecordingAvailable, setIsRecordingAvailable] = useState(false);
   const [isRecording, setIsRecording] = useState(false);
 
-  useEffect(() => {
-    let canceled = false;
-    selectedDevice?.screenCaptureAvailable().then((result) => {
-      if (!canceled) {
-        setIsRecordingAvailable(result);
-      }
-    });
-    return () => {
-      canceled = true;
-    };
-  }, [selectedDevice]);
-
   const handleScreenshot = useCallback(() => {
     setIsTakingScreenshot(true);
     return capture(selectedDevice!)
@@ -87,7 +74,10 @@ export default function ScreenCaptureButtons() {
         title="Take Screenshot"
         type="ghost"
         onClick={handleScreenshot}
-        disabled={!selectedDevice}
+        disabled={
+          !selectedDevice ||
+          !selectedDevice.description.features.screenshotAvailable
+        }
         loading={isTakingScreenshot}
       />
       <AntButton
@@ -95,7 +85,10 @@ export default function ScreenCaptureButtons() {
         title="Make Screen Recording"
         type={isRecording ? 'primary' : 'ghost'}
         onClick={handleRecording}
-        disabled={!selectedDevice || !isRecordingAvailable}
+        disabled={
+          !selectedDevice ||
+          !selectedDevice.description.features.screenCaptureAvailable
+        }
         danger={isRecording}
       />
     </>
diff --git a/desktop/flipper-ui-core/src/chrome/VideoRecordingButton.tsx b/desktop/flipper-ui-core/src/chrome/VideoRecordingButton.tsx
index 365edf010..9319a5bad 100644
--- a/desktop/flipper-ui-core/src/chrome/VideoRecordingButton.tsx
+++ b/desktop/flipper-ui-core/src/chrome/VideoRecordingButton.tsx
@@ -25,14 +25,12 @@ type DispatchFromProps = {};
 
 type State = {
   recording: boolean;
-  recordingEnabled: boolean;
 };
 type Props = OwnProps & StateFromProps & DispatchFromProps;
 
 export default class VideoRecordingButton extends Component<Props, State> {
   state: State = {
     recording: false,
-    recordingEnabled: true,
   };
 
   startRecording = async () => {
@@ -80,7 +78,6 @@ export default class VideoRecordingButton extends Component<Props, State> {
     }
   };
   render() {
-    const {recordingEnabled} = this.state;
     const {selectedDevice} = this.props;
     return (
       <Button
@@ -89,7 +86,10 @@ export default class VideoRecordingButton extends Component<Props, State> {
         pulse={this.state.recording}
         selected={this.state.recording}
         title="Make Screen Recording"
-        disabled={!selectedDevice || !recordingEnabled}
+        disabled={
+          !selectedDevice ||
+          !selectedDevice.description.features.screenCaptureAvailable
+        }
         type={this.state.recording ? 'danger' : 'primary'}>
         <Glyph
           name={this.state.recording ? 'stop-playback' : 'camcorder'}
diff --git a/desktop/flipper-ui-core/src/devices/ArchivedDevice.tsx b/desktop/flipper-ui-core/src/devices/ArchivedDevice.tsx
index 5aeba1557..c8d3c9f45 100644
--- a/desktop/flipper-ui-core/src/devices/ArchivedDevice.tsx
+++ b/desktop/flipper-ui-core/src/devices/ArchivedDevice.tsx
@@ -45,6 +45,10 @@ export default class ArchivedDevice extends BaseDevice {
         os: options.os,
         serial: options.serial,
         icon: 'box',
+        features: {
+          screenCaptureAvailable: false,
+          screenshotAvailable: false,
+        },
       },
     );
     this.connected.set(false);
diff --git a/desktop/flipper-ui-core/src/devices/BaseDevice.tsx b/desktop/flipper-ui-core/src/devices/BaseDevice.tsx
index 7d1e5c0f6..a393c16a2 100644
--- a/desktop/flipper-ui-core/src/devices/BaseDevice.tsx
+++ b/desktop/flipper-ui-core/src/devices/BaseDevice.tsx
@@ -235,32 +235,15 @@ export default class BaseDevice implements Device {
     return this.flipperServer.exec('device-navigate', this.serial, location);
   }
 
-  async screenshotAvailable(): Promise<boolean> {
-    if (this.isArchived) {
-      return false;
-    }
-    return this.flipperServer.exec('device-supports-screenshot', this.serial);
-  }
-
-  async screenshot(): Promise<Uint8Array> {
-    if (this.isArchived) {
-      return new Uint8Array();
+  async screenshot(): Promise<Uint8Array | undefined> {
+    if (!this.description.features.screenshotAvailable || this.isArchived) {
+      return;
     }
     return Base64.toUint8Array(
       await this.flipperServer.exec('device-take-screenshot', this.serial),
     );
   }
 
-  async screenCaptureAvailable(): Promise<boolean> {
-    if (this.isArchived) {
-      return false;
-    }
-    return this.flipperServer.exec(
-      'device-supports-screencapture',
-      this.serial,
-    );
-  }
-
   async startScreenCapture(destination: string): Promise<void> {
     return this.flipperServer.exec(
       'device-start-screencapture',
diff --git a/desktop/flipper-ui-core/src/devices/TestDevice.tsx b/desktop/flipper-ui-core/src/devices/TestDevice.tsx
index d7b33dee1..9d6040604 100644
--- a/desktop/flipper-ui-core/src/devices/TestDevice.tsx
+++ b/desktop/flipper-ui-core/src/devices/TestDevice.tsx
@@ -26,6 +26,10 @@ export class TestDevice extends BaseDevice {
       title,
       os,
       specs,
+      features: {
+        screenCaptureAvailable: false,
+        screenshotAvailable: false,
+      },
     });
   }
 
diff --git a/desktop/flipper-ui-core/src/utils/screenshot.tsx b/desktop/flipper-ui-core/src/utils/screenshot.tsx
index 0fffed3da..a44d7d9ba 100644
--- a/desktop/flipper-ui-core/src/utils/screenshot.tsx
+++ b/desktop/flipper-ui-core/src/utils/screenshot.tsx
@@ -11,6 +11,7 @@ import BaseDevice from '../devices/BaseDevice';
 import {reportPlatformFailures} from 'flipper-common';
 import {getRenderHostInstance} from '../RenderHost';
 import {getFlipperLib, path} from 'flipper-plugin';
+import {assertNotNull} from './assertNotNull';
 
 export function getCaptureLocation() {
   return (
@@ -27,7 +28,7 @@ export function getFileName(extension: 'png' | 'mp4'): string {
 
 export async function capture(device: BaseDevice): Promise<string> {
   if (!device.connected.get()) {
-    console.log('Skipping screenshot for disconnected device');
+    console.info('Skipping screenshot for disconnected device');
     return '';
   }
   const pngPath = path.join(getCaptureLocation(), getFileName('png'));
@@ -36,9 +37,16 @@ export async function capture(device: BaseDevice): Promise<string> {
     // again to write in a file, probably easier to change screenshot api to `device.screenshot(): path`
     device
       .screenshot()
-      .then((buffer) =>
-        getFlipperLib().remoteServerContext.fs.writeFileBinary(pngPath, buffer),
-      )
+      .then((buffer) => {
+        assertNotNull(
+          buffer,
+          `Device ${device.description.deviceType}:${device.description.os} does not support taking screenshots`,
+        );
+        return getFlipperLib().remoteServerContext.fs.writeFileBinary(
+          pngPath,
+          buffer,
+        );
+      })
       .then(() => pngPath),
     'captureScreenshot',
   );
diff --git a/desktop/plugins/public/navigation/plugin.tsx b/desktop/plugins/public/navigation/plugin.tsx
index fcd816756..4167ffe97 100644
--- a/desktop/plugins/public/navigation/plugin.tsx
+++ b/desktop/plugins/public/navigation/plugin.tsx
@@ -73,7 +73,7 @@ export function plugin(client: PluginClient<Events, Methods>) {
     });
 
     const screenshot = await client.device.screenshot();
-    if (screenshot.byteLength === 0) {
+    if (!screenshot) {
       console.warn(
         '[navigation] Could not retrieve valid screenshot from the device.',
       );