Revert "[Image Capture, Android] Fix various threading issues"

This reverts commit 87b3a36a.

Reason for revert: Causes crash issue https://bugs.chromium.org/p/chromium/issues/detail?id=874745

Original change's description:
> [Image Capture, Android] Fix various threading issues
> 
> This CL simplifies the threading of class VideoCaptureCamera2.java
> and by doing so resolves potential issues caused by concurrent
> access to member variables and Android video API calls.
> 
> See https://bugs.chromium.org/p/chromium/issues/detail?id=857530 for
> details on issues.
> 
> The simplified model is to (still) have the constructor and public API
> calls happen on a native thread, and to use a single dedicated thread
> owned by the class instance to post to, do work on, and call back into
> the native code.
> 
> This CL is part of a series, see Design Doc at
> https://docs.google.com/document/d/1h1kva4VR1gaV3HVXaSYZFY41icfaB58j-WnHmJdyqc8/edit?usp=sharing
> 
> Bug: 857530
> Change-Id: I75ffcc4a14f2395d833d80f300acef7b456676e8
> Reviewed-on: https://chromium-review.googlesource.com/1117857
> Commit-Queue: Christian Fremerey <chfremer@chromium.org>
> Reviewed-by: Emircan Uysaler <emircan@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#576155}

TBR=emircan@chromium.org,chfremer@chromium.org

# Not skipping CQ checks because original CL landed > 1 day ago.

