From: csagan5 <32685696+csagan5@users.noreply.github.com>
Date: Tue, 5 May 2020 07:22:20 +0200
Subject: AImageReader CFI crash mitigations

Revert "gpu/android: Remove setup for disabling AImageReader."
This reverts commit dcd5a39518246eb999f1cc63bf1ec95d93fd5b2f.

Revert "Remove flags to enable/disable AImageReader."
This reverts commit 463fa0f2e3b9e418bc26e2c8954463f0b0f76634.

Restore GPU bug blacklist for AImageReader on ARM and Qualcomm CPUs

Restore the AImageReader blacklist for ARM/Qualcomm chipsets which causes
crashes on Android 9 and 10 (at different code locations).

See discussions at:
* https://github.com/bromite/bromite/issues/445
* https://github.com/bromite/bromite/issues/814
* https://github.com/bromite/bromite/issues/1005

License: GPL-3.0-only - https://spdx.org/licenses/GPL-3.0-only.html
---
 base/android/android_image_reader_compat.cc |  8 +++++++-
 base/android/android_image_reader_compat.h  |  4 ++++
 chrome/browser/flag-metadata.json           |  6 +++---
 gpu/config/gpu_driver_bug_list.json         | 16 ++++++++++++++++
 gpu/config/gpu_finch_features.cc            |  5 +++++
 gpu/config/gpu_finch_features.h             |  1 +
 gpu/config/gpu_util.cc                      |  8 ++++++++
 gpu/config/gpu_workaround_list.txt          |  1 +
 gpu/ipc/service/gpu_init.cc                 |  5 +++++
 gpu/ipc/service/stream_texture_android.cc   | 11 ++++++++++-
 media/base/media_switches.cc                |  4 ++++
 media/base/media_switches.h                 |  1 +
 12 files changed, 65 insertions(+), 5 deletions(-)

