[DevTools] Refactor tethering to allow for minimal changes when moving to generated handler

content/browser/devtools/tethering_handler.cc

Index: content/browser/devtools/tethering_handler.cc
diff --git a/content/browser/devtools/tethering_handler.cc b/content/browser/devtools/tethering_handler.cc
index 4135c48ec0fed8a81a2a108bb6817f89c6cd146e..5bbd3d023a2474249019f35a26176e43e1342ab0 100644
--- a/content/browser/devtools/tethering_handler.cc
+++ b/content/browser/devtools/tethering_handler.cc
@@ -257,12 +257,15 @@ class TetheringHandler::TetheringImpl {
       DevToolsHttpHandlerDelegate* delegate);
   ~TetheringImpl();
 
-  void Bind(scoped_refptr<DevToolsProtocol::Command> command);
-  void Unbind(scoped_refptr<DevToolsProtocol::Command> command);
+  void Bind(scoped_refptr<DevToolsProtocol::Command> command, int port);
+  void Unbind(scoped_refptr<DevToolsProtocol::Command> command, int port);
   void Accepted(int port, const std::string& name);
 
  private:
-  void SendAsyncResponse(scoped_refptr<DevToolsProtocol::Response> response);
+  void SendBindSuccess(scoped_refptr<DevToolsProtocol::Command> command);
+  void SendUnbindSuccess(scoped_refptr<DevToolsProtocol::Command> command);
+  void SendInternalError(scoped_refptr<DevToolsProtocol::Command> command,
+                         const std::string& message);
 
   base::WeakPtr<TetheringHandler> handler_;
   DevToolsHttpHandlerDelegate* delegate_;
@@ -284,16 +287,10 @@ TetheringHandler::TetheringImpl::~TetheringImpl() {
 }
 
 void TetheringHandler::TetheringImpl::Bind(
-    scoped_refptr<DevToolsProtocol::Command> command) {
-  const std::string& portParamName = devtools::Tethering::bind::kParamPort;
-  int port = GetPort(command, portParamName);
-  if (port == 0) {
-    SendAsyncResponse(command->InvalidParamResponse(portParamName));
-    return;
-  }
-
+    scoped_refptr<DevToolsProtocol::Command> command,
+    int port) {
   if (bound_sockets_.find(port) != bound_sockets_.end()) {
-    SendAsyncResponse(command->InternalErrorResponse("Port already bound"));
+    SendInternalError(command, "Port already bound");
     return;
   }
 
@@ -301,32 +298,27 @@ void TetheringHandler::TetheringImpl::Bind(
       &TetheringHandler::TetheringImpl::Accepted, base::Unretained(this));
   scoped_ptr<BoundSocket> bound_socket(new BoundSocket(callback, delegate_));
   if (!bound_socket->Listen(port)) {
-    SendAsyncResponse(command->InternalErrorResponse("Could not bind port"));
+    SendInternalError(command, "Could not bind port");
     return;
   }
 
   bound_sockets_[port] = bound_socket.release();
-  SendAsyncResponse(command->SuccessResponse(NULL));
+  SendBindSuccess(command);
 }
 
 void TetheringHandler::TetheringImpl::Unbind(
-    scoped_refptr<DevToolsProtocol::Command> command) {
-  const std::string& portParamName = devtools::Tethering::unbind::kParamPort;
-  int port = GetPort(command, portParamName);
-  if (port == 0) {
-    SendAsyncResponse(command->InvalidParamResponse(portParamName));
-    return;
-  }
+    scoped_refptr<DevToolsProtocol::Command> command,
+    int port) {
 
   BoundSockets::iterator it = bound_sockets_.find(port);
   if (it == bound_sockets_.end()) {
-    SendAsyncResponse(command->InternalErrorResponse("Port is not bound"));
+    SendInternalError(command, "Port is not bound");
     return;
   }
 
   delete it->second;
   bound_sockets_.erase(it);
-  SendAsyncResponse(command->SuccessResponse(NULL));
+  SendUnbindSuccess(command);
 }
 
 void TetheringHandler::TetheringImpl::Accepted(
@@ -337,14 +329,33 @@ void TetheringHandler::TetheringImpl::Accepted(
       base::Bind(&TetheringHandler::Accepted, handler_, port, name));
 }
 
