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

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

BUG=405566
Committed: https://crrev.com/221e24f1e925269d58787d25d7d6d003341b1795
Cr-Commit-Position: refs/heads/master@{#300102}

[vkuzkokov, 2014-10-17 13:11:03]

vkuzkokov@chromium.org changed reviewers:
+ dgozman@chromium.org

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

https://codereview.chromium.org/645033005/diff/1/content/browser/devtools/tet...
File content/browser/devtools/tethering_handler.cc (right):

https://codereview.chromium.org/645033005/diff/1/content/browser/devtools/tet...
content/browser/devtools/tethering_handler.cc:332: void
TetheringHandler::TetheringImpl::SendBindSuccess(
You can inline SendBindSuccess and SendUnbindSuccess.

https://codereview.chromium.org/645033005/diff/1/content/browser/devtools/tet...
content/browser/devtools/tethering_handler.cc:410: const std::string&
portParamName = devtools::Tethering::bind::kParamPort;
Perhaps, we can move this check before |Activate()| call.

https://codereview.chromium.org/645033005/diff/1/content/browser/devtools/tet...
content/browser/devtools/tethering_handler.cc:429: const std::string&
portParamName = devtools::Tethering::unbind::kParamPort;
ditto

https://codereview.chromium.org/645033005/diff/1/content/browser/devtools/tet...
content/browser/devtools/tethering_handler.cc:441: void
TetheringHandler::SendBindSuccess(
What's the point of two equal methods?

[vkuzkokov, 2014-10-17 13:30:44]

https://codereview.chromium.org/645033005/diff/1/content/browser/devtools/tet...
File content/browser/devtools/tethering_handler.cc (right):

https://codereview.chromium.org/645033005/diff/1/content/browser/devtools/tet...
content/browser/devtools/tethering_handler.cc:332: void
TetheringHandler::TetheringImpl::SendBindSuccess(
On 2014/10/17 13:15:16, dgozman wrote:
> You can inline SendBindSuccess and SendUnbindSuccess.

Done.

https://codereview.chromium.org/645033005/diff/1/content/browser/devtools/tet...
content/browser/devtools/tethering_handler.cc:410: const std::string&
portParamName = devtools::Tethering::bind::kParamPort;
On 2014/10/17 13:15:16, dgozman wrote:
> Perhaps, we can move this check before |Activate()| call.

Done.

https://codereview.chromium.org/645033005/diff/1/content/browser/devtools/tet...
content/browser/devtools/tethering_handler.cc:429: const std::string&
portParamName = devtools::Tethering::unbind::kParamPort;
On 2014/10/17 13:15:16, dgozman wrote:
> ditto

Done.

https://codereview.chromium.org/645033005/diff/1/content/browser/devtools/tet...
content/browser/devtools/tethering_handler.cc:441: void
TetheringHandler::SendBindSuccess(
On 2014/10/17 13:15:16, dgozman wrote:
> What's the point of two equal methods?

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

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

lgtm

https://codereview.chromium.org/645033005/diff/1/content/browser/devtools/tet...
File content/browser/devtools/tethering_handler.cc (right):

https://codereview.chromium.org/645033005/diff/1/content/browser/devtools/tet...
content/browser/devtools/tethering_handler.cc:441: void
TetheringHandler::SendBindSuccess(
On 2014/10/17 13:30:43, vkuzkokov wrote:
> On 2014/10/17 13:15:16, dgozman wrote:
> > What's the point of two equal methods?
> 
> They will be calling different methods of devtools::tethering::Client.

Oh, didn't think about that.

https://codereview.chromium.org/645033005/diff/20001/content/browser/devtools...
File content/browser/devtools/tethering_handler.cc (right):

https://codereview.chromium.org/645033005/diff/20001/content/browser/devtools...
content/browser/devtools/tethering_handler.cc:289: int port) {
|port| should fit previous line.

https://codereview.chromium.org/645033005/diff/20001/content/browser/devtools...
content/browser/devtools/tethering_handler.cc:312: int port) {
ditto

[vkuzkokov, 2014-10-17 13:43:38]

https://codereview.chromium.org/645033005/diff/20001/content/browser/devtools...
File content/browser/devtools/tethering_handler.cc (right):

https://codereview.chromium.org/645033005/diff/20001/content/browser/devtools...
content/browser/devtools/tethering_handler.cc:289: int port) {
On 2014/10/17 13:34:24, dgozman wrote:
> |port| should fit previous line.

Done.

https://codereview.chromium.org/645033005/diff/20001/content/browser/devtools...
content/browser/devtools/tethering_handler.cc:312: int port) {
On 2014/10/17 13:34:24, dgozman wrote:
> ditto

Done.

[vkuzkokov, 2014-10-17 13:44:30]

The CQ bit was checked by vkuzkokov@chromium.org

[commit-bot: I haz the power, 2014-10-17 13:45:03]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/645033005/40001

[commit-bot: I haz the power, 2014-10-17 14:33:08]

Committed patchset #3 (id:40001)

[commit-bot: I haz the power, 2014-10-17 14:33:48]

Patchset 3 (id:??) landed as
https://crrev.com/221e24f1e925269d58787d25d7d6d003341b1795
Cr-Commit-Position: refs/heads/master@{#300102}