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

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@276987 0039d316-1c4b-4281-b951-d872f2087c98

3 files changed, 32 insertions, 11 deletions

diff --git a/media/midi/midi_manager.cc b/media/midi/midi_manager.cc
index aba8441..c53eef4 100644
--- a/media/midi/midi_manager.cc
+++ b/media/midi/midi_manager.cc
@@ -43,8 +43,7 @@ void MidiManager::StartSession(MidiManagerClient* client, int client_id) {
       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::StartSession(MidiManagerClient* client, int client_id) {
 
 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 @@ void MidiManager::CompleteInitializationInternal(MidiResult result) {
 
   base::AutoLock auto_lock(lock_);
   DCHECK(clients_.empty());
-  DCHECK(!pending_clients_.empty());
   DCHECK(!initialized_);
   initialized_ = true;
   result_ = result;
@@ -141,8 +138,8 @@ void MidiManager::CompleteInitializationInternal(MidiResult result) {
        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/media/midi/midi_manager.h b/media/midi/midi_manager.h
index f6c424b..9fd7a21 100644
--- a/media/midi/midi_manager.h
+++ b/media/midi/midi_manager.h
@@ -152,7 +152,7 @@ class MEDIA_EXPORT MidiManager {
   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/media/midi/midi_manager_unittest.cc b/media/midi/midi_manager_unittest.cc
index c129ca1..0350813 100644
--- a/media/midi/midi_manager_unittest.cc
+++ b/media/midi/midi_manager_unittest.cc
@@ -171,18 +171,26 @@ TEST_F(MidiManagerTest, StartAndEndSessionWithError) {
 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 @@ TEST_F(MidiManagerTest, TooManyPendingSessions) {
     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));