Set RemoteDebuggingServer port based on "remote-debugging-port" flag.

Set RemoteDebuggingServer port based on "remote-debugging-port" flag.

RemoteDebuggingServer now has a fixed port based on command line flag.
Enabling and disabling remote debugging is now controlled by the
enable_remote_debugging preference.

R=gunsch@chromium.org
BUG=internal b/18790937

Committed: https://crrev.com/ebc9cdbdef3e4542ae2b26bddc134b8e28ad6215
Cr-Commit-Position: refs/heads/master@{#334626}

[gunsch, 2015-01-08 03:42:53]

gunsch@chromium.org changed reviewers:
+ byungchul@chromium.org

[gunsch, 2015-01-08 03:46:52]

Please loop Byungchul in on future remote-debugging changes.

Byungchul: I think we removed the possibility of changing the remote debugging
port via app config, can you confirm?

If so, the changes lgtm % nit and Byungchul's approval

https://codereview.chromium.org/793663007/diff/1/chromecast/browser/devtools/...
File chromecast/browser/devtools/remote_debugging_server.cc (right):

https://codereview.chromium.org/793663007/diff/1/chromecast/browser/devtools/...
chromecast/browser/devtools/remote_debugging_server.cc:134: LOG(INFO) << "Stop
devtools: port=" << port_;
Make the Start/Stop devtools either both LOG(INFO) or both VLOG(1)

[byungchul, 2015-01-08 06:28:56]

Yes, we always use 9222 ignoring port number from DCS.

https://codereview.chromium.org/793663007/diff/1/chromecast/browser/devtools/...
File chromecast/browser/devtools/remote_debugging_server.cc (right):

https://codereview.chromium.org/793663007/diff/1/chromecast/browser/devtools/...
chromecast/browser/devtools/remote_debugging_server.cc:108: unsigned port;
Init to 0. StringToUint may not set port on error.

And, why unsigned? why not just int?

https://codereview.chromium.org/793663007/diff/1/chromecast/browser/devtools/...
chromecast/browser/devtools/remote_debugging_server.cc:109:
base::StringToUint(port_str, &port);
Do we support port=0 case which disables remote debugging regardless of DCS
config?

[derekjchow1, 2015-01-08 17:55:43]

https://codereview.chromium.org/793663007/diff/1/chromecast/browser/devtools/...
File chromecast/browser/devtools/remote_debugging_server.cc (right):

https://codereview.chromium.org/793663007/diff/1/chromecast/browser/devtools/...
chromecast/browser/devtools/remote_debugging_server.cc:108: unsigned port;
On 2015/01/08 06:28:56, byungchul wrote:
> Init to 0. StringToUint may not set port on error.

Done.

> And, why unsigned? why not just int?

Done.

https://codereview.chromium.org/793663007/diff/1/chromecast/browser/devtools/...
chromecast/browser/devtools/remote_debugging_server.cc:109:
base::StringToUint(port_str, &port);
On 2015/01/08 06:28:56, byungchul wrote:
> Do we support port=0 case which disables remote debugging regardless of DCS
> config?

Yes. There is a check in OnEnabledChanged that only starts devtools if port_ !=
0.

https://codereview.chromium.org/793663007/diff/1/chromecast/browser/devtools/...
chromecast/browser/devtools/remote_debugging_server.cc:134: LOG(INFO) << "Stop
devtools: port=" << port_;
On 2015/01/08 03:46:52, gunsch wrote:
> Make the Start/Stop devtools either both LOG(INFO) or both VLOG(1)

Done.

[byungchul, 2015-01-08 18:51:22]

https://codereview.chromium.org/793663007/diff/1/chromecast/browser/devtools/...
File chromecast/browser/devtools/remote_debugging_server.cc (right):

https://codereview.chromium.org/793663007/diff/1/chromecast/browser/devtools/...
chromecast/browser/devtools/remote_debugging_server.cc:109:
base::StringToUint(port_str, &port);
On 2015/01/08 17:55:42, derekjchow1 wrote:
> On 2015/01/08 06:28:56, byungchul wrote:
> > Do we support port=0 case which disables remote debugging regardless of DCS
> > config?
> 
> Yes. There is a check in OnEnabledChanged that only starts devtools if port_
!=
> 0.

My question is, if it is by intention or not. Alternative way is to set 9222 for
invalid inputs.

[derekjchow1, 2015-01-12 23:15:56]

https://codereview.chromium.org/793663007/diff/1/chromecast/browser/devtools/...
File chromecast/browser/devtools/remote_debugging_server.cc (right):

https://codereview.chromium.org/793663007/diff/1/chromecast/browser/devtools/...
chromecast/browser/devtools/remote_debugging_server.cc:109:
base::StringToUint(port_str, &port);
On 2015/01/08 18:51:21, byungchul wrote:
> On 2015/01/08 17:55:42, derekjchow1 wrote:
> > On 2015/01/08 06:28:56, byungchul wrote:
> > > Do we support port=0 case which disables remote debugging regardless of
DCS
> > > config?
> > 
> > Yes. There is a check in OnEnabledChanged that only starts devtools if port_
> !=
> > 0.
> 
> My question is, if it is by intention or not. Alternative way is to set 9222
for
> invalid inputs.

Done. Your correct, we should set default port.

[derekjchow1, 2015-06-16 16:44:50]

Ping on this issue. Does this look good to you Byungchul?

[byungchul, 2015-06-16 16:57:25]

https://codereview.chromium.org/793663007/diff/100001/chromecast/browser/devt...
File chromecast/browser/devtools/remote_debugging_server.cc (right):

https://codereview.chromium.org/793663007/diff/100001/chromecast/browser/devt...
chromecast/browser/devtools/remote_debugging_server.cc:159: } else if
(devtools_http_handler_) {
it includes the condition "enabled && evtools_http_handler_" though it might be
fine because OnEnabledChanged() will be called only when enabled changed. But,
still good to add !enabled.

[derekjchow1, 2015-06-16 17:02:49]

https://codereview.chromium.org/793663007/diff/100001/chromecast/browser/devt...
File chromecast/browser/devtools/remote_debugging_server.cc (right):

https://codereview.chromium.org/793663007/diff/100001/chromecast/browser/devt...
chromecast/browser/devtools/remote_debugging_server.cc:159: } else if
(devtools_http_handler_) {
On 2015/06/16 16:57:24, byungchul wrote:
> it includes the condition "enabled && evtools_http_handler_" though it might
be
> fine because OnEnabledChanged() will be called only when enabled changed. But,
> still good to add !enabled.

Done.

[byungchul, 2015-06-16 17:15:41]

lgtm

[derekjchow1, 2015-06-16 17:16:08]

The CQ bit was checked by derekjchow@chromium.org

[derekjchow1, 2015-06-16 17:16:09]

The patchset sent to the CQ was uploaded after l-g-t-m from gunsch@chromium.org
Link to the patchset: https://codereview.chromium.org/793663007/#ps120001
(title: "Address Byungchul's comment.")

[commit-bot: I haz the power, 2015-06-16 17:17:39]

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

[commit-bot: I haz the power, 2015-06-16 17:26:21]

Committed patchset #7 (id:120001)

[commit-bot: I haz the power, 2015-06-16 17:27:29]

Patchset 7 (id:??) landed as
https://crrev.com/ebc9cdbdef3e4542ae2b26bddc134b8e28ad6215
Cr-Commit-Position: refs/heads/master@{#334626}

[LeavMeB, 2016-10-21 15:26:00]

ellis.smith.709@gmail.com changed reviewers:
+ ellis.smith.709@gmail.com

[LeavMeB, 2016-10-21 15:26:00]

BooleanPrefMember enabled

[LeavMeB, 2016-10-21 15:35:19]

SetBoolean kEnableRemoteDebugging

[LeavMeB, 2016-10-21 15:35:19]

SetBoolean kEnableRemoteDebugging