Add unit tests for VideoEffects[Service/Processor]Impl stubs

Other changes:
- switch VideoEffectsServiceImpl to accept an instance of
  VizGpuChannelHostProvider to facilitate writing unit tests

Change-Id: Ie40ff0691356daa62e1aefe2afa6fa4501552dee
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5357701
Reviewed-by: Bryant Chandler <bryantchandler@chromium.org>
Auto-Submit: Piotr Bialecki <bialpio@chromium.org>
Commit-Queue: Victor Miura <vmiura@chromium.org>
Reviewed-by: Tom Sepez <tsepez@chromium.org>
Reviewed-by: Victor Miura <vmiura@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1272875}

content/utility/services.cc

@@ -348,6 +348,22 @@ auto RunVideoCapture(
 }
 
 #if BUILDFLAG(ENABLE_VIDEO_EFFECTS)
+
+class VizGpuChannelHostProvider : public video_effects::GpuChannelHostProvider {
+ public:
+  explicit VizGpuChannelHostProvider(std::unique_ptr<viz::Gpu> viz_gpu)
+      : viz_gpu_(std::move(viz_gpu)) {
+    CHECK(viz_gpu_);
+  }
+
+  scoped_refptr<gpu::GpuChannelHost> GetGpuChannelHost() override {
+    return viz_gpu_->GetGpuChannel();
+  }
+
+ private:
+  std::unique_ptr<viz::Gpu> viz_gpu_;
+};
+
 auto RunVideoEffects(
     mojo::PendingReceiver<video_effects::mojom::VideoEffectsService> receiver) {
   if (base::FeatureList::IsEnabled(media::kCameraMicEffects)) {
@@ -358,7 +374,8 @@ auto RunVideoEffects(
         std::move(remote_gpu), UtilityThread::Get()->GetIOTaskRunner());
 
     return std::make_unique<video_effects::VideoEffectsServiceImpl>(
-        std::move(receiver), std::move(viz_gpu));
+        std::move(receiver),
+        std::make_unique<VizGpuChannelHostProvider>(std::move(viz_gpu)));
   }
 
   return std::unique_ptr<video_effects::VideoEffectsServiceImpl>{};

services/BUILD.gn

@@ -5,6 +5,7 @@
 import("//build/config/features.gni")
 import("//build/config/ui.gni")
 import("//services/screen_ai/buildflags/features.gni")