diff --git a/base/android/android_image_reader_compat.cc b/base/android/android_image_reader_compat.cc
--- a/base/android/android_image_reader_compat.cc
+++ b/base/android/android_image_reader_compat.cc
@@ -23,6 +23,8 @@
 namespace base {
 namespace android {
 
+bool AndroidImageReader::disable_support_ = false;
+
 AndroidImageReader& AndroidImageReader::GetInstance() {
   // C++11 static local variable initialization is
   // thread-safe.
@@ -30,8 +32,12 @@ AndroidImageReader& AndroidImageReader::GetInstance() {
   return instance;
 }
 
+void AndroidImageReader::DisableSupport() {
+  disable_support_ = true;
+}
+
 bool AndroidImageReader::IsSupported() {
-  return is_supported_;
+  return !disable_support_ && is_supported_;
 }
 
 AndroidImageReader::AndroidImageReader() : is_supported_(LoadFunctions()) {}
diff --git a/base/android/android_image_reader_compat.h b/base/android/android_image_reader_compat.h
--- a/base/android/android_image_reader_compat.h
+++ b/base/android/android_image_reader_compat.h
@@ -24,6 +24,9 @@ class BASE_EXPORT AndroidImageReader {
   AndroidImageReader(const AndroidImageReader&) = delete;
   AndroidImageReader& operator=(const AndroidImageReader&) = delete;
 
+  // Disable image reader support.
+  static void DisableSupport();
+
   // Check if the image reader usage is supported. This function returns TRUE
   // if android version is >=OREO, image reader support is not disabled and all
   // the required functions are loaded.
@@ -61,6 +64,7 @@ class BASE_EXPORT AndroidImageReader {
   jobject ANativeWindow_toSurface(JNIEnv* env, ANativeWindow* window);
 
  private:
+  static bool disable_support_;
   friend class base::NoDestructor<AndroidImageReader>;
 
   AndroidImageReader();
diff --git a/chrome/browser/flag-metadata.json b/chrome/browser/flag-metadata.json
--- a/chrome/browser/flag-metadata.json
+++ b/chrome/browser/flag-metadata.json
@@ -2501,9 +2501,9 @@
     "expiry_milestone": 115
   },
   {
-    "name": "enable-image-reader",
-    "owners": [ "vikassoni", "liberato" ],
-    "expiry_milestone": 115
+    "name": "enable-image-reader",          // Bromite: do not expire
+    "owners": [ "vikassoni", "liberato" ],  // flag
+    "expiry_milestone": -1
   },
   {
     "name": "enable-immersive-fullscreen-toolbar",
diff --git a/gpu/config/gpu_driver_bug_list.json b/gpu/config/gpu_driver_bug_list.json
--- a/gpu/config/gpu_driver_bug_list.json
+++ b/gpu/config/gpu_driver_bug_list.json
@@ -3199,6 +3199,22 @@
         "dont_delete_source_texture_for_egl_image"
       ]
     },
+    {
+      "id":335,
+      "cr_bugs": [1051705],
+      "description": "Disable AImageReader on ARM GPUs",
+      "os": {
+        "type": "android",
+        "version": {
+          "op": "<",
+          "value": "10"
+        }
+      },
+      "gl_vendor": "ARM.*|Qualcomm.*",
+      "features": [
+        "disable_aimagereader"
+      ]
+    },
     {
       "id": 336,
       "cr_bugs": [625785],
diff --git a/gpu/config/gpu_finch_features.cc b/gpu/config/gpu_finch_features.cc
--- a/gpu/config/gpu_finch_features.cc
+++ b/gpu/config/gpu_finch_features.cc
@@ -64,6 +64,11 @@ BASE_FEATURE(kUseGles2ForOopR,
 #endif
 );
 
+
+// Use android AImageReader when playing videos with MediaPlayer.
+const base::Feature kAImageReaderMediaPlayer{"AImageReaderMediaPlayer",
+                                  base::FEATURE_ENABLED_BY_DEFAULT};
+
 #if BUILDFLAG(IS_ANDROID)
 // Use android SurfaceControl API for managing display compositor's buffer queue
 // and using overlays on Android. Also used by webview to disable surface
diff --git a/gpu/config/gpu_finch_features.h b/gpu/config/gpu_finch_features.h
--- a/gpu/config/gpu_finch_features.h
+++ b/gpu/config/gpu_finch_features.h
@@ -17,6 +17,7 @@ namespace features {
 GPU_EXPORT BASE_DECLARE_FEATURE(kUseGles2ForOopR);
 
 // All features in alphabetical order. The features should be documented
+GPU_EXPORT extern const base::Feature kAImageReaderMediaPlayer;
 // alongside the definition of their values in the .cc file.
 #if BUILDFLAG(IS_ANDROID)
 GPU_EXPORT BASE_DECLARE_FEATURE(kAndroidSurfaceControl);
diff --git a/gpu/config/gpu_util.cc b/gpu/config/gpu_util.cc
--- a/gpu/config/gpu_util.cc
+++ b/gpu/config/gpu_util.cc
@@ -121,6 +121,9 @@ GpuFeatureStatus GetAndroidSurfaceControlFeatureStatus(
 #if !BUILDFLAG(IS_ANDROID)
   return kGpuFeatureStatusDisabled;
 #else
+  if (blocklisted_features.count(GPU_FEATURE_TYPE_ANDROID_SURFACE_CONTROL))
+    return kGpuFeatureStatusBlocklisted;
+
   if (!gpu_preferences.enable_android_surface_control)
     return kGpuFeatureStatusDisabled;
 
@@ -344,6 +347,11 @@ void AdjustGpuFeatureStatusToWorkarounds(GpuFeatureInfo* gpu_feature_info) {
     gpu_feature_info->status_values[GPU_FEATURE_TYPE_CANVAS_OOP_RASTERIZATION] =
         kGpuFeatureStatusBlocklisted;
   }
+
+  if (gpu_feature_info->IsWorkaroundEnabled(DISABLE_AIMAGEREADER)) {
+    gpu_feature_info->status_values[GPU_FEATURE_TYPE_ANDROID_SURFACE_CONTROL] =
+        kGpuFeatureStatusBlocklisted;
+  }
 }
 
 // Estimates roughly user total disk space by counting in the drives where
diff --git a/gpu/config/gpu_workaround_list.txt b/gpu/config/gpu_workaround_list.txt
--- a/gpu/config/gpu_workaround_list.txt
+++ b/gpu/config/gpu_workaround_list.txt
@@ -15,6 +15,7 @@ decode_encode_srgb_for_generatemipmap
 depth_stencil_renderbuffer_resize_emulation
 disable_2d_canvas_auto_flush
 disable_accelerated_av1_decode
+disable_aimagereader
 disable_accelerated_av1_encode
 disable_accelerated_h264_encode
 disable_accelerated_hevc_decode
diff --git a/gpu/ipc/service/gpu_init.cc b/gpu/ipc/service/gpu_init.cc
--- a/gpu/ipc/service/gpu_init.cc
+++ b/gpu/ipc/service/gpu_init.cc
@@ -626,6 +626,11 @@ bool GpuInit::InitializeAndStartSandbox(base::CommandLine* command_line,
   }
 #endif  // BUILDFLAG(IS_WIN)
 
+  // Disable AImageReader if the workaround is enabled.
+  if (gpu_feature_info_.IsWorkaroundEnabled(DISABLE_AIMAGEREADER)) {
+    base::android::AndroidImageReader::DisableSupport();
+  }
+
   if (gpu_feature_info_.status_values[GPU_FEATURE_TYPE_VULKAN] !=
           kGpuFeatureStatusEnabled ||
       !InitializeVulkan()) {
diff --git a/gpu/ipc/service/stream_texture_android.cc b/gpu/ipc/service/stream_texture_android.cc
--- a/gpu/ipc/service/stream_texture_android.cc
+++ b/gpu/ipc/service/stream_texture_android.cc
@@ -6,6 +6,7 @@
 
 #include <string.h>
 
+#include "base/android/android_image_reader_compat.h"
 #include "base/android/scoped_hardware_buffer_fence_sync.h"
 #include "base/bind.h"
 #include "base/feature_list.h"
@@ -51,7 +52,15 @@ std::unique_ptr<ui::ScopedMakeCurrent> MakeCurrent(
 }
 
 TextureOwner::Mode GetTextureOwnerMode() {
-  return features::IsAImageReaderEnabled()
+  const bool a_image_reader_supported =
+      base::android::AndroidImageReader::GetInstance().IsSupported();
+
+  // TODO(vikassoni) : Currently we have 2 different flags to enable/disable
+  // AImageReader - one for MCVD and other for MediaPlayer here. Merge those 2
+  // flags into a single flag. Keeping the 2 flags separate for now since finch
+  // experiment using this flag is in progress.
+  return a_image_reader_supported && features::IsAImageReaderEnabled() &&
+             base::FeatureList::IsEnabled(features::kAImageReaderMediaPlayer)
              ? TextureOwner::Mode::kAImageReaderInsecure
              : TextureOwner::Mode::kSurfaceTextureInsecure;
 }
diff --git a/media/base/media_switches.cc b/media/base/media_switches.cc
--- a/media/base/media_switches.cc
+++ b/media/base/media_switches.cc
@@ -750,6 +750,10 @@ BASE_FEATURE(kHardwareSecureDecryptionExperiment,
 // Allows automatically disabling hardware secure Content Decryption Module
 // (CDM) after failures or crashes to fallback to software secure CDMs. If this
 // feature is disabled, the fallback will never happen and users could be stuck
+// Enables the Android Image Reader path for Video decoding(for AVDA and MCVD)
+const base::Feature kAImageReaderVideoOutput{"AImageReaderVideoOutput",
+                                             base::FEATURE_ENABLED_BY_DEFAULT};
+
 // in playback failures.
 BASE_FEATURE(kHardwareSecureDecryptionFallback,
              "HardwareSecureDecryptionFallback",
diff --git a/media/base/media_switches.h b/media/base/media_switches.h
--- a/media/base/media_switches.h
+++ b/media/base/media_switches.h
@@ -232,6 +232,7 @@ MEDIA_EXPORT BASE_DECLARE_FEATURE(kVaapiVp8TemporalLayerHWEncoding);
 MEDIA_EXPORT BASE_DECLARE_FEATURE(kVaapiVp9kSVCHWEncoding);
 #endif  // defined(ARCH_CPU_X86_FAMILY) && BUILDFLAG(IS_CHROMEOS)
 MEDIA_EXPORT BASE_DECLARE_FEATURE(kVideoBlitColorAccuracy);
+MEDIA_EXPORT extern const base::Feature kAImageReaderVideoOutput;
 MEDIA_EXPORT BASE_DECLARE_FEATURE(kVp9kSVCHWDecoding);
 MEDIA_EXPORT BASE_DECLARE_FEATURE(kWakeLockOptimisationHiddenMuted);
 MEDIA_EXPORT BASE_DECLARE_FEATURE(kWebContentsCaptureHiDpi);
--
2.25.1
