Review URL: http://codereview.chromium.org/77006

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

7 files changed, 51 insertions, 41 deletions

diff --git a/chrome/browser/debugger/debugger_remote_service.cc b/chrome/browser/debugger/debugger_remote_service.cc
index 65f8bdb..7053ab7 100644
--- a/chrome/browser/debugger/debugger_remote_service.cc
+++ b/chrome/browser/debugger/debugger_remote_service.cc
@@ -154,7 +154,7 @@ void DebuggerRemoteService::SendResponse(const Value& response,
 
 WebContents* DebuggerRemoteService::ToWebContents(int32 tab_uid) {
   const InspectableTabProxy::ControllersMap& navcon_map =
-      delegate_->inspectable_tab_proxy()->controllers_map(false);
+      delegate_->inspectable_tab_proxy()->controllers_map();
   InspectableTabProxy::ControllersMap::const_iterator it =
       navcon_map.find(tab_uid);
   if (it != navcon_map.end()) {
@@ -193,20 +193,20 @@ void DebuggerRemoteService::AttachTab(const std::string& destination,
   StringToInt(destination, &tab_uid);
   if (tab_uid < 0) {
     // Bad tab_uid received from remote debugger (perhaps NaN)
-    response->SetInteger(kDataWide, Result::kUnknownTab);
+    response->SetInteger(kResultWide, Result::kUnknownTab);
     return;
   }
   if (tab_uid == 0) {  // single tab_uid
     // We've been asked to open a new tab with URL.
     // TODO(apavlov): implement
     NOTIMPLEMENTED();
-    response->SetInteger(kDataWide, Result::kUnknownTab);
+    response->SetInteger(kResultWide, Result::kUnknownTab);
     return;
   }
   WebContents* web_contents = ToWebContents(tab_uid);
   if (web_contents == NULL) {
     // No active web contents with tab_uid
-    response->SetInteger(kDataWide, Result::kUnknownTab);
+    response->SetInteger(kResultWide, Result::kUnknownTab);
     return;
   }
   if (g_browser_process->devtools_manager()->GetDevToolsClientHostFor(
@@ -217,29 +217,32 @@ void DebuggerRemoteService::AttachTab(const std::string& destination,
     if (manager != NULL) {
       manager->RegisterDevToolsClientHostFor(*web_contents, client_host);
       manager->ForwardToDevToolsAgent(*client_host, DevToolsAgentMsg_Attach());
-      response->SetInteger(kDataWide, Result::kOk);
+      response->SetInteger(kResultWide, Result::kOk);
     } else {
-      response->SetInteger(kDataWide, Result::kDebuggerError);
+      response->SetInteger(kResultWide, Result::kDebuggerError);
     }
   } else {
     // DevToolsClientHost for this tab already registered
-    response->SetInteger(kDataWide, Result::kIllegalTabState);
+    response->SetInteger(kResultWide, Result::kIllegalTabState);
   }
 }
 
 void DebuggerRemoteService::DetachTab(const std::string& destination,
                                       DictionaryValue* response) {
   int32 tab_uid = -1;
+  int resultCode = -1;
   StringToInt(destination, &tab_uid);
   if (tab_uid == -1) {
     // Bad tab_uid received from remote debugger (NaN)
-    response->SetInteger(kDataWide, Result::kUnknownTab);
+    if (response != NULL) {
+      response->SetInteger(kResultWide, Result::kUnknownTab);
+    }
     return;
   }
   WebContents* web_contents = ToWebContents(tab_uid);
   if (web_contents == NULL) {
     // Unknown tab
-    response->SetInteger(kDataWide, Result::kUnknownTab);
+    resultCode = Result::kUnknownTab;
   } else {
     DevToolsManager* manager = g_browser_process->devtools_manager();
     if (manager != NULL) {
@@ -249,14 +252,17 @@ void DebuggerRemoteService::DetachTab(const std::string& destination,
         manager->ForwardToDevToolsAgent(
             *client_host, DevToolsAgentMsg_Detach());
         client_host->InspectedTabClosing();
-        response->SetInteger(kDataWide, Result::kOk);
+        resultCode = Result::kOk;
       } else {
         // No client host registered
-        response->SetInteger(kDataWide, Result::kUnknownTab);
+        resultCode = Result::kUnknownTab;
       }
     } else {
       // No DevToolsManager
-      response->SetInteger(kResultWide, Result::kDebuggerError);
+      resultCode = Result::kDebuggerError;
     }
   }
+  if (response != NULL) {
+    response->SetInteger(kResultWide, resultCode);
+  }
 }
diff --git a/chrome/browser/debugger/devtools_remote_listen_socket.cc b/chrome/browser/debugger/devtools_remote_listen_socket.cc
index f064c0c..11d38dc 100644
--- a/chrome/browser/debugger/devtools_remote_listen_socket.cc
+++ b/chrome/browser/debugger/devtools_remote_listen_socket.cc
@@ -45,7 +45,8 @@ DevToolsRemoteListenSocket::DevToolsRemoteListenSocket(
     DevToolsRemoteListener* message_listener)
         : ListenSocket(s, del),
           state_(HANDSHAKE),
-          message_listener_(message_listener) {}
+          message_listener_(message_listener),
+          cr_received_(false) {}
 
 void DevToolsRemoteListenSocket::StartNextField() {
   switch (state_) {
@@ -135,19 +136,19 @@ void DevToolsRemoteListenSocket::DispatchRead(char* buf, int len) {
   char* pBuf = buf;
   while (len > 0) {
     if (state_ != PAYLOAD) {
-      while (*pBuf != '\r' && len > 0) {
-        protocol_field_.push_back(*pBuf);
+      if (cr_received_ && *pBuf == '\n') {
+        cr_received_ = false;
         CONSUME_BUFFER_CHAR;
-      }
-      if (*pBuf != '\r') {
-        continue;
       } else {
-        CONSUME_BUFFER_CHAR;
-        if (*pBuf != '\n') {
-          continue;
-        } else {
-          CONSUME_BUFFER_CHAR;  // handle the \r\n series
+        while (*pBuf != '\r' && len > 0) {
+          protocol_field_.push_back(*pBuf);
+          CONSUME_BUFFER_CHAR;
+        }
+        if (*pBuf == '\r') {
+          cr_received_ = true;
+          CONSUME_BUFFER_CHAR;
         }
+        continue;
       }
       switch (state_) {
         case HANDSHAKE:
diff --git a/chrome/browser/debugger/devtools_remote_listen_socket.h b/chrome/browser/debugger/devtools_remote_listen_socket.h
index d671bf2..0f64eec 100644
--- a/chrome/browser/debugger/devtools_remote_listen_socket.h
+++ b/chrome/browser/debugger/devtools_remote_listen_socket.h
@@ -58,6 +58,7 @@ class DevToolsRemoteListenSocket : public ListenSocket {
   std::string payload_;
   int32 remaining_payload_length_;
   DevToolsRemoteListener* message_listener_;
+  bool cr_received_;
 
   DISALLOW_COPY_AND_ASSIGN(DevToolsRemoteListenSocket);
 };
diff --git a/chrome/browser/debugger/devtools_remote_listen_socket_unittest.cc b/chrome/browser/debugger/devtools_remote_listen_socket_unittest.cc
index a59c992..8a78e9f 100644
--- a/chrome/browser/debugger/devtools_remote_listen_socket_unittest.cc
+++ b/chrome/browser/debugger/devtools_remote_listen_socket_unittest.cc
@@ -13,9 +13,11 @@ const int DevToolsRemoteListenSocketTester::kTestPort = 9999;
 
 static const int kReadBufSize = 1024;
 static const char* kChromeDevToolsHandshake = "ChromeDevToolsHandshake\r\n";
-static const char* kSimpleMessage =
+static const char* kSimpleMessagePart1 =
     "Tool:V8Debugger\r\n"
-    "Destination:2\r\n"
+    "Destination:2\r";
+static const char* kSimpleMessagePart2 =
+    "\n"
     "Content-Length:0\r\n"
     "\r\n";
 static const char* kTwoMessages =
@@ -259,7 +261,10 @@ bool DevToolsRemoteListenSocketTester::Send(SOCKET sock,
 void DevToolsRemoteListenSocketTester::TestClientSend() {
   ASSERT_TRUE(Send(test_socket_, kChromeDevToolsHandshake));
   {
-    ASSERT_TRUE(Send(test_socket_, kSimpleMessage));
+    ASSERT_TRUE(Send(test_socket_, kSimpleMessagePart1));
+    // sleep for 10ms to test message split between \r and \n
+    PlatformThread::Sleep(10);
+    ASSERT_TRUE(Send(test_socket_, kSimpleMessagePart2));
     ASSERT_TRUE(NextAction(kDefaultTimeoutMs));
     ASSERT_EQ(ACTION_READ_MESSAGE, last_action_.type());
     const DevToolsRemoteMessage& message = last_action_.message();
diff --git a/chrome/browser/debugger/devtools_remote_service.cc b/chrome/browser/debugger/devtools_remote_service.cc
index 4913f8b..4308d18 100644
--- a/chrome/browser/debugger/devtools_remote_service.cc
+++ b/chrome/browser/debugger/devtools_remote_service.cc
@@ -71,7 +71,7 @@ void DevToolsRemoteService::ProcessJson(DictionaryValue* json,
   } else if (command == DevToolsRemoteServiceCommand::kListTabs) {
     ListValue* data = new ListValue();
     const InspectableTabProxy::ControllersMap& navcon_map =
-        delegate_->inspectable_tab_proxy()->controllers_map(true);
+        delegate_->inspectable_tab_proxy()->controllers_map();
     for (InspectableTabProxy::ControllersMap::const_iterator it =
         navcon_map.begin(), end = navcon_map.end(); it != end; ++it) {
       NavigationEntry* entry = it->second->GetActiveEntry();
diff --git a/chrome/browser/debugger/inspectable_tab_proxy.cc b/chrome/browser/debugger/inspectable_tab_proxy.cc
index a329d6a..c216766 100644
--- a/chrome/browser/debugger/inspectable_tab_proxy.cc
+++ b/chrome/browser/debugger/inspectable_tab_proxy.cc
@@ -79,17 +79,15 @@ void DevToolsClientHostImpl::DebuggerOutput(const std::string& msg) {
 }  // namespace
 
 const InspectableTabProxy::ControllersMap&
-    InspectableTabProxy::controllers_map(bool refresh) {
-  if (refresh || controllers_map_.empty() /* on initial call */) {
-    controllers_map_.clear();
-    for (BrowserList::const_iterator it = BrowserList::begin(),
-         end = BrowserList::end(); it != end; ++it) {
-      TabStripModel* model = (*it)->tabstrip_model();
-      for (int i = 0, size = model->count(); i < size; ++i) {
-        NavigationController& controller =
-            model->GetTabContentsAt(i)->controller();
-        controllers_map_[controller.session_id().id()] = &controller;
-      }
+    InspectableTabProxy::controllers_map() {
+  controllers_map_.clear();
+  for (BrowserList::const_iterator it = BrowserList::begin(),
+       end = BrowserList::end(); it != end; ++it) {
+    TabStripModel* model = (*it)->tabstrip_model();
+    for (int i = 0, size = model->count(); i < size; ++i) {
+      NavigationController& controller =
+          model->GetTabContentsAt(i)->controller();
+      controllers_map_[controller.session_id().id()] = &controller;
     }
   }
   return controllers_map_;
diff --git a/chrome/browser/debugger/inspectable_tab_proxy.h b/chrome/browser/debugger/inspectable_tab_proxy.h
index 7888a7c..fdf82d9 100644
--- a/chrome/browser/debugger/inspectable_tab_proxy.h
+++ b/chrome/browser/debugger/inspectable_tab_proxy.h
@@ -23,9 +23,8 @@ class InspectableTabProxy {
   virtual ~InspectableTabProxy() {}
 
   // Returns a map of NavigationControllerKeys to NavigationControllers
-  // for all Browser instances. If |refresh| == true, the cached map will be
-  // dropped and retrieved anew.
-  const ControllersMap& controllers_map(bool refresh);
+  // for all Browser instances.
+  const ControllersMap& controllers_map();
 
   // Creates a new DevToolsClientHost implementor instance.
   // |id| is the UID of the tab to debug.