+import("//services/video_effects/args.gni")
 import("//testing/test.gni")
 
 if (is_android) {
@@ -104,6 +105,10 @@ test("services_unittests") {
     deps += [ "//services/video_capture:tests" ]
   }
 
+  if (enable_video_effects) {
+    deps += [ "//services/video_effects:tests" ]
+  }
+
   if (is_fuchsia) {
     additional_manifest_fragments = [
       "//build/config/fuchsia/test/mark_vmo_executable.shard.test-cml",

services/video_effects/BUILD.gn

@@ -16,7 +16,10 @@ source_set("service") {
     "video_effects_service_impl.cc",
   ]
 
-  visibility = [ "//content/utility:utility" ]
+  visibility = [
+    "//content/utility:utility",
+    "//services/video_effects:tests",
+  ]
 
   public_deps = [
     "//services/video_effects/public/mojom:mojom",
@@ -31,3 +34,19 @@ buildflag_header("buildflags") {
 
   flags = [ "ENABLE_VIDEO_EFFECTS=$enable_video_effects" ]
 }
+
+source_set("tests") {
+  testonly = true
+
+  sources = [
+    "video_effects_processor_impl_unittest.cc",
+    "video_effects_service_impl_unittest.cc",
+  ]
+
+  deps = [
+    "//base/test:test_support",
+    "//media/capture/mojom:video_effects_manager",
+    "//services/video_effects:service",
+    "//testing/gtest",
+  ]
+}

services/video_effects/DEPS

@@ -1,4 +1,6 @@
 include_rules = [
+  "+gpu/command_buffer/common",
+  "+gpu/ipc/client",
   "+media/base",
   "+media/capture/mojom",
   "+services/viz/public/cpp/gpu",

services/video_effects/video_effects_processor_impl_unittest.cc

@@ -0,0 +1,67 @@
+// Copyright 2024 The Chromium Authors
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include <optional>
+
+#include "services/video_effects/video_effects_processor_impl.h"
+
+#include "base/test/task_environment.h"
+#include "base/test/test_future.h"
+#include "gpu/command_buffer/common/mailbox_holder.h"
+#include "media/capture/mojom/video_capture_buffer.mojom.h"
+#include "media/capture/mojom/video_effects_manager.mojom.h"
+#include "mojo/public/cpp/bindings/pending_receiver.h"
+#include "mojo/public/cpp/bindings/receiver.h"
+#include "mojo/public/cpp/bindings/receiver_set.h"
+#include "services/video_effects/public/mojom/video_effects_processor.mojom.h"
+#include "testing/gmock/include/gmock/gmock.h"
+#include "testing/gtest/include/gtest/gtest.h"
+
+namespace video_effects {
+
+using testing::_;
+
+namespace {
+
+// Helper, returns dummy (but valid) VideoBufferHandlePtr.
+media::mojom::VideoBufferHandlePtr GetDummyVideoBufferHandle() {
+  return media::mojom::VideoBufferHandle::NewMailboxHandles(
+      media::mojom::MailboxBufferHandleSet::New(
+          std::vector<gpu::MailboxHolder>(4)));
+}
+
+class VideoEffectsProcessorTest : public testing::Test {
+  void SetUp() override {
+    processor_impl_.emplace(manager_receiver_.InitWithNewPipeAndPassRemote(),
+                            processor_remote_.BindNewPipeAndPassReceiver());
+  }
+
+ protected:
+  base::test::TaskEnvironment task_environment_;
+
+  // Processor under test (remote and impl):
+  mojo::Remote<mojom::VideoEffectsProcessor> processor_remote_;
+  std::optional<VideoEffectsProcessorImpl> processor_impl_;
+
+  mojo::PendingReceiver<media::mojom::VideoEffectsManager> manager_receiver_;
+};
+
+TEST_F(VideoEffectsProcessorTest, PostProcessRunsAndFails) {
+  // For now, since `VideoEffectsProcessorImpl` is still a stub, we expect a
+  // call to `VideoEffectsProcessor::PostProcess()` to fail.
+
+  base::test::TestFuture<mojom::PostProcessResultPtr> post_process_result;
+
+  processor_remote_->PostProcess(
+      GetDummyVideoBufferHandle(), media::mojom::VideoFrameInfo::New(),
+      GetDummyVideoBufferHandle(), media::VideoPixelFormat::PIXEL_FORMAT_I420,
+      post_process_result.GetCallback());
+
+  mojom::PostProcessResultPtr result = post_process_result.Take();
+  EXPECT_TRUE(result->is_error());
+}
+
+}  // namespace
+
+}  // namespace video_effects

services/video_effects/video_effects_service_impl.cc

@@ -19,9 +19,10 @@ namespace video_effects {
 
 VideoEffectsServiceImpl::VideoEffectsServiceImpl(
     mojo::PendingReceiver<mojom::VideoEffectsService> receiver,
-    std::unique_ptr<viz::Gpu> viz_gpu)
-    : receiver_(this, std::move(receiver)), viz_gpu_(std::move(viz_gpu)) {
-  CHECK(viz_gpu_);
+    std::unique_ptr<GpuChannelHostProvider> gpu_channel_host_provider)
+    : receiver_(this, std::move(receiver)),
+      gpu_channel_host_provider_(std::move(gpu_channel_host_provider)) {
+  CHECK(gpu_channel_host_provider_);
 }
 
 VideoEffectsServiceImpl::~VideoEffectsServiceImpl() = default;

services/video_effects/video_effects_service_impl.h

@@ -8,6 +8,8 @@
 #include <memory>
 #include <vector>
 
+#include "base/memory/scoped_refptr.h"
+#include "gpu/ipc/client/gpu_channel_host.h"
 #include "media/capture/mojom/video_effects_manager.mojom-forward.h"
 #include "mojo/public/cpp/bindings/pending_receiver.h"
 #include "mojo/public/cpp/bindings/pending_remote.h"
@@ -15,24 +17,32 @@
 #include "services/video_effects/public/mojom/video_effects_processor.mojom-forward.h"
 #include "services/video_effects/public/mojom/video_effects_service.mojom.h"
 
-namespace viz {
-class Gpu;
-}
-
 namespace video_effects {
 
 class VideoEffectsProcessorImpl;
 
+// Abstract interface that is used by `VideoEffectsServiceImpl` to obtain
+// instances of `gpu::GpuChannelHost`. Those are then going to be used to
+// create context providers over which the communication to GPU service will
+// happen.
+class GpuChannelHostProvider {
+ public:
+  virtual ~GpuChannelHostProvider() = default;
+
+  virtual scoped_refptr<gpu::GpuChannelHost> GetGpuChannelHost() = 0;
+};
+
 class VideoEffectsServiceImpl : public mojom::VideoEffectsService {
  public:
   // Similarly to `VideoCaptureServiceImpl`, `VideoEfffectsServiceImpl` needs
-  // to receive `viz::Gpu` instance in order to be able to communicate with the
-  // GPU service. This is passed in via `viz_gpu`.
+  // to receive something that returns `gpu::GpuChannelHost` instances in order
+  // to be able to communicate with the GPU service - this is passed in via the
+  // `gpu_channel_host_provider`.
   // `receiver` is the receiving end of the mojo pipe used to communicate with
   // this instance.
   explicit VideoEffectsServiceImpl(
       mojo::PendingReceiver<mojom::VideoEffectsService> receiver,
-      std::unique_ptr<viz::Gpu> viz_gpu);
+      std::unique_ptr<GpuChannelHostProvider> gpu_channel_host_provider);
 
   ~VideoEffectsServiceImpl() override;
 
@@ -45,7 +55,7 @@ class VideoEffectsServiceImpl : public mojom::VideoEffectsService {
   std::vector<std::unique_ptr<VideoEffectsProcessorImpl>> processors_;
 
   mojo::Receiver<mojom::VideoEffectsService> receiver_;
-  std::unique_ptr<viz::Gpu> viz_gpu_;
+  std::unique_ptr<GpuChannelHostProvider> gpu_channel_host_provider_;
 };
 
 }  // namespace video_effects

services/video_effects/video_effects_service_impl_unittest.cc

@@ -0,0 +1,67 @@
+// Copyright 2024 The Chromium Authors
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include <optional>
+
+#include "services/video_effects/video_effects_service_impl.h"
+
+#include "base/test/task_environment.h"
+#include "mojo/public/cpp/bindings/pending_receiver.h"
+#include "mojo/public/cpp/bindings/receiver.h"
+#include "mojo/public/cpp/bindings/receiver_set.h"
+#include "services/video_effects/public/mojom/video_effects_processor.mojom.h"
+#include "services/video_effects/public/mojom/video_effects_service.mojom.h"
+#include "testing/gmock/include/gmock/gmock.h"
+#include "testing/gtest/include/gtest/gtest.h"
+
+namespace video_effects {
+
+using testing::_;
+
+namespace {
+
+class TestGpuChannelHostProvider : public GpuChannelHostProvider {
+ public:
+  TestGpuChannelHostProvider() = default;
+
+  scoped_refptr<gpu::GpuChannelHost> GetGpuChannelHost() override {
+    return nullptr;
+  }
+};
+
+}  // namespace
+
+class VideoEffectsServiceTest : public testing::Test {
+  void SetUp() override {
+    service_impl_.emplace(service_remote_.BindNewPipeAndPassReceiver(),
+                          std::make_unique<TestGpuChannelHostProvider>());
+  }
+
+ protected:
+  base::test::TaskEnvironment task_environment_;
+
+  // Service under test (remote and impl):
+  mojo::Remote<mojom::VideoEffectsService> service_remote_;
+  std::optional<VideoEffectsServiceImpl> service_impl_;
+};
+
+TEST_F(VideoEffectsServiceTest, CreateEffectsProcessorWorks) {
+  // Calling into `VideoEffectsService:::CreateEffectsProcessor()` is expected
+  // to work (irrespective of whether the passed-in pipes are usable or not).
+
+  base::RunLoop run_loop;
+
+  mojo::PendingReceiver<media::mojom::VideoEffectsManager> manager_receiver;
+  mojo::Remote<mojom::VideoEffectsProcessor> processor_remote;
+
+  service_remote_->CreateEffectsProcessor(
+      manager_receiver.InitWithNewPipeAndPassRemote(),
+      processor_remote.BindNewPipeAndPassReceiver());
+
+  run_loop.RunUntilIdle();
+
+  EXPECT_TRUE(processor_remote.is_connected());
+}
+
+}  // namespace video_effects