Support ephemeral ports in DevTools and Telemetry.

In DevTools, if port 0 is requested, query the HttpServer for the port
actually used, and write the port to a well-known location: the file
"DevToolsActivePort" in the user's profile directory. Currently this
handshaking protocol is only implemented for Chromium, not
content_shell, but this is the only scenario in which flakiness has
been observed.

Add a hidden command-line argument to Telemetry's desktop browser
backend which uses this new code path. The bots will specify this
argument. Telemetry must support browsers two releases back, so once
Chrome 37 goes to the Stable channel, the old initialization path will
be removed. Issue 379980 has been filed to remove the old code.

Tested with and without the new --use-devtools-active-port command
line argument with -v. Verified the new code path is taken. Once these
changes are committed, the bots will begin verifying this new code.

BUG=372560
NOTRY=true

Review URL: https://codereview.chromium.org/315503002

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@274935 0039d316-1c4b-4281-b951-d872f2087c98

11 files changed, 122 insertions, 12 deletions

diff --git a/android_webview/native/aw_dev_tools_server.cc b/android_webview/native/aw_dev_tools_server.cc
index 35b6455..6890655 100644
--- a/android_webview/native/aw_dev_tools_server.cc
+++ b/android_webview/native/aw_dev_tools_server.cc
@@ -6,6 +6,7 @@
 
 #include "android_webview/native/aw_contents.h"
 #include "base/bind.h"
+#include "base/files/file_path.h"
 #include "base/json/json_writer.h"
 #include "base/strings/stringprintf.h"
 #include "base/strings/utf_string_conversions.h"
@@ -180,7 +181,8 @@ void AwDevToolsServer::Start() {
           "",
           base::Bind(&content::CanUserConnectToDevTools)),
       base::StringPrintf(kFrontEndURL, content::GetWebKitRevision().c_str()),
