[chromedriver] Send all OnConnected before any OnEvent when connected.

chrome/test/chromedriver/devtools_client_impl.cc

 // Copyright (c) 2012 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "chrome/test/chromedriver/devtools_client_impl.h"
 
 #include "base/bind.h"
 #include "base/json/json_reader.h"
 #include "base/json/json_writer.h"
 #include "base/logging.h"
@@ skipping 108 matching lines @@
 }
 
 Status DevToolsClientImpl::SendCommandInternal(
     const std::string& method,
     const base::DictionaryValue& params,
     scoped_ptr<base::DictionaryValue>* result) {
   if (!connected_) {
     if (!socket_->Connect(url_))
       return Status(kDisconnected, "unable to connect to renderer");
     connected_ = true;
-    for (std::list<DevToolsEventListener*>::iterator iter = listeners_.begin();
-         iter != listeners_.end(); ++iter) {
-      Status status = (*iter)->OnConnected();
-      if (status.IsError())
-        return status;
-    }
+
+    // OnConnected notification will be sent out in method ReceiveNextMessage.
+    listeners_for_on_connected_ = listeners_;

[craigdh, 2013/02/13 19:19:39]

Instead of copying the list, why not just have a boolean flag indicating that
OnConnected has been triggered and needs to be sent to all listeners?

[kkania, 2013/02/13 20:57:34]

then you'd also have to keep track of what listeners you've already notified,
prolly via a int; i think list copy might be simpler

[craigdh, 2013/02/13 21:45:45]

Ah, I see the issue. Agreed.

[chrisgao (Use stgao instead), 2013/02/14 03:49:45]

I also think list copy is easier and simpler.

   }
 
   int command_id = next_id_++;
   base::DictionaryValue command;
   command.SetInteger("id", command_id);
   command.SetString("method", method);
   command.Set("params", params.DeepCopy());
   std::string message;
   base::JSONWriter::Write(&command, &message);
   if (!socket_->Send(message)) {
@@ skipping 18 matching lines @@
   result->reset(cmd_response_map_[command_id]);
   cmd_response_map_.erase(command_id);
   return Status(kOk);
 }
 
 Status DevToolsClientImpl::ReceiveNextMessage(
     int expected_id,
     internal::InspectorMessageType* type,
     internal::InspectorEvent* event,
     internal::InspectorCommandResponse* response) {
+  while (!listeners_for_on_connected_.empty()) {
+    DevToolsEventListener* listener = listeners_for_on_connected_.front();
+    listeners_for_on_connected_.pop_front();
+    Status status = listener->OnConnected();
+    if (status.IsError())
+      return status;
+  }
+  // The message might be received already when processing other commands sent
+  // from DevToolsEventListener::OnConnected.
+  if (cmd_response_map_.find(expected_id) != cmd_response_map_.end()

[craigdh, 2013/02/13 19:19:39]

Consider making this a private function, something like
HasReceivedCommandResponse(int cmd_id). It'd be a little more clear what you're
checking for and make the code more readable.

[chrisgao (Use stgao instead), 2013/02/14 03:49:45]

Good idea.

+      && cmd_response_map_[expected_id])
+    return Status(kOk);
+
   std::string message;
   if (!socket_->ReceiveNextMessage(&message)) {
     connected_ = false;
     return Status(kDisconnected,
                   "unable to receive message from renderer");
   }
   if (!parser_func_.Run(message, expected_id, type, event, response))
     return Status(kUnknownError, "bad inspector message: " + message);
   if (*type == internal::kEventMessageType)
     return NotifyEventListeners(event->method, *event->params);
@@ skipping 64 matching lines @@
     if (unscoped_result)
       command_response->result.reset(unscoped_result->DeepCopy());
     else
       base::JSONWriter::Write(unscoped_error, &command_response->error);
     return true;
   }
   return false;
 }
 
 }  // namespace internal