-void TetheringHandler::TetheringImpl::SendAsyncResponse(
-    scoped_refptr<DevToolsProtocol::Response> response) {
+void TetheringHandler::TetheringImpl::SendBindSuccess(

[dgozman, 2014/10/17 13:15:16]

You can inline SendBindSuccess and SendUnbindSuccess.

[vkuzkokov, 2014/10/17 13:30:43]

Done.

+    scoped_refptr<DevToolsProtocol::Command> command) {
+  BrowserThread::PostTask(
+      BrowserThread::UI,
+      FROM_HERE,
+      base::Bind(&TetheringHandler::SendBindSuccess, handler_, command));
+}
+
+void TetheringHandler::TetheringImpl::SendUnbindSuccess(
+    scoped_refptr<DevToolsProtocol::Command> command) {
+  BrowserThread::PostTask(
+      BrowserThread::UI,
+      FROM_HERE,
+      base::Bind(&TetheringHandler::SendUnbindSuccess, handler_, command));
+}
+
+void TetheringHandler::TetheringImpl::SendInternalError(
+    scoped_refptr<DevToolsProtocol::Command> command,
+    const std::string& message) {
   BrowserThread::PostTask(
       BrowserThread::UI,
       FROM_HERE,
-      base::Bind(&TetheringHandler::SendAsyncResponse, handler_, response));
+      base::Bind(&TetheringHandler::SendInternalError, handler_,
+                 command, message));
 }
 
+
 // TetheringHandler ----------------------------------------------------------
 
 // static
@@ -396,9 +407,15 @@ TetheringHandler::OnBind(scoped_refptr<DevToolsProtocol::Command> command) {
         "Tethering is used by another connection");
   }
   DCHECK(impl_);
+  const std::string& portParamName = devtools::Tethering::bind::kParamPort;

[dgozman, 2014/10/17 13:15:16]

Perhaps, we can move this check before |Activate()| call.

[vkuzkokov, 2014/10/17 13:30:43]

Done.

+  int port = GetPort(command, portParamName);
+  if (port == 0)
+    return command->InvalidParamResponse(portParamName);
+
   message_loop_proxy_->PostTask(
       FROM_HERE,
-      base::Bind(&TetheringImpl::Bind, base::Unretained(impl_), command));
+      base::Bind(&TetheringImpl::Bind, base::Unretained(impl_),
+                 command, port));
   return command->AsyncResponsePromise();
 }
 
@@ -409,10 +426,32 @@ TetheringHandler::OnUnbind(scoped_refptr<DevToolsProtocol::Command> command) {
         "Tethering is used by another connection");
   }
   DCHECK(impl_);
+  const std::string& portParamName = devtools::Tethering::unbind::kParamPort;

[dgozman, 2014/10/17 13:15:16]

ditto

[vkuzkokov, 2014/10/17 13:30:43]

Done.

+  int port = GetPort(command, portParamName);
+  if (port == 0)
+    return command->InvalidParamResponse(portParamName);
+
   message_loop_proxy_->PostTask(
       FROM_HERE,
-      base::Bind(&TetheringImpl::Unbind, base::Unretained(impl_), command));
+      base::Bind(&TetheringImpl::Unbind, base::Unretained(impl_),
+                 command, port));
   return command->AsyncResponsePromise();
 }
 
+void TetheringHandler::SendBindSuccess(

[dgozman, 2014/10/17 13:15:16]

What's the point of two equal methods?

[vkuzkokov, 2014/10/17 13:30:43]

They will be calling different methods of devtools::tethering::Client.

[dgozman, 2014/10/17 13:34:24]

Oh, didn't think about that.

+    scoped_refptr<DevToolsProtocol::Command> command) {
+  SendAsyncResponse(command->SuccessResponse(nullptr));
+}
+
+void TetheringHandler::SendUnbindSuccess(
+    scoped_refptr<DevToolsProtocol::Command> command) {
+  SendAsyncResponse(command->SuccessResponse(nullptr));
+}
+
+void TetheringHandler::SendInternalError(
+    scoped_refptr<DevToolsProtocol::Command> command,
+    const std::string& message) {
+  SendAsyncResponse(command->InternalErrorResponse(message));
+}
+
 }  // namespace content