[WebMIDI] Mojofy and remove midi_messages.h

- Converts IPC messages in midi_messages.h to mojo interfaces
  (in midi_service.mojom)
- Updates MidiHost and MidiMessageFilter to use mojo
- Changes MidiMessageFilter to do all work on the main task runner
- Renames MidiMessageFilter to MidiClientImpl

Bug: 582328
Change-Id: I1a62af005dc31118aad0081c30b6fbd1f63068a3
Reviewed-on: https://chromium-review.googlesource.com/c/1238815
Commit-Queue: Adithya Srinivasan <adithyas@chromium.org>
Reviewed-by: Kinuko Yasuda <kinuko@chromium.org>
Reviewed-by: Takashi Toyoshima <toyoshim@chromium.org>
Cr-Commit-Position: refs/heads/master@{#611747}

content/browser/media/midi_host.cc

@@ -12,11 +12,12 @@
 #include "content/browser/bad_message.h"
 #include "content/browser/browser_main_loop.h"
 #include "content/browser/child_process_security_policy_impl.h"
-#include "content/common/media/midi_messages.h"
 #include "content/public/browser/content_browser_client.h"
+#include "content/public/browser/render_process_host.h"
 #include "media/midi/message_util.h"
 #include "media/midi/midi_message_queue.h"
 #include "media/midi/midi_service.h"
+#include "mojo/public/cpp/bindings/strong_binding.h"
 
 namespace content {
 namespace {
@@ -42,129 +43,69 @@ using midi::mojom::PortState;
 using midi::mojom::Result;
 
 MidiHost::MidiHost(int renderer_process_id, midi::MidiService* midi_service)
-    : BrowserMessageFilter(MidiMsgStart),
-      renderer_process_id_(renderer_process_id),
+    : renderer_process_id_(renderer_process_id),
       has_sys_ex_permission_(false),
-      is_session_requested_(false),
       midi_service_(midi_service),
       sent_bytes_in_flight_(0),
       bytes_sent_since_last_acknowledgement_(0),
-      output_port_count_(0) {
+      output_port_count_(0),
+      midi_session_(this) {
   DCHECK(midi_service_);
 }
 
-MidiHost::~MidiHost() = default;
-
-void MidiHost::OnChannelClosing() {
-  // If we get here the MidiHost is going to be destroyed soon.  Prevent any
-  // subsequent calls from MidiService by closing our session.
-  // If Send() is called from a different thread (e.g. a separate thread owned
-  // by the MidiService implementation), it will get posted to the IO thread.
-  // There is a race condition here if our refcount is 0 and we're about to or
-  // have already entered OnDestruct().
-  if (is_session_requested_ && midi_service_) {
-    midi_service_->EndSession(this);
-    is_session_requested_ = false;
-  }
-}
-
-void MidiHost::OnDestruct() const {
-  BrowserThread::DeleteOnIOThread::Destruct(this);
-}
-
-// IPC Messages handler
-bool MidiHost::OnMessageReceived(const IPC::Message& message) {
-  bool handled = true;
-  IPC_BEGIN_MESSAGE_MAP(MidiHost, message)
-    IPC_MESSAGE_HANDLER(MidiHostMsg_StartSession, OnStartSession)
-    IPC_MESSAGE_HANDLER(MidiHostMsg_SendData, OnSendData)
-    IPC_MESSAGE_HANDLER(MidiHostMsg_EndSession, OnEndSession)
-    IPC_MESSAGE_UNHANDLED(handled = false)
-  IPC_END_MESSAGE_MAP()
-
-  return handled;
-}
-
-void MidiHost::OnStartSession() {
-  is_session_requested_ = true;
-  if (midi_service_)
-    midi_service_->StartSession(this);
-}
-
-void MidiHost::OnSendData(uint32_t port,
-                          const std::vector<uint8_t>& data,
-                          base::TimeTicks timestamp) {
-  {
-    base::AutoLock auto_lock(output_port_count_lock_);
-    if (output_port_count_ <= port) {
-      bad_message::ReceivedBadMessage(this, bad_message::MH_INVALID_MIDI_PORT);
-      return;
-    }
-  }
-
-  if (data.empty())
-    return;
-
-  // Blink running in a renderer checks permission to raise a SecurityError
-  // in JavaScript. The actual permission check for security purposes
-  // happens here in the browser process.
-  if (!has_sys_ex_permission_ && base::ContainsValue(data, kSysExByte)) {
-    bad_message::ReceivedBadMessage(this, bad_message::MH_SYS_EX_PERMISSION);
-    return;
-  }
-
-  if (!IsValidWebMIDIData(data))
-    return;
-
-  {
-    base::AutoLock auto_lock(in_flight_lock_);
-    // Sanity check that we won't send too much data.
-    // TODO(yukawa): Consider to send an error event back to the renderer
-    // after some future discussion in W3C.
-    if (data.size() + sent_bytes_in_flight_ > kMaxInFlightBytes)
-      return;
-    sent_bytes_in_flight_ += data.size();
-  }
-  if (midi_service_)
-    midi_service_->DispatchSendMidiData(this, port, data, timestamp);
+MidiHost::~MidiHost() {
+  DCHECK_CURRENTLY_ON(BrowserThread::IO);
+  if (midi_client_ && midi_service_)
+    EndSession();
 }
 
-void MidiHost::OnEndSession() {
-  is_session_requested_ = false;
-  if (midi_service_)
-    midi_service_->EndSession(this);
+// static
+void MidiHost::BindRequest(int render_process_id,
+                           midi::MidiService* midi_service,
+                           midi::mojom::MidiSessionProviderRequest request) {
+  DCHECK_CURRENTLY_ON(BrowserThread::IO);
+  mojo::MakeStrongBinding(
+      base::WrapUnique(new MidiHost(render_process_id, midi_service)),
+      std::move(request));
 }
 
 void MidiHost::CompleteStartSession(Result result) {
-  DCHECK(is_session_requested_);
+  DCHECK_CURRENTLY_ON(BrowserThread::IO);
+  DCHECK(midi_client_);
   if (result == Result::OK) {
     // ChildSecurityPolicy is set just before OnStartSession by
     // MidiDispatcherHost. So we can safely cache the policy.
-    has_sys_ex_permission_ = ChildProcessSecurityPolicyImpl::GetInstance()->
-        CanSendMidiSysExMessage(renderer_process_id_);
+    has_sys_ex_permission_ =
+        ChildProcessSecurityPolicyImpl::GetInstance()->CanSendMidiSysExMessage(
+            renderer_process_id_);
+    midi_session_.Bind(std::move(pending_session_request_));
+    midi_session_.set_connection_error_handler(
+        base::BindOnce(&MidiHost::EndSession, base::Unretained(this)));
   }
-  Send(new MidiMsg_SessionStarted(result));
+  midi_client_->SessionStarted(result);
 }
 
 void MidiHost::AddInputPort(const midi::mojom::PortInfo& info) {
   base::AutoLock auto_lock(messages_queues_lock_);
   // MidiMessageQueue is created later in ReceiveMidiData().
   received_messages_queues_.push_back(nullptr);
-  Send(new MidiMsg_AddInputPort(info));
+  CallClient(&midi::mojom::MidiSessionClient::AddInputPort,
+             midi::mojom::PortInfo::New(info));
 }
 
 void MidiHost::AddOutputPort(const midi::mojom::PortInfo& info) {
   base::AutoLock auto_lock(output_port_count_lock_);
   output_port_count_++;
-  Send(new MidiMsg_AddOutputPort(info));
+  CallClient(&midi::mojom::MidiSessionClient::AddOutputPort,
+             midi::mojom::PortInfo::New(info));
 }
 
 void MidiHost::SetInputPortState(uint32_t port, PortState state) {
-  Send(new MidiMsg_SetInputPortState(port, state));
+  CallClient(&midi::mojom::MidiSessionClient::SetInputPortState, port, state);
 }
 
 void MidiHost::SetOutputPortState(uint32_t port, PortState state) {
-  Send(new MidiMsg_SetOutputPortState(port, state));
+  CallClient(&midi::mojom::MidiSessionClient::SetOutputPortState, port, state);
 }
 
 void MidiHost::ReceiveMidiData(uint32_t port,
@@ -191,12 +132,13 @@ void MidiHost::ReceiveMidiData(uint32_t port,
 
     // MIDI devices may send a system exclusive messages even if the renderer
     // doesn't have a permission to receive it. Don't kill the renderer as
-    // OnSendData() does.
+    // SendData() does.
     if (message[0] == kSysExByte && !has_sys_ex_permission_)
       continue;
 
     // Send to the renderer.
-    Send(new MidiMsg_DataReceived(port, message, timestamp));
+    CallClient(&midi::mojom::MidiSessionClient::DataReceived, port, message,
+               timestamp);
   }
 }
 
@@ -213,8 +155,8 @@ void MidiHost::AccumulateMidiBytesSent(size_t n) {
 
   if (bytes_sent_since_last_acknowledgement_ >=
       kAcknowledgementThresholdBytes) {
-    Send(new MidiMsg_AcknowledgeSentData(
-        bytes_sent_since_last_acknowledgement_));
+    CallClient(&midi::mojom::MidiSessionClient::AcknowledgeSentData,
+               bytes_sent_since_last_acknowledgement_);
     bytes_sent_since_last_acknowledgement_ = 0;
   }
 }
@@ -223,4 +165,82 @@ void MidiHost::Detach() {
   midi_service_ = nullptr;
 }
 
+void MidiHost::StartSession(midi::mojom::MidiSessionRequest request,
+                            midi::mojom::MidiSessionClientPtr client) {
+  DCHECK_CURRENTLY_ON(BrowserThread::IO);
+  DCHECK(!pending_session_request_);
+  // Checks to see if |midi_session_| isn't already bound to another
+  // MidiSessionRequest.
+  DCHECK(!midi_session_);
+  pending_session_request_ = std::move(request);
+
+  DCHECK(!midi_client_);
+  midi_client_ = std::move(client);
+
+  if (midi_service_)
+    midi_service_->StartSession(this);
+}
+
+void MidiHost::SendData(uint32_t port,
+                        const std::vector<uint8_t>& data,
+                        base::TimeTicks timestamp) {
+  DCHECK_CURRENTLY_ON(BrowserThread::IO);
+  {
+    base::AutoLock auto_lock(output_port_count_lock_);
+    if (output_port_count_ <= port) {
+      bad_message::ReceivedBadMessage(
+          RenderProcessHost::FromID(renderer_process_id_),
+          bad_message::MH_INVALID_MIDI_PORT);
+      return;
+    }
+  }
+
+  if (data.empty())
+    return;
+
+  // Blink running in a renderer checks permission to raise a SecurityError
+  // in JavaScript. The actual permission check for security purposes
+  // happens here in the browser process.
+  if (!has_sys_ex_permission_ && base::ContainsValue(data, kSysExByte)) {
+    bad_message::ReceivedBadMessage(
+        RenderProcessHost::FromID(renderer_process_id_),
+        bad_message::MH_SYS_EX_PERMISSION);
+    return;
+  }
+
+  if (!IsValidWebMIDIData(data))
+    return;
+
+  {
+    base::AutoLock auto_lock(in_flight_lock_);
+    // Sanity check that we won't send too much data.
+    // TODO(yukawa): Consider to send an error event back to the renderer
+    // after some future discussion in W3C.
+    if (data.size() + sent_bytes_in_flight_ > kMaxInFlightBytes)
+      return;
+    sent_bytes_in_flight_ += data.size();
+  }
+  if (midi_service_)
+    midi_service_->DispatchSendMidiData(this, port, data, timestamp);
+}
+
+template <typename Method, typename... Params>
+void MidiHost::CallClient(Method method, Params... params) {
+  if (!BrowserThread::CurrentlyOn(BrowserThread::IO)) {
+    base::PostTaskWithTraits(
+        FROM_HERE, {BrowserThread::IO},
+        base::BindOnce(&MidiHost::CallClient<Method, Params...>, AsWeakPtr(),
+                       method, std::move(params)...));
+    return;
+  }
+  (midi_client_.get()->*method)(std::move(params)...);
+}
+
+void MidiHost::EndSession() {
+  if (midi_service_)
+    midi_service_->EndSession(this);
+  midi_client_.reset();
+  midi_session_.Close();
+};
+
 }  // namespace content

content/browser/media/midi_host.h

@@ -15,11 +15,16 @@
 #include "base/macros.h"
 #include "base/memory/ref_counted.h"
 #include "base/synchronization/lock.h"
+#include "base/task/post_task.h"
+#include "base/thread_annotations.h"
+#include "base/tuple.h"
 #include "content/common/content_export.h"
 #include "content/public/browser/browser_message_filter.h"
+#include "content/public/browser/browser_task_traits.h"
 #include "content/public/browser/browser_thread.h"
 #include "media/midi/midi_manager.h"
 #include "media/midi/midi_service.mojom.h"
+#include "mojo/public/cpp/bindings/binding.h"
 
 namespace midi {
 class MidiService;
@@ -28,18 +33,22 @@ class MidiMessageQueue;
 
 namespace content {
 
-class CONTENT_EXPORT MidiHost : public BrowserMessageFilter,
-                                public midi::MidiManagerClient {
+class CONTENT_EXPORT MidiHost : public midi::MidiManagerClient,
+                                public midi::mojom::MidiSessionProvider,
+                                public midi::mojom::MidiSession,
+                                public base::SupportsWeakPtr<MidiHost> {
  public:
-  // Called from UI thread from the owner of this object.
-  MidiHost(int renderer_process_id, midi::MidiService* midi_service);
+  ~MidiHost() override;
 
-  // BrowserMessageFilter implementation.
-  void OnChannelClosing() override;
-  void OnDestruct() const override;
-  bool OnMessageReceived(const IPC::Message& message) override;
+  // Creates an instance of MidiHost and binds |request| to the instance using
+  // a strong binding. Should be called on the IO thread.
+  static void BindRequest(int render_process_id,
+                          midi::MidiService* midi_service,
+                          midi::mojom::MidiSessionProviderRequest request);
 
-  // MidiManagerClient implementation.
+  // MidiManagerClient implementation. These methods can be called on any thread
+  // by platform specific implementations of MidiManager, so use locks
+  // appropriately.
   void CompleteStartSession(midi::mojom::Result result) override;
   void AddInputPort(const midi::mojom::PortInfo& info) override;
   void AddOutputPort(const midi::mojom::PortInfo& info) override;
@@ -52,22 +61,25 @@ class CONTENT_EXPORT MidiHost : public BrowserMessageFilter,
   void AccumulateMidiBytesSent(size_t n) override;
   void Detach() override;
 
-  // Start session to access MIDI hardware.
-  void OnStartSession();
-
-  // Data to be sent to a MIDI output port.
-  void OnSendData(uint32_t port,
-                  const std::vector<uint8_t>& data,
-                  base::TimeTicks timestamp);
+  // midi::mojom::MidiSessionProvider implementation.
+  void StartSession(midi::mojom::MidiSessionRequest session_request,
+                    midi::mojom::MidiSessionClientPtr client) override;
 
-  void OnEndSession();
+  // midi::mojom::MidiSession implementation.
+  void SendData(uint32_t port,
+                const std::vector<uint8_t>& data,
+                base::TimeTicks timestamp) override;
 
  protected:
-  ~MidiHost() override;
+  MidiHost(int renderer_process_id, midi::MidiService* midi_service);
 
  private:
-  friend class base::DeleteHelper<MidiHost>;
-  friend class BrowserThread;
+  // Use this to call methods on |midi_client_|. It makes sure that midi_client_
+  // is only accessed on the IO thread.
+  template <typename Method, typename... Params>
+  void CallClient(Method method, Params... params);
+
+  void EndSession();
 
   int renderer_process_id_;
 
@@ -75,23 +87,20 @@ class CONTENT_EXPORT MidiHost : public BrowserMessageFilter,
   // messages.
   bool has_sys_ex_permission_;
 
-  // Represents if a session is requested to start.
-  bool is_session_requested_;
-
   // |midi_service_| manages a MidiManager instance that talks to
   // platform-specific MIDI APIs.  It can be nullptr after detached.
   midi::MidiService* midi_service_;
 
   // Buffers where data sent from each MIDI input port is stored.
-  std::vector<std::unique_ptr<midi::MidiMessageQueue>>
-      received_messages_queues_;
+  std::vector<std::unique_ptr<midi::MidiMessageQueue>> received_messages_queues_
+      GUARDED_BY(messages_queues_lock_);
 
   // Protects access to |received_messages_queues_|;
   base::Lock messages_queues_lock_;
 
   // The number of bytes sent to the platform-specific MIDI sending
   // system, but not yet completed.
-  size_t sent_bytes_in_flight_;
+  size_t sent_bytes_in_flight_ GUARDED_BY(in_flight_lock_);
 
   // The number of bytes successfully sent since the last time
   // we've acknowledged back to the renderer.
@@ -101,11 +110,22 @@ class CONTENT_EXPORT MidiHost : public BrowserMessageFilter,
   base::Lock in_flight_lock_;
 
   // How many output port exists.
-  uint32_t output_port_count_;
+  uint32_t output_port_count_ GUARDED_BY(output_port_count_lock_);
 
   // Protects access to |output_port_count_|.
   base::Lock output_port_count_lock_;
 
+  // Stores a session request sent from the renderer until CompleteStartSession
+  // is called.
+  midi::mojom::MidiSessionRequest pending_session_request_;
+
+  // Bound on the IO thread if a session is successfully started by MidiService.
+  mojo::Binding<midi::mojom::MidiSession> midi_session_;
+
+  // Bound on the IO thread and should only be called there. Use CallClient to
+  // call midi::mojom::MidiSessionClient methods.
+  midi::mojom::MidiSessionClientPtr midi_client_;
+
   DISALLOW_COPY_AND_ASSIGN(MidiHost);
 };
 

content/browser/media/midi_host_unittest.cc

@@ -11,8 +11,8 @@
 #include "base/memory/weak_ptr.h"
 #include "base/run_loop.h"
 #include "base/strings/stringprintf.h"
-#include "base/test/scoped_task_environment.h"
-#include "content/common/media/midi_messages.h"
+#include "content/public/test/mock_render_process_host.h"
+#include "content/public/test/test_browser_context.h"
 #include "content/public/test/test_browser_thread_bundle.h"
 #include "media/midi/midi_manager.h"
 #include "media/midi/midi_service.h"
@@ -24,7 +24,6 @@ namespace {
 using midi::mojom::PortState;
 
 const uint8_t kNoteOn[] = {0x90, 0x3c, 0x7f};
-const int kRenderProcessId = 0;
 
 enum MidiEventType {
   DISPATCH_SEND_MIDI_DATA,
@@ -60,10 +59,8 @@ class FakeMidiManager : public midi::MidiManager {
                             uint32_t port_index,
                             const std::vector<uint8_t>& data,
                             base::TimeTicks timestamp) override {
-    events_.push_back(MidiEvent(DISPATCH_SEND_MIDI_DATA,
-                                port_index,
-                                data,
-                                timestamp));
+    events_.push_back(
+        MidiEvent(DISPATCH_SEND_MIDI_DATA, port_index, data, timestamp));
   }
   std::vector<MidiEvent> events_;
 
@@ -102,31 +99,30 @@ class MidiHostForTesting : public MidiHost {
  public:
   MidiHostForTesting(int renderer_process_id, midi::MidiService* midi_service)
       : MidiHost(renderer_process_id, midi_service) {}
-
- private:
   ~MidiHostForTesting() override {}
 
-  // BrowserMessageFilter implementation.
-  // Override ShutdownForBadMessage() to do nothing since the original
-  // implementation to kill a malicious renderer process causes a check failure
-  // in unit tests.
-  void ShutdownForBadMessage() override {}
-
+ private:
   DISALLOW_COPY_AND_ASSIGN(MidiHostForTesting);
 };
 
 class MidiHostTest : public testing::Test {
  public:
   MidiHostTest() : data_(kNoteOn, kNoteOn + base::size(kNoteOn)), port_id_(0) {
+    browser_context_ = std::make_unique<TestBrowserContext>();
+    rph_ = std::make_unique<MockRenderProcessHost>(browser_context_.get());
     std::unique_ptr<FakeMidiManagerFactory> factory =
         std::make_unique<FakeMidiManagerFactory>();
     factory_ = factory->GetWeakPtr();
     service_ = std::make_unique<midi::MidiService>(std::move(factory));
-    host_ = new MidiHostForTesting(kRenderProcessId, service_.get());
-    host_->OnStartSession();
+    host_ = std::make_unique<MidiHostForTesting>(rph_->GetID(), service_.get());
+    midi::mojom::MidiSessionClientPtr ptr;
+    midi::mojom::MidiSessionClientRequest request = mojo::MakeRequest(&ptr);
+    midi::mojom::MidiSessionRequest session_request =
+        mojo::MakeRequest(&session_);
+    host_->StartSession(std::move(session_request), std::move(ptr));
   }
   ~MidiHostTest() override {
-    host_->OnEndSession();
+    session_.reset();
     service_->Shutdown();
     RunLoopUntilIdle();
   }
@@ -144,9 +140,7 @@ class MidiHostTest : public testing::Test {
   }
 
   void OnSendData(uint32_t port) {
-    std::unique_ptr<IPC::Message> message(
-        new MidiHostMsg_SendData(port, data_, base::TimeTicks()));
-    host_->OnMessageReceived(*message.get());
+    host_->SendData(port, data_, base::TimeTicks());
   }
 
   size_t GetEventSize() const {
@@ -169,14 +163,19 @@ class MidiHostTest : public testing::Test {
     run_loop.RunUntilIdle();
   }
 
+  int GetNumberOfBadMessages() { return rph_->bad_msg_count(); }
+
  private:
   TestBrowserThreadBundle thread_bundle_;
+  std::unique_ptr<BrowserContext> browser_context_;
+  std::unique_ptr<MockRenderProcessHost> rph_;
 
   std::vector<uint8_t> data_;
   int32_t port_id_;
   base::WeakPtr<FakeMidiManagerFactory> factory_;
   std::unique_ptr<midi::MidiService> service_;
-  scoped_refptr<MidiHostForTesting> host_;
+  std::unique_ptr<MidiHostForTesting> host_;
+  midi::mojom::MidiSessionPtr session_;
 
   DISALLOW_COPY_AND_ASSIGN(MidiHostTest);
 };
@@ -200,6 +199,7 @@ TEST_F(MidiHostTest, OutputPortCheck) {
   OnSendData(port1);
   RunLoopUntilIdle();
   EXPECT_EQ(1U, GetEventSize());
+  EXPECT_EQ(1, GetNumberOfBadMessages());
 
   // Two output ports are available from now on.
   AddOutputPort();
@@ -213,4 +213,4 @@ TEST_F(MidiHostTest, OutputPortCheck) {
   CheckSendEventAt(2, port1);
 }
 
-}  // namespace conent
+}  // namespace content

content/browser/renderer_host/render_process_host_impl.cc

@@ -2001,8 +2001,6 @@ void RenderProcessHostImpl::CreateMessageFilters() {
     AddFilter(resource_message_filter_.get());
   }
 
-  AddFilter(
-      new MidiHost(GetID(), BrowserMainLoop::GetInstance()->midi_service()));
   AddFilter(new DOMStorageMessageFilter(
       storage_partition_impl_->GetDOMStorageContext()));
 
@@ -2179,6 +2177,10 @@ void RenderProcessHostImpl::RegisterMojoInterfaces() {
       base::BindRepeating(&FileSystemManagerImpl::BindRequest,
                           base::Unretained(file_system_manager_impl_.get())));
 
+  registry->AddInterface(base::BindRepeating(
+      &MidiHost::BindRequest, GetID(),
+      base::Unretained(BrowserMainLoop::GetInstance()->midi_service())));
+
   if (gpu_client_) {
     // |gpu_client_| outlives the registry, because its destruction is posted to
     // IO thread from the destructor of |this|.

content/common/BUILD.gn

@@ -2,7 +2,6 @@
 # Use of this source code is governed by a BSD-style license that can be
 # found in the LICENSE file.
 
-import("features.gni")
 import("//build/buildflag_header.gni")
 import("//build/config/features.gni")
 import("//build/config/ui.gni")
@@ -11,6 +10,7 @@ import("//mojo/public/tools/bindings/mojom.gni")
 import("//ppapi/buildflags/buildflags.gni")
 import("//sandbox/features.gni")
 import("//tools/ipc_fuzzer/ipc_fuzzer.gni")
+import("features.gni")
 if (is_mac) {
   import("//build/config/mac/mac_sdk.gni")
 }
@@ -191,7 +191,6 @@ source_set("common") {
     "media/media_player_messages_android.h",
     "media/media_stream_controls.cc",
     "media/media_stream_controls.h",
-    "media/midi_messages.h",
     "media/peer_connection_tracker_messages.h",
     "media/video_capture.h",
     "mime_sniffing_throttle.cc",

content/common/content_message_generator.h

@@ -52,11 +52,6 @@
 #ifndef CONTENT_COMMON_MEDIA_MEDIA_PLAYER_DELEGATE_MESSAGES_H_
 #error "Failed to include content/common/media/media_player_delegate_messages.h"
 #endif
-#undef CONTENT_COMMON_MEDIA_MIDI_MESSAGES_H_
-#include "content/common/media/midi_messages.h"
-#ifndef CONTENT_COMMON_MEDIA_MIDI_MESSAGES_H_
-#error "Failed to include content/common/media/midi_messages.h"
-#endif
 #undef CONTENT_COMMON_MEDIA_PEER_CONNECTION_TRACKER_MESSAGES_H_
 #include "content/common/media/peer_connection_tracker_messages.h"
 #ifndef CONTENT_COMMON_MEDIA_PEER_CONNECTION_TRACKER_MESSAGES_H_

content/common/media/midi_messages.h

-// Copyright (c) 2013 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 CONTENT_COMMON_MEDIA_MIDI_MESSAGES_H_
-#define CONTENT_COMMON_MEDIA_MIDI_MESSAGES_H_
-
-// IPC messages for access to MIDI hardware.
-
-// TODO(toyoshim): Mojofication is working in progress. Until the work is
-// finished, this file temporarily depends on midi_service.mojom.h.
-// Once the migration is finished, this file will be removed.
-// http://crbug.com/582327
-
-#include <stdint.h>
-
-#include "content/common/content_export.h"
-#include "ipc/ipc_message_macros.h"
-#include "ipc/param_traits_macros.h"
-#include "media/midi/midi_service.mojom.h"
-#include "url/gurl.h"
-
-#undef IPC_MESSAGE_EXPORT
-#define IPC_MESSAGE_EXPORT CONTENT_EXPORT
-#define IPC_MESSAGE_START MidiMsgStart
-
-IPC_ENUM_TRAITS_MAX_VALUE(midi::mojom::PortState, midi::mojom::PortState::LAST)
-
-IPC_STRUCT_TRAITS_BEGIN(midi::mojom::PortInfo)
-  IPC_STRUCT_TRAITS_MEMBER(id)
-  IPC_STRUCT_TRAITS_MEMBER(manufacturer)
-  IPC_STRUCT_TRAITS_MEMBER(name)
-  IPC_STRUCT_TRAITS_MEMBER(version)
-  IPC_STRUCT_TRAITS_MEMBER(state)
-IPC_STRUCT_TRAITS_END()
-
-IPC_ENUM_TRAITS_MAX_VALUE(midi::mojom::Result, midi::mojom::Result::MAX)
-
-// Messages for IPC between MidiMessageFilter and MidiHost.
-
-// Renderer request to browser for access to MIDI services.
-IPC_MESSAGE_CONTROL0(MidiHostMsg_StartSession)
-
-IPC_MESSAGE_CONTROL3(MidiHostMsg_SendData,
-                     uint32_t /* port */,
-                     std::vector<uint8_t> /* data */,
-                     base::TimeTicks /* timestamp */)
-
-IPC_MESSAGE_CONTROL0(MidiHostMsg_EndSession)
-
-// Messages sent from the browser to the renderer.
-
-IPC_MESSAGE_CONTROL1(MidiMsg_AddInputPort,
-                     midi::mojom::PortInfo /* input port */)
-
-IPC_MESSAGE_CONTROL1(MidiMsg_AddOutputPort,
-                     midi::mojom::PortInfo /* output port */)
-
-IPC_MESSAGE_CONTROL2(MidiMsg_SetInputPortState,
-                     uint32_t /* port */,
-                     midi::mojom::PortState /* state */)
-
-IPC_MESSAGE_CONTROL2(MidiMsg_SetOutputPortState,
-                     uint32_t /* port */,
-                     midi::mojom::PortState /* state */)
-
-IPC_MESSAGE_CONTROL1(MidiMsg_SessionStarted, midi::mojom::Result /* result */)
-
-IPC_MESSAGE_CONTROL3(MidiMsg_DataReceived,
-                     uint32_t /* port */,
-                     std::vector<uint8_t> /* data */,
-                     base::TimeTicks /* timestamp */)
-
-IPC_MESSAGE_CONTROL1(MidiMsg_AcknowledgeSentData, uint32_t /* bytes sent */)
-
-#endif  // CONTENT_COMMON_MEDIA_MIDI_MESSAGES_H_

content/public/app/mojo/content_browser_manifest.json

@@ -75,6 +75,7 @@
           "media.mojom.VideoDecodePerfHistory",
           "memory_coordinator.mojom.MemoryCoordinatorHandle",
           "metrics.mojom.SingleSampleMetricsProvider",
+          "midi.mojom.MidiSessionProvider",
           "network.mojom.P2PSocketManager",
           "network.mojom.MdnsResponder",
           "network.mojom.URLLoaderFactory",

content/renderer/BUILD.gn

@@ -254,8 +254,8 @@ target(link_target_type, "renderer") {
     "media/media_factory.h",
     "media/media_permission_dispatcher.cc",
     "media/media_permission_dispatcher.h",
-    "media/midi/midi_message_filter.cc",
-    "media/midi/midi_message_filter.h",
+    "media/midi/midi_session_client_impl.cc",
+    "media/midi/midi_session_client_impl.h",
     "media/midi/renderer_webmidiaccessor_impl.cc",
     "media/midi/renderer_webmidiaccessor_impl.h",
     "media/render_media_client.cc",

content/renderer/media/midi/midi_message_filter.h

-// Copyright (c) 2013 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 CONTENT_RENDERER_MEDIA_MIDI_MIDI_MESSAGE_FILTER_H_
-#define CONTENT_RENDERER_MEDIA_MIDI_MIDI_MESSAGE_FILTER_H_
-
-#include <stddef.h>
-#include <stdint.h>
-
-#include <memory>
-#include <set>
-#include <vector>
-
-#include "base/macros.h"
-#include "content/common/content_export.h"
-#include "ipc/message_filter.h"
-#include "media/midi/midi_service.mojom.h"
-#include "third_party/blink/public/platform/modules/webmidi/web_midi_accessor_client.h"
-
-namespace base {
-class SingleThreadTaskRunner;
-}
-
-namespace content {
-
-// MessageFilter that handles MIDI messages. Created on render thread, and
-// host multiple clients running on multiple frames on IO thread.
-// Web MIDI intentionally uses MessageFilter (in a renderer process) and
-// BrowserMessageFilter (in the browser process) to intercept MIDI messages and
-// process them on IO thread in the browser process since these messages are
-// time critical. Non-critical operations like permission management are
-// handled in MidiDispatcher.
-class CONTENT_EXPORT MidiMessageFilter : public IPC::MessageFilter {
- public:
-  explicit MidiMessageFilter(
-      scoped_refptr<base::SingleThreadTaskRunner> io_task_runner);
-
-  // Each client registers for MIDI access here.
-  // If permission is granted, then the client's
-  void AddClient(blink::WebMIDIAccessorClient* client);
-  void RemoveClient(blink::WebMIDIAccessorClient* client);
-
-  // A client will only be able to call this method if it has a suitable
-  // output port (from addOutputPort()).
-  void SendMidiData(uint32_t port,
-                    const uint8_t* data,
-                    size_t length,
-                    base::TimeTicks timestamp);
-
-  // IO task runner associated with this message filter.
-  base::SingleThreadTaskRunner* io_task_runner() const {
-    return io_task_runner_.get();
-  }
-
- protected:
-  ~MidiMessageFilter() override;
-
- private:
-  void StartSessionOnIOThread();
-
-  void SendMidiDataOnIOThread(uint32_t port,
-                              const std::vector<uint8_t>& data,
-                              base::TimeTicks timestamp);
-
-  void EndSessionOnIOThread();
-
-  // Sends an IPC message using |sender_|.
-  void Send(IPC::Message* message);
-
-  // IPC::MessageFilter override. Called on |io_task_runner|.
-  bool OnMessageReceived(const IPC::Message& message) override;
-  void OnFilterAdded(IPC::Channel* channel) override;
-  void OnFilterRemoved() override;
-  void OnChannelClosing() override;
-
-  // Called when the browser process has approved (or denied) access to
-  // MIDI hardware.
-  void OnSessionStarted(midi::mojom::Result result);
-
-  // These functions are called in 2 cases:
-  //  (1) Just before calling |OnSessionStarted|, to notify the recipient about
-  //      existing ports.
-  //  (2) To notify the recipient that a new device was connected and that new
-  //      ports have been created.
-  void OnAddInputPort(midi::mojom::PortInfo info);
-  void OnAddOutputPort(midi::mojom::PortInfo info);
-
-  // These functions are called to notify the recipient that a device that is
-  // notified via OnAddInputPort() or OnAddOutputPort() gets disconnected, or
-  // connected again.
-  void OnSetInputPortState(uint32_t port, midi::mojom::PortState state);
-  void OnSetOutputPortState(uint32_t port, midi::mojom::PortState state);
-
-  // Called when the browser process has sent MIDI data containing one or
-  // more messages.
-  void OnDataReceived(uint32_t port,
-                      const std::vector<uint8_t>& data,
-                      base::TimeTicks timestamp);
-
-  // From time-to-time, the browser incrementally informs us of how many bytes
-  // it has successfully sent. This is part of our throttling process to avoid
-  // sending too much data before knowing how much has already been sent.
-  void OnAcknowledgeSentData(size_t bytes_sent);
-
-  // Following methods, Handle*, run on |main_task_runner_|.
-  void HandleClientAdded(midi::mojom::Result result);
-
-  void HandleAddInputPort(midi::mojom::PortInfo info);
-  void HandleAddOutputPort(midi::mojom::PortInfo info);
-  void HandleSetInputPortState(uint32_t port, midi::mojom::PortState state);
-  void HandleSetOutputPortState(uint32_t port, midi::mojom::PortState state);
-
-  void HandleDataReceived(uint32_t port,
-                          const std::vector<uint8_t>& data,
-                          base::TimeTicks timestamp);
-
-  void HandleAckknowledgeSentData(size_t bytes_sent);
-
-  // IPC sender for Send(); must only be accessed on |io_task_runner_|.
-  IPC::Sender* sender_;
-
-  // Task runner on which IPC calls are driven.
-  const scoped_refptr<base::SingleThreadTaskRunner> io_task_runner_;
-
-  // Main task runner.
-  scoped_refptr<base::SingleThreadTaskRunner> main_task_runner_;
-
-  /*
-   * Notice: Following members are designed to be accessed only on
-   * |main_task_runner_|.
-   */
-  // Keeps track of all MIDI clients. This should be std::set so that various
-  // for-loops work correctly. To change the type, make sure that the new type
-  // is safe to modify the container inside for-loops.
-  typedef std::set<blink::WebMIDIAccessorClient*> ClientsSet;
-  ClientsSet clients_;
-
-  // Represents clients that are waiting for a session being open.
-  // Note: std::vector is not safe to invoke callbacks inside iterator based
-  // for-loops.
-  typedef std::vector<blink::WebMIDIAccessorClient*> ClientsQueue;
-  ClientsQueue clients_waiting_session_queue_;
-
-  // Represents a result on starting a session. Can be accessed only on
-  midi::mojom::Result session_result_;
-
-  // Holds MidiPortInfoList for input ports and output ports.
-  std::vector<midi::mojom::PortInfo> inputs_;
-  std::vector<midi::mojom::PortInfo> outputs_;
-
-  size_t unacknowledged_bytes_sent_;
-
-  DISALLOW_COPY_AND_ASSIGN(MidiMessageFilter);
-};
-
-}  // namespace content
-
-#endif  // CONTENT_RENDERER_MEDIA_MIDI_MIDI_MESSAGE_FILTER_H_

content/renderer/media/midi/midi_session_client_impl.h

+// Copyright (c) 2013 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 CONTENT_RENDERER_MEDIA_MIDI_MIDI_SESSION_CLIENT_IMPL_H_
+#define CONTENT_RENDERER_MEDIA_MIDI_MIDI_SESSION_CLIENT_IMPL_H_
+
+#include <stddef.h>
+#include <stdint.h>
+
+#include <memory>
+#include <set>
+#include <vector>
+
+#include "base/macros.h"
+#include "content/common/content_export.h"
+#include "ipc/message_filter.h"
+#include "media/midi/midi_service.mojom.h"
+#include "mojo/public/cpp/bindings/binding.h"
+#include "third_party/blink/public/platform/modules/webmidi/web_midi_accessor_client.h"
+
+namespace content {
+
+// Created on render thread, and hosts multiple clients running on multiple
+// frames. All operations are carried out on |main_task_runner_|.
+class CONTENT_EXPORT MidiSessionClientImpl
+    : public midi::mojom::MidiSessionClient {
+ public:
+  explicit MidiSessionClientImpl();
+  ~MidiSessionClientImpl() override;
+
+  // midi::mojom::MidiSessionClient implementation.
+  // All of the following methods are run on the main thread.
+  void AddInputPort(midi::mojom::PortInfoPtr info) override;
+  void AddOutputPort(midi::mojom::PortInfoPtr info) override;
+  void SetInputPortState(uint32_t port, midi::mojom::PortState state) override;
+  void SetOutputPortState(uint32_t port, midi::mojom::PortState state) override;
+  void SessionStarted(midi::mojom::Result result) override;
+  void AcknowledgeSentData(uint32_t bytes) override;
+  void DataReceived(uint32_t port,
+                    const std::vector<uint8_t>& data,
+                    base::TimeTicks timestamp) override;
+
+  // Each client registers for MIDI access here.
+  void AddClient(blink::WebMIDIAccessorClient* client);
+  void RemoveClient(blink::WebMIDIAccessorClient* client);
+
+  // Called to clean up if browser terminates session.
+  void RemoveClients();
+
+  // A client will only be able to call this method if it has a suitable
+  // output port (from AddOutputPort()).
+  void SendMidiData(uint32_t port,
+                    const uint8_t* data,
+                    size_t length,
+                    base::TimeTicks timestamp);
+
+ private:
+  midi::mojom::MidiSessionProvider& GetMidiSessionProvider();
+  midi::mojom::MidiSession& GetMidiSession();
+
+  // Keeps track of all MIDI clients. This should be std::set so that various
+  // for-loops work correctly. To change the type, make sure that the new type
+  // is safe to modify the container inside for-loops.
+  typedef std::set<blink::WebMIDIAccessorClient*> ClientsSet;
+  ClientsSet clients_;
+
+  // Represents clients that are waiting for a session being open.
+  // Note: std::vector is not safe to invoke callbacks inside iterator based
+  // for-loops.
+  typedef std::vector<blink::WebMIDIAccessorClient*> ClientsQueue;
+  ClientsQueue clients_waiting_session_queue_;
+
+  // Represents a result on starting a session.
+  midi::mojom::Result session_result_;
+
+  // Holds MidiPortInfoList for input ports and output ports.
+  std::vector<midi::mojom::PortInfo> inputs_;
+  std::vector<midi::mojom::PortInfo> outputs_;
+
+  size_t unacknowledged_bytes_sent_;
+
+  midi::mojom::MidiSessionProviderPtr midi_session_provider_;
+  midi::mojom::MidiSessionPtr midi_session_;
+  mojo::Binding<midi::mojom::MidiSessionClient> binding_;
+
+  DISALLOW_COPY_AND_ASSIGN(MidiSessionClientImpl);
+};
+
+}  // namespace content
+
+#endif  // CONTENT_RENDERER_MEDIA_MIDI_MIDI_SESSION_CLIENT_IMPL_H_

content/renderer/media/midi/renderer_webmidiaccessor_impl.cc

@@ -5,7 +5,7 @@
 #include "content/renderer/media/midi/renderer_webmidiaccessor_impl.h"
 
 #include "base/logging.h"
-#include "content/renderer/media/midi/midi_message_filter.h"
+#include "content/renderer/media/midi/midi_session_client_impl.h"
 #include "content/renderer/render_thread_impl.h"
 
 namespace content {
@@ -18,11 +18,11 @@ RendererWebMIDIAccessorImpl::RendererWebMIDIAccessorImpl(
 
 RendererWebMIDIAccessorImpl::~RendererWebMIDIAccessorImpl() {
   if (is_client_added_)
-    midi_message_filter()->RemoveClient(client_);
+    midi_session_client_impl()->RemoveClient(client_);
 }
 
 void RendererWebMIDIAccessorImpl::StartSession() {
-  midi_message_filter()->AddClient(client_);
+  midi_session_client_impl()->AddClient(client_);
   is_client_added_ = true;
 }
 
@@ -30,15 +30,11 @@ void RendererWebMIDIAccessorImpl::SendMIDIData(unsigned port_index,
                                                const unsigned char* data,
                                                size_t length,
                                                base::TimeTicks timestamp) {
-  midi_message_filter()->SendMidiData(
-      port_index,
-      data,
-      length,
-      timestamp);
+  midi_session_client_impl()->SendMidiData(port_index, data, length, timestamp);
 }
 
-MidiMessageFilter* RendererWebMIDIAccessorImpl::midi_message_filter() {
-  return RenderThreadImpl::current()->midi_message_filter();
+MidiSessionClientImpl* RendererWebMIDIAccessorImpl::midi_session_client_impl() {
+  return RenderThreadImpl::current()->midi_session_client_impl();
 }
 
 }  // namespace content

content/renderer/media/midi/renderer_webmidiaccessor_impl.h

@@ -14,7 +14,7 @@
 
 namespace content {
 
-class MidiMessageFilter;
+class MidiSessionClientImpl;
 
 class RendererWebMIDIAccessorImpl
     : public blink::WebMIDIAccessor {
@@ -35,7 +35,7 @@ class RendererWebMIDIAccessorImpl
 
   bool is_client_added_;
 
-  MidiMessageFilter* midi_message_filter();
+  MidiSessionClientImpl* midi_session_client_impl();
 
   DISALLOW_COPY_AND_ASSIGN(RendererWebMIDIAccessorImpl);
 };

content/renderer/render_thread_impl.cc

@@ -93,7 +93,7 @@
 #include "content/renderer/low_memory_mode_controller.h"
 #include "content/renderer/media/audio/audio_renderer_mixer_manager.h"
 #include "content/renderer/media/gpu/gpu_video_accelerator_factories_impl.h"
-#include "content/renderer/media/midi/midi_message_filter.h"
+#include "content/renderer/media/midi/midi_session_client_impl.h"
 #include "content/renderer/media/render_media_client.h"
 #include "content/renderer/media/stream/aec_dump_message_filter.h"
 #include "content/renderer/media/stream/media_stream_center.h"
@@ -788,8 +788,7 @@ void RenderThreadImpl::Init() {
 
   audio_output_ipc_factory_.emplace(GetIOTaskRunner());
 
-  midi_message_filter_ = new MidiMessageFilter(GetIOTaskRunner());
-  AddFilter(midi_message_filter_.get());
+  midi_session_client_impl_ = std::make_unique<MidiSessionClientImpl>();
 
 #if defined(USE_AURA)
   if (features::IsMultiProcessMash())

content/renderer/render_thread_impl.h

@@ -126,7 +126,7 @@ class DomStorageDispatcher;
 class FrameSwapMessageQueue;
 class GpuVideoAcceleratorFactoriesImpl;
 class LowMemoryModeController;
-class MidiMessageFilter;
+class MidiSessionClientImpl;
 class P2PSocketDispatcher;
 class PeerConnectionDependencyFactory;
 class PeerConnectionTracker;
@@ -325,8 +325,8 @@ class CONTENT_EXPORT RenderThreadImpl
     return dom_storage_dispatcher_.get();
   }
 
-  MidiMessageFilter* midi_message_filter() {
-    return midi_message_filter_.get();
+  MidiSessionClientImpl* midi_session_client_impl() {
+    return midi_session_client_impl_.get();
   }
 
   ResourceDispatcher* resource_dispatcher() const {
@@ -602,7 +602,7 @@ class CONTENT_EXPORT RenderThreadImpl
   std::unique_ptr<URLLoaderThrottleProvider> url_loader_throttle_provider_;
 
   // Used on the renderer and IPC threads.
-  scoped_refptr<MidiMessageFilter> midi_message_filter_;
+  std::unique_ptr<MidiSessionClientImpl> midi_session_client_impl_;
 
   std::unique_ptr<BrowserPluginManager> browser_plugin_manager_;
 

media/midi/BUILD.gn

@@ -172,6 +172,10 @@ mojom("mojo") {
   sources = [
     "midi_service.mojom",
   ]
+
+  deps = [
+    "//mojo/public/mojom/base",
+  ]
 }
 
 test("midi_unittests") {

media/midi/midi_service.mojom

@@ -4,6 +4,8 @@
 
 module midi.mojom;
 
+import "mojo/public/mojom/base/time.mojom";
+
 enum Result {
   NOT_INITIALIZED,
   OK,
@@ -39,5 +41,45 @@ struct PortInfo {
   PortState state;
 };
 
-// TODO(toyoshim): MidiService should be declared here.
-// http://crbug.com/582327
+// Interface for MIDI related browser to renderer messages.
+interface MidiSessionClient {
+  // These functions are called in 2 cases:
+  //  (1) Just before calling |SessionStarted|, to notify the recipient about
+  //      existing ports.
+  //  (2) To notify the recipient that a new device was connected and that new
+  //      ports have been created.
+  AddInputPort(PortInfo info);
+  AddOutputPort(PortInfo info);
+
+  // Used to notify clients when a device is disconnected or reconnected. The
+  // ports correspond to ports already sent to the client using AddInputPort/
+  // AddOutputPort.
+  SetInputPortState(uint32 port, PortState state);
+  SetOutputPortState(uint32 port, PortState state);
+
+  // Called in response to StartSession and indicates if a connection with
+  // MIDI hardware was successfully made.
+  SessionStarted(Result result);
+
+  // Used to inform the client incrementally of how many bytes have been
+  // successfully sent. This is only called after the client calls SendData().
+  AcknowledgeSentData(uint32 bytes);
+
+  // Called to send MIDI data to the client.
+  DataReceived(uint32 port,
+               array<uint8> data,
+               mojo_base.mojom.TimeTicks timestamp);
+};
+
+// Interface used by the renderer to start a MIDI session in the browser.
+interface MidiSessionProvider {
+  // Start session to access MIDI hardware.
+  StartSession(MidiSession& request, MidiSessionClient client);
+};
+
+// Represents an active MIDI session.
+interface MidiSession {
+  // Send data to a MIDI output port. The output port should be a port already
+  // sent to the client (via AddOutputPort).
+  SendData(uint32 port, array<uint8> data, mojo_base.mojom.TimeTicks timestamp);
+};