Mojo framework for AndroidOverlay.

media/mojo/clients/mojo_android_overlay_factory.h

Index: media/mojo/clients/mojo_android_overlay_factory.h
diff --git a/media/mojo/clients/mojo_android_overlay_factory.h b/media/mojo/clients/mojo_android_overlay_factory.h
new file mode 100644
index 0000000000000000000000000000000000000000..676ace1aee005677374c47e3aec7523bd9c340e6
--- /dev/null
+++ b/media/mojo/clients/mojo_android_overlay_factory.h
@@ -0,0 +1,43 @@
+// Copyright 2017 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.
+
+#ifndef MEDIA_BASE_MOJO_ANDROID_OVERLAY_FACTORY_H_
+#define MEDIA_BASE_MOJO_ANDROID_OVERLAY_FACTORY_H_
+
+#include "base/macros.h"
+#include "media/base/android/android_overlay_factory.h"
+#include "media/mojo/interfaces/android_overlay.mojom.h"
+
+namespace service_manager {
+namespace mojom {
+class InterfaceProvider;
+}
+}
+
+namespace media {
+
+// AndroidOverlayFactory implementation that memorizes the render frame with
+// which the overlays are associated.
+class MojoAndroidOverlayFactory : public AndroidOverlayFactory {
+ public:
+  MojoAndroidOverlayFactory(
+      service_manager::mojom::InterfaceProvider* interface_provider,
+      int render_frame_id,
+      int renderer_pid);
+  ~MojoAndroidOverlayFactory() override;
+
+  std::unique_ptr<AndroidOverlay> CreateOverlay(
+      const AndroidOverlay::Config& config) override;
+
+ private:
+  service_manager::mojom::InterfaceProvider* interface_provider_;

[dcheng, 2017/02/28 05:22:03]

Nit: I personally prefer to mark these const to make sure they aren't
accidentally mutated.

[liberato (no reviews please), 2017/03/06 07:51:04]

Done.

+  int render_frame_id_;

[dcheng, 2017/02/28 05:22:03]

This feels like a layering violation. //media can't depend on //content since
//content depends on //media, but this embeds the concept of render frames
inside //media.

[liberato (no reviews please), 2017/03/06 07:51:05]

yeah, it is.  i'd like to remove them both; they're just implementation details
anyway that the client shouldn't know or care about.  what i'd really like to
see is render frame create the factory in the browser, and hand that off to the
gpu process.  however, we can't since there's still IPC involved renderer -> GPU
that won't let us hand off mojo objects.  plus, i think it might route messages
from gpu -> renderer -> browser if we do that (+rockot: am i correct about
that?)

one could probably have the render frame get an UnguessableToken from the
browser, rather than the mojo interface, and send that to the gpu process.  i
tried something similar in a previous CL (TL;DR: adds no security, none needed
anyway), but it wasn't in the context of layering, just security.

any way one looks at it, it's just trading one int for another -- we'd name
(|render_frame_id_|, |renderer_pid_|) to something new (|token|), and add some
maps on the browser side to map it between the token and the original data. 
nothing would really be all that different; it'd be a differently named opaque
value.

granted, we could remove the need for the gpu process to get the pid from the
ipc channel.  it'll also work better for other future use-cases for overlays in
which there is no render frame.  the client doesn't need to know that.

so, how about this: i'll replace these two ints with one unguessable token now. 
this token will be created, in the next CL, by the provider in response to some
call from RenderFrameHostImpl.  the token will be sent by RFHI to the
RenderFrame, and passed to the gpu process over IPC.  gpu will put it back into
this config when talking to the provider.

later, if RFHI isn't the only thing that can create an overlay, that'll fit
right in with no client changes.

+  int renderer_pid_;
+
+  DISALLOW_COPY_AND_ASSIGN(MojoAndroidOverlayFactory);
+};
+
+}  // namespace media
+
+#endif  // MEDIA_BASE_MOJO_ANDROID_OVERLAY_H_