Reland "Expose effects processor from MediaEffectsService up to content client"

This is a reland of commit fd87acad32

Original CL was failing in CI because of a missing dependency from
content/browser:browser to services/video_effects/public/mojom:mojom
target (now required because of a transitive include). This was
missing because content/brower:browser does not depend on the target
where the public_deps entry was added. The reland addresses it by
explicitly adding the dependency, compare PS1 and PS2 to see the
change.

Original change's description:
> Expose effects processor from MediaEffectsService up to content client
>
> Additionally, add unit tests for VideoEffectsProcessor that mostly
> mimic the tests that have been written for Video Effects Manager. This
> also includes a basic fake implementation of VideoEffectsService and
> VideoEffectsProcessor.
>
> MediaEffectsService header file also now exposes a way to override the
> VideoEffectsService for testing, as otherwise it'd have to spawn a new
> utility process, and this is not desired in unittests.
>
> Bug: b:328118837
> Change-Id: Id096ecb09e0059d660896d743b80c991915dc4ff
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5353891
> Reviewed-by: Bryant Chandler <bryantchandler@chromium.org>
> Reviewed-by: Alex Moshchuk <alexmos@chromium.org>
> Commit-Queue: Piotr Bialecki <bialpio@chromium.org>
> Reviewed-by: Daniel Cheng <dcheng@chromium.org>
> Cr-Commit-Position: refs/heads/main@{#1278592}

Bug: b:328118837
Change-Id: I7373387ed8478a3df654196c88b831421e82709d
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5401456
Commit-Queue: Piotr Bialecki <bialpio@chromium.org>
Reviewed-by: Alex Moshchuk <alexmos@chromium.org>
Reviewed-by: Daniel Cheng <dcheng@chromium.org>
Reviewed-by: Theresa Sullivan <twellington@chromium.org>
Reviewed-by: Bryant Chandler <bryantchandler@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1279231}

chrome/browser/DEPS

