Remove the --allow-no-sandbox-job flag in favor of automatic recognition.

content/common/sandbox_win.cc

Index: content/common/sandbox_win.cc
diff --git a/content/common/sandbox_win.cc b/content/common/sandbox_win.cc
index 633012ff60d455883dfef18d8b80aa9474a3d51b..fe9fbd9503ba5fc6e7e85d34bd88df8e013a7970 100644
--- a/content/common/sandbox_win.cc
+++ b/content/common/sandbox_win.cc
@@ -250,10 +250,7 @@ base::string16 PrependWindowsSessionPath(const base::char16* object) {
 }
 
 // Checks if the sandbox should be let to run without a job object assigned.
-bool ShouldSetJobLevel(const base::CommandLine& cmd_line) {
-  if (!cmd_line.HasSwitch(switches::kAllowNoSandboxJob))
-    return true;
-
+bool ShouldSetJobLevel() {
   // Windows 8 allows nested jobs so we don't need to check if we are in other
   // job.
   if (base::win::GetVersion() >= base::win::VERSION_WIN8)
@@ -277,6 +274,17 @@ bool ShouldSetJobLevel(const base::CommandLine& cmd_line) {
   if (job_info.BasicLimitInformation.LimitFlags & JOB_OBJECT_LIMIT_BREAKAWAY_OK)
     return true;
 
+  // Lastly in place of the flag which was supposed to be used only for running
+  // Chrome in remote sessions we do this check explicitly here.
+  // According to MS this flag can be false for a remote session only on Windows

[Will Harris, 2017/02/03 17:38:50]

can you link the source?

[pastarmovj, 2017/02/06 13:05:27]

Done.

+  // Server 2012 and newer so if we do the check last we should be on the safe

[Will Harris, 2017/02/03 17:38:50]

we can't just check for this version of OS instead?

How come we can't just detect the Job, as above? Why is that code not correctly
detecting the failure?

[pastarmovj, 2017/02/06 13:05:27]

Not sure what do you mean? What I am trying to say is that if we were to rely on
that on a newer Windows version we might end up assuming we should not allow the
Job escape hatch while we are actually running in a remote session. But at least
according to the documentation this function is always correct for Windows
Server 2008 . The check above on line 256 do check the OS version.

+  // side.
+  if (!::GetSystemMetrics(SM_REMOTESESSION))

[Will Harris, 2017/02/03 17:38:50]

how is this tested? is there a set of automated tests, or a test plan?

[pastarmovj, 2017/02/06 13:05:27]

This is something I will start another thread about this week :)

Right now I tested it by building a fresh branded build and running it on my
Terminal Services installation on SkyTap.

I would love to have some automated testing and have started doing some tests
how to build that but I will love some more input on this however it is beyond
the scope of this CL I think.

+    return true;
+
+  // Allow running without the sandbox in this case. This slightly reduces the
+  // ability of the sandbox to protect its children from spawning new processes
+  // or preventing them from shutting down Windows or accessing the clipboard.
   return false;
 }
 
@@ -566,11 +574,10 @@ bool IsAppContainerEnabled() {
 
 }  // namespace
 
-sandbox::ResultCode SetJobLevel(const base::CommandLine& cmd_line,
-                                sandbox::JobLevel job_level,
+sandbox::ResultCode SetJobLevel(sandbox::JobLevel job_level,
                                 uint32_t ui_exceptions,
                                 sandbox::TargetPolicy* policy) {
-  if (!ShouldSetJobLevel(cmd_line))
+  if (!ShouldSetJobLevel())
     return policy->SetJobLevel(sandbox::JOB_NONE, 0);
 
 #ifdef _WIN64
@@ -684,12 +691,6 @@ sandbox::ResultCode StartSandboxedProcess(
 
   TRACE_EVENT1("startup", "StartProcessWithAccess", "type", type_str);
 
-  // Propagate the --allow-no-job flag if present.
-  if (browser_command_line.HasSwitch(switches::kAllowNoSandboxJob) &&
-      !cmd_line->HasSwitch(switches::kAllowNoSandboxJob)) {
-    cmd_line->AppendSwitch(switches::kAllowNoSandboxJob);
-  }
-
   ProcessDebugFlags(cmd_line);
 
   if ((!delegate->ShouldSandbox()) ||
@@ -750,7 +751,7 @@ sandbox::ResultCode StartSandboxedProcess(
   if (result != sandbox::SBOX_ALL_OK)
     return result;
 
-  result = SetJobLevel(*cmd_line, sandbox::JOB_LOCKDOWN, 0, policy);
+  result = SetJobLevel(sandbox::JOB_LOCKDOWN, 0, policy);
   if (result != sandbox::SBOX_ALL_OK)
     return result;