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

media/midi/midi_manager.cc

Index: media/midi/midi_manager.cc
diff --git a/media/midi/midi_manager.cc b/media/midi/midi_manager.cc
index aba8441ef8f0548ef417dcd477b8e17b934cdf19..b8e20405cf70b535ba54a13c45262016777a4e11 100644
--- a/media/midi/midi_manager.cc
+++ b/media/midi/midi_manager.cc
@@ -12,7 +12,8 @@
 namespace media {
 
 MidiManager::MidiManager()
-    : initialized_(false),
+    : pending_clients_size_(0),
+      initialized_(false),
       result_(MIDI_NOT_SUPPORTED) {
 }
 
@@ -38,13 +39,23 @@ void MidiManager::StartSession(MidiManagerClient* client, int client_id) {
       // Do not accept a new request if the pending client list contains too
       // many clients.
       too_many_pending_clients_exist =
-          pending_clients_.size() >= kMaxPendingClientCount;
+          pending_clients_size_ >= kMaxPendingClientCount;
 
       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));
+        PendingClientMap::iterator pending_client_iterator =

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

pending_clients_[client].push_back(client_id) for this if - and - else block?

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

thanks.
Now that it is multiset, I just insert a pair in both cases.

+            pending_clients_.find(client);
+        if (pending_client_iterator != pending_clients_.end()) {
+          pending_client_iterator->second.push_back(client_id);
+        } else {
+          std::pair<MidiManagerClient*, std::vector<int> > entry =
+              std::pair<MidiManagerClient*, std::vector<int> >(
+                  client, std::vector<int>());
+          entry.second.push_back(client_id);
+          pending_clients_.insert(entry);
+        }
+        pending_clients_size_++;
       }
     }
   }
@@ -81,9 +92,16 @@ 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);
+  ClientList::iterator client_iterator = clients_.find(client);

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

std::set<T>::erase(key) works even when key doesn't exist in the set.

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

Done.

+  if (client_iterator != clients_.end())
+    clients_.erase(client_iterator);
+
+  PendingClientMap::iterator pending_client_iterator =
+      pending_clients_.find(client);
+  if (pending_client_iterator != pending_clients_.end()) {
+    pending_clients_size_ -= pending_client_iterator->second.size();
+    pending_clients_.erase(pending_client_iterator);
+  }
 }
 
 void MidiManager::DispatchSendMidiData(MidiManagerClient* client,
@@ -132,7 +150,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,10 +158,12 @@ 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);
+    for (size_t i = 0; i < it->second.size(); ++i)
+      it->first->CompleteStartSession(it->second[i], result_);
   }
   pending_clients_.clear();
+  pending_clients_size_ = 0;
 }
 
 }  // namespace media