@@ -466,6 +466,8 @@ include_rules = [
   "+services/strings",
   "+services/tracing/public/cpp",
   "+services/video_capture/public",
+  "+services/video_effects/public",
+  "+services/video_effects/test",
   "+services/viz/public",
   "+services/viz/privileged",
   "+skia/ext",

chrome/browser/chrome_content_browser_client.cc

@@ -375,6 +375,7 @@
 #include "services/network/public/cpp/web_sandbox_flags.h"
 #include "services/network/public/mojom/network_service.mojom.h"
 #include "services/network/public/mojom/web_transport.mojom.h"
+#include "services/video_effects/public/mojom/video_effects_processor.mojom-forward.h"
 #include "third_party/blink/public/common/features.h"
 #include "third_party/blink/public/common/loader/url_loader_throttle.h"
 #include "third_party/blink/public/common/navigation/navigation_policy.h"
@@ -8237,6 +8238,15 @@ void ChromeContentBrowserClient::BindVideoEffectsManager(
   media_effects::BindVideoEffectsManager(device_id, browser_context,
                                          std::move(video_effects_manager));
 }
+
+void ChromeContentBrowserClient::BindVideoEffectsProcessor(
+    const std::string& device_id,
+    content::BrowserContext* browser_context,
+    mojo::PendingReceiver<video_effects::mojom::VideoEffectsProcessor>
+        video_effects_processor) {
+  media_effects::BindVideoEffectsProcessor(device_id, browser_context,
+                                           std::move(video_effects_processor));
+}
 #endif  // !BUILDFLAG(IS_ANDROID)
 
 void ChromeContentBrowserClient::PreferenceRankAudioDeviceInfos(

chrome/browser/chrome_content_browser_client.h

@@ -39,6 +39,7 @@
 #include "mojo/public/cpp/bindings/pending_receiver.h"
 #include "mojo/public/cpp/bindings/pending_remote.h"
 #include "services/metrics/public/cpp/ukm_source_id.h"
+#include "services/video_effects/public/mojom/video_effects_processor.mojom-forward.h"
 #include "third_party/blink/public/mojom/worker/shared_worker_info.mojom.h"
 
 class ChromeContentBrowserClientParts;
@@ -996,6 +997,12 @@ class ChromeContentBrowserClient : public content::ContentBrowserClient {
       content::BrowserContext* browser_context,
       mojo::PendingReceiver<media::mojom::VideoEffectsManager>
           video_effects_manager) override;
+
+  void BindVideoEffectsProcessor(
+      const std::string& device_id,
+      content::BrowserContext* browser_context,
+      mojo::PendingReceiver<video_effects::mojom::VideoEffectsProcessor>
+          video_effects_processor) override;
 #endif  // !BUILDFLAG(IS_ANDROID)
 
   void PreferenceRankAudioDeviceInfos(

chrome/browser/chrome_content_browser_client_unittest.cc

@@ -77,6 +77,8 @@
 #include "net/test/cert_test_util.h"
 #include "net/test/test_data_directory.h"
 #include "services/network/test/test_network_context.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"
 #include "third_party/blink/public/common/switches.h"
@@ -90,7 +92,12 @@
 #include "chrome/common/pref_names.h"
 #include "chrome/test/base/browser_with_test_window_test.h"
 #include "chrome/test/base/search_test_utils.h"
+// `gn check` is failing on Android for this particular include even though
+// the (conditional) dependency path exists, adding `nogncheck`:
+#include "components/media_effects/media_effects_service.h"  // nogncheck
 #include "components/password_manager/core/common/password_manager_features.h"
+// Ditto:
+#include "services/video_effects/test/fake_video_effects_service.h"  // nogncheck
 #include "ui/base/page_transition_types.h"
 #else
 #include "base/system/sys_info.h"
@@ -630,6 +637,26 @@ TEST_F(ChromeContentBrowserClientTest, BindVideoEffectsManager) {
   // result means that the plumbing worked.
   EXPECT_FALSE(configuration_future.Get().is_null());
 }
+
+TEST_F(ChromeContentBrowserClientTest, BindVideoEffectsProcessor) {
+  mojo::Remote<video_effects::mojom::VideoEffectsService> service;
+  video_effects::FakeVideoEffectsService fake_effects_service(
+      service.BindNewPipeAndPassReceiver());
+  auto service_reset = SetVideoEffectsServiceRemoteForTesting(&service);
+
+  std::unique_ptr<base::test::TestFuture<void>> effects_processor_future =
+      fake_effects_service.GetEffectsProcessorCreationFuture();
+
+  TestChromeContentBrowserClient test_content_browser_client;
+  mojo::Remote<video_effects::mojom::VideoEffectsProcessor>
+      video_effects_processor;
+  test_content_browser_client.BindVideoEffectsProcessor(
+      "test_device_id", &profile_,
+      video_effects_processor.BindNewPipeAndPassReceiver());
+
+  EXPECT_TRUE(effects_processor_future->Wait());
+  EXPECT_TRUE(video_effects_processor.is_connected());
+}
 #endif  // !BUILDFLAG(IS_ANDROID)
 
 TEST_F(ChromeContentBrowserClientTest, PreferenceRankAudioDeviceInfos) {

chrome/test/BUILD.gn

@@ -8285,6 +8285,7 @@ test("unit_tests") {
       "//components/lens:buildflags",
       "//components/lens:lens_mojo",
       "//components/manta",
+      "//components/media_effects:media_effects",
       "//components/media_effects/test:test_support",
       "//components/media_message_center:test_support",
       "//components/media_router/common:test_support",
@@ -8314,6 +8315,7 @@ test("unit_tests") {
       "//services/device/public/cpp/bluetooth",
       "//services/metrics/public/cpp:ukm_builders",
       "//services/network:test_support",
+      "//services/video_effects/test:test_support",
       "//third_party/crashpad/crashpad/util",
       "//third_party/libaddressinput",
       "//third_party/lzma_sdk/google:unit_tests",

components/media_effects/BUILD.gn

@@ -1,4 +1,8 @@
 source_set("media_effects") {
+  public_deps = [
+    "//media/capture/mojom:video_effects_manager",
+    "//services/video_effects/public/mojom:mojom",
+  ]
   deps = [
     "//base",
     "//components/keyed_service/content",
@@ -32,6 +36,7 @@ source_set("unit_tests") {
     "//components/prefs:test_support",
     "//components/user_prefs/test:test_support",
     "//content/test:test_support",
+    "//services/video_effects/test:test_support",
     "//testing/gtest",
     "//third_party/mediapipe",
   ]

components/media_effects/DEPS

@@ -11,6 +11,8 @@ include_rules = [
   "+services/audio/public/cpp/fake_system_info.h",
   "+services/audio/public/mojom",
   "+services/video_capture/public/mojom",
+  "+services/video_effects/public/mojom",
+  "+services/video_effects/test",
   "+third_party/mediapipe/buildflags.h",
   "+third_party/mediapipe/src",
   "+ui/gfx/geometry",

components/media_effects/README.md

@@ -1,4 +1,4 @@
 The MediaEffectsService provides the source of truth for media effect config and
 control of camera input from the Video Capture Service via browser-context-keyed effect
 managers that are able to get/set prefs and communicate configuration to
-Browser UI, contents, and the video capture service.
+Browser UI, contents, and the Video Effects Service.

components/media_effects/media_effects_manager_binder.cc

@@ -5,8 +5,10 @@
 #include "components/media_effects/media_effects_manager_binder.h"
 #include "components/media_effects/media_effects_service_factory.h"
 #include "content/public/browser/browser_thread.h"
+#include "services/video_effects/public/mojom/video_effects_processor.mojom.h"
 
 namespace {
+
 void BindVideoEffectsManagerOnUIThread(
     const std::string& device_id,
     content::BrowserContext* browser_context,
@@ -24,10 +26,45 @@ void BindVideoEffectsManagerOnUIThread(
   media_effects_service->BindVideoEffectsManager(
       device_id, std::move(video_effects_manager));
 }
+
+void BindVideoEffectsProcessorOnUIThread(
+    const std::string& device_id,
+    content::BrowserContext* browser_context,
+    mojo::PendingReceiver<video_effects::mojom::VideoEffectsProcessor>
+        video_effects_processor) {
+  DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
+  auto* media_effects_service =
+      MediaEffectsServiceFactory::GetForBrowserContext(browser_context);
+  if (!media_effects_service) {
+    LOG(WARNING) << "Video device not registered because no service was "
+                    "returned for the current BrowserContext";
+    return;
+  }
+
+  media_effects_service->BindVideoEffectsProcessor(
+      device_id, std::move(video_effects_processor));
+}
 }  // namespace
 
 namespace media_effects {
 
+void BindVideoEffectsProcessor(
+    const std::string& device_id,
+    content::BrowserContext* browser_context,
+    mojo::PendingReceiver<video_effects::mojom::VideoEffectsProcessor>
+        video_effects_processor) {
+  if (content::BrowserThread::CurrentlyOn(content::BrowserThread::UI)) {
+    BindVideoEffectsProcessorOnUIThread(device_id, browser_context,
+                                        std::move(video_effects_processor));
+    return;
+  }
+  // The function wasn't called from the UI thread, so post a task.
+  content::GetUIThreadTaskRunner()->PostTask(
+      FROM_HERE,
+      base::BindOnce(&BindVideoEffectsProcessorOnUIThread, device_id,
+                     browser_context, std::move(video_effects_processor)));
+}
+
 void BindVideoEffectsManager(
     const std::string& device_id,
     content::BrowserContext* browser_context,
@@ -44,4 +81,5 @@ void BindVideoEffectsManager(
       base::BindOnce(&BindVideoEffectsManagerOnUIThread, device_id,
                      browser_context, std::move(video_effects_manager)));
 }
+
 }  // namespace media_effects

components/media_effects/media_effects_manager_binder.h

@@ -8,6 +8,7 @@
 #include "content/public/browser/browser_context.h"
 #include "media/capture/mojom/video_effects_manager.mojom-forward.h"
 #include "mojo/public/cpp/bindings/pending_receiver.h"
+#include "services/video_effects/public/mojom/video_effects_processor.mojom-forward.h"
 
 namespace media_effects {
 
@@ -17,6 +18,12 @@ void BindVideoEffectsManager(
     mojo::PendingReceiver<media::mojom::VideoEffectsManager>
         video_effects_manager);
 
+void BindVideoEffectsProcessor(
+    const std::string& device_id,
+    content::BrowserContext* browser_context,
+    mojo::PendingReceiver<video_effects::mojom::VideoEffectsProcessor>
+        video_effects_processor);
+
 }  // namespace media_effects
 
 #endif  // COMPONENTS_MEDIA_EFFECTS_MEDIA_EFFECTS_MANAGER_BINDER_H_

components/media_effects/media_effects_manager_binder_unittest.cc

@@ -4,10 +4,14 @@
 
 #include "components/media_effects/media_effects_manager_binder.h"
 #include "base/test/test_future.h"
+#include "components/media_effects/media_effects_service.h"
 #include "components/user_prefs/test/test_browser_context_with_prefs.h"
 #include "content/public/test/browser_task_environment.h"
 #include "media/capture/mojom/video_effects_manager.mojom.h"
 #include "mojo/public/cpp/bindings/remote.h"
+#include "services/video_effects/public/mojom/video_effects_processor.mojom.h"
+#include "services/video_effects/public/mojom/video_effects_service.mojom.h"
+#include "services/video_effects/test/fake_video_effects_service.h"
 #include "testing/gtest/include/gtest/gtest.h"
 #include "ui/gfx/geometry/insets_f.h"
 
@@ -50,3 +54,25 @@ TEST_F(MediaEffectsManagerBinderTest, BindVideoEffectsManager) {
   EXPECT_EQ(kPaddingRatio, GetConfigurationSync(video_effects_manager)
                                ->framing->padding_ratios.top());
 }
+
+TEST_F(MediaEffectsManagerBinderTest, BindVideoEffectsProcessor) {
+  // Tests that `media_effects::BindVideoEffectsProcessor()` works, i.e.
+  // causes the passed in remote to be connected.
+
+  mojo::Remote<video_effects::mojom::VideoEffectsService> service;
+  video_effects::FakeVideoEffectsService fake_effects_service(
+      service.BindNewPipeAndPassReceiver());
+  auto service_reset = SetVideoEffectsServiceRemoteForTesting(&service);
+
+  auto effects_processor_future =
+      fake_effects_service.GetEffectsProcessorCreationFuture();
+
+  mojo::Remote<video_effects::mojom::VideoEffectsProcessor>
+      video_effects_processor;
+  media_effects::BindVideoEffectsProcessor(
+      "some_device_id", &browser_context_,
+      video_effects_processor.BindNewPipeAndPassReceiver());
+
+  EXPECT_TRUE(effects_processor_future->Wait());
+  EXPECT_TRUE(video_effects_processor.is_connected());
+}

components/media_effects/media_effects_service.cc

@@ -2,8 +2,49 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
+#include <optional>
+
+#include "base/auto_reset.h"
 #include "components/media_effects/media_effects_service.h"
+#include "content/public/browser/service_process_host.h"
 #include "media/capture/mojom/video_effects_manager.mojom.h"
+#include "services/video_effects/public/mojom/video_effects_processor.mojom.h"
+#include "services/video_effects/public/mojom/video_effects_service.mojom.h"
+
+namespace {
+
+static mojo::Remote<video_effects::mojom::VideoEffectsService>*
+    g_service_remote = nullptr;
+
+video_effects::mojom::VideoEffectsService* GetVideoEffectsService() {
+  if (!g_service_remote) {
+    g_service_remote =
+        new mojo::Remote<video_effects::mojom::VideoEffectsService>();
+  }
+
+  if (!g_service_remote->is_bound()) {
+    content::ServiceProcessHost::Launch(
+        g_service_remote->BindNewPipeAndPassReceiver(),
+        content::ServiceProcessHost::Options()
+            .WithDisplayName("Video Effects Service")
+            .Pass());
+
+    g_service_remote->reset_on_disconnect();
+    g_service_remote->reset_on_idle_timeout(base::Seconds(5));
+  }
+
+  return g_service_remote->get();
+}
+
+}  // namespace
+
+base::AutoReset<mojo::Remote<video_effects::mojom::VideoEffectsService>*>
+SetVideoEffectsServiceRemoteForTesting(
+    mojo::Remote<video_effects::mojom::VideoEffectsService>* service_override) {
+  return base::AutoReset<
+      mojo::Remote<video_effects::mojom::VideoEffectsService>*>(
+      &g_service_remote, service_override);
+}
 
 MediaEffectsService::MediaEffectsService(PrefService* prefs) : prefs_(prefs) {}
 
@@ -17,6 +58,22 @@ void MediaEffectsService::BindVideoEffectsManager(
   effects_manager.Bind(std::move(effects_manager_receiver));
 }
 
+void MediaEffectsService::BindVideoEffectsProcessor(
+    const std::string& device_id,
+    mojo::PendingReceiver<video_effects::mojom::VideoEffectsProcessor>
+        effects_processor_receiver) {
+  mojo::PendingRemote<media::mojom::VideoEffectsManager> video_effects_manager;
+  BindVideoEffectsManager(
+      device_id, video_effects_manager.InitWithNewPipeAndPassReceiver());
+
+  auto* video_effects_service = GetVideoEffectsService();
+  CHECK(video_effects_service);
+
+  video_effects_service->CreateEffectsProcessor(
+      device_id, std::move(video_effects_manager),
+      std::move(effects_processor_receiver));
+}
+
 VideoEffectsManagerImpl& MediaEffectsService::GetOrCreateVideoEffectsManager(
     const std::string& device_id) {
   if (auto effects_manager = video_effects_managers_.find(device_id);

components/media_effects/media_effects_service.h

@@ -5,10 +5,18 @@
 #ifndef COMPONENTS_MEDIA_EFFECTS_MEDIA_EFFECTS_SERVICE_H_
 #define COMPONENTS_MEDIA_EFFECTS_MEDIA_EFFECTS_SERVICE_H_
 
+#include "base/auto_reset.h"
 #include "components/keyed_service/core/keyed_service.h"
 #include "components/media_effects/video_effects_manager_impl.h"
 #include "content/public/browser/browser_context.h"
 #include "media/capture/mojom/video_effects_manager.mojom-forward.h"
+#include "services/video_effects/public/mojom/video_effects_processor.mojom.h"
+#include "services/video_effects/public/mojom/video_effects_service.mojom-forward.h"
+
+[[nodiscard]] base::AutoReset<
+    mojo::Remote<video_effects::mojom::VideoEffectsService>*>
+SetVideoEffectsServiceRemoteForTesting(
+    mojo::Remote<video_effects::mojom::VideoEffectsService>* service_override);
 
 class MediaEffectsService : public KeyedService {
  public:
@@ -44,6 +52,30 @@ class MediaEffectsService : public KeyedService {
       mojo::PendingReceiver<media::mojom::VideoEffectsManager>
           effects_manager_receiver);
 
+  // Connects a `VideoEffectsManagerImpl` to the provided
+  // `effects_processor_receiver`. If the keyed profile already has a manager
+  // for the passed `device_id`, then it will be used. Otherwise, a new manager
+  // will be created.
+  //
+  // The device id must be the raw string from
+  // `media::mojom::VideoCaptureDeviceDescriptor::device_id`.
+  //
+  // The manager remote will be sent to the Video Effects Service, where
+  // it will be used to subscribe to the effects configuration. The passed in
+  // pending receiver is going to be used to create a Video Effects Processor
+  // in the Video Effects Service.
+  //
+  // Note that this API does not expose the `VideoEffectsManagerImpl` in any
+  // way. If you need to interact with the manager, call
+  // `BindVideoEffectsManager()` instead.
+  //
+  // Calling this method will launch a new instance of Video Effects Service if
+  // it's not already running.
+  void BindVideoEffectsProcessor(
+      const std::string& device_id,
+      mojo::PendingReceiver<video_effects::mojom::VideoEffectsProcessor>
+          effects_processor_receiver);
+
  private:
   VideoEffectsManagerImpl& GetOrCreateVideoEffectsManager(
       const std::string& device_id);

components/media_effects/media_effects_service_unittest.cc

@@ -8,6 +8,10 @@
 #include "base/test/test_future.h"
 #include "components/user_prefs/test/test_browser_context_with_prefs.h"
 #include "content/public/test/browser_task_environment.h"
+#include "mojo/public/cpp/bindings/remote.h"
+#include "services/video_effects/public/mojom/video_effects_processor.mojom.h"
+#include "services/video_effects/public/mojom/video_effects_service.mojom-forward.h"
+#include "services/video_effects/test/fake_video_effects_service.h"
 #include "testing/gtest/include/gtest/gtest.h"
 #include "ui/gfx/geometry/insets_f.h"
 
@@ -104,6 +108,11 @@ TEST_F(
 TEST_F(
     MediaEffectsServiceTest,
     OnLastReceiverDisconnected_ErasesTheManagerWhenAllReceiversAreDisconnected) {
+  mojo::Remote<video_effects::mojom::VideoEffectsService> service;
+  video_effects::FakeVideoEffectsService fake_effects_service(
+      service.BindNewPipeAndPassReceiver());
+  auto service_reset = SetVideoEffectsServiceRemoteForTesting(&service);
+
   mojo::Remote<media::mojom::VideoEffectsManager> effects_manager1;
   service_.BindVideoEffectsManager(
       kDeviceId, effects_manager1.BindNewPipeAndPassReceiver());
@@ -111,6 +120,13 @@ TEST_F(
   service_.BindVideoEffectsManager(
       kDeviceId, effects_manager2.BindNewPipeAndPassReceiver());
 
+  auto effects_processor_future =
+      fake_effects_service.GetEffectsProcessorCreationFuture();
+
+  mojo::Remote<video_effects::mojom::VideoEffectsProcessor> effects_processor;
+  service_.BindVideoEffectsProcessor(
+      kDeviceId, effects_processor.BindNewPipeAndPassReceiver());
+
   const float kFramingPaddingRatio = 0.234;
 
   SetFramingSync(effects_manager1, kFramingPaddingRatio);
@@ -121,8 +137,18 @@ TEST_F(
   EXPECT_EQ(gfx::InsetsF{kFramingPaddingRatio},
             GetConfigurationSync(effects_manager2)->framing->padding_ratios);
 
+  // Wait for the fake effects service to create the processor:
+  EXPECT_TRUE(effects_processor_future->Wait());
+  ASSERT_EQ(fake_effects_service.GetProcessors().size(), 1u);
+  EXPECT_EQ(
+      gfx::InsetsF{kFramingPaddingRatio},
+      GetConfigurationSync(fake_effects_service.GetProcessors()[kDeviceId]
+                               ->GetVideoEffectsManager())
+          ->framing->padding_ratios);
+
   effects_manager1.reset();
   effects_manager2.reset();
+  fake_effects_service.GetProcessors().erase(kDeviceId);
   // Wait for the reset to complete
   base::RunLoop().RunUntilIdle();
 
@@ -134,3 +160,100 @@ TEST_F(
   // `VideoEffectsManager`.
   EXPECT_TRUE(GetConfigurationSync(effects_manager3)->framing.is_null());
 }
+
+TEST_F(MediaEffectsServiceTest, BindVideoEffectsProcessor) {
+  // Tests that `MediaEffectsService::BindVideoEffectsProcessor()` works, i.e.
+  // causes the passed in remote to be connected.
+
+  mojo::Remote<video_effects::mojom::VideoEffectsService> service;
+  video_effects::FakeVideoEffectsService fake_effects_service(
+      service.BindNewPipeAndPassReceiver());
+  auto service_reset = SetVideoEffectsServiceRemoteForTesting(&service);
+
+  auto effects_processor_future =
+      fake_effects_service.GetEffectsProcessorCreationFuture();
+
+  mojo::Remote<video_effects::mojom::VideoEffectsProcessor> effects_processor;
+  service_.BindVideoEffectsProcessor(
+      kDeviceId, effects_processor.BindNewPipeAndPassReceiver());
+
+  EXPECT_TRUE(effects_processor_future->Wait());
+  EXPECT_TRUE(effects_processor.is_connected());
+  EXPECT_EQ(fake_effects_service.GetProcessors().size(), 1u);
+}
+
+TEST_F(
+    MediaEffectsServiceTest,
+    BindVideoEffectsProcessor_TwoProcessorsWithDifferentIdConnectToDifferentManager) {
+  // Tests that `MediaEffectsService::BindVideoEffectsProcessor()` connects to
+  // a different manager if a different ID was used. This is validated by
+  // checking that the managers return different configurations. We also set a
+  // different config directly via effects manager interface (originating from
+  // a call to `MediaEffectsService::BindVideoEffectsManager()`) so this test
+  // also checks that a correct relationship is established between manager
+  // and processor.
+
+  mojo::Remote<video_effects::mojom::VideoEffectsService> service;
+  video_effects::FakeVideoEffectsService fake_effects_service(
+      service.BindNewPipeAndPassReceiver());
+  auto service_reset = SetVideoEffectsServiceRemoteForTesting(&service);
+
+  mojo::Remote<media::mojom::VideoEffectsManager> effects_manager;
+  service_.BindVideoEffectsManager(
+      "test_device_1", effects_manager.BindNewPipeAndPassReceiver());
+
+  constexpr float kFramingPaddingRatio1 = 0.234;
+  SetFramingSync(effects_manager, kFramingPaddingRatio1);
+
+  EXPECT_EQ(gfx::InsetsF{kFramingPaddingRatio1},
+            GetConfigurationSync(effects_manager)->framing->padding_ratios);
+
+  auto effects_processor_future1 =
+      fake_effects_service.GetEffectsProcessorCreationFuture();
+
+  mojo::Remote<video_effects::mojom::VideoEffectsProcessor> effects_processor1;
+  service_.BindVideoEffectsProcessor(
+      "test_device_2", effects_processor1.BindNewPipeAndPassReceiver());
+  EXPECT_TRUE(effects_processor_future1->Wait());
+  ASSERT_EQ(fake_effects_service.GetProcessors().size(), 1u);
+
+  constexpr float kFramingPaddingRatio2 = 0.345;
+  SetFramingSync(fake_effects_service.GetProcessors()["test_device_2"]
+                     ->GetVideoEffectsManager(),
+                 kFramingPaddingRatio2);
+
+  EXPECT_EQ(gfx::InsetsF{kFramingPaddingRatio2},
+            GetConfigurationSync(
+                fake_effects_service.GetProcessors()["test_device_2"]
+                    ->GetVideoEffectsManager())
+                ->framing->padding_ratios);
+
+  auto effects_processor_future2 =
+      fake_effects_service.GetEffectsProcessorCreationFuture();
+
+  mojo::Remote<video_effects::mojom::VideoEffectsProcessor> effects_processor2;
+  service_.BindVideoEffectsProcessor(
+      "test_device_3", effects_processor2.BindNewPipeAndPassReceiver());
+  EXPECT_TRUE(effects_processor_future2->Wait());
+  ASSERT_EQ(fake_effects_service.GetProcessors().size(), 2u);
+
+  constexpr float kFramingPaddingRatio3 = 0.456;
+  SetFramingSync(fake_effects_service.GetProcessors()["test_device_3"]
+                     ->GetVideoEffectsManager(),
+                 kFramingPaddingRatio3);
+
+  auto padding2 =
+      std::move(GetConfigurationSync(
+                    fake_effects_service.GetProcessors()["test_device_2"]
+                        ->GetVideoEffectsManager())
+                    ->framing->padding_ratios);
+  auto padding3 =
+      std::move(GetConfigurationSync(
+                    fake_effects_service.GetProcessors()["test_device_3"]
+                        ->GetVideoEffectsManager())
+                    ->framing->padding_ratios);
+
+  EXPECT_NE(padding2, padding3);
+  EXPECT_EQ(gfx::InsetsF{kFramingPaddingRatio2}, padding2);
+  EXPECT_EQ(gfx::InsetsF{kFramingPaddingRatio3}, padding3);
+}

content/browser/BUILD.gn

@@ -223,6 +223,7 @@ source_set("browser") {
     "//services/video_capture:lib",
     "//services/video_capture/public/cpp",
     "//services/video_capture/public/mojom:constants",
+    "//services/video_effects/public/mojom:mojom",
     "//services/viz/privileged/mojom",
     "//services/viz/public/cpp/gpu",
     "//services/viz/public/mojom",

content/public/browser/BUILD.gn

@@ -548,6 +548,7 @@ source_set("browser_sources") {
     "//services/tracing/public/cpp",
     "//services/tracing/public/mojom",
     "//services/video_capture/public/mojom",
+    "//services/video_effects/public/mojom:mojom",
     "//services/viz/public/mojom",
 
     # We expose skia headers in the public API.

content/public/browser/DEPS

@@ -20,6 +20,7 @@ include_rules = [
   "+services/resource_coordinator/public",
   "+services/tracing/public/mojom",
   "+services/video_capture/public/mojom",
+  "+services/video_effects/public/mojom",
   "+services/viz/public/mojom",
   "+third_party/jni_zero",
   "+third_party/perfetto/protos/perfetto/config/chrome/scenario_config.gen.h",

content/public/browser/content_browser_client.cc

@@ -85,6 +85,8 @@
 
 #if BUILDFLAG(IS_ANDROID)
 #include "content/public/browser/tts_environment_android.h"
+#else
+#include "services/video_effects/public/mojom/video_effects_processor.mojom-forward.h"
 #endif
 
 using AttributionReportType =
@@ -1669,9 +1671,15 @@ bool ContentBrowserClient::UseOutermostMainFrameOrEmbedderForSubCaptureTargets()
 #if !BUILDFLAG(IS_ANDROID)
 void ContentBrowserClient::BindVideoEffectsManager(
     const std::string& device_id,
-    content::BrowserContext* browser_context,
+    BrowserContext* browser_context,
     mojo::PendingReceiver<media::mojom::VideoEffectsManager>
         video_effects_manager) {}
+
+void ContentBrowserClient::BindVideoEffectsProcessor(
+    const std::string& device_id,
+    BrowserContext* browser_context,
+    mojo::PendingReceiver<video_effects::mojom::VideoEffectsProcessor>
+        video_effects_manager) {}
 #endif  // !BUILDFLAG(IS_ANDROID)
 
 void ContentBrowserClient::PreferenceRankAudioDeviceInfos(

content/public/browser/content_browser_client.h

@@ -90,6 +90,7 @@
 
 #if !BUILDFLAG(IS_ANDROID)
 #include "media/capture/mojom/video_effects_manager.mojom.h"
+#include "services/video_effects/public/mojom/video_effects_processor.mojom-forward.h"
 #endif  // !BUILDFLAG(IS_ANDROID)
 
 namespace net {
@@ -2803,9 +2804,17 @@ class CONTENT_EXPORT ContentBrowserClient {
   // effect settings.
   virtual void BindVideoEffectsManager(
       const std::string& device_id,
-      content::BrowserContext* browser_context,
+      BrowserContext* browser_context,
       mojo::PendingReceiver<media::mojom::VideoEffectsManager>
           video_effects_manager);
+
+  // Allows the embedder to correlate backend media services with profile-keyed
+  // effect settings.
+  virtual void BindVideoEffectsProcessor(
+      const std::string& device_id,
+      BrowserContext* browser_context,
+      mojo::PendingReceiver<video_effects::mojom::VideoEffectsProcessor>
+          video_effects_processor);
 #endif  // !BUILDFLAG(IS_ANDROID)
 
   // Re-order audio device `infos` based on user preference. The ordering will

services/video_effects/DEPS

@@ -4,4 +4,5 @@ include_rules = [
   "+media/base",
   "+media/capture/mojom",
   "+services/viz/public/cpp/gpu",
+  "+services/video_effects/public/mojom",
 ]

services/video_effects/public/mojom/video_effects_service.mojom

@@ -16,13 +16,17 @@ import "services/video_effects/public/mojom/video_effects_processor.mojom";
 interface VideoEffectsService {
   // Creates a Video Effects Processor whose effects configuration will be
   // managed by the passed in `manager`. Note that the created processor will
-  // be bound to `processor`, i.e. it is not going to be returned in `result`.
-  // This is done so that the caller can create both ends of the pipe for
-  // `VideoEffectsProcessor` & send the receiver to Video Effects Service, and
-  // the remote to Video Capture Service.
-  // In case of failure, the remote end of the `processor` will eventually
-  // return `false` from the calls to its `.is_connected()` method.
+  // be bound to `processor`. This is done so that the caller can create both
+  // ends of the pipe for `VideoEffectsProcessor` & send the receiver to Video
+  // Effects Service, and the remote to Video Capture Service.
+  // The `device_id` is a string that uniquely identifies the device for which
+  // the processor is being created. If the service has already created a
+  // processor for a given device id, subsequent attempts to create one will
+  // fail.
+  // In case of failure, the remote end of the `processor` will be
+  // disconnected.
   CreateEffectsProcessor(
+    string device_id,
     pending_remote<media.mojom.VideoEffectsManager> manager,
     pending_receiver<VideoEffectsProcessor> processor);
 };

services/video_effects/test/BUILD.gn

@@ -0,0 +1,19 @@
+# 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.
+
+source_set("test_support") {
+  testonly = true
+
+  sources = [
+    "fake_video_effects_processor.cc",
+    "fake_video_effects_processor.h",
+    "fake_video_effects_service.cc",
+    "fake_video_effects_service.h",
+  ]
+
+  public_deps = [
+    "//base/test:test_support",
+    "//services/video_effects/public/mojom",
+  ]
+}

services/video_effects/test/fake_video_effects_processor.cc

@@ -0,0 +1,33 @@
+// 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 "services/video_effects/test/fake_video_effects_processor.h"
+#include "services/video_effects/public/mojom/video_effects_processor.mojom-shared.h"
+#include "services/video_effects/public/mojom/video_effects_processor.mojom.h"
+
+namespace video_effects {
+
+FakeVideoEffectsProcessor::FakeVideoEffectsProcessor(
+    mojo::PendingReceiver<mojom::VideoEffectsProcessor> processor,
+    mojo::PendingRemote<media::mojom::VideoEffectsManager> manager)
+    : receiver_(this, std::move(processor)), manager_(std::move(manager)) {}
+
+FakeVideoEffectsProcessor::~FakeVideoEffectsProcessor() = default;
+
+void FakeVideoEffectsProcessor::PostProcess(
+    media::mojom::VideoBufferHandlePtr input_frame_data,
+    media::mojom::VideoFrameInfoPtr input_frame_info,
+    media::mojom::VideoBufferHandlePtr result_frame_data,
+    media::VideoPixelFormat result_pixel_format,
+    PostProcessCallback callback) {
+  std::move(callback).Run(
+      mojom::PostProcessResult::NewError(mojom::PostProcessError::kUnknown));
+}
+
+mojo::Remote<media::mojom::VideoEffectsManager>&
+FakeVideoEffectsProcessor::GetVideoEffectsManager() {
+  return manager_;
+}
+
+}  // namespace video_effects

services/video_effects/test/fake_video_effects_processor.h

@@ -0,0 +1,42 @@
+// 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.
+
+#ifndef SERVICES_VIDEO_EFFECTS_TEST_FAKE_VIDEO_EFFECTS_PROCESSOR_H_
+#define SERVICES_VIDEO_EFFECTS_TEST_FAKE_VIDEO_EFFECTS_PROCESSOR_H_
+
+#include "media/capture/mojom/video_effects_manager.mojom.h"
+#include "mojo/public/cpp/bindings/pending_receiver.h"
+#include "mojo/public/cpp/bindings/pending_remote.h"
+#include "mojo/public/cpp/bindings/receiver.h"
+#include "mojo/public/cpp/bindings/remote.h"
+#include "services/video_effects/public/mojom/video_effects_processor.mojom.h"
+
+namespace video_effects {
+
+class FakeVideoEffectsProcessor : public mojom::VideoEffectsProcessor {
+ public:
+  explicit FakeVideoEffectsProcessor(
+      mojo::PendingReceiver<mojom::VideoEffectsProcessor> processor,
+      mojo::PendingRemote<media::mojom::VideoEffectsManager> manager);
+  ~FakeVideoEffectsProcessor() override;
+
+  // mojom::VideoEffectsProcessor implementation
+  void PostProcess(media::mojom::VideoBufferHandlePtr input_frame_data,
+                   media::mojom::VideoFrameInfoPtr input_frame_info,
+                   media::mojom::VideoBufferHandlePtr result_frame_data,
+                   media::VideoPixelFormat result_pixel_format,
+                   PostProcessCallback callback) override;
+
+  // For testing, get the manager that this processor will use to obtain the
+  // video effects configuration:
+  mojo::Remote<media::mojom::VideoEffectsManager>& GetVideoEffectsManager();
+
+ private:
+  mojo::Receiver<mojom::VideoEffectsProcessor> receiver_;
+  mojo::Remote<media::mojom::VideoEffectsManager> manager_;
+};
+
+}  // namespace video_effects
+
+#endif  // SERVICES_VIDEO_EFFECTS_TEST_FAKE_VIDEO_EFFECTS_PROCESSOR_H_

services/video_effects/test/fake_video_effects_service.cc

@@ -0,0 +1,49 @@
+// 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 "services/video_effects/test/fake_video_effects_service.h"
+
+#include <memory>
+
+#include "mojo/public/cpp/bindings/receiver.h"
+#include "services/video_effects/public/mojom/video_effects_processor.mojom.h"
+#include "services/video_effects/test/fake_video_effects_processor.h"
+
+namespace video_effects {
+
+FakeVideoEffectsService::FakeVideoEffectsService(
+    mojo::PendingReceiver<mojom::VideoEffectsService> receiver)
+    : receiver_(this, std::move(receiver)) {}
+
+FakeVideoEffectsService::~FakeVideoEffectsService() = default;
+
+void FakeVideoEffectsService::CreateEffectsProcessor(
+    const std::string& device_id,
+    mojo::PendingRemote<media::mojom::VideoEffectsManager> manager,
+    mojo::PendingReceiver<mojom::VideoEffectsProcessor> processor) {
+  processors_.insert(
+      std::make_pair(device_id, std::make_unique<FakeVideoEffectsProcessor>(
+                                    std::move(processor), std::move(manager))));
+
+  if (effects_processor_creation_cb_) {
+    std::move(effects_processor_creation_cb_).Run();
+  }
+}
+
+std::unique_ptr<base::test::TestFuture<void>>
+FakeVideoEffectsService::GetEffectsProcessorCreationFuture() {
+  CHECK(!effects_processor_creation_cb_);
+
+  std::unique_ptr<base::test::TestFuture<void>> result =
+      std::make_unique<base::test::TestFuture<void>>();
+  effects_processor_creation_cb_ = result->GetCallback();
+  return result;
+}
+
+base::flat_map<std::string, std::unique_ptr<FakeVideoEffectsProcessor>>&
+FakeVideoEffectsService::GetProcessors() {
+  return processors_;
+}
+
+}  // namespace video_effects

services/video_effects/test/fake_video_effects_service.h

@@ -0,0 +1,62 @@
+// 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.
+
+#ifndef SERVICES_VIDEO_EFFECTS_TEST_FAKE_VIDEO_EFFECTS_SERVICE_H_
+#define SERVICES_VIDEO_EFFECTS_TEST_FAKE_VIDEO_EFFECTS_SERVICE_H_
+
+#include <memory>
+#include <vector>
+
+#include "base/containers/flat_map.h"
+#include "base/functional/callback_forward.h"
+#include "base/test/test_future.h"
+#include "mojo/public/cpp/bindings/pending_receiver.h"
+#include "mojo/public/cpp/bindings/pending_remote.h"
+#include "mojo/public/cpp/bindings/receiver.h"
+#include "services/video_effects/public/mojom/video_effects_service.mojom.h"
+#include "services/video_effects/test/fake_video_effects_processor.h"
+
+namespace video_effects {
+
+// Fake implementation of Video Effects Service.
+// Intended to be used in unit tests.
+class FakeVideoEffectsService : public mojom::VideoEffectsService {
+ public:
+  explicit FakeVideoEffectsService(
+      mojo::PendingReceiver<mojom::VideoEffectsService> receiver);
+  ~FakeVideoEffectsService() override;
+
+  // mojom::VideoEffectsService implementation:
+
+  // The fake implementation will ensure that the remote end of the passed in
+  // `processor` receiver will stay connected for as long as the fake is alive.
+  void CreateEffectsProcessor(
+      const std::string& device_id,
+      mojo::PendingRemote<media::mojom::VideoEffectsManager> manager,
+      mojo::PendingReceiver<mojom::VideoEffectsProcessor> processor) override;
+
+  // Returns a test future which will be resolved when the next video effects
+  // processor creation request is fulfilled. There can be at most one
+  // outstanding test feature created at any given time.
+  std::unique_ptr<base::test::TestFuture<void>>
+  GetEffectsProcessorCreationFuture();
+
+  // For testing, get the processors that this service created.
+  base::flat_map<std::string, std::unique_ptr<FakeVideoEffectsProcessor>>&
+  GetProcessors();
+
+ private:
+  mojo::Receiver<mojom::VideoEffectsService> receiver_;
+
+  // Note: indirection via std::unique_ptr is required since the processor is
+  // not copyable and not moveable.
+  base::flat_map<std::string, std::unique_ptr<FakeVideoEffectsProcessor>>
+      processors_;
+
+  base::OnceClosure effects_processor_creation_cb_;
+};
+
+}  // namespace video_effects
+
+#endif  // SERVICES_VIDEO_EFFECTS_TEST_FAKE_VIDEO_EFFECTS_SERVICE_H_

services/video_effects/video_effects_service_impl.cc

@@ -28,13 +28,18 @@ VideoEffectsServiceImpl::VideoEffectsServiceImpl(
 VideoEffectsServiceImpl::~VideoEffectsServiceImpl() = default;
 
 void VideoEffectsServiceImpl::CreateEffectsProcessor(
+    const std::string& device_id,
     mojo::PendingRemote<media::mojom::VideoEffectsManager> manager,
     mojo::PendingReceiver<mojom::VideoEffectsProcessor> processor) {
+  if (processors_.contains(device_id)) {
+    return;
+  }
+
   std::unique_ptr<VideoEffectsProcessorImpl> effects_processor =
       std::make_unique<VideoEffectsProcessorImpl>(std::move(manager),
                                                   std::move(processor));
 
-  processors_.push_back(std::move(effects_processor));
+  processors_.insert(std::make_pair(device_id, std::move(effects_processor)));
 }
 
 }  // namespace video_effects

services/video_effects/video_effects_service_impl.h

@@ -8,6 +8,7 @@
 #include <memory>
 #include <vector>
 
+#include "base/containers/flat_map.h"
 #include "base/memory/scoped_refptr.h"
 #include "gpu/ipc/client/gpu_channel_host.h"
 #include "media/capture/mojom/video_effects_manager.mojom-forward.h"
@@ -48,11 +49,15 @@ class VideoEffectsServiceImpl : public mojom::VideoEffectsService {
 
   // mojom::VideoEffectsService implementation:
   void CreateEffectsProcessor(
+      const std::string& device_id,
       mojo::PendingRemote<media::mojom::VideoEffectsManager> manager,
       mojo::PendingReceiver<mojom::VideoEffectsProcessor> processor) override;
 
  private:
-  std::vector<std::unique_ptr<VideoEffectsProcessorImpl>> processors_;
+  // Mapping from the device ID to processor implementation. Device ID is only
+  // used to deduplicate processor creation requests.
+  base::flat_map<std::string, std::unique_ptr<VideoEffectsProcessorImpl>>
+      processors_;
 
   mojo::Receiver<mojom::VideoEffectsService> receiver_;
   std::unique_ptr<GpuChannelHostProvider> gpu_channel_host_provider_;

services/video_effects/video_effects_service_impl_unittest.cc

@@ -6,6 +6,7 @@
 
 #include "services/video_effects/video_effects_service_impl.h"
 
+#include "base/run_loop.h"
 #include "base/test/task_environment.h"
 #include "mojo/public/cpp/bindings/pending_receiver.h"
 #include "mojo/public/cpp/bindings/receiver.h"
@@ -21,6 +22,8 @@ using testing::_;
 
 namespace {
 
+constexpr char kDeviceId[] = "test_device";
+
 class TestGpuChannelHostProvider : public GpuChannelHostProvider {
  public:
   TestGpuChannelHostProvider() = default;
@@ -50,18 +53,41 @@ 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(),
+      kDeviceId, manager_receiver.InitWithNewPipeAndPassRemote(),
       processor_remote.BindNewPipeAndPassReceiver());
 
-  run_loop.RunUntilIdle();
-
+  base::RunLoop().RunUntilIdle();
   EXPECT_TRUE(processor_remote.is_connected());
 }
 
+TEST_F(VideoEffectsServiceTest, CreateEffectsProcessorWithSameIdFails) {
+  // Calling into `VideoEffectsService:::CreateEffectsProcessor()` is expected
+  // to fail if the same device id is passed.
+
+  mojo::PendingReceiver<media::mojom::VideoEffectsManager> manager_receiver1;
+  mojo::Remote<mojom::VideoEffectsProcessor> processor_remote1;
+
+  service_remote_->CreateEffectsProcessor(
+      kDeviceId, manager_receiver1.InitWithNewPipeAndPassRemote(),
+      processor_remote1.BindNewPipeAndPassReceiver());
+
+  base::RunLoop().RunUntilIdle();
+  ASSERT_TRUE(processor_remote1.is_connected());
+
+  mojo::PendingReceiver<media::mojom::VideoEffectsManager> manager_receiver2;
+  mojo::Remote<mojom::VideoEffectsProcessor> processor_remote2;
+
+  service_remote_->CreateEffectsProcessor(
+      kDeviceId, manager_receiver2.InitWithNewPipeAndPassRemote(),
+      processor_remote2.BindNewPipeAndPassReceiver());
+
+  base::RunLoop().RunUntilIdle();
+  EXPECT_TRUE(processor_remote1.is_connected());
+  EXPECT_FALSE(processor_remote2.is_connected());
+}
+
 }  // namespace video_effects