Resolve MediaCapabilities::decodingInfo promise even if mojo connection dies.

Promise for MediaCapabilities::decodingInfo() is resolved by
blink::WebMediaCapabilitiesQueryCallbacks which is passed to
WebMediaCapabilitiesClientImpl::DecodingInfo in //media/blink,
which, in some cases, means to resolve it in the callback
of an async mojo call. However, if the connections is lost meanwhile,
neither OnSuccess nor OnError is called and, in the result,
the promise resolver remains in pending state.

This patch makes sure the OnError is called when mojo connection
gets dropped.

Bug: 847211
Change-Id: I018792c56a3d3074b042e4318d9a8b85b3c16360
Reviewed-on: https://chromium-review.googlesource.com/1075308Reviewed-by: Kentaro Hara <haraken@chromium.org>
Reviewed-by: Chrome Cunningham <chcunningham@chromium.org>
Reviewed-by: Mounir Lamouri <mlamouri@chromium.org>
Commit-Queue: Chrome Cunningham <chcunningham@chromium.org>
Cr-Commit-Position: refs/heads/master@{#567874}

media/blink/BUILD.gn

@@ -158,6 +158,7 @@ test("media_blink_unittests") {
     "video_frame_compositor_unittest.cc",
     "watch_time_reporter_unittest.cc",
     "webaudiosourceprovider_impl_unittest.cc",
+    "webmediacapabilitiesclient_impl_unittest.cc",
     "webmediaplayer_impl_unittest.cc",
   ]
 

media/blink/webmediacapabilitiesclient_impl.cc

@@ -140,16 +140,33 @@ WebMediaCapabilitiesClientImpl::WebMediaCapabilitiesClientImpl() = default;
 
 WebMediaCapabilitiesClientImpl::~WebMediaCapabilitiesClientImpl() = default;
 
+namespace {
+// This structs wraps WebMediaCapabilitiesQueryCallbacks and makes sure
+// they are called even when the mojo response is not received
+// (connection error) as there is a pending promise waiting for the callback.
+// See https://crbug.com/847211
+struct CallbacksHolder {
+  std::unique_ptr<blink::WebMediaCapabilitiesQueryCallbacks> callbacks;
+
+  CallbacksHolder(CallbacksHolder&&) = default;
+  ~CallbacksHolder() {
+    if (callbacks) {
+      callbacks->OnError();
+    }
+  }
+};
+
 void VideoPerfInfoCallback(
-    std::unique_ptr<blink::WebMediaCapabilitiesQueryCallbacks> callbacks,
+    CallbacksHolder callbacks_holder,
     std::unique_ptr<blink::WebMediaCapabilitiesInfo> info,
     bool is_smooth,
     bool is_power_efficient) {
   DCHECK(info->supported);
   info->smooth = is_smooth;
   info->power_efficient = is_power_efficient;
-  callbacks->OnSuccess(std::move(info));
+  std::move(callbacks_holder.callbacks)->OnSuccess(std::move(info));
 }
+}  // namespace
 
 void WebMediaCapabilitiesClientImpl::DecodingInfo(
     const blink::WebMediaConfiguration& configuration,
@@ -209,8 +226,13 @@ void WebMediaCapabilitiesClientImpl::DecodingInfo(
 
   decode_history_ptr_->GetPerfInfo(
       std::move(features),
-      base::BindOnce(&VideoPerfInfoCallback, std::move(callbacks),
-                     std::move(info)));
+      base::BindOnce(&VideoPerfInfoCallback,
+                     CallbacksHolder{std::move(callbacks)}, std::move(info)));
+}
+
+void WebMediaCapabilitiesClientImpl::BindVideoDecodePerfHistoryForTests(
+    mojom::VideoDecodePerfHistoryPtr decode_history_ptr) {
+  decode_history_ptr_ = std::move(decode_history_ptr);
 }
 
 }  // namespace media

media/blink/webmediacapabilitiesclient_impl.h

@@ -24,6 +24,9 @@ class MEDIA_BLINK_EXPORT WebMediaCapabilitiesClientImpl
       const blink::WebMediaConfiguration&,
       std::unique_ptr<blink::WebMediaCapabilitiesQueryCallbacks>) override;
 