Bug: 857530
Change-Id: Ifc02ccd8b8282b0ec31e2f4f36dbe831eee551a2
Reviewed-on: https://chromium-review.googlesource.com/1185262Reviewed-by: Christian Fremerey <chfremer@chromium.org>
Commit-Queue: Christian Fremerey <chfremer@chromium.org>
Cr-Commit-Position: refs/heads/master@{#585199}

media/capture/video/android/java/src/org/chromium/media/VideoCapture.java

@@ -62,22 +62,14 @@ public abstract class VideoCapture {
     @CalledByNative
     public abstract boolean allocate(int width, int height, int frameRate);
 
-    // Success is indicated by returning true and a callback to
-    // nativeOnStarted(), which may occur synchronously or asynchronously.
-    // Failure can be indicated by one of the following:
-    // * Returning false. In this case no callback to nativeOnStarted() is made.
-    // * Returning true, and asynchronously invoking nativeOnError. In this case
-    //   also no callback to nativeOnStarted() is made.
     @CalledByNative
-    public abstract boolean startCaptureMaybeAsync();
+    public abstract boolean startCapture();
 
-    // Blocks until it is guaranteed that no more frames are sent.
     @CalledByNative
-    public abstract boolean stopCaptureAndBlockUntilStopped();
+    public abstract boolean stopCapture();
 
-    // Replies by calling nativeOnGetPhotoCapabilitiesReply().
     @CalledByNative
-    public abstract void getPhotoCapabilitiesAsync(long callbackId);
+    public abstract PhotoCapabilities getPhotoCapabilities();
 
     /**
      * @param zoom Zoom level, should be ignored if 0.
@@ -103,9 +95,8 @@ public abstract class VideoCapture {
             boolean hasRedEyeReduction, boolean redEyeReduction, int fillLightMode,
             boolean hasTorch, boolean torch, double colorTemperature);
 
-    // Replies by calling nativeOnPhotoTaken().
     @CalledByNative
-    public abstract void takePhotoAsync(long callbackId);
+    public abstract boolean takePhoto(final long callbackId);
 
     @CalledByNative
     public abstract void deallocate();
@@ -173,12 +164,6 @@ public abstract class VideoCapture {
         return orientation;
     }
 
-    // {@link nativeOnPhotoTaken()} needs to be called back if there's any
-    // problem after {@link takePhotoAsync()} has returned true.
-    protected void notifyTakePhotoError(long callbackId) {
-        nativeOnPhotoTaken(mNativeVideoCaptureDeviceAndroid, callbackId, null);
-    }
-
     /**
      * Finds the framerate range matching |targetFramerate|. Tries to find a range with as low of a
      * minimum value as possible to allow the camera adjust based on the lighting conditions.
@@ -245,17 +230,10 @@ public abstract class VideoCapture {
     // Method for VideoCapture implementations to signal an asynchronous error.
     public native void nativeOnError(long nativeVideoCaptureDeviceAndroid, String message);
 
-    public native void nativeOnGetPhotoCapabilitiesReply(
-            long nativeVideoCaptureDeviceAndroid, long callbackId, PhotoCapabilities result);
-
-    // Callback for calls to takePhoto(). This can indicate both success and
-    // failure. Failure is indicated by |data| being null.
+    // Method for VideoCapture implementations to send Photos back to.
     public native void nativeOnPhotoTaken(
             long nativeVideoCaptureDeviceAndroid, long callbackId, byte[] data);
 
     // Method for VideoCapture implementations to report device started event.
     public native void nativeOnStarted(long nativeVideoCaptureDeviceAndroid);
-
-    public native void nativeDCheckCurrentlyOnIncomingTaskRunner(
-            long nativeVideoCaptureDeviceAndroid);
 }

media/capture/video/android/java/src/org/chromium/media/VideoCaptureCamera.java

@@ -146,7 +146,8 @@ public class VideoCaptureCamera
 
             synchronized (mPhotoTakenCallbackLock) {
                 if (mPhotoTakenCallbackId == 0) return;
-                notifyTakePhotoError(mPhotoTakenCallbackId);
+                nativeOnPhotoTaken(
+                        mNativeVideoCaptureDeviceAndroid, mPhotoTakenCallbackId, new byte[0]);
                 mPhotoTakenCallbackId = 0;
             }
         }
@@ -424,9 +425,9 @@ public class VideoCaptureCamera
     }
 
     @Override
-    public boolean startCaptureMaybeAsync() {
+    public boolean startCapture() {
         if (mCamera == null) {
-            Log.e(TAG, "startCaptureAsync: mCamera is null");
+            Log.e(TAG, "startCapture: mCamera is null");
             return false;
         }
 
@@ -443,7 +444,7 @@ public class VideoCaptureCamera
         try {
             mCamera.startPreview();
         } catch (RuntimeException ex) {
-            Log.e(TAG, "startCaptureAsync: Camera.startPreview: " + ex);
+            Log.e(TAG, "startCapture: Camera.startPreview: " + ex);
             return false;
         }
 
@@ -458,9 +459,9 @@ public class VideoCaptureCamera
     }
 
     @Override
-    public boolean stopCaptureAndBlockUntilStopped() {
+    public boolean stopCapture() {
         if (mCamera == null) {
-            Log.e(TAG, "stopCaptureAndBlockUntilStopped: mCamera is null");
+            Log.e(TAG, "stopCapture: mCamera is null");
             return true;
         }
 
@@ -480,7 +481,7 @@ public class VideoCaptureCamera
     }
 
     @Override
-    public void getPhotoCapabilitiesAsync(long callbackId) {
+    public PhotoCapabilities getPhotoCapabilities() {
         final android.hardware.Camera.Parameters parameters = getCameraParameters(mCamera);
         PhotoCapabilities.Builder builder = new PhotoCapabilities.Builder();
         Log.i(TAG, " CAM params: %s", parameters.flatten());
@@ -633,8 +634,7 @@ public class VideoCaptureCamera
             builder.setFillLightModes(integerArrayListToArray(modes));
         }
 
-        nativeOnGetPhotoCapabilitiesReply(
-                mNativeVideoCaptureDeviceAndroid, callbackId, builder.build());
+        return builder.build();
     }
 
     @Override
@@ -776,19 +776,15 @@ public class VideoCaptureCamera
     }
 
     @Override
-    public void takePhotoAsync(final long callbackId) {
+    public boolean takePhoto(final long callbackId) {
         if (mCamera == null || !mIsRunning) {
-            Log.e(TAG, "takePhotoAsync: mCamera is null or is not running");
-            notifyTakePhotoError(callbackId);
-            return;
+            Log.e(TAG, "takePhoto: mCamera is null or is not running");
+            return false;
         }
 
         // Only one picture can be taken at once.
         synchronized (mPhotoTakenCallbackLock) {
-            if (mPhotoTakenCallbackId != 0) {
-                notifyTakePhotoError(callbackId);
-                return;
-            }
+            if (mPhotoTakenCallbackId != 0) return false;
             mPhotoTakenCallbackId = callbackId;
         }
         mPreviewParameters = getCameraParameters(mCamera);
@@ -821,18 +817,18 @@ public class VideoCaptureCamera
             mCamera.setParameters(photoParameters);
         } catch (RuntimeException ex) {
             Log.e(TAG, "setParameters " + ex);
-            notifyTakePhotoError(callbackId);
-            return;
+            return false;
         }
 
         mCamera.takePicture(null, null, null, new CrPictureCallback());
+        return true;
     }
 
     @Override
     public void deallocate() {
         if (mCamera == null) return;
 
-        stopCaptureAndBlockUntilStopped();
+        stopCapture();
         try {
             mCamera.setPreviewTexture(null);
             if (mGlTextures != null) GLES20.glDeleteTextures(1, mGlTextures, 0);

media/capture/video/android/video_capture_device_android.h

@@ -21,7 +21,7 @@
 namespace base {
 class Location;
 class SingleThreadTaskRunner;
-}  // namespace base
+}
 
 namespace media {
 
@@ -91,12 +91,6 @@ class CAPTURE_EXPORT VideoCaptureDeviceAndroid : public VideoCaptureDevice {
                const base::android::JavaParamRef<jobject>& obj,
                const base::android::JavaParamRef<jstring>& message);
 
-  void OnGetPhotoCapabilitiesReply(
-      JNIEnv* env,
-      const base::android::JavaParamRef<jobject>& obj,
-      jlong callback_id,
-      jobject photo_capabilities);
-
   // Implement org.chromium.media.VideoCapture.nativeOnPhotoTaken.
   void OnPhotoTaken(JNIEnv* env,
                     const base::android::JavaParamRef<jobject>& obj,
@@ -106,12 +100,6 @@ class CAPTURE_EXPORT VideoCaptureDeviceAndroid : public VideoCaptureDevice {
   // Implement org.chromium.media.VideoCapture.nativeOnStarted.
   void OnStarted(JNIEnv* env, const base::android::JavaParamRef<jobject>& obj);
 
-  // Implement
-  // org.chromium.media.VideoCapture.nativeDCheckCurrentlyOnIncomingTaskRunner.
-  void DCheckCurrentlyOnIncomingTaskRunner(
-      JNIEnv* env,
-      const base::android::JavaParamRef<jobject>& obj);
-
   void ConfigureForTesting();
 
  protected:
@@ -164,10 +152,9 @@ class CAPTURE_EXPORT VideoCaptureDeviceAndroid : public VideoCaptureDevice {
   base::TimeTicks expected_next_frame_time_;
   base::TimeDelta frame_interval_;
 
-  // List of callbacks for photo API in flight, being served in Java side.
+  // List of |photo_callbacks_| in flight, being served in Java side.
   base::Lock photo_callbacks_lock_;
-  std::list<std::unique_ptr<GetPhotoStateCallback>> get_photo_state_callbacks_;
-  std::list<std::unique_ptr<TakePhotoCallback>> take_photo_callbacks_;
+  std::list<std::unique_ptr<TakePhotoCallback>> photo_callbacks_;
 
   const VideoCaptureDeviceDescriptor device_descriptor_;
   VideoCaptureFormat capture_format_;