-      new AwDevToolsServerDelegate());
+      new AwDevToolsServerDelegate(),
+      base::FilePath());
 }
 
 void AwDevToolsServer::Stop() {
diff --git a/chrome/browser/android/dev_tools_server.cc b/chrome/browser/android/dev_tools_server.cc
index 091e901..ff1acd3 100644
--- a/chrome/browser/android/dev_tools_server.cc
+++ b/chrome/browser/android/dev_tools_server.cc
@@ -13,6 +13,7 @@
 #include "base/callback.h"
 #include "base/command_line.h"
 #include "base/compiler_specific.h"
+#include "base/files/file_path.h"
 #include "base/logging.h"
 #include "base/strings/string_number_conversions.h"
 #include "base/strings/stringprintf.h"
@@ -427,7 +428,8 @@ void DevToolsServer::Start() {
           base::StringPrintf("%s_%d", socket_name_.c_str(), getpid()),
           base::Bind(&content::CanUserConnectToDevTools)),
       base::StringPrintf(kFrontEndURL, content::GetWebKitRevision().c_str()),
-      new DevToolsServerDelegate());
+      new DevToolsServerDelegate(),
+      base::FilePath());
 }
 
 void DevToolsServer::Stop() {
diff --git a/chrome/browser/devtools/remote_debugging_server.cc b/chrome/browser/devtools/remote_debugging_server.cc
index 7c98d7f..211ff1a 100644
--- a/chrome/browser/devtools/remote_debugging_server.cc
+++ b/chrome/browser/devtools/remote_debugging_server.cc
@@ -4,8 +4,10 @@
 
 #include "chrome/browser/devtools/remote_debugging_server.h"
 
+#include "base/path_service.h"
 #include "chrome/browser/devtools/browser_list_tabcontents_provider.h"
 #include "chrome/browser/ui/webui/devtools_ui.h"
+#include "chrome/common/chrome_paths.h"
 #include "content/public/browser/devtools_http_handler.h"
 #include "net/socket/tcp_listen_socket.h"
 
@@ -13,10 +15,20 @@ RemoteDebuggingServer::RemoteDebuggingServer(
     chrome::HostDesktopType host_desktop_type,
     const std::string& ip,
     int port) {
+  base::FilePath output_dir;
+  if (!port) {
+    // The client requested an ephemeral port. Must write the selected
+    // port to a well-known location in the profile directory to
+    // bootstrap the connection process.
+    bool result = PathService::Get(chrome::DIR_USER_DATA, &output_dir);
+    DCHECK(result);
+  }
+
   devtools_http_handler_ = content::DevToolsHttpHandler::Start(
       new net::TCPListenSocketFactory(ip, port),
       "",
-      new BrowserListTabContentsProvider(host_desktop_type));
+      new BrowserListTabContentsProvider(host_desktop_type),
+      output_dir);
 }
 
 RemoteDebuggingServer::~RemoteDebuggingServer() {
diff --git a/content/browser/devtools/devtools_http_handler_impl.cc b/content/browser/devtools/devtools_http_handler_impl.cc
index 5a5bb09..113c2e5 100644
--- a/content/browser/devtools/devtools_http_handler_impl.cc
+++ b/content/browser/devtools/devtools_http_handler_impl.cc
@@ -5,6 +5,7 @@
 #include "content/browser/devtools/devtools_http_handler_impl.h"
 
 #include <algorithm>
+#include <sstream>
 #include <utility>
 
 #include "base/bind.h"
@@ -37,6 +38,7 @@
 #include "net/base/escape.h"
 #include "net/base/io_buffer.h"
 #include "net/base/ip_endpoint.h"
+#include "net/base/net_errors.h"
 #include "net/server/http_server_request_info.h"
 #include "net/server/http_server_response_info.h"
 
@@ -48,6 +50,9 @@ namespace content {
 
 namespace {
 
+const base::FilePath::CharType kDevToolsActivePortFileName[] =
+    FILE_PATH_LITERAL("DevToolsActivePort");
+
 const char kDevToolsHandlerThreadName[] = "Chrome_DevToolsHandlerThread";
 
 const char kThumbUrlPrefix[] = "/thumb/";
@@ -150,11 +155,13 @@ int DevToolsHttpHandler::GetFrontendResourceId(const std::string& name) {
 DevToolsHttpHandler* DevToolsHttpHandler::Start(
     const net::StreamListenSocketFactory* socket_factory,
     const std::string& frontend_url,
-    DevToolsHttpHandlerDelegate* delegate) {
+    DevToolsHttpHandlerDelegate* delegate,
+    const base::FilePath& active_port_output_directory) {
   DevToolsHttpHandlerImpl* http_handler =
       new DevToolsHttpHandlerImpl(socket_factory,
                                   frontend_url,
-                                  delegate);
+                                  delegate,
+                                  active_port_output_directory);
   http_handler->Start();
   return http_handler;
 }
@@ -642,10 +649,12 @@ void DevToolsHttpHandlerImpl::OnCloseUI(int connection_id) {
 DevToolsHttpHandlerImpl::DevToolsHttpHandlerImpl(
     const net::StreamListenSocketFactory* socket_factory,
     const std::string& frontend_url,
-    DevToolsHttpHandlerDelegate* delegate)
+    DevToolsHttpHandlerDelegate* delegate,
+    const base::FilePath& active_port_output_directory)
     : frontend_url_(frontend_url),
       socket_factory_(socket_factory),
-      delegate_(delegate) {
+      delegate_(delegate),
+      active_port_output_directory_(active_port_output_directory) {
   if (frontend_url_.empty())
       frontend_url_ = "/devtools/devtools.html";
 
@@ -656,6 +665,8 @@ DevToolsHttpHandlerImpl::DevToolsHttpHandlerImpl(
 // Runs on the handler thread
 void DevToolsHttpHandlerImpl::Init() {
   server_ = new net::HttpServer(*socket_factory_.get(), this);
+  if (!active_port_output_directory_.empty())
+    WriteActivePortToUserProfile();
 }
 
 // Runs on the handler thread
@@ -675,6 +686,27 @@ void DevToolsHttpHandlerImpl::StopHandlerThread() {
   thread_->Stop();
 }
 
+void DevToolsHttpHandlerImpl::WriteActivePortToUserProfile() {
+  DCHECK(!active_port_output_directory_.empty());
+  net::IPEndPoint endpoint;
+  int err;
+  if ((err = server_->GetLocalAddress(&endpoint)) != net::OK) {
+    LOG(ERROR) << "Error " << err << " getting local address";
+    return;
+  }
+
+  // Write this port to a well-known file in the profile directory
+  // so Telemetry can pick it up.
+  base::FilePath path = active_port_output_directory_.Append(
+      kDevToolsActivePortFileName);
+  std::stringstream port_stream;
+  port_stream << endpoint.port();
+  std::string s = port_stream.str();
+  if (base::WriteFile(path, s.c_str(), s.length()) < 0) {
+    LOG(ERROR) << "Error writing DevTools active port to file";
+  }
+}
+
 void DevToolsHttpHandlerImpl::SendJson(int connection_id,
                                        net::HttpStatusCode status_code,
                                        base::Value* value,
diff --git a/content/browser/devtools/devtools_http_handler_impl.h b/content/browser/devtools/devtools_http_handler_impl.h
index 8e9fe15..d55237d 100644
--- a/content/browser/devtools/devtools_http_handler_impl.h
+++ b/content/browser/devtools/devtools_http_handler_impl.h
@@ -46,7 +46,8 @@ class DevToolsHttpHandlerImpl
   // Takes ownership over |socket_factory|.
   DevToolsHttpHandlerImpl(const net::StreamListenSocketFactory* socket_factory,
                           const std::string& frontend_url,
-                          DevToolsHttpHandlerDelegate* delegate);
+                          DevToolsHttpHandlerDelegate* delegate,
+                          const base::FilePath& active_port_output_directory);
   virtual ~DevToolsHttpHandlerImpl();
   void Start();
 
@@ -90,6 +91,8 @@ class DevToolsHttpHandlerImpl
   void StartHandlerThread();
   void StopHandlerThread();
 
+  void WriteActivePortToUserProfile();
+
   void SendJson(int connection_id,
                 net::HttpStatusCode status_code,
                 base::Value* value,
@@ -119,6 +122,7 @@ class DevToolsHttpHandlerImpl
   typedef std::map<int, DevToolsClientHost*> ConnectionToClientHostMap;
   ConnectionToClientHostMap connection_to_client_host_ui_;
   scoped_ptr<DevToolsHttpHandlerDelegate> delegate_;
+  base::FilePath active_port_output_directory_;
   typedef std::map<std::string, DevToolsTarget*> TargetMap;
   TargetMap target_map_;
   scoped_refptr<DevToolsBrowserTarget> browser_target_;
diff --git a/content/browser/devtools/devtools_http_handler_unittest.cc b/content/browser/devtools/devtools_http_handler_unittest.cc
index f3179f0..5cd189f 100644
--- a/content/browser/devtools/devtools_http_handler_unittest.cc
+++ b/content/browser/devtools/devtools_http_handler_unittest.cc
@@ -106,7 +106,8 @@ TEST_F(DevToolsHttpHandlerTest, TestStartStop) {
           new DummyListenSocketFactory(run_loop.QuitClosure(),
                                        run_loop_2.QuitClosure()),
           std::string(),
-          new DummyDelegate());
+          new DummyDelegate(),
+          base::FilePath());
   // Our dummy socket factory will post a quit message once the server will
   // become ready.
   run_loop.Run();
diff --git a/content/public/browser/devtools_http_handler.h b/content/public/browser/devtools_http_handler.h
index 4af5505..cdceff8 100644
--- a/content/public/browser/devtools_http_handler.h
+++ b/content/public/browser/devtools_http_handler.h
@@ -7,6 +7,7 @@
 
 #include <string>
 
+#include "base/files/file_path.h"
 #include "content/common/content_export.h"
 
 class GURL;
@@ -34,10 +35,15 @@ class DevToolsHttpHandler {
       const std::string& name);
 
   // Takes ownership over |socket_factory| and |delegate|.
+  // If |active_port_output_directory| is non-empty, it is assumed the
+  // socket_factory was initialized with an ephemeral port (0). The
+  // port selected by the OS will be written to a well-known file in
+  // the output directory.
   CONTENT_EXPORT static DevToolsHttpHandler* Start(
       const net::StreamListenSocketFactory* socket_factory,
       const std::string& frontend_url,
-      DevToolsHttpHandlerDelegate* delegate);
+      DevToolsHttpHandlerDelegate* delegate,
+      const base::FilePath& active_port_output_directory);
 
   // Called from the main thread in order to stop protocol handler.
   // Automatically destroys the handler instance.
diff --git a/content/shell/browser/shell_devtools_delegate.cc b/content/shell/browser/shell_devtools_delegate.cc
index 6a8f09e..d316b2a 100644
--- a/content/shell/browser/shell_devtools_delegate.cc
+++ b/content/shell/browser/shell_devtools_delegate.cc
@@ -8,6 +8,7 @@
 
 #include "base/bind.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 "base/strings/utf_string_conversions.h"
@@ -147,7 +148,8 @@ ShellDevToolsDelegate::ShellDevToolsDelegate(BrowserContext* browser_context)
   frontend_url = base::StringPrintf(kFrontEndURL, GetWebKitRevision().c_str());
 #endif
   devtools_http_handler_ =
-      DevToolsHttpHandler::Start(CreateSocketFactory(), frontend_url, this);
+      DevToolsHttpHandler::Start(CreateSocketFactory(), frontend_url, this,
+                                 base::FilePath());
 }
 
 ShellDevToolsDelegate::~ShellDevToolsDelegate() {
diff --git a/tools/telemetry/telemetry/core/backends/chrome/desktop_browser_backend.py b/tools/telemetry/telemetry/core/backends/chrome/desktop_browser_backend.py
index bfbc687..a9f1fa5 100644
--- a/tools/telemetry/telemetry/core/backends/chrome/desktop_browser_backend.py
+++ b/tools/telemetry/telemetry/core/backends/chrome/desktop_browser_backend.py
@@ -7,6 +7,7 @@ import glob
 import heapq
 import logging
 import os
+import os.path
 import shutil
 import subprocess as subprocess
 import sys
@@ -74,6 +75,20 @@ class DesktopBrowserBackend(chrome_browser_backend.ChromeBrowserBackend):
         logging.info("Using profile directory:'%s'." % profile_dir)
         shutil.rmtree(self._tmp_profile_dir)
         shutil.copytree(profile_dir, self._tmp_profile_dir)
+    if self.browser_options.use_devtools_active_port:
+      # No matter whether we're using an existing profile directory or
+      # creating a new one, always delete the well-known file containing
+      # the active DevTools port number.
+      port_file = self._GetDevToolsActivePortPath()
+      if os.path.isfile(port_file):
+        try:
+          os.remove(port_file)
+        except Exception as e:
+          logging.critical('Unable to remove DevToolsActivePort file: %s' % e)
+          sys.exit(1)
+
+  def _GetDevToolsActivePortPath(self):
+    return os.path.join(self.profile_directory, 'DevToolsActivePort')
 
   def _GetCrashServicePipeName(self):
     # Ensure a unique pipe name by using the name of the temp dir.
@@ -119,11 +134,27 @@ class DesktopBrowserBackend(chrome_browser_backend.ChromeBrowserBackend):
     if not self.IsBrowserRunning():
       raise exceptions.ProcessGoneException(
           "Return code: %d" % self._proc.returncode)
+    if self.browser_options.use_devtools_active_port:
+      # The Telemetry user selected the new code path to start DevTools on
+      # an ephemeral port. Wait for the well-known file containing the port
+      # number to exist.
+      port_file = self._GetDevToolsActivePortPath()
+      if not os.path.isfile(port_file):
+        # File isn't ready yet. Return false. Will retry.
+        return False
+      with open(port_file) as f:
+        port_string = f.read()
+        self._port = int(port_string)
+        logging.info('Discovered ephemeral port %s' % self._port)
     return super(DesktopBrowserBackend, self).HasBrowserFinishedLaunching()
 
   def GetBrowserStartupArgs(self):
     args = super(DesktopBrowserBackend, self).GetBrowserStartupArgs()
-    self._port = util.GetUnreservedAvailableLocalPort()
+    if self.browser_options.use_devtools_active_port:
+      self._port = 0
+    else:
+      self._port = util.GetUnreservedAvailableLocalPort()
+    logging.info('Requested remote debugging port: %d' % self._port)
     args.append('--remote-debugging-port=%i' % self._port)
     args.append('--enable-crash-reporter-for-testing')
     args.append('--use-mock-keychain')
diff --git a/tools/telemetry/telemetry/core/browser_options.py b/tools/telemetry/telemetry/core/browser_options.py
index aefaa7f..9d3d20c 100644
--- a/tools/telemetry/telemetry/core/browser_options.py
+++ b/tools/telemetry/telemetry/core/browser_options.py
@@ -204,6 +204,11 @@ class BrowserOptions(object):
 
     self.platform = None
 
+    # Whether to use the new code path for choosing an ephemeral port for
+    # DevTools. The bots set this to true. When Chrome 37 reaches stable,
+    # remove this setting and the old code path. http://crbug.com/379980
+    self.use_devtools_active_port = False
+
   def __repr__(self):
     return str(sorted(self.__dict__.items()))
 
@@ -244,6 +249,11 @@ class BrowserOptions(object):
     group.add_option('--show-stdout',
         action='store_true',
         help='When possible, will display the stdout of the process')
+    # This hidden option is to be removed, and the older code path deleted,
+    # once Chrome 37 reaches Stable. http://crbug.com/379980
+    group.add_option('--use-devtools-active-port',
+        action='store_true',
+        help=optparse.SUPPRESS_HELP)
     parser.add_option_group(group)
 
     group = optparse.OptionGroup(parser, 'Compatibility options')
@@ -274,6 +284,7 @@ class BrowserOptions(object):
         'profile_type',
         'show_stdout',
         'synthetic_gesture_source_type',
+        'use_devtools_active_port',
         ]
     for o in browser_options_list:
       a = getattr(finder_options, o, None)
diff --git a/tools/telemetry/telemetry/core/browser_options_unittest.py b/tools/telemetry/telemetry/core/browser_options_unittest.py
index 8e5cb11..659ed0c 100644
--- a/tools/telemetry/telemetry/core/browser_options_unittest.py
+++ b/tools/telemetry/telemetry/core/browser_options_unittest.py
@@ -83,6 +83,13 @@ class BrowserOptionsTest(unittest.TestCase):
     self.assertEquals(options.browser_options.extra_browser_args,
                       set(['--foo','--bar']))
 
+  def testUseDevToolsActivePort(self):
+    options = browser_options.BrowserFinderOptions()
+    parser = options.CreateParser()
+    parser.parse_args(['--use-devtools-active-port'])
+
+    self.assertEquals(options.browser_options.use_devtools_active_port, True)
+
   def testMergeDefaultValues(self):
     options = browser_options.BrowserFinderOptions()
     options.already_true = True