From c6d5b8ccb5fb6e4282d8f32f37750a70ee02850a Mon Sep 17 00:00:00 2001
From: Pascal Hartig <realpassy@fb.com>
Date: Tue, 1 Oct 2019 10:41:42 -0700
Subject: [PATCH] Fix screen recording on Android

Summary:
There were two issues with the previous approach:

1) We didn't block the shell process, so we would immediately
    start pulling the video from the device as soon as we **started**
    recording it. This meant, you'd usually get the previous recording
    or just an empty file.
2) The stop side doesn't know when it's actually safe to start displaying
    the video as pulling takes time. So we need access to the process
    promise there.

Reviewed By: jknoxville

Differential Revision: D17687827

fbshipit-source-id: 6ad5da52442f1888dd491b2a4c7f7a6b5a7885dd
---
 src/devices/AndroidDevice.tsx | 20 +++++++++++---------
 1 file changed, 11 insertions(+), 9 deletions(-)

diff --git a/src/devices/AndroidDevice.tsx b/src/devices/AndroidDevice.tsx
index e640a70c1..f0bc4d844 100644
--- a/src/devices/AndroidDevice.tsx
+++ b/src/devices/AndroidDevice.tsx
@@ -59,7 +59,7 @@ export default class AndroidDevice extends BaseDevice {
 
   adb: ADBClient;
   pidAppMapping: {[key: number]: string} = {};
-  private recordingDestination?: string;
+  private recordingProcess?: Promise<string>;
 
   supportedColumns(): Array<string> {
     return ['date', 'pid', 'tid', 'tag', 'message', 'type', 'time'];
@@ -134,12 +134,12 @@ export default class AndroidDevice extends BaseDevice {
     await this.executeShell(
       `mkdir -p "${DEVICE_RECORDING_DIR}" && echo -n > "${DEVICE_RECORDING_DIR}/.nomedia"`,
     );
-    this.recordingDestination = destination;
     const recordingLocation = `${DEVICE_RECORDING_DIR}/video.mp4`;
-    this.adb
+    this.recordingProcess = this.adb
       .shell(this.serial, `screenrecord --bugreport "${recordingLocation}"`)
+      .then(adb.util.readAll)
       .then(
-        () =>
+        _ =>
           new Promise((resolve, reject) =>
             this.adb.pull(this.serial, recordingLocation).then(stream => {
               stream.on('end', resolve);
@@ -147,16 +147,18 @@ export default class AndroidDevice extends BaseDevice {
               stream.pipe(createWriteStream(destination));
             }),
           ),
-      );
+      )
+      .then(_ => destination);
   }
 
   async stopScreenCapture(): Promise<string> {
-    const {recordingDestination} = this;
-    if (!recordingDestination) {
+    const {recordingProcess} = this;
+    if (!recordingProcess) {
       return Promise.reject(new Error('Recording was not properly started'));
     }
-    this.recordingDestination = undefined;
     await this.adb.shell(this.serial, `pgrep 'screenrecord' -L 2`);
-    return recordingDestination;
+    const destination = await recordingProcess;
+    this.recordingProcess = undefined;
+    return destination;
   }
 }