remote-debugging-port for guest sessions.

chrome/browser/chromeos/login/chrome_restart_request.cc

 // Copyright 2013 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 "chrome/browser/chromeos/login/chrome_restart_request.h"
 
 #include <vector>
 
 #include "ash/common/ash_switches.h"
 #include "base/base_switches.h"
@@ skipping 120 matching lines @@
     ::switches::kGpuStartupDialog,
     ::switches::kGpuSandboxAllowSysVShm,
     ::switches::kGpuSandboxFailuresFatal,
     ::switches::kGpuSandboxStartEarly,
     ::switches::kNoSandbox,
     ::switches::kNumRasterThreads,
     ::switches::kPpapiFlashArgs,
     ::switches::kPpapiFlashPath,
     ::switches::kPpapiFlashVersion,
     ::switches::kPpapiInProcess,
+    ::switches::kRemoteDebuggingPort,

[emaxx, 2016/07/13 18:25:40]

I'm wondering whether a race between the current Chrome process and the newly
started Chrome process would become possible.
Because probably the dev tools would just fail to start listening on a port if
it's already in use.

And, if this really may happen, I'm not sure how to fix this properly. I don't
see in the RestartChrome function any currently existing place for putting the
necessary cleanup _before_ launching the new process.

[achuithb, 2016/07/14 09:21:14]

It shouldn't be in use though right? I'm assuming chrome shutdown properly
closes the socket. Also, this seems to be no different than restarting the
browser using the same port, which is something I do frequently without seeing
any problems.
In practice, this seems to work ok in my testing. 

--remote-debugging-port is only used for testing, and this change is chromeos
specific, so even if there are some cases where this doesn't work perfectly,
it's still an improvement for debugging guest sessions.

The teardown sequence closes the remote debugging server here:
https://cs.chromium.org/chromium/src/chrome/browser/browser_process_impl.cc?l...

I'm not able to find the exact location where the socket is closed but I could
look into it if you have a strong concern that it is not cleanly closed.

[emaxx, 2016/07/14 18:27:02]

OK, I see: as this code (the GetOffTheRecordCommandLine method) is actually
relevant only for Chrome OS, we have only two cases to consider: the real Chrome
OS and the development "chromeos=1" build.
In the former case, seems that the session manager actually waits until the old
browser process is terminated until opening the new one (so there is no race
condition).
As for the latter case, I'm still thinking there's some probability of having
issues.
But I don't want to block the CL just because of some potential (and not really
harmful) problem in the dev environment.

     ::switches::kRendererStartupDialog,
     ::switches::kRootLayerScrolls,
     ::switches::kEnableShareGroupAsyncTextureUpload,
 #if defined(USE_X11) || defined(USE_OZONE)
     ::switches::kTouchCalibration,
 #endif
     ::switches::kTouchDevices,
     ::switches::kTouchEvents,
     ::switches::kTopChromeMD,
     ::switches::kTraceToConsole,
@@ skipping 207 matching lines @@
     // Relaunch chrome without session manager on dev box.
     ReLaunch(command_line);
     return;
   }
 
   // ChromeRestartRequest deletes itself after request sent to session manager.
   (new ChromeRestartRequest(command_line.argv()))->Start();
 }
 
 }  // namespace chromeos