diff options
author | hashimoto@chromium.org <hashimoto@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98> | 2013-11-27 08:23:31 +0000 |
---|---|---|
committer | hashimoto@chromium.org <hashimoto@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98> | 2013-11-27 08:23:31 +0000 |
commit | ecb10fa1d0b06ba322e0782a1becb67730f69707 (patch) | |
tree | e1b944d00a329f9ef0837870a80d09bd01cfa310 | |
parent | 4e5a81b5606ea6bd7be2f631e52e93d0d3b23902 (diff) | |
download | chromium_src-ecb10fa1d0b06ba322e0782a1becb67730f69707.zip chromium_src-ecb10fa1d0b06ba322e0782a1becb67730f69707.tar.gz chromium_src-ecb10fa1d0b06ba322e0782a1becb67730f69707.tar.bz2 |
Revert of https://codereview.chromium.org/71013004/
Reason for revert: Causing compile failure in chrome_util.cc on "Google Chrome Win" http://build.chromium.org/p/chromium.chrome/builders/Google%20Chrome%20Win/builds/21803
TBR=cpu@chromium.org,jvoung@chromium.org,thakis@chromium.org,sergeyu@chromium.org,grt@chromium.org,gene@chromium.org,youngki@chromium.org,rvargas@chromium.org
NOTREECHECKS=true
NOTRY=true
Review URL: https://codereview.chromium.org/90963002
git-svn-id: svn://svn.chromium.org/chrome/trunk/src@237541 0039d316-1c4b-4281-b951-d872f2087c98
43 files changed, 237 insertions, 222 deletions
diff --git a/base/base.gyp b/base/base.gyp index 8a73064..71b2615 100644 --- a/base/base.gyp +++ b/base/base.gyp @@ -680,6 +680,7 @@ 'win/registry_unittest.cc', 'win/scoped_bstr_unittest.cc', 'win/scoped_comptr_unittest.cc', + 'win/scoped_handle_unittest.cc', 'win/scoped_process_information_unittest.cc', 'win/scoped_variant_unittest.cc', 'win/shortcut_unittest.cc', diff --git a/base/memory/shared_memory_unittest.cc b/base/memory/shared_memory_unittest.cc index a1aa9e32..2b3ad7a 100644 --- a/base/memory/shared_memory_unittest.cc +++ b/base/memory/shared_memory_unittest.cc @@ -425,18 +425,16 @@ TEST(SharedMemoryTest, ShareReadOnly) { EXPECT_EQ(NULL, MapViewOfFile(handle, FILE_MAP_WRITE, 0, 0, 0)) << "Shouldn't be able to map memory writable."; - HANDLE temp_handle; - BOOL rv = ::DuplicateHandle(GetCurrentProcess(), + base::win::ScopedHandle writable_handle; + EXPECT_EQ(0, + ::DuplicateHandle(GetCurrentProcess(), handle, GetCurrentProcess, - &temp_handle, + writable_handle.Receive(), FILE_MAP_ALL_ACCESS, false, - 0); - EXPECT_EQ(FALSE, rv) + 0)) << "Shouldn't be able to duplicate the handle into a writable one."; - if (rv) - base::win::ScopedHandle writable_handle(temp_handle); #else #error Unexpected platform; write a test that tries to make 'handle' writable. #endif // defined(OS_POSIX) || defined(OS_WIN) diff --git a/base/process/launch.h b/base/process/launch.h index f5664fb..ac2df5e 100644 --- a/base/process/launch.h +++ b/base/process/launch.h @@ -21,7 +21,6 @@ #include "base/posix/file_descriptor_shuffle.h" #elif defined(OS_WIN) #include <windows.h> -#include "base/win/scoped_handle.h" #endif class CommandLine; @@ -147,7 +146,7 @@ BASE_EXPORT bool LaunchProcess(const CommandLine& cmdline, // cmdline = "c:\windows\explorer.exe" -foo "c:\bar\" BASE_EXPORT bool LaunchProcess(const string16& cmdline, const LaunchOptions& options, - win::ScopedHandle* process_handle); + ProcessHandle* process_handle); #elif defined(OS_POSIX) // A POSIX-specific version of LaunchProcess that takes an argv array diff --git a/base/process/launch_win.cc b/base/process/launch_win.cc index 34f84c3..da913ef 100644 --- a/base/process/launch_win.cc +++ b/base/process/launch_win.cc @@ -103,7 +103,7 @@ void RouteStdioToConsole() { bool LaunchProcess(const string16& cmdline, const LaunchOptions& options, - win::ScopedHandle* process_handle) { + ProcessHandle* process_handle) { STARTUPINFO startup_info = {}; startup_info.cb = sizeof(startup_info); if (options.empty_desktop_name) @@ -136,7 +136,7 @@ bool LaunchProcess(const string16& cmdline, if (options.force_breakaway_from_job_) flags |= CREATE_BREAKAWAY_FROM_JOB; - PROCESS_INFORMATION temp_process_info = {}; + base::win::ScopedProcessInformation process_info; if (options.as_user) { flags |= CREATE_UNICODE_ENVIRONMENT; @@ -152,7 +152,7 @@ bool LaunchProcess(const string16& cmdline, const_cast<wchar_t*>(cmdline.c_str()), NULL, NULL, options.inherit_handles, flags, enviroment_block, NULL, &startup_info, - &temp_process_info); + process_info.Receive()); DestroyEnvironmentBlock(enviroment_block); if (!launched) { DPLOG(ERROR); @@ -162,12 +162,11 @@ bool LaunchProcess(const string16& cmdline, if (!CreateProcess(NULL, const_cast<wchar_t*>(cmdline.c_str()), NULL, NULL, options.inherit_handles, flags, NULL, NULL, - &startup_info, &temp_process_info)) { + &startup_info, process_info.Receive())) { DPLOG(ERROR); return false; } } - base::win::ScopedProcessInformation process_info(temp_process_info); if (options.job_handle) { if (0 == AssignProcessToJobObject(options.job_handle, @@ -185,7 +184,7 @@ bool LaunchProcess(const string16& cmdline, // If the caller wants the process handle, we won't close it. if (process_handle) - process_handle->Set(process_info.TakeProcessHandle()); + *process_handle = process_info.TakeProcessHandle(); return true; } @@ -193,13 +192,7 @@ bool LaunchProcess(const string16& cmdline, bool LaunchProcess(const CommandLine& cmdline, const LaunchOptions& options, ProcessHandle* process_handle) { - if (!process_handle) - return LaunchProcess(cmdline.GetCommandLineString(), options, NULL); - - win::ScopedHandle process; - bool rv = LaunchProcess(cmdline.GetCommandLineString(), options, &process); - *process_handle = process.Take(); - return rv; + return LaunchProcess(cmdline.GetCommandLineString(), options, process_handle); } bool SetJobObjectLimitFlags(HANDLE job_object, DWORD limit_flags) { @@ -240,7 +233,8 @@ bool GetAppOutput(const CommandLine& cl, std::string* output) { FilePath::StringType writable_command_line_string(cl.GetCommandLineString()); - STARTUPINFO start_info = {}; + base::win::ScopedProcessInformation proc_info; + STARTUPINFO start_info = { 0 }; start_info.cb = sizeof(STARTUPINFO); start_info.hStdOutput = out_write; @@ -250,16 +244,14 @@ bool GetAppOutput(const CommandLine& cl, std::string* output) { start_info.dwFlags |= STARTF_USESTDHANDLES; // Create the child process. - PROCESS_INFORMATION temp_process_info = {}; if (!CreateProcess(NULL, &writable_command_line_string[0], NULL, NULL, TRUE, // Handles are inherited. - 0, NULL, NULL, &start_info, &temp_process_info)) { + 0, NULL, NULL, &start_info, proc_info.Receive())) { NOTREACHED() << "Failed to start process"; return false; } - base::win::ScopedProcessInformation proc_info(temp_process_info); // Close our writing end of pipe now. Otherwise later read would not be able // to detect end of child's output. diff --git a/base/win/scoped_handle.h b/base/win/scoped_handle.h index 0d038e0..d3f5c22 100644 --- a/base/win/scoped_handle.h +++ b/base/win/scoped_handle.h @@ -39,6 +39,22 @@ class GenericScopedHandle { public: typedef typename Traits::Handle Handle; + // Helper object to contain the effect of Receive() to the function that needs + // a pointer, and allow proper tracking of the handle. + class Receiver { + public: + explicit Receiver(GenericScopedHandle* owner) + : handle_(Traits::NullHandle()), + owner_(owner) {} + ~Receiver() { owner_->Set(handle_); } + + operator Handle*() { return &handle_; } + + private: + Handle handle_; + GenericScopedHandle* owner_; + }; + GenericScopedHandle() : handle_(Traits::NullHandle()) {} explicit GenericScopedHandle(Handle handle) : handle_(Traits::NullHandle()) { @@ -86,6 +102,16 @@ class GenericScopedHandle { return handle_; } + // This method is intended to be used with functions that require a pointer to + // a destination handle, like so: + // void CreateRequiredHandle(Handle* out_handle); + // ScopedHandle a; + // CreateRequiredHandle(a.Receive()); + Receiver Receive() { + DCHECK(!Traits::IsHandleValid(handle_)) << "Handle must be NULL"; + return Receiver(this); + } + // Transfers ownership away from this object. Handle Take() { Handle temp = handle_; diff --git a/base/win/scoped_handle_unittest.cc b/base/win/scoped_handle_unittest.cc new file mode 100644 index 0000000..e0d8b46 --- /dev/null +++ b/base/win/scoped_handle_unittest.cc @@ -0,0 +1,31 @@ +// 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 "base/win/scoped_handle.h" +#include "testing/gtest/include/gtest/gtest.h" + +void CreateHandle(int value, HANDLE* result) { + *result = reinterpret_cast<HANDLE>(value); +} + +TEST(ScopedHandleTest, Receive) { + base::win::ScopedHandle handle; + int value = 51; + + { + // This is not really the expected use case, but it is a very explicit test. + base::win::ScopedHandle::Receiver a = handle.Receive(); + HANDLE* pointer = a; + *pointer = reinterpret_cast<HANDLE>(value); + } + + EXPECT_EQ(handle.Get(), reinterpret_cast<HANDLE>(value)); + HANDLE to_discard = handle.Take(); + + // The standard use case: + value = 183; + CreateHandle(value, handle.Receive()); + EXPECT_EQ(handle.Get(), reinterpret_cast<HANDLE>(value)); + to_discard = handle.Take(); +} diff --git a/base/win/scoped_process_information.cc b/base/win/scoped_process_information.cc index bb24637..cb7a30e 100644 --- a/base/win/scoped_process_information.cc +++ b/base/win/scoped_process_information.cc @@ -15,7 +15,7 @@ namespace { // Duplicates source into target, returning true upon success. |target| is // guaranteed to be untouched in case of failure. Succeeds with no side-effects // if source is NULL. -bool CheckAndDuplicateHandle(HANDLE source, ScopedHandle* target) { +bool CheckAndDuplicateHandle(HANDLE source, HANDLE* target) { if (!source) return true; @@ -26,7 +26,7 @@ bool CheckAndDuplicateHandle(HANDLE source, ScopedHandle* target) { DPLOG(ERROR) << "Failed to duplicate a handle."; return false; } - target->Set(temp); + *target = temp; return true; } @@ -36,15 +36,15 @@ ScopedProcessInformation::ScopedProcessInformation() : process_id_(0), thread_id_(0) { } -ScopedProcessInformation::ScopedProcessInformation( - const PROCESS_INFORMATION& process_info) : process_id_(0), thread_id_(0) { - Set(process_info); -} - ScopedProcessInformation::~ScopedProcessInformation() { Close(); } +ScopedProcessInformation::Receiver ScopedProcessInformation::Receive() { + DCHECK(!IsValid()) << "process_information_ must be NULL"; + return Receiver(this); +} + bool ScopedProcessInformation::IsValid() const { return process_id_ || process_handle_.Get() || thread_id_ || thread_handle_.Get(); @@ -72,8 +72,10 @@ bool ScopedProcessInformation::DuplicateFrom( DCHECK(!IsValid()) << "target ScopedProcessInformation must be NULL"; DCHECK(other.IsValid()) << "source ScopedProcessInformation must be valid"; - if (CheckAndDuplicateHandle(other.process_handle(), &process_handle_) && - CheckAndDuplicateHandle(other.thread_handle(), &thread_handle_)) { + if (CheckAndDuplicateHandle(other.process_handle(), + process_handle_.Receive()) && + CheckAndDuplicateHandle(other.thread_handle(), + thread_handle_.Receive())) { process_id_ = other.process_id(); thread_id_ = other.thread_id(); return true; diff --git a/base/win/scoped_process_information.h b/base/win/scoped_process_information.h index 2e24054..1f404c2 100644 --- a/base/win/scoped_process_information.h +++ b/base/win/scoped_process_information.h @@ -18,10 +18,33 @@ namespace win { // structures. Allows clients to take ownership of either handle independently. class BASE_EXPORT ScopedProcessInformation { public: + // Helper object to contain the effect of Receive() to the funtion that needs + // a pointer. + class Receiver { + public: + explicit Receiver(ScopedProcessInformation* owner) + : info_(), + owner_(owner) {} + ~Receiver() { owner_->Set(info_); } + + operator PROCESS_INFORMATION*() { return &info_; } + + private: + PROCESS_INFORMATION info_; + ScopedProcessInformation* owner_; + }; + ScopedProcessInformation(); - explicit ScopedProcessInformation(const PROCESS_INFORMATION& process_info); ~ScopedProcessInformation(); + // Returns an object that may be passed to API calls such as CreateProcess. + // DCHECKs that the object is not currently holding any handles. + // HANDLEs stored in the returned PROCESS_INFORMATION will be owned by this + // instance. + // The intended use case is something like this: + // if (::CreateProcess(..., startup_info, scoped_proces_info.Receive())) + Receiver Receive(); + // Returns true iff this instance is holding a thread and/or process handle. bool IsValid() const; diff --git a/base/win/scoped_process_information_unittest.cc b/base/win/scoped_process_information_unittest.cc index 550076e..b8ffc44 100644 --- a/base/win/scoped_process_information_unittest.cc +++ b/base/win/scoped_process_information_unittest.cc @@ -19,13 +19,11 @@ const DWORD kThreadId = 1234; const HANDLE kProcessHandle = reinterpret_cast<HANDLE>(7651); const HANDLE kThreadHandle = reinterpret_cast<HANDLE>(1567); -void MockCreateProcess(base::win::ScopedProcessInformation* process_info) { - PROCESS_INFORMATION process_information = {}; - process_information.dwProcessId = kProcessId; - process_information.dwThreadId = kThreadId; - process_information.hProcess = kProcessHandle; - process_information.hThread = kThreadHandle; - process_info->Set(process_information); +void MockCreateProcess(PROCESS_INFORMATION* process_info) { + process_info->dwProcessId = kProcessId; + process_info->dwThreadId = kThreadId; + process_info->hProcess = kProcessHandle; + process_info->hThread = kThreadHandle; } } // namespace @@ -64,7 +62,7 @@ TEST_F(ScopedProcessInformationTest, InitiallyInvalid) { TEST_F(ScopedProcessInformationTest, Receive) { base::win::ScopedProcessInformation process_info; - MockCreateProcess(&process_info); + MockCreateProcess(process_info.Receive()); EXPECT_TRUE(process_info.IsValid()); EXPECT_EQ(kProcessId, process_info.process_id()); @@ -76,7 +74,7 @@ TEST_F(ScopedProcessInformationTest, Receive) { TEST_F(ScopedProcessInformationTest, TakeProcess) { base::win::ScopedProcessInformation process_info; - MockCreateProcess(&process_info); + MockCreateProcess(process_info.Receive()); HANDLE process = process_info.TakeProcessHandle(); EXPECT_EQ(kProcessHandle, process); @@ -88,7 +86,7 @@ TEST_F(ScopedProcessInformationTest, TakeProcess) { TEST_F(ScopedProcessInformationTest, TakeThread) { base::win::ScopedProcessInformation process_info; - MockCreateProcess(&process_info); + MockCreateProcess(process_info.Receive()); HANDLE thread = process_info.TakeThreadHandle(); EXPECT_EQ(kThreadHandle, thread); @@ -100,7 +98,7 @@ TEST_F(ScopedProcessInformationTest, TakeThread) { TEST_F(ScopedProcessInformationTest, TakeBoth) { base::win::ScopedProcessInformation process_info; - MockCreateProcess(&process_info); + MockCreateProcess(process_info.Receive()); HANDLE process = process_info.TakeProcessHandle(); HANDLE thread = process_info.TakeThreadHandle(); @@ -110,7 +108,7 @@ TEST_F(ScopedProcessInformationTest, TakeBoth) { TEST_F(ScopedProcessInformationTest, TakeWholeStruct) { base::win::ScopedProcessInformation process_info; - MockCreateProcess(&process_info); + MockCreateProcess(process_info.Receive()); PROCESS_INFORMATION to_discard = process_info.Take(); EXPECT_EQ(kProcessId, to_discard.dwProcessId); @@ -121,11 +119,8 @@ TEST_F(ScopedProcessInformationTest, TakeWholeStruct) { } TEST_F(ScopedProcessInformationTest, Duplicate) { - PROCESS_INFORMATION temp_process_information; - DoCreateProcess("ReturnSeven", &temp_process_information); base::win::ScopedProcessInformation process_info; - process_info.Set(temp_process_information); - + DoCreateProcess("ReturnSeven", process_info.Receive()); base::win::ScopedProcessInformation duplicate; duplicate.DuplicateFrom(process_info); @@ -151,17 +146,15 @@ TEST_F(ScopedProcessInformationTest, Duplicate) { } TEST_F(ScopedProcessInformationTest, Set) { - base::win::ScopedProcessInformation base_process_info; + PROCESS_INFORMATION base_process_info = {}; MockCreateProcess(&base_process_info); - PROCESS_INFORMATION base_struct = base_process_info.Take(); - base::win::ScopedProcessInformation process_info; - process_info.Set(base_struct); + process_info.Set(base_process_info); EXPECT_EQ(kProcessId, process_info.process_id()); EXPECT_EQ(kThreadId, process_info.thread_id()); EXPECT_EQ(kProcessHandle, process_info.process_handle()); EXPECT_EQ(kThreadHandle, process_info.thread_handle()); - base_struct = process_info.Take(); + base_process_info = process_info.Take(); } diff --git a/base/win/startup_information_unittest.cc b/base/win/startup_information_unittest.cc index d637ebd..1903564 100644 --- a/base/win/startup_information_unittest.cc +++ b/base/win/startup_information_unittest.cc @@ -38,6 +38,7 @@ TEST_F(StartupInformationTest, InheritStdOut) { if (base::win::GetVersion() < base::win::VERSION_VISTA) return; + base::win::ScopedProcessInformation process_info; base::win::StartupInformation startup_info; HANDLE section = ::CreateFileMappingW(INVALID_HANDLE_VALUE, NULL, @@ -64,13 +65,10 @@ TEST_F(StartupInformationTest, InheritStdOut) { std::wstring cmd_line = this->MakeCmdLine("FireInheritedEvents", false).GetCommandLineString(); - PROCESS_INFORMATION temp_process_info = {}; ASSERT_TRUE(::CreateProcess(NULL, const_cast<wchar_t*>(cmd_line.c_str()), NULL, NULL, true, EXTENDED_STARTUPINFO_PRESENT, NULL, NULL, startup_info.startup_info(), - &temp_process_info)) << ::GetLastError(); - base::win::ScopedProcessInformation process_info(temp_process_info); - + process_info.Receive())) << ::GetLastError(); // Only the first event should be signalled EXPECT_EQ(WAIT_OBJECT_0, ::WaitForMultipleObjects(2, events, false, 4000)); diff --git a/chrome/browser/extensions/api/messaging/native_process_launcher_win.cc b/chrome/browser/extensions/api/messaging/native_process_launcher_win.cc index 31fbdc8..fd7345c 100644 --- a/chrome/browser/extensions/api/messaging/native_process_launcher_win.cc +++ b/chrome/browser/extensions/api/messaging/native_process_launcher_win.cc @@ -136,7 +136,7 @@ bool NativeProcessLauncher::LaunchNativeProcess( base::LaunchOptions options; options.start_hidden = true; - base::win::ScopedHandle cmd_handle; + base::ProcessHandle cmd_handle; if (!base::LaunchProcess(command.c_str(), options, &cmd_handle)) { LOG(ERROR) << "Error launching process " << command_line.GetProgram().MaybeAsASCII(); @@ -148,13 +148,14 @@ bool NativeProcessLauncher::LaunchNativeProcess( bool stdin_connected = ConnectNamedPipe(stdin_pipe.Get(), NULL) ? TRUE : GetLastError() == ERROR_PIPE_CONNECTED; if (!stdout_connected || !stdin_connected) { - base::KillProcess(cmd_handle.Get(), 0, false); + base::KillProcess(cmd_handle, 0, false); + base::CloseProcessHandle(cmd_handle); LOG(ERROR) << "Failed to connect IO pipes when starting " << command_line.GetProgram().MaybeAsASCII(); return false; } - *process_handle = cmd_handle.Take(); + *process_handle = cmd_handle; *read_file = stdout_pipe.Take(); *write_file = stdin_pipe.Take(); diff --git a/chrome/browser/first_run/upgrade_util_win.cc b/chrome/browser/first_run/upgrade_util_win.cc index 244773f..729bfcb 100644 --- a/chrome/browser/first_run/upgrade_util_win.cc +++ b/chrome/browser/first_run/upgrade_util_win.cc @@ -252,13 +252,14 @@ bool SwapNewChromeExeIfPresent() { std::wstring rename_cmd; if (key.ReadValue(google_update::kRegRenameCmdField, &rename_cmd) == ERROR_SUCCESS) { - base::win::ScopedHandle handle; + base::ProcessHandle handle; base::LaunchOptions options; options.wait = true; options.start_hidden = true; if (base::LaunchProcess(rename_cmd, options, &handle)) { DWORD exit_code; ::GetExitCodeProcess(handle, &exit_code); + ::CloseHandle(handle); if (exit_code == installer::RENAME_SUCCESSFUL) return true; } diff --git a/chrome/installer/setup/setup_util.cc b/chrome/installer/setup/setup_util.cc index da46e4f..611c53c 100644 --- a/chrome/installer/setup/setup_util.cc +++ b/chrome/installer/setup/setup_util.cc @@ -452,13 +452,11 @@ bool IsUninstallSuccess(InstallStatus install_status) { ScopedTokenPrivilege::ScopedTokenPrivilege(const wchar_t* privilege_name) : is_enabled_(false) { - HANDLE temp_handle; if (!::OpenProcessToken(::GetCurrentProcess(), TOKEN_ADJUST_PRIVILEGES | TOKEN_QUERY, - &temp_handle)) { + token_.Receive())) { return; } - token_.Set(temp_handle); LUID privilege_luid; if (!::LookupPrivilegeValue(NULL, privilege_name, &privilege_luid)) { diff --git a/chrome/installer/setup/setup_util_unittest.cc b/chrome/installer/setup/setup_util_unittest.cc index 389cc1e..647d90f 100644 --- a/chrome/installer/setup/setup_util_unittest.cc +++ b/chrome/installer/setup/setup_util_unittest.cc @@ -55,15 +55,13 @@ static const wchar_t kTestedPrivilege[] = SE_RESTORE_NAME; // Returns true if the current process' token has privilege |privilege_name| // enabled. bool CurrentProcessHasPrivilege(const wchar_t* privilege_name) { - HANDLE temp_handle; + base::win::ScopedHandle token; if (!::OpenProcessToken(::GetCurrentProcess(), TOKEN_QUERY, - &temp_handle)) { + token.Receive())) { ADD_FAILURE(); return false; } - base::win::ScopedHandle token(temp_handle); - // First get the size of the buffer needed for |privileges| below. DWORD size; EXPECT_FALSE(::GetTokenInformation(token, TokenPrivileges, NULL, 0, &size)); diff --git a/chrome/installer/test/alternate_version_generator.cc b/chrome/installer/test/alternate_version_generator.cc index d00da4f..886ff06 100644 --- a/chrome/installer/test/alternate_version_generator.cc +++ b/chrome/installer/test/alternate_version_generator.cc @@ -210,13 +210,13 @@ bool MappedFile::Initialize(base::PlatformFile file) { bool RunProcessAndWait(const wchar_t* exe_path, const std::wstring& cmdline, int* exit_code) { bool result = true; - base::win::ScopedHandle process; + base::ProcessHandle process; base::LaunchOptions options; options.wait = true; options.start_hidden = true; if (base::LaunchProcess(cmdline, options, &process)) { if (exit_code) { - if (!GetExitCodeProcess(process.Get(), + if (!GetExitCodeProcess(process, reinterpret_cast<DWORD*>(exit_code))) { PLOG(DFATAL) << "Failed getting the exit code for \"" << cmdline << "\"."; @@ -229,6 +229,7 @@ bool RunProcessAndWait(const wchar_t* exe_path, const std::wstring& cmdline, result = false; } + CloseHandle(process); return result; } diff --git a/chrome/installer/util/google_update_util.cc b/chrome/installer/util/google_update_util.cc index 630cd0f..2448ba4 100644 --- a/chrome/installer/util/google_update_util.cc +++ b/chrome/installer/util/google_update_util.cc @@ -95,9 +95,9 @@ bool LaunchProcessAndWaitWithTimeout(const string16& cmd_string, bool success = false; base::win::ScopedHandle process; int exit_code = 0; - VLOG(0) << "Launching: " << cmd_string; + LOG(INFO) << "Launching: " << cmd_string; if (!base::LaunchProcess(cmd_string, base::LaunchOptions(), - &process)) { + process.Receive())) { PLOG(ERROR) << "Failed to launch (" << cmd_string << ")"; } else if (!base::WaitForExitCodeWithTimeout(process, &exit_code, timeout)) { // The GetExitCodeProcess failed or timed-out. @@ -174,7 +174,7 @@ bool GetGoogleUpdateUntrustedData( } // namespace bool EnsureUserLevelGoogleUpdatePresent() { - VLOG(0) << "Ensuring Google Update is present at user-level."; + LOG(INFO) << "Ensuring Google Update is present at user-level."; bool success = false; if (IsGoogleUpdatePresent(false)) { diff --git a/chrome/test/ui/ui_test_suite.cc b/chrome/test/ui/ui_test_suite.cc index 1d02df5..50cd2d5 100644 --- a/chrome/test/ui/ui_test_suite.cc +++ b/chrome/test/ui/ui_test_suite.cc @@ -70,13 +70,11 @@ void UITestSuite::LoadCrashService() { base::LaunchOptions launch_options; launch_options.job_handle = job_handle_.Get(); base::FilePath crash_service = exe_dir.Append(L"crash_service.exe"); - base::win::ScopedHandle crash_service_handle; if (!base::LaunchProcess(crash_service.value(), base::LaunchOptions(), - &crash_service_handle)) { + &crash_service_)) { LOG(ERROR) << "Couldn't start crash_service.exe, so this ui_tests run " << "won't tell you if any test crashes!"; return; } - crash_service_ = crash_service_handle.Take(); } #endif diff --git a/chrome_frame/ready_mode/internal/registry_ready_mode_state.cc b/chrome_frame/ready_mode/internal/registry_ready_mode_state.cc index bb1dedc..0a66e7d 100644 --- a/chrome_frame/ready_mode/internal/registry_ready_mode_state.cc +++ b/chrome_frame/ready_mode/internal/registry_ready_mode_state.cc @@ -40,11 +40,11 @@ HANDLE LaunchCommandDirectly(const std::wstring& command_field) { std::wstring command_line; if (version_key.ReadValue(command_field.c_str(), &command_line) == ERROR_SUCCESS) { - base::win::ScopedHandle launched_process; + HANDLE launched_process = NULL; base::LaunchOptions options; options.start_hidden = true; if (base::LaunchProcess(command_line, options, &launched_process)) { - return launched_process.Take(); + return launched_process; } } } diff --git a/chrome_frame/test/chrome_frame_test_utils.cc b/chrome_frame/test/chrome_frame_test_utils.cc index c614214..6c7fb48 100644 --- a/chrome_frame/test/chrome_frame_test_utils.cc +++ b/chrome_frame/test/chrome_frame_test_utils.cc @@ -640,7 +640,7 @@ base::ProcessHandle StartCrashService() { return NULL; } - base::win::ScopedHandle crash_service; + base::ProcessHandle crash_service = NULL; VLOG(1) << "Starting crash_service.exe so you know if a test crashes!"; @@ -656,7 +656,7 @@ base::ProcessHandle StartCrashService() { if (DetectRunningCrashService(kCrashServiceStartupTimeoutMs)) { VLOG(1) << "crash_service.exe is ready for clients in " << (base::Time::Now() - start).InMilliseconds() << " ms."; - return crash_service.Take(); + return crash_service; } else { LOG(ERROR) << "crash_service.exe failed to accept client connections " "within " << kCrashServiceStartupTimeoutMs << " ms. " @@ -664,8 +664,8 @@ base::ProcessHandle StartCrashService() { // First check to see if it's even still running just to minimize the // likelihood of spurious error messages from KillProcess. - if (WAIT_OBJECT_0 != ::WaitForSingleObject(crash_service.Get(), 0)) { - base::KillProcess(crash_service.Get(), 0, false); + if (WAIT_OBJECT_0 != ::WaitForSingleObject(crash_service, 0)) { + base::KillProcess(crash_service, 0, false); } return NULL; } diff --git a/chrome_frame/test_utils.cc b/chrome_frame/test_utils.cc index b8884e7..ef78046 100644 --- a/chrome_frame/test_utils.cc +++ b/chrome_frame/test_utils.cc @@ -88,7 +88,7 @@ void ScopedChromeFrameRegistrar::DoRegistration( int entrypoint_index = 0; base::LaunchOptions launch_options; - base::win::ScopedHandle process_handle; + base::ProcessHandle process_handle = INVALID_HANDLE_VALUE; int exit_code = -1; if (registration_type == PER_USER) @@ -111,12 +111,13 @@ void ScopedChromeFrameRegistrar::DoRegistration( << "Failed to register or unregister DLL with command: " << registration_command; } else { + base::win::ScopedHandle rundll32(process_handle); if (!base::WaitForExitCodeWithTimeout( - process_handle.Get(), &exit_code, + process_handle, &exit_code, base::TimeDelta::FromMilliseconds(kDllRegistrationTimeoutMs))) { LOG(ERROR) << "Timeout waiting to register or unregister DLL with " "command: " << registration_command; - base::KillProcess(process_handle.Get(), 0, false); + base::KillProcess(process_handle, 0, false); NOTREACHED() << "Aborting test due to registration failure."; } } diff --git a/cloud_print/service/win/chrome_launcher.cc b/cloud_print/service/win/chrome_launcher.cc index 35d760e..51501df 100644 --- a/cloud_print/service/win/chrome_launcher.cc +++ b/cloud_print/service/win/chrome_launcher.cc @@ -67,23 +67,22 @@ void CloseChrome(HANDLE process, DWORD thread_id) { } bool LaunchProcess(const CommandLine& cmdline, - base::win::ScopedHandle* process_handle, + base::ProcessHandle* process_handle, DWORD* thread_id) { STARTUPINFO startup_info = {}; startup_info.cb = sizeof(startup_info); startup_info.dwFlags = STARTF_USESHOWWINDOW; startup_info.wShowWindow = SW_SHOW; - PROCESS_INFORMATION temp_process_info = {}; + base::win::ScopedProcessInformation process_info; if (!CreateProcess(NULL, const_cast<wchar_t*>(cmdline.GetCommandLineString().c_str()), NULL, NULL, - FALSE, 0, NULL, NULL, &startup_info, &temp_process_info)) { + FALSE, 0, NULL, NULL, &startup_info, process_info.Receive())) { return false; } - base::win::ScopedProcessInformation process_info(temp_process_info); if (process_handle) - process_handle->Set(process_info.TakeProcessHandle()); + *process_handle = process_info.TakeProcessHandle(); if (thread_id) *thread_id = process_info.thread_id(); @@ -241,7 +240,7 @@ void ChromeLauncher::Run() { base::win::ScopedHandle chrome_handle; base::Time started = base::Time::Now(); DWORD thread_id = 0; - LaunchProcess(cmd, &chrome_handle, &thread_id); + LaunchProcess(cmd, chrome_handle.Receive(), &thread_id); HANDLE handles[] = {stop_event_.handle(), chrome_handle}; DWORD wait_result = WAIT_TIMEOUT; @@ -318,7 +317,7 @@ std::string ChromeLauncher::CreateServiceStateFile( base::win::ScopedHandle chrome_handle; DWORD thread_id = 0; - if (!LaunchProcess(cmd, &chrome_handle, &thread_id)) { + if (!LaunchProcess(cmd, chrome_handle.Receive(), &thread_id)) { LOG(ERROR) << "Unable to launch Chrome."; return result; } diff --git a/cloud_print/virtual_driver/win/install/setup.cc b/cloud_print/virtual_driver/win/install/setup.cc index 1c87ca2..6e0df24 100644 --- a/cloud_print/virtual_driver/win/install/setup.cc +++ b/cloud_print/virtual_driver/win/install/setup.cc @@ -97,7 +97,7 @@ void SpoolerServiceCommand(const char* command) { base::LaunchOptions options; options.wait = true; options.start_hidden = true; - VLOG(0) << command_line.GetCommandLineString(); + LOG(INFO) << command_line.GetCommandLineString(); base::LaunchProcess(command_line, options, NULL); } @@ -140,8 +140,7 @@ HRESULT RegisterPortMonitor(bool install, const base::FilePath& install_path) { options.wait = true; base::win::ScopedHandle regsvr32_handle; - if (!base::LaunchProcess(command_line.GetCommandLineString(), options, - ®svr32_handle)) { + if (!base::LaunchProcess(command_line, options, regsvr32_handle.Receive())) { LOG(ERROR) << "Unable to launch regsvr32.exe."; return HRESULT_FROM_WIN32(ERROR_NOT_SUPPORTED); } @@ -541,8 +540,8 @@ int WINAPI WinMain(__in HINSTANCE hInstance, CommandLine::Init(0, NULL); HRESULT retval = cloud_print::ExecuteCommands(); - VLOG(0) << _com_error(retval).ErrorMessage() << " HRESULT=0x" << - std::setbase(16) << retval; + LOG(INFO) << _com_error(retval).ErrorMessage() << " HRESULT=0x" << + std::setbase(16) << retval; // Installer is silent by default as required by Google Update. if (CommandLine::ForCurrentProcess()->HasSwitch("verbose")) { diff --git a/components/nacl/browser/nacl_process_host.cc b/components/nacl/browser/nacl_process_host.cc index aa79f4f..d40af96 100644 --- a/components/nacl/browser/nacl_process_host.cc +++ b/components/nacl/browser/nacl_process_host.cc @@ -977,7 +977,7 @@ bool NaClProcessHost::AttachDebugExceptionHandler(const std::string& info, debug_exception_handler_requested_ = true; base::ProcessId nacl_pid = base::GetProcId(process_->GetData().handle); - base::ProcessHandle temp_handle; + base::win::ScopedHandle process_handle; // We cannot use process_->GetData().handle because it does not have // the necessary access rights. We open the new handle here rather // than in the NaCl broker process in case the NaCl loader process @@ -996,11 +996,10 @@ bool NaClProcessHost::AttachDebugExceptionHandler(const std::string& info, base::kProcessAccessVMWrite | base::kProcessAccessDuplicateHandle | base::kProcessAccessWaitForTermination, - &temp_handle)) { + process_handle.Receive())) { LOG(ERROR) << "Failed to get process handle"; return false; } - base::win::ScopedHandle process_handle(temp_handle); attach_debug_exception_handler_reply_msg_.reset(reply_msg); // If the NaCl loader is 64-bit, the process running its debug diff --git a/content/common/sandbox_win.cc b/content/common/sandbox_win.cc index f5c86f7..13b3bd4 100644 --- a/content/common/sandbox_win.cc +++ b/content/common/sandbox_win.cc @@ -467,14 +467,13 @@ BOOL WINAPI DuplicateHandlePatch(HANDLE source_process_handle, if (!::IsProcessInJob(target_process_handle, NULL, &is_in_job)) { // We need a handle with permission to check the job object. if (ERROR_ACCESS_DENIED == ::GetLastError()) { - HANDLE temp_handle; + base::win::ScopedHandle process; CHECK(g_iat_orig_duplicate_handle(::GetCurrentProcess(), target_process_handle, ::GetCurrentProcess(), - &temp_handle, + process.Receive(), PROCESS_QUERY_INFORMATION, FALSE, 0)); - base::win::ScopedHandle process(temp_handle); CHECK(::IsProcessInJob(process, NULL, &is_in_job)); } } @@ -484,11 +483,10 @@ BOOL WINAPI DuplicateHandlePatch(HANDLE source_process_handle, CHECK(!inherit_handle) << kDuplicateHandleWarning; // Duplicate the handle again, to get the final permissions. - HANDLE temp_handle; + base::win::ScopedHandle handle; CHECK(g_iat_orig_duplicate_handle(target_process_handle, *target_handle, - ::GetCurrentProcess(), &temp_handle, + ::GetCurrentProcess(), handle.Receive(), 0, FALSE, DUPLICATE_SAME_ACCESS)); - base::win::ScopedHandle handle(temp_handle); // Callers use CHECK macro to make sure we get the right stack. CheckDuplicateHandle(handle); @@ -602,6 +600,7 @@ base::ProcessHandle StartSandboxedProcess( return process; } + base::win::ScopedProcessInformation target; sandbox::TargetPolicy* policy = g_broker_services->CreatePolicy(); sandbox::MitigationFlags mitigations = sandbox::MITIGATION_HEAP_TERMINATE | @@ -673,13 +672,11 @@ base::ProcessHandle StartSandboxedProcess( TRACE_EVENT_BEGIN_ETW("StartProcessWithAccess::LAUNCHPROCESS", 0, 0); - PROCESS_INFORMATION temp_process_info = {}; result = g_broker_services->SpawnTarget( - cmd_line->GetProgram().value().c_str(), - cmd_line->GetCommandLineString().c_str(), - policy, &temp_process_info); + cmd_line->GetProgram().value().c_str(), + cmd_line->GetCommandLineString().c_str(), + policy, target.Receive()); policy->Release(); - base::win::ScopedProcessInformation target(temp_process_info); TRACE_EVENT_END_ETW("StartProcessWithAccess::LAUNCHPROCESS", 0, 0); diff --git a/device/bluetooth/bluetooth_task_manager_win.cc b/device/bluetooth/bluetooth_task_manager_win.cc index 3347f0f..0dc0a59 100644 --- a/device/bluetooth/bluetooth_task_manager_win.cc +++ b/device/bluetooth/bluetooth_task_manager_win.cc @@ -211,12 +211,10 @@ void BluetoothTaskManagerWin::PollAdapter() { { sizeof(BLUETOOTH_FIND_RADIO_PARAMS) }; if (adapter_handle_) adapter_handle_.Close(); - HANDLE temp_adapter_handle; HBLUETOOTH_RADIO_FIND handle = BluetoothFindFirstRadio( - &adapter_param, &temp_adapter_handle); + &adapter_param, adapter_handle_.Receive()); if (handle) { - adapter_handle_.Set(temp_adapter_handle); GetKnownDevices(); BluetoothFindRadioClose(handle); } diff --git a/net/base/net_util_win.cc b/net/base/net_util_win.cc index b44836c..bbe1655 100644 --- a/net/base/net_util_win.cc +++ b/net/base/net_util_win.cc @@ -61,17 +61,6 @@ struct WlanApi { close_handle_func; } - template <typename T> - DWORD OpenHandle(DWORD client_version, DWORD* cur_version, T* handle) const { - HANDLE temp_handle; - DWORD result = open_handle_func(client_version, NULL, cur_version, - &temp_handle); - if (result != ERROR_SUCCESS) - return result; - handle->Set(temp_handle); - return ERROR_SUCCESS; - } - HMODULE module; WlanOpenHandleFunc open_handle_func; WlanEnumInterfacesFunc enum_interfaces_func; @@ -247,7 +236,8 @@ WifiPHYLayerProtocol GetWifiPHYLayerProtocol() { WlanHandle client; DWORD cur_version = 0; const DWORD kMaxClientVersion = 2; - DWORD result = wlanapi.OpenHandle(kMaxClientVersion, &cur_version, &client); + DWORD result = wlanapi.open_handle_func(kMaxClientVersion, NULL, &cur_version, + client.Receive()); if (result != ERROR_SUCCESS) return WIFI_PHY_LAYER_PROTOCOL_NONE; diff --git a/printing/backend/win_helper.h b/printing/backend/win_helper.h index db66bf8..0f6b7ef 100644 --- a/printing/backend/win_helper.h +++ b/printing/backend/win_helper.h @@ -46,10 +46,9 @@ class ScopedPrinterHandle base::win::VerifierTraits> { public: bool OpenPrinter(const wchar_t* printer) { - HANDLE temp_handle; // ::OpenPrinter may return error but assign some value into handle. - if (::OpenPrinter(const_cast<LPTSTR>(printer), &temp_handle, NULL)) { - Set(temp_handle); + if (!::OpenPrinter(const_cast<LPTSTR>(printer), Receive(), NULL)) { + Take(); } return IsValid(); } @@ -57,6 +56,10 @@ class ScopedPrinterHandle private: typedef base::win::GenericScopedHandle<PrinterHandleTraits, base::win::VerifierTraits> Base; + // Hide Receive to avoid assigning handle when ::OpenPrinter returned error. + Base::Receiver Receive() { + return Base::Receive(); + } }; class PrinterChangeHandleTraits { diff --git a/remoting/host/desktop_session_proxy.cc b/remoting/host/desktop_session_proxy.cc index 058a2f4..9e52f8d 100644 --- a/remoting/host/desktop_session_proxy.cc +++ b/remoting/host/desktop_session_proxy.cc @@ -230,9 +230,9 @@ bool DesktopSessionProxy::AttachToDesktop( #if defined(OS_WIN) // On Windows: |desktop_process| is a valid handle, but |desktop_pipe| needs // to be duplicated from the desktop process. - HANDLE temp_handle; + base::win::ScopedHandle pipe; if (!DuplicateHandle(desktop_process_, desktop_pipe, GetCurrentProcess(), - &temp_handle, 0, FALSE, DUPLICATE_SAME_ACCESS)) { + pipe.Receive(), 0, FALSE, DUPLICATE_SAME_ACCESS)) { LOG_GETLASTERROR(ERROR) << "Failed to duplicate the desktop-to-network" " pipe handle"; @@ -240,7 +240,6 @@ bool DesktopSessionProxy::AttachToDesktop( base::CloseProcessHandle(desktop_process); return false; } - base::win::ScopedHandle pipe(temp_handle); IPC::ChannelHandle desktop_channel_handle(pipe); diff --git a/remoting/host/setup/daemon_installer_win.cc b/remoting/host/setup/daemon_installer_win.cc index 176efa1..dbb72192 100644 --- a/remoting/host/setup/daemon_installer_win.cc +++ b/remoting/host/setup/daemon_installer_win.cc @@ -300,7 +300,7 @@ void DaemonCommandLineInstallerWin::Install() { kOmahaLanguage)); base::LaunchOptions options; - if (!base::LaunchProcess(command_line, options, &process_)) { + if (!base::LaunchProcess(command_line, options, process_.Receive())) { result = GetLastError(); Done(HRESULT_FROM_WIN32(result)); return; diff --git a/remoting/host/win/chromoting_module.cc b/remoting/host/win/chromoting_module.cc index 630c6c2..724ce7e 100644 --- a/remoting/host/win/chromoting_module.cc +++ b/remoting/host/win/chromoting_module.cc @@ -42,13 +42,12 @@ base::LazyInstance<scoped_refptr<AutoThreadTaskRunner> > g_module_task_runner = // Lowers the process integrity level such that it does not exceed |max_level|. // |max_level| is expected to be one of SECURITY_MANDATORY_XXX constants. bool LowerProcessIntegrityLevel(DWORD max_level) { - HANDLE temp_handle; + base::win::ScopedHandle token; if (!OpenProcessToken(GetCurrentProcess(), TOKEN_QUERY | TOKEN_WRITE, - &temp_handle)) { + token.Receive())) { PLOG(ERROR) << "OpenProcessToken() failed"; return false; } - base::win::ScopedHandle token(temp_handle); TypedBuffer<TOKEN_MANDATORY_LABEL> mandatory_label; DWORD length = 0; diff --git a/remoting/host/win/launch_process_with_token.cc b/remoting/host/win/launch_process_with_token.cc index f45332c..e579c09 100644 --- a/remoting/host/win/launch_process_with_token.cc +++ b/remoting/host/win/launch_process_with_token.cc @@ -122,26 +122,26 @@ bool ConnectToExecutionServer(uint32 session_id, // Copies the process token making it a primary impersonation token. // The returned handle will have |desired_access| rights. bool CopyProcessToken(DWORD desired_access, ScopedHandle* token_out) { - HANDLE temp_handle; + ScopedHandle process_token; if (!OpenProcessToken(GetCurrentProcess(), TOKEN_DUPLICATE | desired_access, - &temp_handle)) { + process_token.Receive())) { LOG_GETLASTERROR(ERROR) << "Failed to open process token"; return false; } - ScopedHandle process_token(temp_handle); + ScopedHandle copied_token; if (!DuplicateTokenEx(process_token, desired_access, NULL, SecurityImpersonation, TokenPrimary, - &temp_handle)) { + copied_token.Receive())) { LOG_GETLASTERROR(ERROR) << "Failed to duplicate the process token"; return false; } - token_out->Set(temp_handle); + *token_out = copied_token.Pass(); return true; } @@ -467,7 +467,7 @@ bool LaunchProcessWithToken(const base::FilePath& binary, if (desktop_name) startup_info.lpDesktop = const_cast<char16*>(desktop_name); - PROCESS_INFORMATION temp_process_info = {}; + base::win::ScopedProcessInformation process_info; BOOL result = CreateProcessAsUser(user_token, application_name.c_str(), const_cast<LPWSTR>(command_line.c_str()), @@ -478,7 +478,7 @@ bool LaunchProcessWithToken(const base::FilePath& binary, NULL, NULL, &startup_info, - &temp_process_info); + process_info.Receive()); // CreateProcessAsUser will fail on XP and W2K3 with ERROR_PIPE_NOT_CONNECTED // if the user hasn't logged to the target session yet. In such a case @@ -502,7 +502,7 @@ bool LaunchProcessWithToken(const base::FilePath& binary, command_line, creation_flags, desktop_name, - &temp_process_info); + process_info.Receive()); } else { // Restore the error status returned by CreateProcessAsUser(). result = FALSE; @@ -516,8 +516,6 @@ bool LaunchProcessWithToken(const base::FilePath& binary, return false; } - base::win::ScopedProcessInformation process_info(temp_process_info); - CHECK(process_info.IsValid()); process_out->Set(process_info.TakeProcessHandle()); thread_out->Set(process_info.TakeThreadHandle()); diff --git a/remoting/host/win/unprivileged_process_delegate.cc b/remoting/host/win/unprivileged_process_delegate.cc index 006b9e5..dcdad94 100644 --- a/remoting/host/win/unprivileged_process_delegate.cc +++ b/remoting/host/win/unprivileged_process_delegate.cc @@ -74,12 +74,11 @@ const char kWorkerThreadSd[] = "O:SYG:SYD:(A;;GA;;;SY)(A;;0x120801;;;BA)"; // process. bool CreateRestrictedToken(ScopedHandle* token_out) { // Create a token representing LocalService account. - HANDLE temp_handle; + ScopedHandle token; if (!LogonUser(L"LocalService", L"NT AUTHORITY", NULL, LOGON32_LOGON_SERVICE, - LOGON32_PROVIDER_DEFAULT, &temp_handle)) { + LOGON32_PROVIDER_DEFAULT, token.Receive())) { return false; } - ScopedHandle token(temp_handle); sandbox::RestrictedToken restricted_token; if (restricted_token.Init(token) != ERROR_SUCCESS) @@ -98,12 +97,8 @@ bool CreateRestrictedToken(ScopedHandle* token_out) { } // Return the resulting token. - if (restricted_token.GetRestrictedTokenHandle(&temp_handle) == - ERROR_SUCCESS) { - token_out->Set(temp_handle); - return true; - } - return false; + return restricted_token.GetRestrictedTokenHandle(token_out->Receive()) == + ERROR_SUCCESS; } // Creates a window station with a given name and the default desktop giving @@ -278,13 +273,12 @@ void UnprivilegedProcessDelegate::LaunchProcess( base::AutoLock lock(g_inherit_handles_lock.Get()); // Create a connected IPC channel. - HANDLE temp_handle; - if (!CreateConnectedIpcChannel(io_task_runner_, this, &temp_handle, + ScopedHandle client; + if (!CreateConnectedIpcChannel(io_task_runner_, this, client.Receive(), &server)) { ReportFatalError(); return; } - ScopedHandle client(temp_handle); // Convert the handle value into a decimal integer. Handle values are 32bit // even on 64bit platforms. @@ -403,11 +397,11 @@ void UnprivilegedProcessDelegate::ReportProcessLaunched( // query information about the process and duplicate handles. DWORD desired_access = SYNCHRONIZE | PROCESS_DUP_HANDLE | PROCESS_QUERY_INFORMATION; - HANDLE temp_handle; + ScopedHandle limited_handle; if (!DuplicateHandle(GetCurrentProcess(), worker_process_, GetCurrentProcess(), - &temp_handle, + limited_handle.Receive(), desired_access, FALSE, 0)) { @@ -415,7 +409,6 @@ void UnprivilegedProcessDelegate::ReportProcessLaunched( ReportFatalError(); return; } - ScopedHandle limited_handle(temp_handle); event_handler_->OnProcessLaunched(limited_handle.Pass()); } diff --git a/remoting/host/win/worker_process_launcher_unittest.cc b/remoting/host/win/worker_process_launcher_unittest.cc index cd92b11..3737f8b 100644 --- a/remoting/host/win/worker_process_launcher_unittest.cc +++ b/remoting/host/win/worker_process_launcher_unittest.cc @@ -343,7 +343,7 @@ void WorkerProcessLauncherTest::DoLaunchProcess() { STARTUPINFOW startup_info = { 0 }; startup_info.cb = sizeof(startup_info); - PROCESS_INFORMATION temp_process_info = {}; + base::win::ScopedProcessInformation process_information; ASSERT_TRUE(CreateProcess(NULL, notepad, NULL, // default process attibutes @@ -353,8 +353,7 @@ void WorkerProcessLauncherTest::DoLaunchProcess() { NULL, // no environment NULL, // default current directory &startup_info, - &temp_process_info)); - base::win::ScopedProcessInformation process_information(temp_process_info); + process_information.Receive())); worker_process_.Set(process_information.TakeProcessHandle()); ASSERT_TRUE(worker_process_.IsValid()); @@ -369,15 +368,14 @@ void WorkerProcessLauncherTest::DoLaunchProcess() { this, task_runner_)); - HANDLE temp_handle; + ScopedHandle copy; ASSERT_TRUE(DuplicateHandle(GetCurrentProcess(), worker_process_, GetCurrentProcess(), - &temp_handle, + copy.Receive(), 0, FALSE, DUPLICATE_SAME_ACCESS)); - ScopedHandle copy(temp_handle); event_handler_->OnProcessLaunched(copy.Pass()); } diff --git a/remoting/host/win/wts_session_process_delegate.cc b/remoting/host/win/wts_session_process_delegate.cc index 0781649..8cb7325 100644 --- a/remoting/host/win/wts_session_process_delegate.cc +++ b/remoting/host/win/wts_session_process_delegate.cc @@ -513,11 +513,11 @@ void WtsSessionProcessDelegate::Core::ReportProcessLaunched( // query information about the process and duplicate handles. DWORD desired_access = SYNCHRONIZE | PROCESS_DUP_HANDLE | PROCESS_QUERY_INFORMATION; - HANDLE temp_handle; + ScopedHandle limited_handle; if (!DuplicateHandle(GetCurrentProcess(), worker_process_, GetCurrentProcess(), - &temp_handle, + limited_handle.Receive(), desired_access, FALSE, 0)) { @@ -525,7 +525,6 @@ void WtsSessionProcessDelegate::Core::ReportProcessLaunched( ReportFatalError(); return; } - ScopedHandle limited_handle(temp_handle); event_handler_->OnProcessLaunched(limited_handle.Pass()); } diff --git a/sandbox/win/src/Wow64.cc b/sandbox/win/src/Wow64.cc index a710d75..39108e5 100644 --- a/sandbox/win/src/Wow64.cc +++ b/sandbox/win/src/Wow64.cc @@ -157,11 +157,10 @@ bool Wow64::RunWowHelper(void* buffer) { STARTUPINFO startup_info = {0}; startup_info.cb = sizeof(startup_info); - PROCESS_INFORMATION temp_process_info = {}; + base::win::ScopedProcessInformation process_info; if (!::CreateProcess(NULL, writable_command.get(), NULL, NULL, FALSE, 0, NULL, - NULL, &startup_info, &temp_process_info)) + NULL, &startup_info, process_info.Receive())) return false; - base::win::ScopedProcessInformation process_info(temp_process_info); DWORD reason = ::WaitForSingleObject(process_info.process_handle(), INFINITE); diff --git a/sandbox/win/src/job_unittest.cc b/sandbox/win/src/job_unittest.cc index 3b2b331..597c86c 100644 --- a/sandbox/win/src/job_unittest.cc +++ b/sandbox/win/src/job_unittest.cc @@ -164,11 +164,10 @@ TEST(JobTest, ProcessInJob) { wchar_t notepad[] = L"notepad"; STARTUPINFO si = { sizeof(si) }; - PROCESS_INFORMATION temp_process_info = {}; + base::win::ScopedProcessInformation pi; result = ::CreateProcess(NULL, notepad, NULL, NULL, FALSE, 0, NULL, NULL, &si, - &temp_process_info); + pi.Receive()); ASSERT_TRUE(result); - base::win::ScopedProcessInformation pi(temp_process_info); ASSERT_EQ(ERROR_SUCCESS, job.AssignProcessToJob(pi.process_handle())); // Get the job handle. diff --git a/sandbox/win/src/policy_target_test.cc b/sandbox/win/src/policy_target_test.cc index 1e29df2..ee28260 100644 --- a/sandbox/win/src/policy_target_test.cc +++ b/sandbox/win/src/policy_target_test.cc @@ -150,11 +150,10 @@ SBOX_TESTS_COMMAND int PolicyTargetTest_process(int argc, wchar_t **argv) { // Use default values to create a new process. STARTUPINFO startup_info = {0}; startup_info.cb = sizeof(startup_info); - PROCESS_INFORMATION temp_process_info = {}; + base::win::ScopedProcessInformation process_info; if (!::CreateProcessW(L"foo.exe", L"foo.exe", NULL, NULL, FALSE, 0, - NULL, NULL, &startup_info, &temp_process_info)) + NULL, NULL, &startup_info, process_info.Receive())) return SBOX_TEST_SUCCEEDED; - base::win::ScopedProcessInformation process_info(temp_process_info); return SBOX_TEST_FAILED; } @@ -240,14 +239,11 @@ TEST(PolicyTargetTest, DesktopPolicy) { TargetPolicy* policy = broker->CreatePolicy(); policy->SetAlternateDesktop(false); policy->SetTokenLevel(USER_INTERACTIVE, USER_LOCKDOWN); - PROCESS_INFORMATION temp_process_info = {}; result = broker->SpawnTarget(prog_name, arguments.c_str(), policy, - &temp_process_info); + target.Receive()); policy->Release(); EXPECT_EQ(SBOX_ALL_OK, result); - if (result == SBOX_ALL_OK) - target.Set(temp_process_info); EXPECT_EQ(1, ::ResumeThread(target.thread_handle())); @@ -303,14 +299,11 @@ TEST(PolicyTargetTest, WinstaPolicy) { TargetPolicy* policy = broker->CreatePolicy(); policy->SetAlternateDesktop(true); policy->SetTokenLevel(USER_INTERACTIVE, USER_LOCKDOWN); - PROCESS_INFORMATION temp_process_info = {}; result = broker->SpawnTarget(prog_name, arguments.c_str(), policy, - &temp_process_info); + target.Receive()); policy->Release(); EXPECT_EQ(SBOX_ALL_OK, result); - if (result == SBOX_ALL_OK) - target.Set(temp_process_info); EXPECT_EQ(1, ::ResumeThread(target.thread_handle())); diff --git a/sandbox/win/src/process_policy_test.cc b/sandbox/win/src/process_policy_test.cc index a03e0be..babe321 100644 --- a/sandbox/win/src/process_policy_test.cc +++ b/sandbox/win/src/process_policy_test.cc @@ -50,12 +50,8 @@ sandbox::SboxTestResult CreateProcessHelper(const string16& exe, // Create the process with the unicode version of the API. sandbox::SboxTestResult ret1 = sandbox::SBOX_TEST_FAILED; - PROCESS_INFORMATION temp_process_info = {}; - if (::CreateProcessW(exe_name, const_cast<wchar_t*>(cmd_line), NULL, NULL, - FALSE, 0, NULL, NULL, &si, &temp_process_info)) { - pi.Set(temp_process_info); - ret1 = sandbox::SBOX_TEST_SUCCEEDED; - } else { + if (!::CreateProcessW(exe_name, const_cast<wchar_t*>(cmd_line), NULL, NULL, + FALSE, 0, NULL, NULL, &si, pi.Receive())) { DWORD last_error = GetLastError(); if ((ERROR_NOT_ENOUGH_QUOTA == last_error) || (ERROR_ACCESS_DENIED == last_error) || @@ -64,6 +60,8 @@ sandbox::SboxTestResult CreateProcessHelper(const string16& exe, } else { ret1 = sandbox::SBOX_TEST_FAILED; } + } else { + ret1 = sandbox::SBOX_TEST_SUCCEEDED; } pi.Close(); @@ -75,13 +73,10 @@ sandbox::SboxTestResult CreateProcessHelper(const string16& exe, std::string narrow_cmd_line; if (cmd_line) narrow_cmd_line = base::SysWideToMultiByte(cmd_line, CP_UTF8); - if (::CreateProcessA( + if (!::CreateProcessA( exe_name ? base::SysWideToMultiByte(exe_name, CP_UTF8).c_str() : NULL, cmd_line ? const_cast<char*>(narrow_cmd_line.c_str()) : NULL, - NULL, NULL, FALSE, 0, NULL, NULL, &sia, &temp_process_info)) { - pi.Set(temp_process_info); - ret2 = sandbox::SBOX_TEST_SUCCEEDED; - } else { + NULL, NULL, FALSE, 0, NULL, NULL, &sia, pi.Receive())) { DWORD last_error = GetLastError(); if ((ERROR_NOT_ENOUGH_QUOTA == last_error) || (ERROR_ACCESS_DENIED == last_error) || @@ -90,6 +85,8 @@ sandbox::SboxTestResult CreateProcessHelper(const string16& exe, } else { ret2 = sandbox::SBOX_TEST_FAILED; } + } else { + ret2 = sandbox::SBOX_TEST_SUCCEEDED; } if (ret1 == ret2) @@ -218,14 +215,13 @@ SBOX_TESTS_COMMAND int Process_GetChildProcessToken(int argc, wchar_t **argv) { string16 path = MakeFullPathToSystem32(argv[0]); + base::win::ScopedProcessInformation pi; STARTUPINFOW si = {sizeof(si)}; - PROCESS_INFORMATION temp_process_info = {}; if (!::CreateProcessW(path.c_str(), NULL, NULL, NULL, FALSE, CREATE_SUSPENDED, - NULL, NULL, &si, &temp_process_info)) { + NULL, NULL, &si, pi.Receive())) { return SBOX_TEST_FAILED; } - base::win::ScopedProcessInformation pi(temp_process_info); HANDLE token = NULL; BOOL result = diff --git a/sandbox/win/src/restricted_token_utils.cc b/sandbox/win/src/restricted_token_utils.cc index f30a8a6..5f1a246 100644 --- a/sandbox/win/src/restricted_token_utils.cc +++ b/sandbox/win/src/restricted_token_utils.cc @@ -183,7 +183,7 @@ DWORD StartRestrictedProcessInJob(wchar_t *command_line, // Start the process STARTUPINFO startup_info = {0}; - PROCESS_INFORMATION temp_process_info = {}; + base::win::ScopedProcessInformation process_info; DWORD flags = CREATE_SUSPENDED; if (base::win::GetVersion() < base::win::VERSION_WIN8) { @@ -202,10 +202,9 @@ DWORD StartRestrictedProcessInJob(wchar_t *command_line, NULL, // Use the environment of the caller. NULL, // Use current directory of the caller. &startup_info, - &temp_process_info)) { + process_info.Receive())) { return ::GetLastError(); } - base::win::ScopedProcessInformation process_info(temp_process_info); // Change the token of the main thread of the new process for the // impersonation token with more rights. diff --git a/sandbox/win/src/target_process.cc b/sandbox/win/src/target_process.cc index a2d630c..9300cce 100644 --- a/sandbox/win/src/target_process.cc +++ b/sandbox/win/src/target_process.cc @@ -131,7 +131,8 @@ DWORD TargetProcess::Create(const wchar_t* exe_path, flags |= CREATE_BREAKAWAY_FROM_JOB; } - PROCESS_INFORMATION temp_process_info = {}; + base::win::ScopedProcessInformation process_info; + if (!::CreateProcessAsUserW(lockdown_token_, exe_path, cmd_line.get(), @@ -142,10 +143,9 @@ DWORD TargetProcess::Create(const wchar_t* exe_path, NULL, // Use the environment of the caller. NULL, // Use current directory of the caller. startup_info.startup_info(), - &temp_process_info)) { + process_info.Receive())) { return ::GetLastError(); } - base::win::ScopedProcessInformation process_info(temp_process_info); lockdown_token_.Close(); DWORD win_result = ERROR_SUCCESS; diff --git a/tools/gn/function_exec_script.cc b/tools/gn/function_exec_script.cc index cca62a3..b382575 100644 --- a/tools/gn/function_exec_script.cc +++ b/tools/gn/function_exec_script.cc @@ -85,7 +85,8 @@ bool ExecProcess(const CommandLine& cmdline, base::FilePath::StringType cmdline_str(cmdline.GetCommandLineString()); - STARTUPINFO start_info = {}; + base::win::ScopedProcessInformation proc_info; + STARTUPINFO start_info = { 0 }; start_info.cb = sizeof(STARTUPINFO); start_info.hStdOutput = out_write; @@ -97,17 +98,15 @@ bool ExecProcess(const CommandLine& cmdline, start_info.dwFlags |= STARTF_USESTDHANDLES; // Create the child process. - PROCESS_INFORMATION temp_process_info = {}; if (!CreateProcess(NULL, &cmdline_str[0], NULL, NULL, TRUE, // Handles are inherited. 0, NULL, startup_dir.value().c_str(), - &start_info, &temp_process_info)) { + &start_info, proc_info.Receive())) { return false; } - base::win::ScopedProcessInformation proc_info(temp_process_info); // Close our writing end of pipes now. Otherwise later read would not be able // to detect end of child's output. diff --git a/win8/delegate_execute/command_execute_impl.cc b/win8/delegate_execute/command_execute_impl.cc index f52037b..5f6cc19 100644 --- a/win8/delegate_execute/command_execute_impl.cc +++ b/win8/delegate_execute/command_execute_impl.cc @@ -487,13 +487,12 @@ HRESULT CommandExecuteImpl::LaunchDesktopChrome() { AtlTrace("Formatted command line is %ls\n", command_line.c_str()); - PROCESS_INFORMATION temp_process_info = {}; + base::win::ScopedProcessInformation proc_info; BOOL ret = CreateProcess(chrome_exe_.value().c_str(), const_cast<LPWSTR>(command_line.c_str()), NULL, NULL, FALSE, 0, NULL, NULL, &start_info_, - &temp_process_info); + proc_info.Receive()); if (ret) { - base::win::ScopedProcessInformation proc_info(temp_process_info); AtlTrace("Process id is %d\n", proc_info.process_id()); AllowSetForegroundWindow(proc_info.process_id()); } else { diff --git a/win8/test/metro_registration_helper.cc b/win8/test/metro_registration_helper.cc index 75319a9..bcf5644 100644 --- a/win8/test/metro_registration_helper.cc +++ b/win8/test/metro_registration_helper.cc @@ -54,12 +54,11 @@ bool RegisterTestDefaultBrowser() { register_command.AppendArg("/RegServer"); base::win::ScopedHandle register_handle; - if (base::LaunchProcess(register_command.GetCommandLineString(), - base::LaunchOptions(), - ®ister_handle)) { + if (base::LaunchProcess(register_command, base::LaunchOptions(), + register_handle.Receive())) { int ret = 0; if (base::WaitForExitCodeWithTimeout( - register_handle.Get(), &ret, + register_handle, &ret, base::TimeDelta::FromSeconds(kRegistrationTimeoutSeconds))) { if (ret == 0) { return true; |