diff options
Diffstat (limited to 'base')
| -rw-r--r-- | base/base.gyp | 1 | ||||
| -rw-r--r-- | base/memory/shared_memory_unittest.cc | 12 | ||||
| -rw-r--r-- | base/process/launch.h | 3 | ||||
| -rw-r--r-- | base/process/launch_win.cc | 26 | ||||
| -rw-r--r-- | base/win/scoped_handle.h | 26 | ||||
| -rw-r--r-- | base/win/scoped_handle_unittest.cc | 31 | ||||
| -rw-r--r-- | base/win/scoped_process_information.cc | 20 | ||||
| -rw-r--r-- | base/win/scoped_process_information.h | 25 | ||||
| -rw-r--r-- | base/win/scoped_process_information_unittest.cc | 35 | ||||
| -rw-r--r-- | base/win/startup_information_unittest.cc | 6 |
10 files changed, 61 insertions, 124 deletions
diff --git a/base/base.gyp b/base/base.gyp index 71b2615..8a73064 100644 --- a/base/base.gyp +++ b/base/base.gyp @@ -680,7 +680,6 @@ '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 2b3ad7a..a1aa9e32 100644 --- a/base/memory/shared_memory_unittest.cc +++ b/base/memory/shared_memory_unittest.cc @@ -425,16 +425,18 @@ TEST(SharedMemoryTest, ShareReadOnly) { EXPECT_EQ(NULL, MapViewOfFile(handle, FILE_MAP_WRITE, 0, 0, 0)) << "Shouldn't be able to map memory writable."; - base::win::ScopedHandle writable_handle; - EXPECT_EQ(0, - ::DuplicateHandle(GetCurrentProcess(), + HANDLE temp_handle; + BOOL rv = ::DuplicateHandle(GetCurrentProcess(), handle, GetCurrentProcess, - writable_handle.Receive(), + &temp_handle, FILE_MAP_ALL_ACCESS, false, - 0)) + 0); + EXPECT_EQ(FALSE, rv) << "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 ac2df5e..f5664fb 100644 --- a/base/process/launch.h +++ b/base/process/launch.h @@ -21,6 +21,7 @@ #include "base/posix/file_descriptor_shuffle.h" #elif defined(OS_WIN) #include <windows.h> +#include "base/win/scoped_handle.h" #endif class CommandLine; @@ -146,7 +147,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, - ProcessHandle* process_handle); + win::ScopedHandle* 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 da913ef..34f84c3 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, - ProcessHandle* process_handle) { + win::ScopedHandle* 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; - base::win::ScopedProcessInformation process_info; + PROCESS_INFORMATION temp_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, - process_info.Receive()); + &temp_process_info); DestroyEnvironmentBlock(enviroment_block); if (!launched) { DPLOG(ERROR); @@ -162,11 +162,12 @@ 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, process_info.Receive())) { + &startup_info, &temp_process_info)) { DPLOG(ERROR); return false; } } + base::win::ScopedProcessInformation process_info(temp_process_info); if (options.job_handle) { if (0 == AssignProcessToJobObject(options.job_handle, @@ -184,7 +185,7 @@ bool LaunchProcess(const string16& cmdline, // If the caller wants the process handle, we won't close it. if (process_handle) - *process_handle = process_info.TakeProcessHandle(); + process_handle->Set(process_info.TakeProcessHandle()); return true; } @@ -192,7 +193,13 @@ bool LaunchProcess(const string16& cmdline, bool LaunchProcess(const CommandLine& cmdline, const LaunchOptions& options, ProcessHandle* process_handle) { - return LaunchProcess(cmdline.GetCommandLineString(), options, 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; } bool SetJobObjectLimitFlags(HANDLE job_object, DWORD limit_flags) { @@ -233,8 +240,7 @@ bool GetAppOutput(const CommandLine& cl, std::string* output) { FilePath::StringType writable_command_line_string(cl.GetCommandLineString()); - base::win::ScopedProcessInformation proc_info; - STARTUPINFO start_info = { 0 }; + STARTUPINFO start_info = {}; start_info.cb = sizeof(STARTUPINFO); start_info.hStdOutput = out_write; @@ -244,14 +250,16 @@ 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, proc_info.Receive())) { + 0, NULL, NULL, &start_info, &temp_process_info)) { 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 d3f5c22..0d038e0 100644 --- a/base/win/scoped_handle.h +++ b/base/win/scoped_handle.h @@ -39,22 +39,6 @@ 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()) { @@ -102,16 +86,6 @@ 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 deleted file mode 100644 index e0d8b46..0000000 --- a/base/win/scoped_handle_unittest.cc +++ /dev/null @@ -1,31 +0,0 @@ -// 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 cb7a30e..bb24637 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, HANDLE* target) { +bool CheckAndDuplicateHandle(HANDLE source, ScopedHandle* target) { if (!source) return true; @@ -26,7 +26,7 @@ bool CheckAndDuplicateHandle(HANDLE source, HANDLE* target) { DPLOG(ERROR) << "Failed to duplicate a handle."; return false; } - *target = temp; + target->Set(temp); return true; } @@ -36,13 +36,13 @@ ScopedProcessInformation::ScopedProcessInformation() : process_id_(0), thread_id_(0) { } -ScopedProcessInformation::~ScopedProcessInformation() { - Close(); +ScopedProcessInformation::ScopedProcessInformation( + const PROCESS_INFORMATION& process_info) : process_id_(0), thread_id_(0) { + Set(process_info); } -ScopedProcessInformation::Receiver ScopedProcessInformation::Receive() { - DCHECK(!IsValid()) << "process_information_ must be NULL"; - return Receiver(this); +ScopedProcessInformation::~ScopedProcessInformation() { + Close(); } bool ScopedProcessInformation::IsValid() const { @@ -72,10 +72,8 @@ 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_.Receive()) && - CheckAndDuplicateHandle(other.thread_handle(), - thread_handle_.Receive())) { + if (CheckAndDuplicateHandle(other.process_handle(), &process_handle_) && + CheckAndDuplicateHandle(other.thread_handle(), &thread_handle_)) { 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 1f404c2..2e24054 100644 --- a/base/win/scoped_process_information.h +++ b/base/win/scoped_process_information.h @@ -18,33 +18,10 @@ 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 b8ffc44..550076e 100644 --- a/base/win/scoped_process_information_unittest.cc +++ b/base/win/scoped_process_information_unittest.cc @@ -19,11 +19,13 @@ const DWORD kThreadId = 1234; const HANDLE kProcessHandle = reinterpret_cast<HANDLE>(7651); const HANDLE kThreadHandle = reinterpret_cast<HANDLE>(1567); -void MockCreateProcess(PROCESS_INFORMATION* process_info) { - process_info->dwProcessId = kProcessId; - process_info->dwThreadId = kThreadId; - process_info->hProcess = kProcessHandle; - process_info->hThread = kThreadHandle; +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); } } // namespace @@ -62,7 +64,7 @@ TEST_F(ScopedProcessInformationTest, InitiallyInvalid) { TEST_F(ScopedProcessInformationTest, Receive) { base::win::ScopedProcessInformation process_info; - MockCreateProcess(process_info.Receive()); + MockCreateProcess(&process_info); EXPECT_TRUE(process_info.IsValid()); EXPECT_EQ(kProcessId, process_info.process_id()); @@ -74,7 +76,7 @@ TEST_F(ScopedProcessInformationTest, Receive) { TEST_F(ScopedProcessInformationTest, TakeProcess) { base::win::ScopedProcessInformation process_info; - MockCreateProcess(process_info.Receive()); + MockCreateProcess(&process_info); HANDLE process = process_info.TakeProcessHandle(); EXPECT_EQ(kProcessHandle, process); @@ -86,7 +88,7 @@ TEST_F(ScopedProcessInformationTest, TakeProcess) { TEST_F(ScopedProcessInformationTest, TakeThread) { base::win::ScopedProcessInformation process_info; - MockCreateProcess(process_info.Receive()); + MockCreateProcess(&process_info); HANDLE thread = process_info.TakeThreadHandle(); EXPECT_EQ(kThreadHandle, thread); @@ -98,7 +100,7 @@ TEST_F(ScopedProcessInformationTest, TakeThread) { TEST_F(ScopedProcessInformationTest, TakeBoth) { base::win::ScopedProcessInformation process_info; - MockCreateProcess(process_info.Receive()); + MockCreateProcess(&process_info); HANDLE process = process_info.TakeProcessHandle(); HANDLE thread = process_info.TakeThreadHandle(); @@ -108,7 +110,7 @@ TEST_F(ScopedProcessInformationTest, TakeBoth) { TEST_F(ScopedProcessInformationTest, TakeWholeStruct) { base::win::ScopedProcessInformation process_info; - MockCreateProcess(process_info.Receive()); + MockCreateProcess(&process_info); PROCESS_INFORMATION to_discard = process_info.Take(); EXPECT_EQ(kProcessId, to_discard.dwProcessId); @@ -119,8 +121,11 @@ TEST_F(ScopedProcessInformationTest, TakeWholeStruct) { } TEST_F(ScopedProcessInformationTest, Duplicate) { + PROCESS_INFORMATION temp_process_information; + DoCreateProcess("ReturnSeven", &temp_process_information); base::win::ScopedProcessInformation process_info; - DoCreateProcess("ReturnSeven", process_info.Receive()); + process_info.Set(temp_process_information); + base::win::ScopedProcessInformation duplicate; duplicate.DuplicateFrom(process_info); @@ -146,15 +151,17 @@ TEST_F(ScopedProcessInformationTest, Duplicate) { } TEST_F(ScopedProcessInformationTest, Set) { - PROCESS_INFORMATION base_process_info = {}; + base::win::ScopedProcessInformation base_process_info; MockCreateProcess(&base_process_info); + PROCESS_INFORMATION base_struct = base_process_info.Take(); + base::win::ScopedProcessInformation process_info; - process_info.Set(base_process_info); + process_info.Set(base_struct); 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_process_info = process_info.Take(); + base_struct = process_info.Take(); } diff --git a/base/win/startup_information_unittest.cc b/base/win/startup_information_unittest.cc index 1903564..d637ebd 100644 --- a/base/win/startup_information_unittest.cc +++ b/base/win/startup_information_unittest.cc @@ -38,7 +38,6 @@ 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, @@ -65,10 +64,13 @@ 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(), - process_info.Receive())) << ::GetLastError(); + &temp_process_info)) << ::GetLastError(); + base::win::ScopedProcessInformation process_info(temp_process_info); + // Only the first event should be signalled EXPECT_EQ(WAIT_OBJECT_0, ::WaitForMultipleObjects(2, events, false, 4000)); |