Skip to content

Commit

Permalink
Manage MIDI related objects and sessions' lifecycles correctly
Browse files Browse the repository at this point in the history
Following changes are needed to maintain MIDIPortInfoList contents
consistent in each MidiMessageFilter instances when MidiManager
update the list asynchronously.

Essential changes:
- Call EndSession iff a session is open.
- Do not use client_id to communicate between browser and renderer.
  Multiple clients are managed in a MidiMessageFilter existing in
  each renderer.

Minor changes:
- Remove midi_manager_ checks in MIDIHost, and add CHECK insteads.
- Rename MidiManagerFilter::StartSession to MidiManagerFilter::AddClient
  to be aligned with RemoveClient. A session can be shared with multiple
  clients.

After this change, MidiManager::EndSession is called from each Renderer
and RendererHost at the right timing, e.g., GC collects the last MIDIAccess
reference in JavaScript. See, also http://crbug.com/424859.

BUG=424859

Review URL: https://codereview.chromium.org/662853003

Cr-Commit-Position: refs/heads/master@{#300851}
  • Loading branch information
toyoshim authored and Commit bot committed Oct 23, 2014
1 parent 7a0ddbb commit 71fcb15
Show file tree
Hide file tree
Showing 11 changed files with 240 additions and 213 deletions.
26 changes: 16 additions & 10 deletions content/browser/media/midi_host.cc
Original file line number Diff line number Diff line change
Expand Up @@ -52,13 +52,16 @@ MidiHost::MidiHost(int renderer_process_id, media::MidiManager* midi_manager)
: BrowserMessageFilter(MidiMsgStart),
renderer_process_id_(renderer_process_id),
has_sys_ex_permission_(false),
is_session_requested_(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);
}

Expand All @@ -72,23 +75,21 @@ bool MidiHost::OnMessageReceived(const IPC::Message& message) {
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;

Expand Down Expand Up @@ -117,7 +118,13 @@ void MidiHost::OnSendData(uint32 port,
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;

Expand All @@ -132,8 +139,7 @@ void MidiHost::CompleteStartSession(int client_id, media::MidiResult result) {
CanSendMidiSysExMessage(renderer_process_id_);
}

Send(new MidiMsg_SessionStarted(client_id,
result,
Send(new MidiMsg_SessionStarted(result,
input_ports,
output_ports));
}
Expand Down
10 changes: 8 additions & 2 deletions content/browser/media/midi_host.h
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
#include "base/memory/ref_counted.h"
#include "base/memory/scoped_ptr.h"
#include "base/memory/scoped_vector.h"
#include "base/synchronization/lock.h"
#include "content/common/content_export.h"
#include "content/public/browser/browser_message_filter.h"
#include "content/public/browser/browser_thread.h"
Expand All @@ -35,21 +36,23 @@ class CONTENT_EXPORT MidiHost
bool OnMessageReceived(const IPC::Message& message) override;

// MidiManagerClient implementation.
void CompleteStartSession(int client_id, media::MidiResult result) override;
void CompleteStartSession(media::MidiResult result) override;
void ReceiveMidiData(uint32 port,
const uint8* data,
size_t length,
double timestamp) override;
void AccumulateMidiBytesSent(size_t n) override;

// Start session to access MIDI hardware.
void OnStartSession(int client_id);
void OnStartSession();

// Data to be sent to a MIDI output port.
void OnSendData(uint32 port,
const std::vector<uint8>& data,
double timestamp);

void OnEndSession();

private:
FRIEND_TEST_ALL_PREFIXES(MidiHostTest, IsValidWebMIDIData);
friend class base::DeleteHelper<MidiHost>;
Expand All @@ -70,6 +73,9 @@ class CONTENT_EXPORT MidiHost
// messages.
bool has_sys_ex_permission_;

// Represents if a session is requested to start.
bool is_session_requested_;

// |midi_manager_| talks to the platform-specific MIDI APIs.
// It can be NULL if the platform (or our current implementation)
// does not support MIDI. If not supported then a call to
Expand Down
8 changes: 4 additions & 4 deletions content/common/media/midi_messages.h
Original file line number Diff line number Diff line change
Expand Up @@ -43,18 +43,18 @@ IPC_MESSAGE_ROUTED2(MidiMsg_SysExPermissionApproved,
// Messages for IPC between MidiMessageFilter and MidiHost.

// Renderer request to browser for access to MIDI services.
IPC_MESSAGE_CONTROL1(MidiHostMsg_StartSession,
int /* client id */)
IPC_MESSAGE_CONTROL0(MidiHostMsg_StartSession)

IPC_MESSAGE_CONTROL3(MidiHostMsg_SendData,
uint32 /* port */,
std::vector<uint8> /* data */,
double /* timestamp */)

IPC_MESSAGE_CONTROL0(MidiHostMsg_EndSession)

// Messages sent from the browser to the renderer.

IPC_MESSAGE_CONTROL4(MidiMsg_SessionStarted,
int /* client id */,
IPC_MESSAGE_CONTROL3(MidiMsg_SessionStarted,
media::MidiResult /* result */,
media::MidiPortInfoList /* input ports */,
media::MidiPortInfoList /* output ports */)
Expand Down
Loading

0 comments on commit 71fcb15

Please sign in to comment.