Convert accelerated JPEG Decoder IPC to Mojo

This CL adds implementation for GJDA's mojom interface which
was introduced in [1]. GJDA interface that runs in the gpu
process is exposed to the browser process via ui::Gpu.
The mojo connection between browser and gpu is set up by a
call to ui::Gpu::CreateJpegDecodeAccelerator().
Existing JPEG decoder IPC messages between browser and gpu
have been replaced with mojo calls to GJDA interface.

GJDA unittest has been updated to use the new mojo implementation.

[1] https://codereview.chromium.org/2905823002/

Bug: 699255
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: I158c2f7c7d77f9a042fef0630c4ac07160c6e903
Reviewed-on: https://chromium-review.googlesource.com/525672Reviewed-by: Miguel Casas <mcasas@chromium.org>
Reviewed-by: Ken Rockot <rockot@chromium.org>
Reviewed-by: Tom Sepez <tsepez@chromium.org>
Reviewed-by: Antoine Labour <piman@chromium.org>
Reviewed-by: Dan Sanders <sandersd@chromium.org>
Commit-Queue: Chandan Padhi <c.padhi@samsung.com>
Cr-Commit-Position: refs/heads/master@{#517304}

components/viz/service/gl/gpu_service_impl.cc

@@ -34,7 +34,7 @@
 #include "ipc/ipc_sync_channel.h"
 #include "ipc/ipc_sync_message_filter.h"
 #include "media/gpu/gpu_video_encode_accelerator_factory.h"
-#include "media/gpu/ipc/service/gpu_jpeg_decode_accelerator_factory_provider.h"
+#include "media/gpu/ipc/service/gpu_jpeg_decode_accelerator.h"
 #include "media/gpu/ipc/service/gpu_video_decode_accelerator.h"
 #include "media/gpu/ipc/service/gpu_video_encode_accelerator.h"
 #include "media/gpu/ipc/service/media_gpu_channel_manager.h"
@@ -234,8 +234,7 @@ void GpuServiceImpl::RecordLogMessage(int severity,
 void GpuServiceImpl::CreateJpegDecodeAccelerator(
     media::mojom::GpuJpegDecodeAcceleratorRequest jda_request) {
   DCHECK(io_runner_->BelongsToCurrentThread());
-  // TODO(c.padhi): Implement this, see https://crbug.com/699255.
-  NOTIMPLEMENTED();
+  media::GpuJpegDecodeAccelerator::Create(std::move(jda_request));
 }
 
 void GpuServiceImpl::CreateVideoEncodeAcceleratorProvider(

content/browser/renderer_host/media/video_capture_gpu_jpeg_decoder.cc

@@ -10,8 +10,8 @@
 #include "base/command_line.h"
 #include "base/logging.h"
 #include "base/metrics/histogram_macros.h"
-#include "base/single_thread_task_runner.h"
 #include "base/strings/stringprintf.h"
+#include "base/synchronization/waitable_event.h"
 #include "base/threading/thread_task_runner_handle.h"
 #include "base/trace_event/trace_event.h"
 #include "build/build_config.h"
@@ -19,11 +19,11 @@
 #include "content/browser/gpu/gpu_process_host.h"
 #include "content/public/browser/browser_thread.h"
 #include "content/public/common/content_switches.h"
-#include "gpu/ipc/client/gpu_channel_host.h"
+#include "media/base/bind_to_current_loop.h"
 #include "media/base/media_switches.h"
 #include "media/base/video_frame.h"
-#include "media/gpu/ipc/client/gpu_jpeg_decode_accelerator_host.h"
 #include "mojo/public/cpp/system/platform_handle.h"
+#include "services/ui/public/cpp/gpu/gpu.h"
 
 namespace content {
 
@@ -35,18 +35,26 @@ VideoCaptureGpuJpegDecoder::VideoCaptureGpuJpegDecoder(
       has_received_decoded_frame_(false),
       next_bitstream_buffer_id_(0),
       in_buffer_id_(media::JpegDecodeAccelerator::kInvalidBitstreamBufferId),
-      decoder_status_(INIT_PENDING) {}
+      decoder_status_(INIT_PENDING),
+      weak_ptr_factory_(this) {}
 
 VideoCaptureGpuJpegDecoder::~VideoCaptureGpuJpegDecoder() {
   DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
 
-  // |decoder_| guarantees no more JpegDecodeAccelerator::Client callbacks
-  // on IO thread after deletion.
-  decoder_.reset();
-
-  // |gpu_channel_host_| should outlive |decoder_|, so |gpu_channel_host_|
-  // must be released after |decoder_| has been destroyed.
-  gpu_channel_host_ = nullptr;
+  // |this| was set as |decoder_|'s client. |decoder_| has to be deleted before
+  // this destructor returns to ensure that it doesn't call back into its
+  // client. Hence, we wait here while we delete |decoder_| on the IO thread.
+  if (decoder_) {
+    base::WaitableEvent event(base::WaitableEvent::ResetPolicy::MANUAL,
+                              base::WaitableEvent::InitialState::NOT_SIGNALED);
+    // base::Unretained is safe because |this| will be valid until |event|
+    // is signaled.
+    BrowserThread::PostTask(
+        BrowserThread::IO, FROM_HERE,
+        base::Bind(&VideoCaptureGpuJpegDecoder::DestroyDecoderOnIOThread,
+                   base::Unretained(this), &event));
+    event.Wait();
+  }
 }
 
 void VideoCaptureGpuJpegDecoder::Initialize() {
@@ -70,11 +78,10 @@ void VideoCaptureGpuJpegDecoder::Initialize() {
     return;
   }
 
-  const scoped_refptr<base::SingleThreadTaskRunner> current_task_runner(
-      base::ThreadTaskRunnerHandle::Get());
-  BrowserThread::PostTask(BrowserThread::UI, FROM_HERE,
-                          base::BindOnce(&EstablishGpuChannelOnUIThread,
-                                         current_task_runner, AsWeakPtr()));
+  BrowserThread::PostTask(
+      BrowserThread::IO, FROM_HERE,
+      base::Bind(&RequestGPUInfoOnIOThread, base::ThreadTaskRunnerHandle::Get(),
+                 weak_ptr_factory_.GetWeakPtr()));
 }
 
 VideoCaptureGpuJpegDecoder::STATUS VideoCaptureGpuJpegDecoder::GetStatus()
@@ -180,7 +187,12 @@ void VideoCaptureGpuJpegDecoder::DecodeCapturedData(
                    base::Passed(&out_buffer.access_permission),
                    base::Passed(&out_frame_info));
   }
-  decoder_->Decode(in_buffer, std::move(out_frame));
+
+  // base::Unretained is safe because |decoder_| is deleted on the IO thread.
+  BrowserThread::PostTask(BrowserThread::IO, FROM_HERE,
+                          base::Bind(&media::JpegDecodeAccelerator::Decode,
+                                     base::Unretained(decoder_.get()),
+                                     in_buffer, std::move(out_frame)));
 }
 
 void VideoCaptureGpuJpegDecoder::VideoFrameReady(int32_t bitstream_buffer_id) {
@@ -247,61 +259,59 @@ void VideoCaptureGpuJpegDecoder::DidReceiveGPUInfoOnIOThread(
     base::WeakPtr<VideoCaptureGpuJpegDecoder> weak_this,
     const gpu::GPUInfo& gpu_info) {
   DCHECK_CURRENTLY_ON(BrowserThread::IO);
-  // TODO(c.padhi): Implement this, see http://crbug.com/699255.
-  NOTIMPLEMENTED();
-}
-
-// static
-void VideoCaptureGpuJpegDecoder::EstablishGpuChannelOnUIThread(
-    const scoped_refptr<base::SingleThreadTaskRunner>& task_runner,
-    base::WeakPtr<VideoCaptureGpuJpegDecoder> weak_this) {
-  DCHECK_CURRENTLY_ON(BrowserThread::UI);
 
-  BrowserMainLoop::GetInstance()
-      ->gpu_channel_establish_factory()
-      ->EstablishGpuChannel(base::Bind(
-          &VideoCaptureGpuJpegDecoder::GpuChannelEstablishedOnUIThread,
-          task_runner, weak_this));
-}
+  media::mojom::GpuJpegDecodeAcceleratorPtr remote_decoder;
 
-// static
-void VideoCaptureGpuJpegDecoder::GpuChannelEstablishedOnUIThread(
-    const scoped_refptr<base::SingleThreadTaskRunner>& task_runner,
-    base::WeakPtr<VideoCaptureGpuJpegDecoder> weak_this,
-    scoped_refptr<gpu::GpuChannelHost> gpu_channel_host) {
-  DCHECK_CURRENTLY_ON(BrowserThread::UI);
+  if (gpu_info.jpeg_decode_accelerator_supported) {
+    GpuProcessHost* host =
+        GpuProcessHost::Get(GpuProcessHost::GPU_PROCESS_KIND_SANDBOXED, false);
+    if (host) {
+      host->gpu_service()->CreateJpegDecodeAccelerator(
+          mojo::MakeRequest(&remote_decoder));
+    }
+  }
 
   task_runner->PostTask(
       FROM_HERE,
-      base::BindOnce(&VideoCaptureGpuJpegDecoder::FinishInitialization,
-                     weak_this, std::move(gpu_channel_host)));
+      base::Bind(&VideoCaptureGpuJpegDecoder::FinishInitialization, weak_this,
+                 base::Passed(remote_decoder.PassInterface())));
 }
 
 void VideoCaptureGpuJpegDecoder::FinishInitialization(
-    scoped_refptr<gpu::GpuChannelHost> gpu_channel_host) {
+    media::mojom::GpuJpegDecodeAcceleratorPtrInfo unbound_remote_decoder) {
   TRACE_EVENT0("gpu", "VideoCaptureGpuJpegDecoder::FinishInitialization");
   DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
+
   base::AutoLock lock(lock_);
-  if (!gpu_channel_host) {
-    LOG(ERROR) << "Failed to establish GPU channel for JPEG decoder";
-  } else if (gpu_channel_host->gpu_info().jpeg_decode_accelerator_supported) {
-    gpu_channel_host_ = std::move(gpu_channel_host);
-    scoped_refptr<base::SingleThreadTaskRunner> io_task_runner =
-        BrowserThread::GetTaskRunnerForThread(BrowserThread::IO);
-
-    int32_t route_id = gpu_channel_host_->GenerateRouteID();
-    std::unique_ptr<media::GpuJpegDecodeAcceleratorHost> decoder(
-        new media::GpuJpegDecodeAcceleratorHost(gpu_channel_host_.get(),
-                                                route_id, io_task_runner));
-    if (decoder->Initialize(this)) {
-      gpu_channel_host_->AddRouteWithTaskRunner(
-          route_id, decoder->GetReceiver(), io_task_runner);
-      decoder_ = std::move(decoder);
-    } else {
-      DLOG(ERROR) << "Failed to initialize JPEG decoder";
-    }
+  if (unbound_remote_decoder.is_valid()) {
+    decoder_ = base::MakeUnique<media::GpuJpegDecodeAcceleratorHost>(
+        BrowserThread::GetTaskRunnerForThread(BrowserThread::IO),
+        std::move(unbound_remote_decoder));
+
+    // base::Unretained is safe because |decoder_| is deleted on the IO thread.
+    BrowserThread::PostTask(
+        BrowserThread::IO, FROM_HERE,
+        base::Bind(&media::JpegDecodeAccelerator::InitializeAsync,
+                   base::Unretained(decoder_.get()), this,
+                   media::BindToCurrentLoop(base::Bind(
+                       &VideoCaptureGpuJpegDecoder::OnInitializationDone,
+                       weak_ptr_factory_.GetWeakPtr()))));
+  } else {
+    OnInitializationDone(false);
   }
-  decoder_status_ = decoder_ ? INIT_PASSED : FAILED;
+}
+
+void VideoCaptureGpuJpegDecoder::OnInitializationDone(bool success) {
+  TRACE_EVENT0("gpu", "VideoCaptureGpuJpegDecoder::OnInitializationDone");
+  DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
+
+  base::AutoLock lock(lock_);
+  if (!success) {
+    BrowserThread::DeleteSoon(BrowserThread::IO, FROM_HERE, decoder_.release());
+    DLOG(ERROR) << "Failed to initialize JPEG decoder";
+  }
+
+  decoder_status_ = success ? INIT_PASSED : FAILED;
   RecordInitDecodeUMA_Locked();
 }
 
@@ -315,4 +325,11 @@ void VideoCaptureGpuJpegDecoder::RecordInitDecodeUMA_Locked() {
                         decoder_status_ == INIT_PASSED);
 }
 
+void VideoCaptureGpuJpegDecoder::DestroyDecoderOnIOThread(
+    base::WaitableEvent* event) {
+  DCHECK_CURRENTLY_ON(BrowserThread::IO);
+  decoder_.reset();
+  event->Signal();
+}
+
 }  // namespace content

content/browser/renderer_host/media/video_capture_gpu_jpeg_decoder.h

@@ -13,18 +13,15 @@
 
 #include "base/callback.h"
 #include "base/macros.h"
-#include "base/memory/ref_counted.h"
 #include "base/memory/weak_ptr.h"
 #include "base/sequence_checker.h"
-#include "base/single_thread_task_runner.h"
-#include "base/threading/thread.h"
 #include "content/common/content_export.h"
 #include "gpu/config/gpu_info.h"
 #include "media/capture/video/video_capture_jpeg_decoder.h"
-#include "media/video/jpeg_decode_accelerator.h"
+#include "media/gpu/ipc/client/gpu_jpeg_decode_accelerator_host.h"
 
-namespace gpu {
-class GpuChannelHost;
+namespace base {
+class WaitableEvent;
 }
 
 namespace content {
@@ -38,10 +35,9 @@ namespace content {
 // the IO thread.
 class CONTENT_EXPORT VideoCaptureGpuJpegDecoder
     : public media::VideoCaptureJpegDecoder,
-      public media::JpegDecodeAccelerator::Client,
-      public base::SupportsWeakPtr<VideoCaptureGpuJpegDecoder> {
+      public media::JpegDecodeAccelerator::Client {
  public:
-  // |decode_done_cb| is called on the IO thread when decode succeed. This can
+  // |decode_done_cb| is called on the IO thread when decode succeeds. This can
   // be on any thread. |decode_done_cb| is never called after
   // VideoCaptureGpuJpegDecoder is destroyed.
   VideoCaptureGpuJpegDecoder(
@@ -76,18 +72,9 @@ class CONTENT_EXPORT VideoCaptureGpuJpegDecoder
       base::WeakPtr<VideoCaptureGpuJpegDecoder> weak_this,
       const gpu::GPUInfo& gpu_info);
 
-  // Initialization helper, to establish GPU channel.
-  static void EstablishGpuChannelOnUIThread(
-      const scoped_refptr<base::SingleThreadTaskRunner>& task_runner,
-      base::WeakPtr<VideoCaptureGpuJpegDecoder> weak_this);
-
-  static void GpuChannelEstablishedOnUIThread(
-      const scoped_refptr<base::SingleThreadTaskRunner>& task_runner,
-      base::WeakPtr<VideoCaptureGpuJpegDecoder> weak_this,
-      scoped_refptr<gpu::GpuChannelHost> established_channel_host);
-
   void FinishInitialization(
-      scoped_refptr<gpu::GpuChannelHost> gpu_channel_host);
+      media::mojom::GpuJpegDecodeAcceleratorPtrInfo unbound_remote_decoder);
+  void OnInitializationDone(bool success);
 
   // Returns true if the decoding of last frame is not finished yet.
   bool IsDecoding_Locked() const;
@@ -95,7 +82,7 @@ class CONTENT_EXPORT VideoCaptureGpuJpegDecoder
   // Records |decoder_status_| to histogram.
   void RecordInitDecodeUMA_Locked();
 
-  scoped_refptr<gpu::GpuChannelHost> gpu_channel_host_;
+  void DestroyDecoderOnIOThread(base::WaitableEvent* event);
 
   // The underlying JPEG decode accelerator.
   std::unique_ptr<media::JpegDecodeAccelerator> decoder_;
@@ -126,6 +113,8 @@ class CONTENT_EXPORT VideoCaptureGpuJpegDecoder
 
   SEQUENCE_CHECKER(sequence_checker_);
 
+  base::WeakPtrFactory<VideoCaptureGpuJpegDecoder> weak_ptr_factory_;
+
   DISALLOW_COPY_AND_ASSIGN(VideoCaptureGpuJpegDecoder);
 };
 

media/gpu/BUILD.gn

@@ -600,12 +600,11 @@ test("video_decode_accelerator_service_unittest") {
   deps = [
     ":gpu",
     "//base",
-    "//base/test:run_all_unittests",
     "//base/test:test_support",
     "//gpu:test_support",
     "//media/gpu/ipc/common",
     "//media/gpu/ipc/service",
-    "//testing/gmock",
+    "//media/test:run_all_unittests",
     "//testing/gtest",
     "//ui/gfx:test_support",
     "//ui/gfx/geometry",

media/gpu/ipc/DEPS

 include_rules = [
   "+gpu/ipc/common",
   "+ipc",
+  "+mojo/public/cpp",
   "-media/base/media_export.h",
 ]

media/gpu/ipc/client/gpu_jpeg_decode_accelerator_host.h

@@ -10,35 +10,25 @@
 #include <memory>
 
 #include "base/macros.h"
-#include "base/memory/weak_ptr.h"
-#include "base/sequence_checker.h"
+#include "media/gpu/mojo/jpeg_decoder.mojom.h"
 #include "media/video/jpeg_decode_accelerator.h"
 
 namespace base {
 class SingleThreadTaskRunner;
 }
 
-namespace gpu {
-class GpuChannelHost;
-}
-
-namespace IPC {
-class Listener;
-class Message;
-}
-
 namespace media {
 
 // TODO(c.padhi): Move GpuJpegDecodeAcceleratorHost to media/gpu/mojo, see
 // http://crbug.com/699255.
-// This class is used to talk to JpegDecodeAccelerator in the GPU process
-// through IPC messages.
+// A JpegDecodeAccelerator, for use in the browser process, that proxies to a
+// mojom::GpuJpegDecodeAccelerator. Created on the owner's thread, otherwise
+// operating and deleted on the IO thread.
 class GpuJpegDecodeAcceleratorHost : public JpegDecodeAccelerator {
  public:
   GpuJpegDecodeAcceleratorHost(
-      scoped_refptr<gpu::GpuChannelHost> channel,
-      int32_t route_id,
-      const scoped_refptr<base::SingleThreadTaskRunner>& io_task_runner);
+      scoped_refptr<base::SingleThreadTaskRunner> io_task_runner,
+      mojom::GpuJpegDecodeAcceleratorPtrInfo jpeg_decoder);
   ~GpuJpegDecodeAcceleratorHost() override;
 
   // JpegDecodeAccelerator implementation.
@@ -50,24 +40,25 @@ class GpuJpegDecodeAcceleratorHost : public JpegDecodeAccelerator {
               const scoped_refptr<VideoFrame>& video_frame) override;
   bool IsSupported() override;
 
-  base::WeakPtr<IPC::Listener> GetReceiver();
-
  private:
-  class Receiver;
-
-  void Send(IPC::Message* message);
-
-  scoped_refptr<gpu::GpuChannelHost> channel_;
-
-  // Route ID for the associated decoder in the GPU process.
-  int32_t decoder_route_id_;
-
-  // GPU IO task runner.
+  void OnInitializeDone(InitCB init_cb,
+                        JpegDecodeAccelerator::Client* client,
+                        bool success);
+  void OnDecodeAck(int32_t bitstream_buffer_id,
+                   JpegDecodeAccelerator::Error error);
+  void OnLostConnectionToJpegDecoder();
+
+  // Browser IO task runner.
   scoped_refptr<base::SingleThreadTaskRunner> io_task_runner_;
 
-  std::unique_ptr<Receiver> receiver_;
+  Client* client_ = nullptr;
+
+  // Used to safely pass the GpuJpegDecodeAcceleratorPtr from one thread
+  // to another. It is set in the constructor and consumed in
+  // InitializeAsync().
+  mojom::GpuJpegDecodeAcceleratorPtrInfo jpeg_decoder_info_;
 
-  SEQUENCE_CHECKER(sequence_checker_);
+  mojom::GpuJpegDecodeAcceleratorPtr jpeg_decoder_;
 
   DISALLOW_COPY_AND_ASSIGN(GpuJpegDecodeAcceleratorHost);
 };

media/gpu/ipc/common/media_messages.h

@@ -22,13 +22,6 @@
 
 #define IPC_MESSAGE_START MediaMsgStart
 
-IPC_STRUCT_BEGIN(AcceleratedJpegDecoderMsg_Decode_Params)
-  IPC_STRUCT_MEMBER(media::BitstreamBuffer, input_buffer)
-  IPC_STRUCT_MEMBER(gfx::Size, coded_size)
-  IPC_STRUCT_MEMBER(base::SharedMemoryHandle, output_video_frame_handle)
-  IPC_STRUCT_MEMBER(uint32_t, output_buffer_size)
-IPC_STRUCT_END()
-
 IPC_STRUCT_BEGIN(AcceleratedVideoEncoderMsg_Encode_Params)
   IPC_STRUCT_MEMBER(int32_t, frame_id)
   IPC_STRUCT_MEMBER(base::TimeDelta, timestamp)
@@ -196,34 +189,3 @@ IPC_MESSAGE_ROUTED1(AcceleratedVideoEncoderHostMsg_NotifyError,
 
 // Send destroy request to the encoder.
 IPC_MESSAGE_ROUTED0(AcceleratedVideoEncoderMsg_Destroy)
-
-//------------------------------------------------------------------------------
-// Accelerated JPEG Decoder Messages
-// These messages are sent from the Browser process to GPU process.
-
-// Create and initialize a hardware jpeg decoder using the specified route_id.
-// Created decoders should be freed with AcceleratedJpegDecoderMsg_Destroy when
-// no longer needed.
-IPC_SYNC_MESSAGE_CONTROL1_1(GpuChannelMsg_CreateJpegDecoder,
-                            int32_t /* route_id */,
-                            bool /* succeeded */)
-
-// Decode one JPEG image from shared memory |input_buffer_handle| with size
-// |input_buffer_size|. The input buffer is associated with |input_buffer_id|
-// and the size of JPEG image is |coded_size|. Decoded I420 frame data will
-// be put onto shared memory associated with |output_video_frame_handle|
-// with size limit |output_buffer_size|.
-IPC_MESSAGE_ROUTED1(AcceleratedJpegDecoderMsg_Decode,
-                    AcceleratedJpegDecoderMsg_Decode_Params)
-
-// Send destroy request to the decoder.
-IPC_MESSAGE_ROUTED0(AcceleratedJpegDecoderMsg_Destroy)
-
-//------------------------------------------------------------------------------
-// Accelerated JPEG Decoder Host Messages
-// These messages are sent from the GPU process to Browser process.
-//
-// Report decode status.
-IPC_MESSAGE_ROUTED2(AcceleratedJpegDecoderHostMsg_DecodeAck,
-                    int32_t, /* bitstream_buffer_id */
-                    media::JpegDecodeAccelerator::Error /* error */)

media/gpu/ipc/common/media_param_traits_macros.h

@@ -9,14 +9,11 @@
 #include "ipc/ipc_message_macros.h"
 #include "media/base/ipc/media_param_traits.h"
 #include "media/gpu/ipc/common/create_video_encoder_params.h"
-#include "media/video/jpeg_decode_accelerator.h"
 #include "media/video/video_decode_accelerator.h"
 #include "media/video/video_encode_accelerator.h"
 #include "ui/gfx/ipc/color/gfx_param_traits.h"
 #include "ui/gfx/ipc/geometry/gfx_param_traits.h"
 
-IPC_ENUM_TRAITS_MAX_VALUE(media::JpegDecodeAccelerator::Error,
-                          media::JpegDecodeAccelerator::LARGEST_ERROR_ENUM)
 IPC_ENUM_TRAITS_MAX_VALUE(media::VideoEncodeAccelerator::Error,
                           media::VideoEncodeAccelerator::kErrorMax)
 

media/gpu/ipc/service/gpu_jpeg_decode_accelerator.h

@@ -11,53 +11,34 @@
 
 #include "base/macros.h"
 #include "base/memory/ref_counted.h"
-#include "base/memory/weak_ptr.h"
-#include "base/sequence_checker.h"
-#include "base/synchronization/waitable_event.h"
-#include "ipc/ipc_listener.h"
-#include "ipc/ipc_sender.h"
+#include "base/threading/thread_checker.h"
 #include "media/gpu/ipc/service/gpu_jpeg_decode_accelerator_factory_provider.h"
 #include "media/gpu/mojo/jpeg_decoder.mojom.h"
 #include "media/video/jpeg_decode_accelerator.h"
 
-namespace base {
-class SingleThreadTaskRunner;
-}
-
-namespace gpu {
-class FilteredSender;
-}
-
 namespace media {
 
 // TODO(c.padhi): Move GpuJpegDecodeAccelerator to media/gpu/mojo, see
 // http://crbug.com/699255.
-class GpuJpegDecodeAccelerator
-    : public IPC::Sender,
-      public mojom::GpuJpegDecodeAccelerator,
-      public base::SupportsWeakPtr<GpuJpegDecodeAccelerator> {
+// Implementation of a mojom::GpuJpegDecodeAccelerator which runs in the GPU
+// process, and wraps a JpegDecodeAccelerator.
+class GpuJpegDecodeAccelerator : public mojom::GpuJpegDecodeAccelerator,
+                                 public JpegDecodeAccelerator::Client {
  public:
-  // |channel| must outlive this object.
-  // This convenience constructor internally calls
-  // GpuJpegDecodeAcceleratorFactoryProvider::GetAcceleratorFactories() to
-  // fill |accelerator_factory_functions_|.
-  GpuJpegDecodeAccelerator(
-      gpu::FilteredSender* channel,
-      scoped_refptr<base::SingleThreadTaskRunner> io_task_runner);
-
-  // |channel| must outlive this object.
-  GpuJpegDecodeAccelerator(
-      gpu::FilteredSender* channel,
-      scoped_refptr<base::SingleThreadTaskRunner> io_task_runner,
-      std::vector<GpuJpegDecodeAcceleratorFactoryProvider::CreateAcceleratorCB>
-          accelerator_factory_functions);
+  static void Create(mojom::GpuJpegDecodeAcceleratorRequest request);
+
   ~GpuJpegDecodeAccelerator() override;
 
-  void AddClient(int32_t route_id, base::Callback<void(bool)> response);
+  // JpegDecodeAccelerator::Client implementation.
+  void VideoFrameReady(int32_t buffer_id) override;
+  void NotifyError(int32_t buffer_id,
+                   JpegDecodeAccelerator::Error error) override;
 
-  void NotifyDecodeStatus(int32_t route_id,
-                          int32_t bitstream_buffer_id,
-                          JpegDecodeAccelerator::Error error);
+ private:
+  // This constructor internally calls
+  // GpuJpegDecodeAcceleratorFactoryProvider::GetAcceleratorFactories() to
+  // fill |accelerator_factory_functions_|.
+  GpuJpegDecodeAccelerator();
 
   // mojom::GpuJpegDecodeAccelerator implementation.
   void Initialize(InitializeCallback callback) override;
@@ -68,35 +49,20 @@ class GpuJpegDecodeAccelerator
               DecodeCallback callback) override;
   void Uninitialize() override;
 
-  // Function to delegate sending to actual sender.
-  bool Send(IPC::Message* message) override;
-
- private:
-  class Client;
-  class MessageFilter;
-
-  void ClientRemoved();
+  void NotifyDecodeStatus(int32_t bitstream_buffer_id,
+                          JpegDecodeAccelerator::Error error);
 
-  std::vector<GpuJpegDecodeAcceleratorFactoryProvider::CreateAcceleratorCB>
+  const std::vector<
+      GpuJpegDecodeAcceleratorFactoryProvider::CreateAcceleratorCB>
       accelerator_factory_functions_;
 
-  gpu::FilteredSender* channel_;
-
-  // The message filter to run JpegDecodeAccelerator::Decode on IO thread.
-  scoped_refptr<MessageFilter> filter_;
-
-  // GPU child task runner.
-  scoped_refptr<base::SingleThreadTaskRunner> child_task_runner_;
-
-  // GPU IO task runner.
-  scoped_refptr<base::SingleThreadTaskRunner> io_task_runner_;
+  DecodeCallback decode_cb_;
 
-  // Number of clients added to |filter_|.
-  int client_number_;
+  std::unique_ptr<JpegDecodeAccelerator> accelerator_;
 
-  SEQUENCE_CHECKER(sequence_checker_);
+  THREAD_CHECKER(thread_checker_);
 
-  DISALLOW_IMPLICIT_CONSTRUCTORS(GpuJpegDecodeAccelerator);
+  DISALLOW_COPY_AND_ASSIGN(GpuJpegDecodeAccelerator);
 };
 
 }  // namespace media

media/gpu/ipc/service/gpu_jpeg_decode_accelerator_unittest.cc

@@ -3,81 +3,44 @@
 // found in the LICENSE file.
 
 #include "media/gpu/ipc/service/gpu_jpeg_decode_accelerator.h"
+
 #include "base/bind.h"
+#include "base/command_line.h"
 #include "base/run_loop.h"
-#include "base/single_thread_task_runner.h"
 #include "base/test/scoped_task_environment.h"
 #include "base/threading/thread.h"
-#include "gpu/ipc/service/gpu_channel.h"
-#include "media/gpu/ipc/common/media_messages.h"
-#include "testing/gmock/include/gmock/gmock.h"
+#include "media/base/media_switches.h"
+#include "mojo/public/cpp/system/platform_handle.h"
 #include "testing/gtest/include/gtest/gtest.h"
 
-using testing::_;
-using testing::SaveArg;
-using testing::InvokeWithoutArgs;
-using testing::Return;
-
 namespace media {
 
-static const size_t kOutputFrameSizeInBytes = 1024;
-static const gfx::Size kDummyFrameCodedSize(10, 10);
-
-class MockFilteredSender : public gpu::FilteredSender {
- public:
-  MOCK_METHOD1(AddFilter, void(IPC::MessageFilter*));
-  MOCK_METHOD1(RemoveFilter, void(IPC::MessageFilter*));
-  MOCK_METHOD1(Send, bool(IPC::Message*));
-};
+static const int32_t kArbitraryBitstreamBufferId = 123;
 
-class MockJpegDecodeAccelerator : public media::JpegDecodeAccelerator {
- public:
-  MOCK_METHOD1(Initialize, bool(Client*));
-  MOCK_METHOD2(Decode,
-               void(const BitstreamBuffer&,
-                    const scoped_refptr<media::VideoFrame>&));
-  MOCK_METHOD0(IsSupported, bool());
-};
-
-// Test fixture for the unit that is created via the constructor of
+// Test fixture for the unit that is created via the mojom interface for
 // class GpuJpegDecodeAccelerator. Uses a FakeJpegDecodeAccelerator to
 // simulate the actual decoding without the need for special hardware.
 class GpuJpegDecodeAcceleratorTest : public ::testing::Test {
  public:
-  GpuJpegDecodeAcceleratorTest() : io_thread_("io") {}
+  GpuJpegDecodeAcceleratorTest() {}
   ~GpuJpegDecodeAcceleratorTest() override {}
 
   void SetUp() override {
-    output_frame_memory_.CreateAndMapAnonymous(kOutputFrameSizeInBytes);
-    io_thread_.Start();
-    std::vector<GpuJpegDecodeAcceleratorFactoryProvider::CreateAcceleratorCB>
-        accelerator_factory_functions;
-    accelerator_factory_functions.push_back(
-        base::Bind(&GpuJpegDecodeAcceleratorTest::GetMockJpegDecodeAccelerator,
-                   base::Unretained(this)));
-    system_under_test_ = base::MakeUnique<GpuJpegDecodeAccelerator>(
-        &gpu_channel_, io_thread_.task_runner(), accelerator_factory_functions);
+    base::CommandLine::ForCurrentProcess()->AppendSwitch(
+        switches::kUseFakeJpegDecodeAccelerator);
   }
 
-  MOCK_METHOD1(GetMockJpegDecodeAccelerator,
-               std::unique_ptr<JpegDecodeAccelerator>(
-                   scoped_refptr<base::SingleThreadTaskRunner>));
-
-  void SendStubFrame(IPC::MessageFilter* message_filter, int32_t route_id) {
-    AcceleratedJpegDecoderMsg_Decode_Params decode_params;
-    decode_params.output_video_frame_handle = output_frame_memory_.handle();
-    decode_params.output_buffer_size =
-        static_cast<uint32_t>(kOutputFrameSizeInBytes);
-    decode_params.coded_size = kDummyFrameCodedSize;
-    AcceleratedJpegDecoderMsg_Decode message(route_id, decode_params);
-    message_filter->OnMessageReceived(message);
+  void OnInitializeDone(const base::Closure& continuation, bool success) {
+    EXPECT_TRUE(success);
+    continuation.Run();
   }
 
- protected:
-  base::Thread io_thread_;
-  MockFilteredSender gpu_channel_;
-  std::unique_ptr<GpuJpegDecodeAccelerator> system_under_test_;
-  base::SharedMemory output_frame_memory_;
+  void OnDecodeAck(const base::Closure& continuation,
+                   int32_t bitstream_buffer_id,
+                   JpegDecodeAccelerator::Error error) {
+    EXPECT_EQ(kArbitraryBitstreamBufferId, bitstream_buffer_id);
+    continuation.Run();
+  }
 
  private:
   // This is required to allow base::ThreadTaskRunnerHandle::Get() from the
@@ -85,48 +48,43 @@ class GpuJpegDecodeAcceleratorTest : public ::testing::Test {
   base::test::ScopedTaskEnvironment scoped_task_environment_;
 };
 
-// Tests that the communication for decoding a frame between the caller of
-// IPC:MessageFilter and the media::JpegDecodeAccelerator works as expected.
-TEST_F(GpuJpegDecodeAcceleratorTest, DecodeFrameCallArrivesAtDecoder) {
-  static const int32_t kArbitraryRouteId = 123;
-  auto io_task_runner = io_thread_.task_runner();
-  auto main_task_runner = base::ThreadTaskRunnerHandle::Get();
-  auto decoder = base::MakeUnique<MockJpegDecodeAccelerator>();
-  auto* decoder_ptr = decoder.get();
-  ON_CALL(*decoder, Initialize(_)).WillByDefault(Return(true));
-
-  scoped_refptr<IPC::MessageFilter> message_filter = nullptr;
-  EXPECT_CALL(*this, GetMockJpegDecodeAccelerator(_))
-      .WillOnce(InvokeWithoutArgs([&decoder]() { return std::move(decoder); }));
-  EXPECT_CALL(gpu_channel_, AddFilter(_)).WillOnce(SaveArg<0>(&message_filter));
+TEST_F(GpuJpegDecodeAcceleratorTest, InitializeAndDecode) {
+  mojom::GpuJpegDecodeAcceleratorPtr jpeg_decoder;
+  GpuJpegDecodeAccelerator::Create(mojo::MakeRequest(&jpeg_decoder));
+
   base::RunLoop run_loop;
-  auto response_cb = base::Bind(
-      [](base::RunLoop* run_loop,
-         scoped_refptr<base::SingleThreadTaskRunner> main_task_runner,
-         bool succeeded) {
-        EXPECT_TRUE(succeeded);
-        if (main_task_runner->BelongsToCurrentThread() == false) {
-          DLOG(INFO) << "Response arrived on thread other than main thread.";
-          main_task_runner->PostTask(FROM_HERE, run_loop->QuitClosure());
-          return;
-        }
-        run_loop->Quit();
-      },
-      &run_loop, main_task_runner);
-  system_under_test_->AddClient(kArbitraryRouteId, response_cb);
+  jpeg_decoder->Initialize(
+      base::Bind(&GpuJpegDecodeAcceleratorTest::OnInitializeDone,
+                 base::Unretained(this), run_loop.QuitClosure()));
   run_loop.Run();
-  ASSERT_TRUE(message_filter);
 
-  // Send a AcceleratedJpegDecoderMsg_Decode to the |message_filter| on the
-  // io_thread.
+  const size_t kInputBufferSizeInBytes = 512;
+  const size_t kOutputFrameSizeInBytes = 1024;
+  const gfx::Size kDummyFrameCodedSize(10, 10);
+  const char kKeyId[] = "key id";
+  const char kIv[] = "0123456789abcdef";
+  std::vector<SubsampleEntry> subsamples;
+  subsamples.push_back(SubsampleEntry(10, 5));
+  subsamples.push_back(SubsampleEntry(15, 7));
+
   base::RunLoop run_loop2;
-  EXPECT_CALL(*decoder_ptr, Decode(_, _));
-  io_task_runner->PostTaskAndReply(
-      FROM_HERE,
-      base::Bind(&GpuJpegDecodeAcceleratorTest::SendStubFrame,
-                 base::Unretained(this), base::RetainedRef(message_filter),
-                 kArbitraryRouteId),
-      run_loop2.QuitClosure());
+  base::SharedMemory shm;
+  ASSERT_TRUE(shm.CreateAndMapAnonymous(kInputBufferSizeInBytes));
+
+  mojo::ScopedSharedBufferHandle output_frame_handle =
+      mojo::SharedBufferHandle::Create(kOutputFrameSizeInBytes);
+
+  BitstreamBuffer bitstream_buffer(
+      kArbitraryBitstreamBufferId,
+      base::SharedMemory::DuplicateHandle(shm.handle()),
+      kInputBufferSizeInBytes);
+  bitstream_buffer.SetDecryptConfig(DecryptConfig(kKeyId, kIv, subsamples));
+
+  jpeg_decoder->Decode(
+      bitstream_buffer, kDummyFrameCodedSize, std::move(output_frame_handle),
+      base::checked_cast<uint32_t>(kOutputFrameSizeInBytes),
+      base::Bind(&GpuJpegDecodeAcceleratorTest::OnDecodeAck,
+                 base::Unretained(this), run_loop2.QuitClosure()));
   run_loop2.Run();
 }
 

media/gpu/ipc/service/media_gpu_channel.cc

@@ -104,49 +104,11 @@ bool MediaGpuChannel::OnMessageReceived(const IPC::Message& message) {
     IPC_MESSAGE_FORWARD_DELAY_REPLY(
         GpuCommandBufferMsg_CreateVideoEncoder, &helper,
         MediaGpuChannelDispatchHelper::OnCreateVideoEncoder)
-    IPC_MESSAGE_HANDLER_DELAY_REPLY(GpuChannelMsg_CreateJpegDecoder,
-                                    OnCreateJpegDecoder)
     IPC_MESSAGE_UNHANDLED(handled = false)
   IPC_END_MESSAGE_MAP()
   return handled;
 }
 
-namespace {
-
-void SendCreateJpegDecoderResult(
-    std::unique_ptr<IPC::Message> reply_message,
-    scoped_refptr<base::SingleThreadTaskRunner> io_task_runner,
-    base::WeakPtr<IPC::Sender> channel,
-    scoped_refptr<MediaGpuChannelFilter> filter,
-    bool result) {
-  GpuChannelMsg_CreateJpegDecoder::WriteReplyParams(reply_message.get(),
-                                                    result);
-  if (io_task_runner->BelongsToCurrentThread()) {
-    filter->Send(reply_message.release());
-  } else if (channel) {
-    channel->Send(reply_message.release());
-  }
-}
-
-}  // namespace
-
-void MediaGpuChannel::OnCreateJpegDecoder(int32_t route_id,
-                                          IPC::Message* reply_msg) {
-  std::unique_ptr<IPC::Message> msg(reply_msg);
-  if (!jpeg_decoder_) {
-    // The lifetime of |jpeg_decoder_| is managed by a gpu::GpuChannel. The
-    // GpuChannels destroy all the GpuJpegDecodeAccelerator that they own when
-    // they are destroyed. Therefore, passing |channel_| as a raw pointer is
-    // safe.
-    jpeg_decoder_.reset(
-        new GpuJpegDecodeAccelerator(channel_, channel_->io_task_runner()));
-  }
-  jpeg_decoder_->AddClient(
-      route_id,
-      base::Bind(&SendCreateJpegDecoderResult, base::Passed(&msg),
-                 channel_->io_task_runner(), channel_->AsWeakPtr(), filter_));
-}
-
 void MediaGpuChannel::OnCreateVideoDecoder(
     int32_t command_buffer_route_id,
     const VideoDecodeAccelerator::Config& config,

media/gpu/ipc/service/media_gpu_channel.h

@@ -11,7 +11,6 @@
 #include "ipc/ipc_listener.h"
 #include "ipc/ipc_sender.h"
 #include "media/base/android_overlay_mojo_factory.h"
-#include "media/gpu/ipc/service/gpu_jpeg_decode_accelerator.h"
 #include "media/video/video_decode_accelerator.h"
 
 namespace media {
@@ -44,7 +43,6 @@ class MediaGpuChannel : public IPC::Listener, public IPC::Sender {
   bool OnMessageReceived(const IPC::Message& message) override;
 
   // Message handlers.
-  void OnCreateJpegDecoder(int32_t route_id, IPC::Message* reply_msg);
   void OnCreateVideoDecoder(int32_t command_buffer_route_id,
                             const VideoDecodeAccelerator::Config& config,
                             int32_t route_id,
@@ -55,7 +53,6 @@ class MediaGpuChannel : public IPC::Listener, public IPC::Sender {
 
   gpu::GpuChannel* const channel_;
   scoped_refptr<MediaGpuChannelFilter> filter_;
-  std::unique_ptr<GpuJpegDecodeAccelerator> jpeg_decoder_;
   AndroidOverlayMojoFactoryCB overlay_factory_cb_;
 
   DISALLOW_COPY_AND_ASSIGN(MediaGpuChannel);

media/gpu/mojo/jpeg_decoder_typemap_traits.cc

@@ -66,8 +66,14 @@ bool EnumTraits<media::mojom::DecodeError,
 mojo::ScopedSharedBufferHandle
 StructTraits<media::mojom::BitstreamBufferDataView, media::BitstreamBuffer>::
     memory_handle(const media::BitstreamBuffer& input) {
-  return mojo::WrapSharedMemoryHandle(input.handle(), input.handle().GetSize(),
-                                      false);
+  base::SharedMemoryHandle input_handle =
+      base::SharedMemory::DuplicateHandle(input.handle());
+  if (!base::SharedMemory::IsHandleValid(input_handle)) {
+    DLOG(ERROR) << "Failed to duplicate handle of BitstreamBuffer";
+    return mojo::ScopedSharedBufferHandle();
+  }
+  return mojo::WrapSharedMemoryHandle(input_handle, input.size(),
+                                      true /* read_only */);
 }
 
 // static
@@ -104,8 +110,10 @@ bool StructTraits<
   media::BitstreamBuffer bitstream_buffer(
       input.id(), memory_handle, input.size(),
       base::checked_cast<off_t>(input.offset()), timestamp);
-  bitstream_buffer.SetDecryptConfig(
-      media::DecryptConfig(key_id, iv, subsamples));
+  if (key_id.size()) {
+    bitstream_buffer.SetDecryptConfig(
+        media::DecryptConfig(key_id, iv, subsamples));
+  }
   *output = bitstream_buffer;
 
   return true;