Web MIDI: MidiManager crashes if a session is ended while initializing

Web MIDI: MidiManager crashes if a session is ended while initializing

If a MidiManagerClient was destructed while an asynchronous MidiManager
initialization performs, the browser process crashes when the
initilization is finished. On EndSession(), MidiManager should remove
the client even from |pending_clients_|.

Also, this change fixes a problem that CompleteStartSession() may not be
invoked on a MidiManagerClient if its |client_id| is confclicting with
another MidiManagerClient.

BUG=382522
TEST=media_unittests --gtest_filter='Midi*'

Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=276987

[Takashi Toyoshima, 2014-06-11 09:00:41]

PTAL.
This security bug is marked as a stable blocker.

[yhirano, 2014-06-11 12:11:57]

https://codereview.chromium.org/323323002/diff/10004/media/midi/midi_manager.cc
File media/midi/midi_manager.cc (right):

https://codereview.chromium.org/323323002/diff/10004/media/midi/midi_manager....
media/midi/midi_manager.cc:47: PendingClientMap::iterator
pending_client_iterator =
pending_clients_[client].push_back(client_id) for this if - and - else block?

https://codereview.chromium.org/323323002/diff/10004/media/midi/midi_manager....
media/midi/midi_manager.cc:95: ClientList::iterator client_iterator =
clients_.find(client);
std::set<T>::erase(key) works even when key doesn't exist in the set.

https://codereview.chromium.org/323323002/diff/10004/media/midi/midi_manager.h
File media/midi/midi_manager.h (right):

https://codereview.chromium.org/323323002/diff/10004/media/midi/midi_manager....
media/midi/midi_manager.h:155: typedef std::map<MidiManagerClient*,
std::vector<int> > PendingClientMap;
[opt] You can use std::multimap<K, V> instead of std::map<K, std::vector<V>>. If
you do so, you can remove pending_clients_size_.

[Takashi Toyoshima, 2014-06-12 04:41:02]

Thanks!
multimap helps me to simplify it.
PTAL.

https://codereview.chromium.org/323323002/diff/10004/media/midi/midi_manager.cc
File media/midi/midi_manager.cc (right):

https://codereview.chromium.org/323323002/diff/10004/media/midi/midi_manager....
media/midi/midi_manager.cc:47: PendingClientMap::iterator
pending_client_iterator =
thanks.
Now that it is multiset, I just insert a pair in both cases.

https://codereview.chromium.org/323323002/diff/10004/media/midi/midi_manager....
media/midi/midi_manager.cc:95: ClientList::iterator client_iterator =
clients_.find(client);
On 2014/06/11 12:11:57, yhirano wrote:
> std::set<T>::erase(key) works even when key doesn't exist in the set.

Done.

https://codereview.chromium.org/323323002/diff/10004/media/midi/midi_manager.h
File media/midi/midi_manager.h (right):

https://codereview.chromium.org/323323002/diff/10004/media/midi/midi_manager....
media/midi/midi_manager.h:155: typedef std::map<MidiManagerClient*,
std::vector<int> > PendingClientMap;
On 2014/06/11 12:11:57, yhirano wrote:
> [opt] You can use std::multimap<K, V> instead of std::map<K, std::vector<V>>.
If
> you do so, you can remove pending_clients_size_.

Done.

[yhirano, 2014-06-12 04:49:10]

lgtm

https://codereview.chromium.org/323323002/diff/30001/media/midi/midi_manager.cc
File media/midi/midi_manager.cc (right):

https://codereview.chromium.org/323323002/diff/30001/media/midi/midi_manager....
media/midi/midi_manager.cc:47: std::pair<MidiManagerClient*, int>(client,
client_id));
You can use std::make_pair.

[Takashi Toyoshima, 2014-06-12 04:54:31]

https://codereview.chromium.org/323323002/diff/30001/media/midi/midi_manager.cc
File media/midi/midi_manager.cc (right):

https://codereview.chromium.org/323323002/diff/30001/media/midi/midi_manager....
media/midi/midi_manager.cc:47: std::pair<MidiManagerClient*, int>(client,
client_id));
On 2014/06/12 04:49:10, yhirano wrote:
> You can use std::make_pair.

Done.

[Takashi Toyoshima, 2014-06-12 04:54:46]

The CQ bit was checked by toyoshim@chromium.org

[Takashi Toyoshima, 2014-06-12 23:28:44]

The CQ bit was unchecked by toyoshim@chromium.org

[Takashi Toyoshima, 2014-06-12 23:28:49]

The CQ bit was checked by toyoshim@chromium.org

[commit-bot: I haz the power, 2014-06-13 02:26:44]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/toyoshim@chromium.org/323323002/50001

[commit-bot: I haz the power, 2014-06-13 08:23:14]

FYI, CQ is re-trying this CL (attempt #1).
The failing builders are:
  android_dbg_triggered_tests on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/android_dbg_triggered...)

[commit-bot: I haz the power, 2014-06-13 09:40:36]

Change committed as 276987