Web MIDI: distributes MIDIPort information asynchronously

content/renderer/media/midi_message_filter.cc

 // 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.
 
 #include "content/renderer/media/midi_message_filter.h"
 
 #include <algorithm>
 
 #include "base/bind.h"
 #include "base/debug/trace_event.h"
@@ skipping 42 matching lines @@
 void MidiMessageFilter::RemoveClient(blink::WebMIDIAccessorClient* client) {
   base::AutoLock auto_lock(lock_);
   clients_.erase(client);
   ClientsQueue::iterator it = std::find(clients_waiting_session_queue_.begin(),
                                         clients_waiting_session_queue_.end(),
                                         client);
   if (it != clients_waiting_session_queue_.end())
     clients_waiting_session_queue_.erase(it);
   if (clients_.empty() && clients_waiting_session_queue_.empty()) {
     session_result_ = media::MIDI_NOT_INITIALIZED;
+    inputs_.clear();
+    outputs_.clear();
     io_message_loop_->PostTask(FROM_HERE,
         base::Bind(&MidiMessageFilter::EndSessionOnIOThread, this));
   }
 }
 
 void MidiMessageFilter::SendMidiData(uint32 port,
                                      const uint8* data,
                                      size_t length,
                                      double timestamp) {
   if (length > kMaxUnacknowledgedBytesSent) {
@@ skipping 45 matching lines @@
   } else {
     sender_->Send(message);
   }
 }
 
 bool MidiMessageFilter::OnMessageReceived(const IPC::Message& message) {
   DCHECK(io_message_loop_->BelongsToCurrentThread());
   bool handled = true;
   IPC_BEGIN_MESSAGE_MAP(MidiMessageFilter, message)
     IPC_MESSAGE_HANDLER(MidiMsg_SessionStarted, OnSessionStarted)
+    IPC_MESSAGE_HANDLER(MidiMsg_AddInputPort, OnAddInputPort)
+    IPC_MESSAGE_HANDLER(MidiMsg_AddOutputPort, OnAddOutputPort)
     IPC_MESSAGE_HANDLER(MidiMsg_DataReceived, OnDataReceived)
     IPC_MESSAGE_HANDLER(MidiMsg_AcknowledgeSentData, OnAcknowledgeSentData)
     IPC_MESSAGE_UNHANDLED(handled = false)
   IPC_END_MESSAGE_MAP()
   return handled;
 }
 
 void MidiMessageFilter::OnFilterAdded(IPC::Sender* sender) {
   DCHECK(io_message_loop_->BelongsToCurrentThread());
   sender_ = sender;
 }
 
 void MidiMessageFilter::OnFilterRemoved() {
   DCHECK(io_message_loop_->BelongsToCurrentThread());
   // Once removed, a filter will not be used again.  At this time all
   // delegates must be notified so they release their reference.
   OnChannelClosing();
 }
 
 void MidiMessageFilter::OnChannelClosing() {
   DCHECK(io_message_loop_->BelongsToCurrentThread());
   sender_ = NULL;
 }
 
-void MidiMessageFilter::OnSessionStarted(media::MidiResult result,
-                                         MidiPortInfoList inputs,
-                                         MidiPortInfoList outputs) {
+void MidiMessageFilter::OnSessionStarted(media::MidiResult result) {
   TRACE_EVENT0("midi", "MidiMessageFilter::OnSessionStarted");
   DCHECK(io_message_loop_->BelongsToCurrentThread());
   base::AutoLock auto_lock(lock_);
   session_result_ = result;
-  inputs_ = inputs;
-  outputs_ = outputs;
   // Handle on the main JS thread.
   for (blink::WebMIDIAccessorClient* client : clients_waiting_session_queue_) {
     main_message_loop_->PostTask(FROM_HERE,
         base::Bind(&MidiMessageFilter::HandleClientAdded, this, client));
   }
   clients_waiting_session_queue_.clear();
 }
 
+void MidiMessageFilter::OnAddInputPort(media::MidiPortInfo info) {
+  DCHECK(io_message_loop_->BelongsToCurrentThread());
+  base::AutoLock auto_lock(lock_);
+  inputs_.push_back(info);
+  // TODO(toyoshim): Notify to clients that were already added.
+}
+
+void MidiMessageFilter::OnAddOutputPort(media::MidiPortInfo info) {
+  DCHECK(io_message_loop_->BelongsToCurrentThread());
+  base::AutoLock auto_lock(lock_);
+  outputs_.push_back(info);
+  // TODO(toyoshim): Notify to clients that were already added.
+}
+
 void MidiMessageFilter::OnDataReceived(uint32 port,
                                        const std::vector<uint8>& data,
                                        double timestamp) {
   TRACE_EVENT0("midi", "MidiMessageFilter::OnDataReceived");
   DCHECK(io_message_loop_->BelongsToCurrentThread());
   main_message_loop_->PostTask(
       FROM_HERE,
       base::Bind(&MidiMessageFilter::HandleDataReceived, this,
                  port, data, timestamp));
 }
 
 void MidiMessageFilter::OnAcknowledgeSentData(size_t bytes_sent) {
   DCHECK(io_message_loop_->BelongsToCurrentThread());
   DCHECK_GE(unacknowledged_bytes_sent_, bytes_sent);
   if (unacknowledged_bytes_sent_ >= bytes_sent)
     unacknowledged_bytes_sent_ -= bytes_sent;
 }
 
 void MidiMessageFilter::HandleClientAdded(
     blink::WebMIDIAccessorClient* client) {
   TRACE_EVENT0("midi", "MidiMessageFilter::HandleClientAdded");
   DCHECK(main_message_loop_->BelongsToCurrentThread());
   media::MidiResult result;
   {
     base::AutoLock auto_lock(lock_);
     result = session_result_;
   }

[Takashi Toyoshima, 2014/10/20 18:31:12]

Originally, I protect line 210:230 with the lock_ at once because inputs_ and
outputs_ may be updated in OnAddInputPort() or OnAddOutputPort() on IO thread
after MidiManager supports device detection in later CL.

So, one solution to avoid a race on inputs_ and outputs_ here is to use lock_
from 207 to 234 though it is slightly long living lock including callback
invocations.

Another solution is to pass inputs_.size() and outputs_.size() from caller to
this function. In that case, we can handle the right count of inputs_ and
outputs_ here. Newer ports that are added just now can be handled in the next
task since |client| is added to |clients_| at the end of this function, and
OnAddInputPort() posts HandleAddInputPort task that adds new port to all clients
including this client.

Note that we never decrease the number of ports. So the index < size() is always
valid and accessing by valid index to std::vector from multiple threads is safe.

Let's talk offline which approach is better here.

   if (result == media::MIDI_OK) {
     // Add the client's input and output ports.
     const bool active = true;
     for (media::MidiPortInfo info : inputs_) {
       client->didAddInputPort(
           base::UTF8ToUTF16(info.id),
           base::UTF8ToUTF16(info.manufacturer),
           base::UTF8ToUTF16(info.name),
           base::UTF8ToUTF16(info.version),
           true);
@@ skipping 39 matching lines @@
                                            double timestamp) {
   TRACE_EVENT0("midi", "MidiMessageFilter::HandleDataReceived");
   DCHECK(main_message_loop_->BelongsToCurrentThread());
   DCHECK(!data.empty());
 
   for (blink::WebMIDIAccessorClient* client : clients_)
     client->didReceiveMIDIData(port, &data[0], data.size(), timestamp);
 }
 
 }  // namespace content