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*'
Review URL: https://codereview.chromium.org/323323002
Cr-Mirrored-From: https://chromium.googlesource.com/chromium/src
Cr-Mirrored-Commit: 8b055378f2640dfc13f42d1d4edbe80f5aa771d8
diff --git a/midi_manager.cc b/midi_manager.cc
index aba8441..c53eef4 100644
--- a/midi_manager.cc
+++ b/midi_manager.cc
@@ -43,8 +43,7 @@
if (!too_many_pending_clients_exist) {
// Call StartInitialization() only for the first request.
session_needs_initialization = pending_clients_.empty();
- pending_clients_.insert(
- std::pair<int, MidiManagerClient*>(client_id, client));
+ pending_clients_.insert(std::make_pair(client, client_id));
}
}
}
@@ -81,9 +80,8 @@
void MidiManager::EndSession(MidiManagerClient* client) {
base::AutoLock auto_lock(lock_);
- ClientList::iterator i = clients_.find(client);
- if (i != clients_.end())
- clients_.erase(i);
+ clients_.erase(client);
+ pending_clients_.erase(client);
}
void MidiManager::DispatchSendMidiData(MidiManagerClient* client,
@@ -132,7 +130,6 @@
base::AutoLock auto_lock(lock_);
DCHECK(clients_.empty());
- DCHECK(!pending_clients_.empty());
DCHECK(!initialized_);
initialized_ = true;
result_ = result;
@@ -141,8 +138,8 @@
it != pending_clients_.end();
++it) {
if (result_ == MIDI_OK)
- clients_.insert(it->second);
- it->second->CompleteStartSession(it->first, result_);
+ clients_.insert(it->first);
+ it->first->CompleteStartSession(it->second, result_);
}
pending_clients_.clear();
}
diff --git a/midi_manager.h b/midi_manager.h
index f6c424b..9fd7a21 100644
--- a/midi_manager.h
+++ b/midi_manager.h
@@ -152,7 +152,7 @@
ClientList clients_;
// Keeps track of all clients who are waiting for CompleteStartSession().
- typedef std::map<int, MidiManagerClient*> PendingClientMap;
+ typedef std::multimap<MidiManagerClient*, int> PendingClientMap;
PendingClientMap pending_clients_;
// Keeps a SingleThreadTaskRunner of the thread that calls StartSession in
diff --git a/midi_manager_unittest.cc b/midi_manager_unittest.cc
index c129ca1..0350813 100644
--- a/midi_manager_unittest.cc
+++ b/midi_manager_unittest.cc
@@ -171,18 +171,26 @@
TEST_F(MidiManagerTest, StartMultipleSessions) {
scoped_ptr<FakeMidiManagerClient> client1;
scoped_ptr<FakeMidiManagerClient> client2;
+ scoped_ptr<FakeMidiManagerClient> client3;
client1.reset(new FakeMidiManagerClient(0));
client2.reset(new FakeMidiManagerClient(1));
+ client3.reset(new FakeMidiManagerClient(1));
StartTheFirstSession(client1.get());
StartTheNthSession(client2.get(), 2);
+ StartTheNthSession(client3.get(), 3);
CompleteInitialization(MIDI_OK);
EXPECT_EQ(MIDI_OK, client1->WaitForResult());
EXPECT_EQ(MIDI_OK, client2->WaitForResult());
- EndSession(client1.get(), 2U, 1U);
- EndSession(client2.get(), 1U, 0U);
+ EXPECT_EQ(MIDI_OK, client3->WaitForResult());
+ EndSession(client1.get(), 3U, 2U);
+ EndSession(client2.get(), 2U, 1U);
+ EndSession(client3.get(), 1U, 0U);
}
+// TODO(toyoshim): Add a test for a MidiManagerClient that has multiple
+// sessions with multiple client_id.
+
TEST_F(MidiManagerTest, TooManyPendingSessions) {
// Push as many client requests for starting session as possible.
ScopedVector<FakeMidiManagerClient> many_existing_clients;
@@ -217,6 +225,22 @@
EndSession(many_existing_clients[i], sessions, sessions - 1);
}
+TEST_F(MidiManagerTest, AbortSession) {
+ // A client starting a session can be destructed while an asynchronous
+ // initialization is performed.
+ scoped_ptr<FakeMidiManagerClient> client;
+ client.reset(new FakeMidiManagerClient(0));
+
+ StartTheFirstSession(client.get());
+ EndSession(client.get(), 0, 0);
+ client.reset();
+
+ // Following function should not call the destructed |client| function.
+ CompleteInitialization(MIDI_OK);
+ base::RunLoop run_loop;
+ run_loop.RunUntilIdle();
+}
+
TEST_F(MidiManagerTest, CreateMidiManager) {
scoped_ptr<FakeMidiManagerClient> client;
client.reset(new FakeMidiManagerClient(0));