Improve screen capture experience

Summary:
Some improvements on the previous diff that were too much for just comments.

- Remove an unnecessary `touch` that I commented on in the previous diff.
- Ensure that the recording button is not set to "recording" if there's an error during the setup.
- More reliable way of checking the file size on the device before pulling.

Still not done: Show the error more prominently in the UI. Out of scope for this but if we could come up with a design for showing this (perhaps in place of the recording button) when something went wrong, that would be more obvious than having the error caret show up at the top of the screen.

Reviewed By: jknoxville

Differential Revision: D19723627

fbshipit-source-id: 20babcc1482e5a9ac829ff6d6ae7a731a3454fa0

src/chrome/ScreenCaptureButtons.tsx

@@ -102,11 +102,7 @@ class ScreenCaptureButtons extends Component<Props, State> {
       return;
     }
     const videoPath = path.join(CAPTURE_LOCATION, getFileName('mp4'));
-    await selectedDevice.startScreenCapture(videoPath);
-
-    this.setState({
-      recording: true,
-    });
+    return selectedDevice.startScreenCapture(videoPath);
   };
 
   stopRecording = async () => {
@@ -114,18 +110,28 @@ class ScreenCaptureButtons extends Component<Props, State> {
     if (!selectedDevice) {
       return;
     }
-    const path = await selectedDevice.stopScreenCapture();
-    this.setState({
-      recording: false,
+    const path = await selectedDevice.stopScreenCapture().catch(e => {
+      console.error(e);
     });
-    openFile(path);
+    path ? openFile(path) : 0;
   };
 
   onRecordingClicked = () => {
     if (this.state.recording) {
       this.stopRecording();
+      this.setState({
+        recording: false,
+      });
     } else {
-      this.startRecording();
+      this.setState({
+        recording: true,
+      });
+      this.startRecording().catch(e => {
+        this.setState({
+          recording: false,
+        });
+        console.error(e);
+      });
     }
   };
 

src/chrome/VideoRecordingButton.tsx

@@ -46,10 +46,15 @@ export default class VideoRecordingButton extends Component<Props, State> {
       .toISOString()
       .replace(/:/g, '')}.mp4`;
     const videoPath = path.join(flipperDirectory, fileName);
-    await selectedDevice.startScreenCapture(videoPath);
     this.setState({
       recording: true,
     });
+    selectedDevice.startScreenCapture(videoPath).catch(e => {
+      console.error('Screen recording failed:', e);
+      this.setState({
+        recording: false,
+      });
+    });
   };
 
   stopRecording = async () => {

src/devices/AndroidDevice.tsx

@@ -132,6 +132,23 @@ export default class AndroidDevice extends BaseDevice {
     }
   }
 
+  private async isValidFile(filePath: string): Promise<boolean> {
+    const fileSize = await this.adb
+      .shell(this.serial, `du "${filePath}"`)
+      .then(adb.util.readAll)
+      .then((output: Buffer) =>
+        output
+          .toString()
+          .trim()
+          .split('\t'),
+      )
+      .then(x => Number(x[0]));
+
+    // 4 is what an empty file (touch file) already takes up, so it's
+    // definitely not a valid video file.
+    return fileSize > 4;
+  }
+
   async startScreenCapture(destination: string) {
     await this.executeShell(
       `mkdir -p "${DEVICE_RECORDING_DIR}" && echo -n > "${DEVICE_RECORDING_DIR}/.nomedia"`,
@@ -140,17 +157,28 @@ export default class AndroidDevice extends BaseDevice {
     this.recordingProcess = this.adb
       .shell(this.serial, `screenrecord --bugreport "${recordingLocation}"`)
       .then(adb.util.readAll)
+      .then(async output => {
+        const isValid = await this.isValidFile(recordingLocation);
+        if (!isValid) {
+          const outputMessage = output.toString().trim();
+          throw new Error(
+            'Recording was not properly started: \n' + outputMessage,
+          );
+        }
+      })
       .then(
         _ =>
-          new Promise((resolve, reject) =>
+          new Promise((resolve, reject) => {
             this.adb.pull(this.serial, recordingLocation).then(stream => {
               stream.on('end', resolve);
               stream.on('error', reject);
               stream.pipe(createWriteStream(destination));
-            }),
-          ),
+            });
+          }),
       )
       .then(_ => destination);
+
+    return this.recordingProcess.then(_ => {});
   }
 
   async stopScreenCapture(): Promise<string> {

src/devices/BaseDevice.tsx

@@ -166,7 +166,7 @@ export default class BaseDevice {
     return false;
   }
 
-  async startScreenCapture(_destination: string) {
+  async startScreenCapture(_destination: string): Promise<void> {
     throw new Error('startScreenCapture not implemented on BaseDevice ');
   }