Manage MIDI related objects and sessions' lifecycles correctly

content/browser/media/midi_host.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/browser/media/midi_host.h"
 
 #include "base/bind.h"
 #include "base/bind_helpers.h"
 #include "base/debug/trace_event.h"
 #include "base/process/process.h"
@@ skipping 30 matching lines @@
 
 bool IsSystemRealTimeMessage(uint8 data) {
   return 0xf8 <= data && data <= 0xff;
 }
 
 }  // namespace
 
 using media::kSysExByte;
 using media::kEndOfSysExByte;
 
 MidiHost::MidiHost(int renderer_process_id, media::MidiManager* midi_manager)

[yhirano, 2014/10/20 12:15:14]

Please initialize |is_session_requested_|.

[Takashi Toyoshima, 2014/10/20 17:14:43]

Oh, thanks!
But, I'm surprised since there are no compile errors and no try failures on
this.

     : BrowserMessageFilter(MidiMsgStart),
       renderer_process_id_(renderer_process_id),
       has_sys_ex_permission_(false),
       midi_manager_(midi_manager),
       sent_bytes_in_flight_(0),
       bytes_sent_since_last_acknowledgement_(0) {
+  CHECK(midi_manager_);
 }
 
 MidiHost::~MidiHost() {
-  if (midi_manager_)
+  // Close an open session, or abort opening a session.
+  if (is_session_requested_)
     midi_manager_->EndSession(this);
 }
 
 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(int client_id) {
-  if (midi_manager_)
-    midi_manager_->StartSession(this, client_id);
+void MidiHost::OnStartSession() {
+  is_session_requested_ = true;
+  midi_manager_->StartSession(this);
 }
 
 void MidiHost::OnSendData(uint32 port,
                           const std::vector<uint8>& data,
                           double timestamp) {
-  if (!midi_manager_)
-    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_ &&
       std::find(data.begin(), data.end(), kSysExByte) != data.end()) {
     RecordAction(base::UserMetricsAction("BadMessageTerminate_MIDI"));
     BadMessageReceived();
     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();
   }
   midi_manager_->DispatchSendMidiData(this, port, data, timestamp);
 }
 
-void MidiHost::CompleteStartSession(int client_id, media::MidiResult result) {
+void MidiHost::OnEndSession() {
+  is_session_requested_ = false;
+  midi_manager_->EndSession(this);
+}
+
+void MidiHost::CompleteStartSession(media::MidiResult result) {
+  DCHECK(is_session_requested_);
   MidiPortInfoList input_ports;
   MidiPortInfoList output_ports;
 
   if (result == media::MIDI_OK) {
     input_ports = midi_manager_->input_ports();
     output_ports = midi_manager_->output_ports();
     received_messages_queues_.clear();
     received_messages_queues_.resize(input_ports.size());
     // 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_);
   }
 
-  Send(new MidiMsg_SessionStarted(client_id,
-                                  result,
+  Send(new MidiMsg_SessionStarted(result,
                                   input_ports,
                                   output_ports));
 }
 
 void MidiHost::ReceiveMidiData(
     uint32 port,
     const uint8* data,
     size_t length,
     double timestamp) {
   TRACE_EVENT0("midi", "MidiHost::ReceiveMidiData");
@@ skipping 69 matching lines @@
     }
     waiting_data_length = media::GetMidiMessageLength(current);
     if (waiting_data_length == 0)
       return false;  // Error: |current| should have been a valid status byte.
     --waiting_data_length;  // Found status byte
   }
   return waiting_data_length == 0 && !in_sysex;
 }
 
 }  // namespace content