Install callbacks for the AVDA sync init path.

Previously, we didn't bother to install callbacks for the sync
init path for |surface_chooser_|, because there wasn't any need
for the chooser to choose anything.  However, now that we request
compositor feedback for overlay promotion, we still might ask the
chooser to choose.  When it does, it'll crash without a callback.

This CL installs the callbacks so that the chooser is in a good
state even in the sync path.  Then, it doesn't matter if we ask it
questions, since it will (correctly) answer "use SurfaceTexture".
It also adds a check to the appropriate unit test.

Bug: 772899, 772613
Cq-Include-Trybots: master.tryserver.chromium.android:android_optional_gpu_tests_rel;master.tryserver.chromium.linux:linux_optional_gpu_tests_rel;master.tryserver.chromium.mac:mac_optional_gpu_tests_rel;master.tryserver.chromium.win:win_optional_gpu_tests_rel
Change-Id: Ie0eb6bf5febcdb21119b734d3f2483e4d5aa9d70
Reviewed-on: https://chromium-review.googlesource.com/706250
Commit-Queue: Frank Liberato <[email protected]>
Reviewed-by: Thomas Guilbert <[email protected]>
Cr-Commit-Position: refs/heads/master@{#507441}

Author: liberato-at-chromium

media/gpu/android/android_video_decode_accelerator.cc

@@ -409,6 +409,12 @@ void AndroidVideoDecodeAccelerator::StartSurfaceChooser() {
 
   surface_chooser_state_.is_fullscreen = config_.overlay_info.is_fullscreen;
 
+  surface_chooser_->SetClientCallbacks(
+      base::Bind(&AndroidVideoDecodeAccelerator::OnSurfaceTransition,
+                 weak_this_factory_.GetWeakPtr()),
+      base::Bind(&AndroidVideoDecodeAccelerator::OnSurfaceTransition,
+                 weak_this_factory_.GetWeakPtr(), nullptr));
+
   // Handle the sync path, which must use SurfaceTexture anyway.  Note that we
   // check both |during_initialize_| and |deferred_initialization_pending_|,
   // since we might get here during deferred surface creation.  In that case,
@@ -426,6 +432,9 @@ void AndroidVideoDecodeAccelerator::StartSurfaceChooser() {
   if (during_initialize_ && !deferred_initialization_pending_) {
     DCHECK(!config_.overlay_info.HasValidSurfaceId());
     DCHECK(!config_.overlay_info.HasValidRoutingToken());
+    // Note that we might still send feedback to |surface_chooser_|, which might
+    // call us back.  However, it will only ever tell us to use SurfaceTexture,
+    // since we have no overlay factory anyway.
     OnSurfaceTransition(nullptr);
     return;
   }
@@ -448,11 +457,6 @@ void AndroidVideoDecodeAccelerator::StartSurfaceChooser() {
   // the synchronous case.  It will be soon, though.  For pre-M, we rely on the
   // fact that |surface_chooser_| won't tell us to use a SurfaceTexture while
   // waiting for an overlay to become ready, for example.
-  surface_chooser_->SetClientCallbacks(
-      base::Bind(&AndroidVideoDecodeAccelerator::OnSurfaceTransition,
-                 weak_this_factory_.GetWeakPtr()),
-      base::Bind(&AndroidVideoDecodeAccelerator::OnSurfaceTransition,
-                 weak_this_factory_.GetWeakPtr(), nullptr));
   surface_chooser_->UpdateState(std::move(factory), surface_chooser_state_);
 }
 

media/gpu/android/android_video_decode_accelerator_unittest.cc

@@ -237,7 +237,11 @@ TEST_F(AndroidVideoDecodeAcceleratorTest,
   config_.is_deferred_initialization_allowed = false;
 
   EXPECT_CALL(*codec_allocator_, MockCreateMediaCodecSync(_, _));
+  // AVDA must set client callbacks even in sync mode, so that the chooser is
+  // in a sane state.  crbug.com/772899 .
+  EXPECT_CALL(*chooser_, MockSetClientCallbacks());
   ASSERT_TRUE(InitializeAVDA());
+  testing::Mock::VerifyAndClearExpectations(chooser_);
 }
 
 TEST_F(AndroidVideoDecodeAcceleratorTest, FailingToCreateACodecSyncIsAnError) {