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

chromecast/browser/devtools/remote_debugging_server.cc

Index: chromecast/browser/devtools/remote_debugging_server.cc
diff --git a/chromecast/browser/devtools/remote_debugging_server.cc b/chromecast/browser/devtools/remote_debugging_server.cc
index 60b49333c32005c6d8eb78f4ebdbb26099017e4f..44763241930d7311d938504ee1ebb393660f3b23 100644
--- a/chromecast/browser/devtools/remote_debugging_server.cc
+++ b/chromecast/browser/devtools/remote_debugging_server.cc
@@ -8,6 +8,7 @@
 #include "base/bind_helpers.h"
 #include "base/command_line.h"
 #include "base/files/file_path.h"
+#include "base/strings/string_number_conversions.h"
 #include "base/strings/stringprintf.h"
 #include "chromecast/browser/cast_browser_process.h"
 #include "chromecast/browser/devtools/cast_dev_tools_delegate.h"
@@ -93,48 +94,45 @@ std::string GetFrontendUrl() {
 
 }  // namespace
 
-RemoteDebuggingServer::RemoteDebuggingServer() : port_(0) {
+RemoteDebuggingServer::RemoteDebuggingServer()
+    : port_(kDefaultRemoteDebuggingPort) {
   DCHECK(content::BrowserThread::CurrentlyOn(content::BrowserThread::UI));
-  pref_port_.Init(prefs::kRemoteDebuggingPort,
-                  CastBrowserProcess::GetInstance()->pref_service(),
-                  base::Bind(&RemoteDebuggingServer::OnPortChanged,
-                             base::Unretained(this)));
+  pref_enabled_.Init(prefs::kEnableRemoteDebugging,
+                     CastBrowserProcess::GetInstance()->pref_service(),
+                     base::Bind(&RemoteDebuggingServer::OnEnabledChanged,
+                                base::Unretained(this)));
+
+  std::string port_str = base::CommandLine::ForCurrentProcess()
+      ->GetSwitchValueASCII(switches::kRemoteDebuggingPort);
+  if (!port_str.empty()) {
+    unsigned port;

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

Init to 0. StringToUint may not set port on error.

And, why unsigned? why not just int?

[derekjchow1, 2015/01/08 17:55:42]

Done.
Done.

+    base::StringToUint(port_str, &port);

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

Do we support port=0 case which disables remote debugging regardless of DCS
config?

[derekjchow1, 2015/01/08 17:55:42]

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

[byungchul, 2015/01/08 18:51:21]

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:55]

Done. Your correct, we should set default port. 

+    port_ = port;
+  }
 
   // Starts new dev tools, clearing port number saved in config.
   // Remote debugging in production must be triggered only by config server.
-  pref_port_.SetValue(ShouldStartImmediately() ?
-                      kDefaultRemoteDebuggingPort : 0);
-  OnPortChanged();
+  pref_enabled_.SetValue(ShouldStartImmediately() && port_ != 0);
+  OnEnabledChanged();
 }
 
 RemoteDebuggingServer::~RemoteDebuggingServer() {
-  pref_port_.SetValue(0);
-  OnPortChanged();
+  pref_enabled_.SetValue(false);
+  OnEnabledChanged();
 }
 
-void RemoteDebuggingServer::OnPortChanged() {
-  uint16 new_port = static_cast<uint16>(std::max(*pref_port_, 0));
-  VLOG(1) << "OnPortChanged called: old_port=" << port_
-          << ", new_port=" << new_port;
-
-  if (new_port == port_) {
-    VLOG(1) << "Port has not been changed. Ignore silently.";
-    return;
-  }
-
-  if (devtools_http_handler_) {
-    LOG(INFO) << "Stop old devtools: port=" << port_;
-    devtools_http_handler_.reset();
-  }
-
-  port_ = new_port;
-  if (port_ > 0) {
+void RemoteDebuggingServer::OnEnabledChanged() {
+  bool enabled = *pref_enabled_ && port_ != 0;
+  if (enabled && !devtools_http_handler_) {
+    VLOG(1) << "Start devtools: port=" << port_;
     devtools_http_handler_.reset(content::DevToolsHttpHandler::Start(
         CreateSocketFactory(port_),
         GetFrontendUrl(),
         new CastDevToolsDelegate(),
         base::FilePath()));
-    LOG(INFO) << "Devtools started: port=" << port_;
+  } else if (devtools_http_handler_) {
+    LOG(INFO) << "Stop devtools: port=" << port_;

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

Make the Start/Stop devtools either both LOG(INFO) or both VLOG(1)

[derekjchow1, 2015/01/08 17:55:42]

Done.

+    devtools_http_handler_.reset();
   }
 }