+  void BindVideoDecodePerfHistoryForTests(
+      mojom::VideoDecodePerfHistoryPtr decode_history_ptr);
+
  private:
   mojom::VideoDecodePerfHistoryPtr decode_history_ptr_;
 

media/blink/webmediacapabilitiesclient_impl_unittest.cc

+// Copyright 2018 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include <memory>
+
+#include "base/run_loop.h"
+#include "base/test/scoped_task_environment.h"
+#include "media/blink/webmediacapabilitiesclient_impl.h"
+#include "media/mojo/interfaces/video_decode_perf_history.mojom.h"
+#include "mojo/public/cpp/bindings/binding.h"
+#include "testing/gmock/include/gmock/gmock.h"
+#include "testing/gtest/include/gtest/gtest.h"
+#include "third_party/blink/public/platform/modules/media_capabilities/web_media_capabilities_info.h"
+#include "third_party/blink/public/platform/modules/media_capabilities/web_media_configuration.h"
+
+using ::testing::_;
+
+namespace media {
+
+class MockVideoDecodePerfHistory : public mojom::VideoDecodePerfHistory {
+ public:
+  explicit MockVideoDecodePerfHistory(
+      mojom::VideoDecodePerfHistoryPtr* decode_perf_history_ptr)
+      : binding_(this, mojo::MakeRequest(decode_perf_history_ptr)) {}
+
+  MOCK_METHOD2(GetPerfInfo,
+               void(mojom::PredictionFeaturesPtr, GetPerfInfoCallback));
+
+  void CloseMojoBinding() { binding_.Close(); }
+
+ private:
+  mojo::Binding<mojom::VideoDecodePerfHistory> binding_;
+};
+
+class MockWebMediaCapabilitiesQueryCallbacks
+    : public blink::WebMediaCapabilitiesQueryCallbacks {
+ public:
+  ~MockWebMediaCapabilitiesQueryCallbacks() override = default;
+
+  void OnSuccess(std::unique_ptr<blink::WebMediaCapabilitiesInfo>) override {}
+  MOCK_METHOD0(OnError, void());
+};
+
+// Verify that query callback is called even if mojo connection is lost while
+// waiting for the result of mojom.VideoDecodePerfHistory.GetPerfInfo() call.
+// See https://crbug.com/847211
+TEST(WebMediaCapabilitiesClientImplTest, RunCallbackEvenIfMojoDisconnects) {
+  static const blink::WebVideoConfiguration kFakeVideoConfiguration{
+      blink::WebString::FromASCII("video/webm"),                 // mime type
+      blink::WebString::FromASCII("vp09.00.51.08.01.01.01.01"),  // codec
+      1920,                                                      // width
+      1080,                                                      // height
+      2661034,                                                   // bitrate
+      25,                                                        // framerate
+  };
+
+  static const blink::WebMediaConfiguration kFakeMediaConfiguration{
+      blink::MediaConfigurationType::kFile,
+      base::nullopt,            // audio configuration
+      kFakeVideoConfiguration,  // video configuration
+  };
+
+  using ::testing::InvokeWithoutArgs;
+
+  mojom::VideoDecodePerfHistoryPtr decode_perf_history_ptr;
+  MockVideoDecodePerfHistory decode_perf_history_impl(&decode_perf_history_ptr);
+
+  ASSERT_TRUE(decode_perf_history_ptr.is_bound());
+
+  WebMediaCapabilitiesClientImpl media_capabilities_client_impl;
+  media_capabilities_client_impl.BindVideoDecodePerfHistoryForTests(
+      std::move(decode_perf_history_ptr));
+
+  auto query_callbacks =
+      std::make_unique<MockWebMediaCapabilitiesQueryCallbacks>();
+
+  EXPECT_CALL(decode_perf_history_impl, GetPerfInfo(_, _))
+      .WillOnce(
+          InvokeWithoutArgs(&decode_perf_history_impl,
+                            &MockVideoDecodePerfHistory::CloseMojoBinding));
+
+  EXPECT_CALL(*query_callbacks, OnError());
+
+  media_capabilities_client_impl.DecodingInfo(kFakeMediaConfiguration,
+                                              std::move(query_callbacks));
+
+  base::RunLoop().RunUntilIdle();
+}
+
+}  // namespace media