Skip to content

Commit

Permalink
Make SharedMemoryHandle a class on windows.
Browse files Browse the repository at this point in the history
This CL is intended to be a refactor and should not introduce any behavior
changes.

Previously, SharedMemoryhandle was typedefed to HANDLE. Making it a class allows
us to add metainformation about the process in which the HANDLE is valid. This
will be used in the future by Chrome's IPC system to automatically duplicate
HANDLEs into their destination process.

BUG=493414, 535028

Review URL: https://codereview.chromium.org/1320783002

Cr-Commit-Position: refs/heads/master@{#350932}
  • Loading branch information
erikchen authored and Commit bot committed Sep 25, 2015
1 parent 997afdd commit 5ea2ab7
Show file tree
Hide file tree
Showing 37 changed files with 300 additions and 134 deletions.
1 change: 1 addition & 0 deletions base/base.gypi
Original file line number Diff line number Diff line change
Expand Up @@ -361,6 +361,7 @@
'memory/shared_memory_android.cc',
'memory/shared_memory_handle.h',
'memory/shared_memory_handle_mac.cc',
'memory/shared_memory_handle_win.cc',
'memory/shared_memory_mac.cc',
'memory/shared_memory_nacl.cc',
'memory/shared_memory_posix.cc',
Expand Down
1 change: 1 addition & 0 deletions base/memory/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@ source_set("memory") {
"shared_memory_android.cc",
"shared_memory_handle.h",
"shared_memory_handle_mac.cc",
"shared_memory_handle_win.cc",
"shared_memory_mac.cc",
"shared_memory_nacl.cc",
"shared_memory_posix.cc",
Expand Down
47 changes: 44 additions & 3 deletions base/memory/shared_memory_handle.h
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@

#if defined(OS_WIN)
#include <windows.h>
#include "base/process/process_handle.h"
#elif defined(OS_MACOSX)
#include <sys/types.h>
#include "base/base_export.h"
Expand All @@ -25,10 +26,50 @@ class Pickle;

// SharedMemoryHandle is a platform specific type which represents
// the underlying OS handle to a shared memory segment.
#if defined(OS_WIN)
typedef HANDLE SharedMemoryHandle;
#elif defined(OS_POSIX) && !defined(OS_MACOSX)
#if defined(OS_POSIX) && !defined(OS_MACOSX)
typedef FileDescriptor SharedMemoryHandle;
#elif defined(OS_WIN)
class BASE_EXPORT SharedMemoryHandle {
public:
// The default constructor returns an invalid SharedMemoryHandle.
SharedMemoryHandle();
SharedMemoryHandle(HANDLE h, base::ProcessId pid);

// Standard copy constructor. The new instance shares the underlying OS
// primitives.
SharedMemoryHandle(const SharedMemoryHandle& handle);

// Standard assignment operator. The updated instance shares the underlying
// OS primitives.
SharedMemoryHandle& operator=(const SharedMemoryHandle& handle);

// Comparison operators.
bool operator==(const SharedMemoryHandle& handle) const;
bool operator!=(const SharedMemoryHandle& handle) const;

// Closes the underlying OS resources.
void Close() const;

// Whether the underlying OS primitive is valid.
bool IsValid() const;

// Whether |pid_| is the same as the current process's id.
bool BelongsToCurrentProcess() const;

// Whether handle_ needs to be duplicated into the destination process when
// an instance of this class is passed over a Chrome IPC channel.
bool NeedsBrokering() const;

HANDLE GetHandle() const;
base::ProcessId GetPID() const;

private:
HANDLE handle_;

// The process in which |handle_| is valid and can be used. If |handle_| is
// invalid, this will be kNullProcessId.
base::ProcessId pid_;
};
#else
class BASE_EXPORT SharedMemoryHandle {
public:
Expand Down
68 changes: 68 additions & 0 deletions base/memory/shared_memory_handle_win.cc
Original file line number Diff line number Diff line change
@@ -0,0 +1,68 @@
// Copyright 2015 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/memory/shared_memory_handle.h"

#include "base/logging.h"

namespace base {

SharedMemoryHandle::SharedMemoryHandle()
: handle_(nullptr), pid_(kNullProcessId) {}

SharedMemoryHandle::SharedMemoryHandle(HANDLE h, base::ProcessId pid)
: handle_(h), pid_(pid) {}

SharedMemoryHandle::SharedMemoryHandle(const SharedMemoryHandle& handle)
: handle_(handle.handle_), pid_(handle.pid_) {}

SharedMemoryHandle& SharedMemoryHandle::operator=(
const SharedMemoryHandle& handle) {
if (this == &handle)
return *this;

handle_ = handle.handle_;
pid_ = handle.pid_;
return *this;
}

bool SharedMemoryHandle::operator==(const SharedMemoryHandle& handle) const {
// Invalid handles are always equal.
if (!IsValid() && !handle.IsValid())
return true;

return handle_ == handle.handle_ && pid_ == handle.pid_;
}

bool SharedMemoryHandle::operator!=(const SharedMemoryHandle& handle) const {
return !(*this == handle);
}

void SharedMemoryHandle::Close() const {
DCHECK(handle_ != nullptr);
DCHECK(BelongsToCurrentProcess());
::CloseHandle(handle_);
}

bool SharedMemoryHandle::IsValid() const {
return handle_ != nullptr;
}

bool SharedMemoryHandle::BelongsToCurrentProcess() const {
return pid_ == base::GetCurrentProcId();
}

bool SharedMemoryHandle::NeedsBrokering() const {
return false;
}

HANDLE SharedMemoryHandle::GetHandle() const {
return handle_;
}

base::ProcessId SharedMemoryHandle::GetPID() const {
return pid_;
}

} // namespace base
22 changes: 7 additions & 15 deletions base/memory/shared_memory_unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -369,28 +369,20 @@ TEST(SharedMemoryTest, ShareReadOnly) {
EXPECT_EQ(0, munmap(writable, readonly_shmem.mapped_size()));

#elif defined(OS_WIN)
EXPECT_EQ(NULL, MapViewOfFile(handle, FILE_MAP_WRITE, 0, 0, 0))
EXPECT_EQ(NULL, MapViewOfFile(handle.GetHandle(), FILE_MAP_WRITE, 0, 0, 0))
<< "Shouldn't be able to map memory writable.";

HANDLE temp_handle;
BOOL rv = ::DuplicateHandle(GetCurrentProcess(),
handle,
GetCurrentProcess(),
&temp_handle,
FILE_MAP_ALL_ACCESS,
false,
0);
BOOL rv = ::DuplicateHandle(GetCurrentProcess(), handle.GetHandle(),
GetCurrentProcess(), &temp_handle,
FILE_MAP_ALL_ACCESS, false, 0);
EXPECT_EQ(FALSE, rv)
<< "Shouldn't be able to duplicate the handle into a writable one.";
if (rv)
win::ScopedHandle writable_handle(temp_handle);
rv = ::DuplicateHandle(GetCurrentProcess(),
handle,
GetCurrentProcess(),
&temp_handle,
FILE_MAP_READ,
false,
0);
rv = ::DuplicateHandle(GetCurrentProcess(), handle.GetHandle(),
GetCurrentProcess(), &temp_handle, FILE_MAP_READ,
false, 0);
EXPECT_EQ(TRUE, rv)
<< "Should be able to duplicate the handle into a readable one.";
if (rv)
Expand Down
40 changes: 20 additions & 20 deletions base/memory/shared_memory_win.cc
Original file line number Diff line number Diff line change
Expand Up @@ -45,11 +45,12 @@ SharedMemory::SharedMemory(const std::wstring& name)
}

SharedMemory::SharedMemory(const SharedMemoryHandle& handle, bool read_only)
: mapped_file_(handle),
: mapped_file_(handle.GetHandle()),
mapped_size_(0),
memory_(NULL),
read_only_(read_only),
requested_size_(0) {
DCHECK(!handle.IsValid() || handle.BelongsToCurrentProcess());
}

SharedMemory::SharedMemory(const SharedMemoryHandle& handle,
Expand All @@ -60,11 +61,9 @@ SharedMemory::SharedMemory(const SharedMemoryHandle& handle,
memory_(NULL),
read_only_(read_only),
requested_size_(0) {
::DuplicateHandle(process, handle,
GetCurrentProcess(), &mapped_file_,
read_only_ ? FILE_MAP_READ : FILE_MAP_READ |
FILE_MAP_WRITE,
FALSE, 0);
::DuplicateHandle(
process, handle.GetHandle(), GetCurrentProcess(), &mapped_file_,
read_only_ ? FILE_MAP_READ : FILE_MAP_READ | FILE_MAP_WRITE, FALSE, 0);
}

SharedMemory::~SharedMemory() {
Expand All @@ -74,18 +73,17 @@ SharedMemory::~SharedMemory() {

// static
bool SharedMemory::IsHandleValid(const SharedMemoryHandle& handle) {
return handle != NULL;
return handle.IsValid();
}

// static
SharedMemoryHandle SharedMemory::NULLHandle() {
return NULL;
return SharedMemoryHandle();
}

// static
void SharedMemory::CloseHandle(const SharedMemoryHandle& handle) {
DCHECK(handle != NULL);
::CloseHandle(handle);
handle.Close();
}

// static
Expand All @@ -98,13 +96,15 @@ size_t SharedMemory::GetHandleLimit() {
// static
SharedMemoryHandle SharedMemory::DuplicateHandle(
const SharedMemoryHandle& handle) {
DCHECK(handle.BelongsToCurrentProcess());
HANDLE duped_handle;
ProcessHandle process = GetCurrentProcess();
SharedMemoryHandle duped_handle;
BOOL success = ::DuplicateHandle(process, handle, process, &duped_handle, 0,
FALSE, DUPLICATE_SAME_ACCESS);
BOOL success =
::DuplicateHandle(process, handle.GetHandle(), process, &duped_handle, 0,
FALSE, DUPLICATE_SAME_ACCESS);
if (success)
return duped_handle;
return NULLHandle();
return SharedMemoryHandle(duped_handle, GetCurrentProcId());
return SharedMemoryHandle();
}

bool SharedMemory::CreateAndMapAnonymous(size_t size) {
Expand Down Expand Up @@ -230,7 +230,7 @@ bool SharedMemory::ShareToProcessCommon(ProcessHandle process,
SharedMemoryHandle* new_handle,
bool close_self,
ShareMode share_mode) {
*new_handle = 0;
*new_handle = SharedMemoryHandle();
DWORD access = FILE_MAP_READ;
DWORD options = 0;
HANDLE mapped_file = mapped_file_;
Expand All @@ -245,28 +245,28 @@ bool SharedMemory::ShareToProcessCommon(ProcessHandle process,
}

if (process == GetCurrentProcess() && close_self) {
*new_handle = mapped_file;
*new_handle = SharedMemoryHandle(mapped_file, base::GetCurrentProcId());
return true;
}

if (!::DuplicateHandle(GetCurrentProcess(), mapped_file, process, &result,
access, FALSE, options)) {
return false;
}
*new_handle = result;
*new_handle = SharedMemoryHandle(result, base::GetProcId(process));
return true;
}


void SharedMemory::Close() {
if (mapped_file_ != NULL) {
CloseHandle(mapped_file_);
::CloseHandle(mapped_file_);
mapped_file_ = NULL;
}
}

SharedMemoryHandle SharedMemory::handle() const {
return mapped_file_;
return SharedMemoryHandle(mapped_file_, base::GetCurrentProcId());
}

} // namespace base
4 changes: 3 additions & 1 deletion components/mus/gles2/command_buffer_driver.cc
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
#include "base/bind.h"
#include "base/macros.h"
#include "base/memory/shared_memory.h"
#include "base/process/process_handle.h"
#include "components/mus/gles2/command_buffer_type_conversions.h"
#include "components/mus/gles2/gpu_memory_tracker.h"
#include "components/mus/gles2/gpu_state.h"
Expand Down Expand Up @@ -243,7 +244,8 @@ void CommandBufferDriver::CreateImage(int32_t id,
}

#if defined(OS_WIN)
gfx_handle.handle = platform_handle;
gfx_handle.handle =
base::SharedMemoryHandle(platform_handle, base::GetCurrentProcId());
#else
gfx_handle.handle = base::FileDescriptor(platform_handle, false);
#endif
Expand Down
2 changes: 1 addition & 1 deletion components/nacl/loader/nacl_ipc_adapter.cc
Original file line number Diff line number Diff line change
Expand Up @@ -545,7 +545,7 @@ bool NaClIPCAdapter::RewriteMessage(const IPC::Message& msg, uint32_t type) {
uint32_t size = iter->size();
nacl_desc.reset(new NaClDescWrapper(NaClDescImcShmMake(
#if defined(OS_WIN)
shm_handle,
shm_handle.GetHandle(),
#else
base::SharedMemory::GetFdFromSharedMemoryHandle(shm_handle),
#endif
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -111,7 +111,7 @@ bool PrintWebViewHelper::PrintPagesNative(blink::WebFrame* frame,
printed_page_params.content_area = content_area_in_dpi[i];
Send(new PrintHostMsg_DidPrintPage(routing_id(), printed_page_params));
// Send the rest of the pages with an invalid metafile handle.
printed_page_params.metafile_data_handle = nullptr;
printed_page_params.metafile_data_handle = base::SharedMemoryHandle();
}
return true;
}
Expand Down
2 changes: 1 addition & 1 deletion content/browser/compositor/software_output_device_win.cc
Original file line number Diff line number Diff line change
Expand Up @@ -114,7 +114,7 @@ SkCanvas* SoftwareOutputDeviceWin::BeginPaint(const gfx::Rect& damage_rect) {
if (!contents_) {
HANDLE shared_section = NULL;
if (backing_)
shared_section = backing_->GetSharedMemory()->handle();
shared_section = backing_->GetSharedMemory()->handle().GetHandle();
contents_ = skia::AdoptRef(skia::CreatePlatformCanvas(
viewport_pixel_size_.width(), viewport_pixel_size_.height(), true,
shared_section, skia::CRASH_ON_FAILURE));
Expand Down
7 changes: 3 additions & 4 deletions content/browser/renderer_host/render_process_host_impl.cc
Original file line number Diff line number Diff line change
Expand Up @@ -744,13 +744,12 @@ scoped_ptr<IPC::ChannelProxy> RenderProcessHostImpl::CreateChannelProxy(
VLOG(1) << "Mojo Channel is enabled on host";

return IPC::ChannelProxy::Create(
IPC::ChannelMojo::CreateServerFactory(
mojo_task_runner, channel_id),
IPC::ChannelMojo::CreateServerFactory(mojo_task_runner, channel_id),
this, runner.get());
}

return IPC::ChannelProxy::Create(
channel_id, IPC::Channel::MODE_SERVER, this, runner.get());
return IPC::ChannelProxy::Create(channel_id, IPC::Channel::MODE_SERVER, this,
runner.get());
}

void RenderProcessHostImpl::CreateMessageFilters() {
Expand Down
12 changes: 8 additions & 4 deletions content/common/dwrite_font_platform_win.cc
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
#include <windows.h>

#include <dwrite.h>
#include <limits>
#include <map>
#include <string>
#include <utility>
Expand Down Expand Up @@ -937,10 +938,13 @@ bool FontCollectionLoader::LoadCacheFile() {
if (font_cache_handle_string.empty())
return false;

base::SharedMemoryHandle font_cache_handle = NULL;
base::StringToUint(font_cache_handle_string,
reinterpret_cast<unsigned int*>(&font_cache_handle));
DCHECK(font_cache_handle);
unsigned int handle_uint;
base::StringToUint(font_cache_handle_string, &handle_uint);
DCHECK(handle_uint);
if (handle_uint > static_cast<unsigned int>(std::numeric_limits<long>::max()))
return false;
base::SharedMemoryHandle font_cache_handle(LongToHandle(handle_uint),
base::GetCurrentProcId());

base::SharedMemory* shared_mem = new base::SharedMemory(
font_cache_handle, true);
Expand Down
6 changes: 0 additions & 6 deletions content/common/gpu/client/gpu_channel_host.cc
Original file line number Diff line number Diff line change
Expand Up @@ -334,14 +334,8 @@ base::SharedMemoryHandle GpuChannelHost::ShareToGpuProcess(
return base::SharedMemory::NULLHandle();
peer_pid = channel_->GetPeerPID();
}
#if defined(OS_WIN)
bool success =
BrokerDuplicateHandle(source_handle, peer_pid, &target_handle,
FILE_GENERIC_READ | FILE_GENERIC_WRITE, 0);
#elif defined(OS_MACOSX)
bool success = BrokerDuplicateSharedMemoryHandle(source_handle, peer_pid,
&target_handle);
#endif
if (!success)
return base::SharedMemory::NULLHandle();

Expand Down
Loading

0 comments on commit 5ea2ab7

Please sign in to comment.