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

content/browser/devtools/tethering_handler.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 "content/browser/devtools/tethering_handler.h"
 
 #include "base/bind.h"
 #include "base/callback.h"
 #include "base/stl_util.h"
 #include "base/values.h"
@@ skipping 239 matching lines @@
 
 // TetheringHandler::TetheringImpl -------------------------------------------
 
 class TetheringHandler::TetheringImpl {
  public:
   TetheringImpl(
       base::WeakPtr<TetheringHandler> handler,
       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_;
 
   typedef std::map<int, BoundSocket*> BoundSockets;
   BoundSockets bound_sockets_;
 };
 
 TetheringHandler::TetheringImpl::TetheringImpl(
     base::WeakPtr<TetheringHandler> handler,
     DevToolsHttpHandlerDelegate* delegate)
     : handler_(handler),
       delegate_(delegate) {
 }
 
 TetheringHandler::TetheringImpl::~TetheringImpl() {
   STLDeleteContainerPairSecondPointers(bound_sockets_.begin(),
                                        bound_sockets_.end());
 }
 
 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));
+    scoped_refptr<DevToolsProtocol::Command> command,
+    int port) {
+  if (bound_sockets_.find(port) != bound_sockets_.end()) {
+    SendInternalError(command, "Port already bound");
     return;
   }
 
-  if (bound_sockets_.find(port) != bound_sockets_.end()) {
-    SendAsyncResponse(command->InternalErrorResponse("Port already bound"));
-    return;
-  }
-
   BoundSocket::AcceptedCallback callback = base::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(
     int port, const std::string& name) {
   BrowserThread::PostTask(
       BrowserThread::UI,
       FROM_HERE,
       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::SendAsyncResponse, handler_, response));
+      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::SendInternalError, handler_,
+                 command, message));
+}
+
+
 // TetheringHandler ----------------------------------------------------------
 
 // static
 TetheringHandler::TetheringImpl* TetheringHandler::impl_ = nullptr;
 
 TetheringHandler::TetheringHandler(
     DevToolsHttpHandlerDelegate* delegate,
     scoped_refptr<base::MessageLoopProxy> message_loop_proxy)
     : delegate_(delegate),
       message_loop_proxy_(message_loop_proxy),
@@ skipping 31 matching lines @@
   return true;
 }
 
 scoped_refptr<DevToolsProtocol::Response>
 TetheringHandler::OnBind(scoped_refptr<DevToolsProtocol::Command> command) {
   if (!Activate()) {
     return command->ServerErrorResponse(
         "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();
 }
 
 scoped_refptr<DevToolsProtocol::Response>
 TetheringHandler::OnUnbind(scoped_refptr<DevToolsProtocol::Command> command) {
   if (!Activate()) {
     return command->ServerErrorResponse(
         "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