Use enum class for JobLevel

JobLevel can be an enum class (this means we cannot compile code that
supplies wrong values). As the class must be named, the prefix JOB_
becomes redundant, so entries are renamed to kNone etc.

No functional changes. Bad args test no longer necessary as cannot
compile.

Bug: 1108031
Change-Id: I0abef9a7972b3fce658e37c931fd6b95d3129958
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3533430
Reviewed-by: Will Harris <wfh@chromium.org>
Reviewed-by: Xinghui Lu <xinghuilu@chromium.org>
Reviewed-by: Zhenyao Mo <zmo@chromium.org>
Commit-Queue: Alex Gough <ajgo@chromium.org>
Cr-Commit-Position: refs/heads/main@{#983648}

chrome/chrome_cleaner/ipc/sandbox.cc

@@ -62,7 +62,7 @@ scoped_refptr<sandbox::TargetPolicy> GetSandboxPolicy(
       sandbox::USER_RESTRICTED_SAME_ACCESS, sandbox::USER_LOCKDOWN);
   CHECK_EQ(sandbox::SBOX_ALL_OK, sandbox_result);
 
-  sandbox_result = policy->SetJobLevel(sandbox::JOB_LOCKDOWN, 0);
+  sandbox_result = policy->SetJobLevel(sandbox::JobLevel::kLockdown, 0);
   CHECK_EQ(sandbox::SBOX_ALL_OK, sandbox_result);
 
 #ifdef NDEBUG

content/browser/gpu/gpu_process_host.cc

@@ -404,7 +404,7 @@ class GpuSandboxedProcessLauncherDelegate
       policy->SetTokenLevel(sandbox::USER_RESTRICTED_SAME_ACCESS,
                             sandbox::USER_LIMITED);
       sandbox::policy::SandboxWin::SetJobLevel(
-          cmd_line_, sandbox::JOB_UNPROTECTED, 0, policy);
+          cmd_line_, sandbox::JobLevel::kUnprotected, 0, policy);
     } else {
       policy->SetTokenLevel(sandbox::USER_RESTRICTED_SAME_ACCESS,
                             sandbox::USER_LIMITED);
@@ -416,7 +416,7 @@ class GpuSandboxedProcessLauncherDelegate
       // message pump entirely and just add job restrictions to prevent child
       // processes.
       sandbox::policy::SandboxWin::SetJobLevel(
-          cmd_line_, sandbox::JOB_LIMITED_USER,
+          cmd_line_, sandbox::JobLevel::kLimitedUser,
           JOB_OBJECT_UILIMIT_SYSTEMPARAMETERS | JOB_OBJECT_UILIMIT_DESKTOP |
               JOB_OBJECT_UILIMIT_EXITWINDOWS |
               JOB_OBJECT_UILIMIT_DISPLAYSETTINGS,

content/browser/utility_sandbox_delegate_win.cc

@@ -26,7 +26,7 @@ namespace {
 //  lockdown_level_(sandbox::USER_LOCKDOWN),
 //  initial_level_(sandbox::USER_RESTRICTED_SAME_ACCESS),
 //
-//  job_level_(sandbox::JOB_LOCKDOWN),
+//  job_level_(sandbox::JobLevel::kLockdown),
 //
 //  integrity_level_(sandbox::INTEGRITY_LEVEL_LOW),
 //  delayed_integrity_level_(sandbox::INTEGRITY_LEVEL_UNTRUSTED),
@@ -221,7 +221,7 @@ bool UtilitySandboxedProcessLauncherDelegate::PreSpawnTarget(
     // Unprotected token/job.
     policy->SetTokenLevel(sandbox::USER_UNPROTECTED, sandbox::USER_UNPROTECTED);
     sandbox::policy::SandboxWin::SetJobLevel(
-        cmd_line_, sandbox::JOB_UNPROTECTED, 0, policy);
+        cmd_line_, sandbox::JobLevel::kUnprotected, 0, policy);
   }
 
   if (sandbox_type_ == sandbox::mojom::Sandbox::kMediaFoundationCdm ||

sandbox/policy/win/sandbox_win.cc

@@ -557,7 +557,7 @@ bool IsAppContainerEnabled() {
 
 ResultCode SetJobMemoryLimit(const base::CommandLine& cmd_line,
                              TargetPolicy* policy) {
-  DCHECK_NE(policy->GetJobLevel(), JOB_NONE);
+  DCHECK_NE(policy->GetJobLevel(), JobLevel::kNone);
 
 #ifdef _WIN64
   size_t memory_limit = static_cast<size_t>(kDataSizeLimit);
@@ -784,7 +784,7 @@ ResultCode LaunchWithoutSandbox(
       static Job* job_object = nullptr;
       if (!job_object) {
         job_object = new Job;
-        DWORD result = job_object->Init(JOB_UNPROTECTED, nullptr, 0, 0);
+        DWORD result = job_object->Init(JobLevel::kUnprotected, nullptr, 0, 0);
         if (result != ERROR_SUCCESS) {
           job_object = nullptr;
           return SBOX_ERROR_CANNOT_INIT_JOB;
@@ -832,7 +832,7 @@ ResultCode SandboxWin::SetJobLevel(const base::CommandLine& cmd_line,
                                    uint32_t ui_exceptions,
                                    TargetPolicy* policy) {
   if (!ShouldSetJobLevel(policy->GetAllowNoSandboxJob()))
-    return policy->SetJobLevel(JOB_NONE, 0);
+    return policy->SetJobLevel(JobLevel::kNone, 0);
 
   ResultCode ret = policy->SetJobLevel(job_level, ui_exceptions);
   if (ret != SBOX_ALL_OK)
@@ -1070,7 +1070,7 @@ ResultCode SandboxWin::GeneratePolicyForSandboxedProcess(
   if (result != SBOX_ALL_OK)
     return result;
 
-  result = SetJobLevel(cmd_line, JOB_LOCKDOWN, 0, policy.get());
+  result = SetJobLevel(cmd_line, JobLevel::kLockdown, 0, policy.get());
   if (result != SBOX_ALL_OK)
     return result;
 
@@ -1130,7 +1130,7 @@ ResultCode SandboxWin::GeneratePolicyForSandboxedProcess(
     // Set a policy that would normally allow for process creation. This allows
     // the mf cdm process to launch the protected media pipeline process
     // (mfpmp.exe) without process interception.
-    result = policy->SetJobLevel(JOB_INTERACTIVE, 0);
+    result = policy->SetJobLevel(JobLevel::kInteractive, 0);
     if (result != SBOX_ALL_OK)
       return result;
   }

sandbox/policy/win/sandbox_win_unittest.cc

@@ -385,7 +385,7 @@ TEST_F(SandboxWinTest, GeneratedPolicyTest) {
   // detail, but just that GeneratePolicyForSandboxedProcess generated some kind
   // of valid policy.
   EXPECT_EQ(IntegrityLevel::INTEGRITY_LEVEL_LOW, policy->GetIntegrityLevel());
-  EXPECT_EQ(JobLevel::JOB_LOCKDOWN, policy->GetJobLevel());
+  EXPECT_EQ(JobLevel::kLockdown, policy->GetJobLevel());
   EXPECT_EQ(TokenLevel::USER_LOCKDOWN, policy->GetLockdownTokenLevel());
 }
 

sandbox/win/src/app_container_test.cc

@@ -478,7 +478,7 @@ TEST_F(AppContainerTest, DenyOpenEventForLowBox) {
   if (!features::IsAppContainerSandboxSupported())
     return;
 
-  TestRunner runner(JOB_UNPROTECTED, USER_UNPROTECTED, USER_UNPROTECTED);
+  TestRunner runner(JobLevel::kUnprotected, USER_UNPROTECTED, USER_UNPROTECTED);
 
   EXPECT_EQ(SBOX_ALL_OK, runner.GetPolicy()->SetLowBox(kAppContainerSid));
   // Run test once, this ensures the app container directory exists, we
@@ -492,7 +492,8 @@ TEST_F(AppContainerTest, DenyOpenEventForLowBox) {
       ::CreateEvent(nullptr, false, false, event_name.c_str()));
   ASSERT_TRUE(event.IsValid());
 
-  TestRunner runner2(JOB_UNPROTECTED, USER_UNPROTECTED, USER_UNPROTECTED);
+  TestRunner runner2(JobLevel::kUnprotected, USER_UNPROTECTED,
+                     USER_UNPROTECTED);
   EXPECT_EQ(SBOX_TEST_DENIED, runner2.RunTest(L"AppContainerEvent_Open test"));
 }
 
@@ -524,7 +525,7 @@ TEST_F(AppContainerTest, NoCapabilities) {
     return;
 
   policy_->SetTokenLevel(USER_UNPROTECTED, USER_UNPROTECTED);
-  policy_->SetJobLevel(JOB_NONE, 0);
+  policy_->SetJobLevel(JobLevel::kNone, 0);
 
   CreateProcess();
   auto security_capabilities = container_->GetSecurityCapabilities();
@@ -540,7 +541,7 @@ TEST_F(AppContainerTest, NoCapabilitiesRestricted) {
     return;
 
   policy_->SetTokenLevel(USER_LOCKDOWN, USER_RESTRICTED_SAME_ACCESS);
-  policy_->SetJobLevel(JOB_NONE, 0);
+  policy_->SetJobLevel(JobLevel::kNone, 0);
 
   CreateProcess();
   auto security_capabilities = container_->GetSecurityCapabilities();
@@ -559,7 +560,7 @@ TEST_F(AppContainerTest, WithCapabilities) {
   container_->AddCapability(
       base::win::WellKnownCapability::kInternetClientServer);
   policy_->SetTokenLevel(USER_UNPROTECTED, USER_UNPROTECTED);
-  policy_->SetJobLevel(JOB_NONE, 0);
+  policy_->SetJobLevel(JobLevel::kNone, 0);
 
   CreateProcess();
   auto security_capabilities = container_->GetSecurityCapabilities();
@@ -578,7 +579,7 @@ TEST_F(AppContainerTest, WithCapabilitiesRestricted) {
   container_->AddCapability(
       base::win::WellKnownCapability::kInternetClientServer);
   policy_->SetTokenLevel(USER_LOCKDOWN, USER_RESTRICTED_SAME_ACCESS);
-  policy_->SetJobLevel(JOB_NONE, 0);
+  policy_->SetJobLevel(JobLevel::kNone, 0);
 
   CreateProcess();
   auto security_capabilities = container_->GetSecurityCapabilities();
@@ -601,7 +602,7 @@ TEST_F(AppContainerTest, WithImpersonationCapabilities) {
   container_->AddImpersonationCapability(
       base::win::WellKnownCapability::kPicturesLibrary);
   policy_->SetTokenLevel(USER_UNPROTECTED, USER_UNPROTECTED);
-  policy_->SetJobLevel(JOB_NONE, 0);
+  policy_->SetJobLevel(JobLevel::kNone, 0);
 
   CreateProcess();
   auto security_capabilities = container_->GetSecurityCapabilities();
@@ -620,7 +621,7 @@ TEST_F(AppContainerTest, NoCapabilitiesLPAC) {
 
   container_->SetEnableLowPrivilegeAppContainer(true);
   policy_->SetTokenLevel(USER_UNPROTECTED, USER_UNPROTECTED);
-  policy_->SetJobLevel(JOB_NONE, 0);
+  policy_->SetJobLevel(JobLevel::kNone, 0);
 
   CreateProcess();
   auto security_capabilities = container_->GetSecurityCapabilities();

sandbox/win/src/broker_services.cc

@@ -502,7 +502,7 @@ ResultCode BrokerServicesBase::SpawnTarget(const wchar_t* exe_path,
   startup_info->SetMitigations(policy_base->GetProcessMitigations());
 
   if (base::win::GetVersion() >= base::win::Version::WIN10_TH2 &&
-      policy_base->GetJobLevel() <= JOB_LIMITED_USER) {
+      policy_base->GetJobLevel() <= JobLevel::kLimitedUser) {
     startup_info->SetRestrictChildProcessCreation(true);
   }
 
@@ -549,7 +549,8 @@ ResultCode BrokerServicesBase::SpawnTarget(const wchar_t* exe_path,
     return result;
   }
 
-  if (policy_base->HasJob() && policy_base->GetJobLevel() <= JOB_LIMITED_USER) {
+  if (policy_base->HasJob() &&
+      policy_base->GetJobLevel() <= JobLevel::kLimitedUser) {
     // Restrict the job from containing any processes. Job restrictions
     // are only applied at process creation, so the target process is
     // unaffected.

sandbox/win/src/integrity_level_test.cc

@@ -43,8 +43,8 @@ SBOX_TESTS_COMMAND int CheckIntegrityLevel(int argc, wchar_t** argv) {
 }
 
 std::unique_ptr<TestRunner> LowILRealRunner() {
-  auto runner = std::make_unique<TestRunner>(JOB_LOCKDOWN, USER_INTERACTIVE,
-                                             USER_INTERACTIVE);
+  auto runner = std::make_unique<TestRunner>(
+      JobLevel::kLockdown, USER_INTERACTIVE, USER_INTERACTIVE);
   runner->SetTimeout(INFINITE);
   runner->GetPolicy()->SetAlternateDesktop(true);
   runner->GetPolicy()->SetIntegrityLevel(INTEGRITY_LEVEL_LOW);
@@ -61,8 +61,8 @@ TEST(IntegrityLevelTest, TestLowILReal) {
 }
 
 std::unique_ptr<TestRunner> LowILDelayedRunner() {
-  auto runner = std::make_unique<TestRunner>(JOB_LOCKDOWN, USER_INTERACTIVE,
-                                             USER_INTERACTIVE);
+  auto runner = std::make_unique<TestRunner>(
+      JobLevel::kLockdown, USER_INTERACTIVE, USER_INTERACTIVE);
   runner->SetTimeout(INFINITE);
   runner->GetPolicy()->SetDelayedIntegrityLevel(INTEGRITY_LEVEL_LOW);
   return runner;
@@ -78,15 +78,16 @@ TEST(DelayedIntegrityLevelTest, TestLowILDelayed) {
 }
 
 TEST(IntegrityLevelTest, TestNoILChange) {
-  TestRunner runner(JOB_LOCKDOWN, USER_INTERACTIVE, USER_INTERACTIVE);
+  TestRunner runner(JobLevel::kLockdown, USER_INTERACTIVE, USER_INTERACTIVE);
 
   runner.SetTimeout(INFINITE);
 
   EXPECT_EQ(SBOX_TEST_DENIED, runner.RunTest(L"CheckIntegrityLevel"));
 }
 
 TEST(IntegrityLevelTest, TestUntrustedIL) {
-  TestRunner runner(JOB_LOCKDOWN, USER_RESTRICTED_SAME_ACCESS, USER_LOCKDOWN);
+  TestRunner runner(JobLevel::kLockdown, USER_RESTRICTED_SAME_ACCESS,
+                    USER_LOCKDOWN);
   runner.GetPolicy()->SetIntegrityLevel(INTEGRITY_LEVEL_LOW);
   runner.GetPolicy()->SetDelayedIntegrityLevel(INTEGRITY_LEVEL_UNTRUSTED);
   runner.GetPolicy()->SetLockdownDefaultDacl();
@@ -98,7 +99,8 @@ TEST(IntegrityLevelTest, TestUntrustedIL) {
 }
 
 TEST(IntegrityLevelTest, TestLowIL) {
-  TestRunner runner(JOB_LOCKDOWN, USER_RESTRICTED_SAME_ACCESS, USER_LOCKDOWN);
+  TestRunner runner(JobLevel::kLockdown, USER_RESTRICTED_SAME_ACCESS,
+                    USER_LOCKDOWN);
   runner.GetPolicy()->SetIntegrityLevel(INTEGRITY_LEVEL_LOW);
   runner.GetPolicy()->SetDelayedIntegrityLevel(INTEGRITY_LEVEL_LOW);
   runner.GetPolicy()->SetLockdownDefaultDacl();

sandbox/win/src/job.cc

@@ -36,7 +36,7 @@ DWORD Job::Init(JobLevel security_level,
   // Set the settings for the different security levels. Note: The higher levels
   // inherit from the lower levels.
   switch (security_level) {
-    case JOB_LOCKDOWN: {
+    case JobLevel::kLockdown: {
       jeli.BasicLimitInformation.LimitFlags |=
           JOB_OBJECT_LIMIT_DIE_ON_UNHANDLED_EXCEPTION;
       jbur.UIRestrictionsClass |= JOB_OBJECT_UILIMIT_WRITECLIPBOARD;
@@ -45,19 +45,19 @@ DWORD Job::Init(JobLevel security_level,
       jbur.UIRestrictionsClass |= JOB_OBJECT_UILIMIT_GLOBALATOMS;
       [[fallthrough]];
     }
-    case JOB_LIMITED_USER: {
+    case JobLevel::kLimitedUser: {
       jbur.UIRestrictionsClass |= JOB_OBJECT_UILIMIT_DISPLAYSETTINGS;
       jeli.BasicLimitInformation.LimitFlags |= JOB_OBJECT_LIMIT_ACTIVE_PROCESS;
       jeli.BasicLimitInformation.ActiveProcessLimit = 1;
       [[fallthrough]];
     }
-    case JOB_INTERACTIVE: {
+    case JobLevel::kInteractive: {
       jbur.UIRestrictionsClass |= JOB_OBJECT_UILIMIT_SYSTEMPARAMETERS;
       jbur.UIRestrictionsClass |= JOB_OBJECT_UILIMIT_DESKTOP;
       jbur.UIRestrictionsClass |= JOB_OBJECT_UILIMIT_EXITWINDOWS;
       [[fallthrough]];
     }
-    case JOB_UNPROTECTED: {
+    case JobLevel::kUnprotected: {
       if (memory_limit) {
         jeli.BasicLimitInformation.LimitFlags |=
             JOB_OBJECT_LIMIT_PROCESS_MEMORY;
@@ -68,7 +68,9 @@ DWORD Job::Init(JobLevel security_level,
           JOB_OBJECT_LIMIT_KILL_ON_JOB_CLOSE;
       break;
     }
-    default: { return ERROR_BAD_ARGUMENTS; }
+    case JobLevel::kNone: {
+      return ERROR_BAD_ARGUMENTS;
+    }
   }
 
   if (!::SetInformationJobObject(job_handle_.Get(),

sandbox/win/src/job.h

@@ -8,14 +8,14 @@
 #include <stddef.h>
 
 #include "base/win/scoped_handle.h"
-#include "sandbox/win/src/restricted_token_utils.h"
 
 namespace sandbox {
+enum class JobLevel;
 
 // Handles the creation of job objects based on a security profile.
 // Sample usage:
 //   Job job;
-//   job.Init(JOB_LOCKDOWN, nullptr);  //no job name
+//   job.Init(JobLevel::kLockdown, nullptr);  //no job name
 //   job.AssignProcessToJob(process_handle);
 class Job {
  public:

sandbox/win/src/job_unittest.cc

@@ -7,6 +7,7 @@
 #include "sandbox/win/src/job.h"
 
 #include "base/win/scoped_process_information.h"
+#include "sandbox/win/src/security_level.h"
 #include "testing/gtest/include/gtest/gtest.h"
 
 namespace sandbox {
@@ -18,7 +19,7 @@ TEST(JobTest, TestCreation) {
     // Create the job.
     Job job;
     ASSERT_EQ(static_cast<DWORD>(ERROR_SUCCESS),
-              job.Init(JOB_LOCKDOWN, L"my_test_job_name", 0, 0));
+              job.Init(JobLevel::kLockdown, L"my_test_job_name", 0, 0));
 
     // check if the job exists.
     HANDLE job_handle =
@@ -43,7 +44,7 @@ TEST(JobTest, TestExceptions) {
     // Create the job.
     Job job;
     ASSERT_EQ(static_cast<DWORD>(ERROR_SUCCESS),
-              job.Init(JOB_LOCKDOWN, L"my_test_job_name",
+              job.Init(JobLevel::kLockdown, L"my_test_job_name",
                        JOB_OBJECT_UILIMIT_READCLIPBOARD, 0));
 
     job_handle = job.GetHandle();
@@ -62,7 +63,7 @@ TEST(JobTest, TestExceptions) {
     // Create the job.
     Job job;
     ASSERT_EQ(static_cast<DWORD>(ERROR_SUCCESS),
-              job.Init(JOB_LOCKDOWN, L"my_test_job_name", 0, 0));
+              job.Init(JobLevel::kLockdown, L"my_test_job_name", 0, 0));
 
     job_handle = job.GetHandle();
     ASSERT_TRUE(job_handle != INVALID_HANDLE_VALUE);
@@ -82,9 +83,9 @@ TEST(JobTest, DoubleInit) {
   // Create the job.
   Job job;
   ASSERT_EQ(static_cast<DWORD>(ERROR_SUCCESS),
-            job.Init(JOB_LOCKDOWN, L"my_test_job_name", 0, 0));
+            job.Init(JobLevel::kLockdown, L"my_test_job_name", 0, 0));
   ASSERT_EQ(static_cast<DWORD>(ERROR_ALREADY_INITIALIZED),
-            job.Init(JOB_LOCKDOWN, L"test", 0, 0));
+            job.Init(JobLevel::kLockdown, L"test", 0, 0));
 }
 
 // Tests the error case when we use a method and the object is not yet
@@ -101,37 +102,35 @@ TEST(JobTest, NoInit) {
 TEST(JobTest, SecurityLevel) {
   Job job_lockdown;
   ASSERT_EQ(static_cast<DWORD>(ERROR_SUCCESS),
-            job_lockdown.Init(JOB_LOCKDOWN, L"job_lockdown", 0, 0));
+            job_lockdown.Init(JobLevel::kLockdown, L"job_lockdown", 0, 0));
 
   Job job_limited_user;
-  ASSERT_EQ(static_cast<DWORD>(ERROR_SUCCESS),
-            job_limited_user.Init(JOB_LIMITED_USER, L"job_limited_user", 0, 0));
+  ASSERT_EQ(
+      static_cast<DWORD>(ERROR_SUCCESS),
+      job_limited_user.Init(JobLevel::kLimitedUser, L"job_limited_user", 0, 0));
 
   Job job_interactive;
-  ASSERT_EQ(static_cast<DWORD>(ERROR_SUCCESS),
-            job_interactive.Init(JOB_INTERACTIVE, L"job_interactive", 0, 0));
+  ASSERT_EQ(
+      static_cast<DWORD>(ERROR_SUCCESS),
+      job_interactive.Init(JobLevel::kInteractive, L"job_interactive", 0, 0));
 
   Job job_unprotected;
-  ASSERT_EQ(static_cast<DWORD>(ERROR_SUCCESS),
-            job_unprotected.Init(JOB_UNPROTECTED, L"job_unprotected", 0, 0));
+  ASSERT_EQ(
+      static_cast<DWORD>(ERROR_SUCCESS),
+      job_unprotected.Init(JobLevel::kUnprotected, L"job_unprotected", 0, 0));
 
-  // JOB_NONE means we run without a job object so Init should fail.
+  // JobLevel::kNone means we run without a job object so Init should fail.
   Job job_none;
   ASSERT_EQ(static_cast<DWORD>(ERROR_BAD_ARGUMENTS),
-            job_none.Init(JOB_NONE, L"job_none", 0, 0));
-
-  Job job_bad_args;
-  ASSERT_EQ(static_cast<DWORD>(ERROR_BAD_ARGUMENTS),
-            job_bad_args.Init(static_cast<JobLevel>(JOB_NONE + 1),
-                              L"job_bad_args", 0, 0));
+            job_none.Init(JobLevel::kNone, L"job_none", 0, 0));
 }
 
 // Tests the method "AssignProcessToJob".
 TEST(JobTest, ProcessInJob) {
   // Create the job.
   Job job;
   ASSERT_EQ(static_cast<DWORD>(ERROR_SUCCESS),
-            job.Init(JOB_UNPROTECTED, L"job_test_process", 0, 0));
+            job.Init(JobLevel::kUnprotected, L"job_test_process", 0, 0));
 
   wchar_t notepad[] = L"notepad";
   STARTUPINFO si = {sizeof(si)};