Skip to content

Commit

Permalink
Deprecate named base::SharedMemory.
Browse files Browse the repository at this point in the history
It's hardly used and not implemented on various platforms. (More deprecation
work here to come.)

R=brettw@chromium.org
TBR=sky@chromium.org
BUG=345734

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

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@255148 0039d316-1c4b-4281-b951-d872f2087c98
  • Loading branch information
viettrungluu@chromium.org committed Mar 5, 2014
1 parent 0b2d270 commit ff672b7
Show file tree
Hide file tree
Showing 8 changed files with 41 additions and 31 deletions.
21 changes: 13 additions & 8 deletions base/memory/shared_memory.h
Original file line number Diff line number Diff line change
Expand Up @@ -41,22 +41,25 @@ typedef ino_t SharedMemoryId;

// Options for creating a shared memory object.
struct SharedMemoryCreateOptions {
SharedMemoryCreateOptions() : name(NULL), size(0), open_existing(false),
SharedMemoryCreateOptions() : name_deprecated(NULL), size(0),
open_existing_deprecated(false),
executable(false) {}

// DEPRECATED (crbug.com/345734):
// If NULL, the object is anonymous. This pointer is owned by the caller
// and must live through the call to Create().
const std::string* name;
const std::string* name_deprecated;

// Size of the shared memory object to be created.
// When opening an existing object, this has no effect.
size_t size;

// DEPRECATED (crbug.com/345734):
// If true, and the shared memory already exists, Create() will open the
// existing shared memory and ignore the size parameter. If false,
// shared memory must not exist. This flag is meaningless unless name is
// non-NULL.
bool open_existing;
// shared memory must not exist. This flag is meaningless unless
// name_deprecated is non-NULL.
bool open_existing_deprecated;

// If true, mappings might need to be made executable later.
bool executable;
Expand Down Expand Up @@ -122,16 +125,18 @@ class BASE_EXPORT SharedMemory {
return Create(options);
}

// DEPRECATED (crbug.com/345734):
// Creates or opens a shared memory segment based on a name.
// If open_existing is true, and the shared memory already exists,
// opens the existing shared memory and ignores the size parameter.
// If open_existing is false, shared memory must not exist.
// size is the size of the block to be created.
// Returns true on success, false on failure.
bool CreateNamed(const std::string& name, bool open_existing, size_t size) {
bool CreateNamedDeprecated(
const std::string& name, bool open_existing, size_t size) {
SharedMemoryCreateOptions options;
options.name = &name;
options.open_existing = open_existing;
options.name_deprecated = &name;
options.open_existing_deprecated = open_existing;
options.size = size;
return Create(options);
}
Expand Down
2 changes: 1 addition & 1 deletion base/memory/shared_memory_android.cc
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ bool SharedMemory::Create(const SharedMemoryCreateOptions& options) {

// "name" is just a label in ashmem. It is visible in /proc/pid/maps.
mapped_file_ = ashmem_create_region(
options.name == NULL ? "" : options.name->c_str(),
options.name_deprecated == NULL ? "" : options.name_deprecated->c_str(),
options.size);
if (-1 == mapped_file_) {
DLOG(ERROR) << "Shared memory creation failed";
Expand Down
8 changes: 4 additions & 4 deletions base/memory/shared_memory_posix.cc
Original file line number Diff line number Diff line change
Expand Up @@ -136,9 +136,9 @@ bool SharedMemory::Create(const SharedMemoryCreateOptions& options) {
ScopedFD readonly_fd(&readonly_fd_storage);

FilePath path;
if (options.name == NULL || options.name->empty()) {
if (options.name_deprecated == NULL || options.name_deprecated->empty()) {
// It doesn't make sense to have a open-existing private piece of shmem
DCHECK(!options.open_existing);
DCHECK(!options.open_existing_deprecated);
// Q: Why not use the shm_open() etc. APIs?
// A: Because they're limited to 4mb on OS X. FFFFFFFUUUUUUUUUUU
fp.reset(base::CreateAndOpenTemporaryShmemFile(&path, options.executable));
Expand All @@ -157,7 +157,7 @@ bool SharedMemory::Create(const SharedMemoryCreateOptions& options) {
PLOG(WARNING) << "unlink";
}
} else {
if (!FilePathForMemoryName(*options.name, &path))
if (!FilePathForMemoryName(*options.name_deprecated, &path))
return false;

// Make sure that the file is opened without any permission
Expand All @@ -167,7 +167,7 @@ bool SharedMemory::Create(const SharedMemoryCreateOptions& options) {
// First, try to create the file.
int fd = HANDLE_EINTR(
open(path.value().c_str(), O_RDWR | O_CREAT | O_EXCL, kOwnerOnly));
if (fd == -1 && options.open_existing) {
if (fd == -1 && options.open_existing_deprecated) {
// If this doesn't work, try and open an existing file in append mode.
// Opening an existing file in a world writable directory has two main
// security implications:
Expand Down
24 changes: 12 additions & 12 deletions base/memory/shared_memory_unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,7 @@ class MultipleThreadMain : public PlatformThread::Delegate {
#endif
const uint32 kDataSize = 1024;
SharedMemory memory;
bool rv = memory.CreateNamed(s_test_name_, true, kDataSize);
bool rv = memory.CreateNamedDeprecated(s_test_name_, true, kDataSize);
EXPECT_TRUE(rv);
rv = memory.Map(kDataSize);
EXPECT_TRUE(rv);
Expand Down Expand Up @@ -109,8 +109,8 @@ class MultipleLockThread : public PlatformThread::Delegate {
SharedMemoryHandle handle = NULL;
{
SharedMemory memory1;
EXPECT_TRUE(memory1.CreateNamed("SharedMemoryMultipleLockThreadTest",
true, kDataSize));
EXPECT_TRUE(memory1.CreateNamedDeprecated(
"SharedMemoryMultipleLockThreadTest", true, kDataSize));
EXPECT_TRUE(memory1.ShareToProcess(GetCurrentProcess(), &handle));
// TODO(paulg): Implement this once we have a posix version of
// SharedMemory::ShareToProcess.
Expand Down Expand Up @@ -143,7 +143,7 @@ class MultipleLockThread : public PlatformThread::Delegate {
} // namespace

// Android doesn't support SharedMemory::Open/Delete/
// CreateNamed(openExisting=true)
// CreateNamedDeprecated(openExisting=true)
#if !defined(OS_ANDROID)
TEST(SharedMemoryTest, OpenClose) {
const uint32 kDataSize = 1024;
Expand All @@ -158,7 +158,7 @@ TEST(SharedMemoryTest, OpenClose) {
EXPECT_TRUE(rv);
rv = memory1.Open(test_name, false);
EXPECT_FALSE(rv);
rv = memory1.CreateNamed(test_name, false, kDataSize);
rv = memory1.CreateNamedDeprecated(test_name, false, kDataSize);
EXPECT_TRUE(rv);
rv = memory1.Map(kDataSize);
EXPECT_TRUE(rv);
Expand Down Expand Up @@ -201,10 +201,10 @@ TEST(SharedMemoryTest, OpenExclusive) {
<< Time::Now().ToDoubleT();
std::string test_name = test_name_stream.str();

// Open two handles to a memory segment and check that open_existing works
// as expected.
// Open two handles to a memory segment and check that
// open_existing_deprecated works as expected.
SharedMemory memory1;
bool rv = memory1.CreateNamed(test_name, false, kDataSize);
bool rv = memory1.CreateNamedDeprecated(test_name, false, kDataSize);
EXPECT_TRUE(rv);

// Memory1 knows it's size because it created it.
Expand All @@ -224,11 +224,11 @@ TEST(SharedMemoryTest, OpenExclusive) {

SharedMemory memory2;
// Should not be able to create if openExisting is false.
rv = memory2.CreateNamed(test_name, false, kDataSize2);
rv = memory2.CreateNamedDeprecated(test_name, false, kDataSize2);
EXPECT_FALSE(rv);

// Should be able to create with openExisting true.
rv = memory2.CreateNamed(test_name, true, kDataSize2);
rv = memory2.CreateNamedDeprecated(test_name, true, kDataSize2);
EXPECT_TRUE(rv);

// Memory2 shouldn't know the size because we didn't create it.
Expand Down Expand Up @@ -561,7 +561,7 @@ TEST(SharedMemoryTest, FilePermissionsNamed) {
options.size = kTestSize;
std::string shared_mem_name = "shared_perm_test-" + IntToString(getpid()) +
"-" + Uint64ToString(RandUint64());
options.name = &shared_mem_name;
options.name_deprecated = &shared_mem_name;
// Set a file mode creation mask that gives all permissions.
ScopedUmaskSetter permissive_mask(S_IWGRP | S_IWOTH);

Expand Down Expand Up @@ -613,7 +613,7 @@ class SharedMemoryProcessTest : public MultiProcessTest {
#endif
const uint32 kDataSize = 1024;
SharedMemory memory;
bool rv = memory.CreateNamed(s_test_name_, true, kDataSize);
bool rv = memory.CreateNamedDeprecated(s_test_name_, true, kDataSize);
EXPECT_TRUE(rv);
if (rv != true)
errors++;
Expand Down
5 changes: 3 additions & 2 deletions base/memory/shared_memory_win.cc
Original file line number Diff line number Diff line change
Expand Up @@ -113,7 +113,8 @@ bool SharedMemory::Create(const SharedMemoryCreateOptions& options) {
return false;

size_t rounded_size = (options.size + kSectionMask) & ~kSectionMask;
name_ = ASCIIToWide(options.name == NULL ? "" : *options.name);
name_ = ASCIIToWide(options.name_deprecated == NULL ? "" :
*options.name_deprecated);
mapped_file_ = CreateFileMapping(INVALID_HANDLE_VALUE, NULL,
PAGE_READWRITE, 0, static_cast<DWORD>(rounded_size),
name_.empty() ? NULL : name_.c_str());
Expand All @@ -127,7 +128,7 @@ bool SharedMemory::Create(const SharedMemoryCreateOptions& options) {
// If the file already existed, set requested_size_ to 0 to show that
// we don't know the size.
requested_size_ = 0;
if (!options.open_existing) {
if (!options.open_existing_deprecated) {
Close();
return false;
}
Expand Down
2 changes: 1 addition & 1 deletion base/metrics/stats_table.cc
Original file line number Diff line number Diff line change
Expand Up @@ -217,7 +217,7 @@ SharedMemory* StatsTable::Internal::CreateSharedMemory(const std::string& name,
return shared_memory.release();
#elif defined(OS_WIN)
scoped_ptr<SharedMemory> shared_memory(new SharedMemory());
if (!shared_memory->CreateNamed(name, true, size))
if (!shared_memory->CreateNamedDeprecated(name, true, size))
return NULL;
return shared_memory.release();
#endif
Expand Down
5 changes: 3 additions & 2 deletions chrome/common/service_process_util.cc
Original file line number Diff line number Diff line change
Expand Up @@ -223,8 +223,9 @@ bool ServiceProcessState::CreateSharedData() {
return false;

uint32 alloc_size = sizeof(ServiceProcessSharedData);
if (!shared_mem_service_data->CreateNamed(GetServiceProcessSharedMemName(),
true, alloc_size))
// TODO(viettrungluu): Named shared memory is deprecated (crbug.com/345734).
if (!shared_mem_service_data->CreateNamedDeprecated
(GetServiceProcessSharedMemName(), true, alloc_size))
return false;

if (!shared_mem_service_data->Map(alloc_size))
Expand Down
5 changes: 4 additions & 1 deletion content/child/resource_dispatcher_unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -307,7 +307,10 @@ class DeferredResourceLoadingTest : public ResourceDispatcherTest,
virtual void SetUp() OVERRIDE {
ResourceDispatcherTest::SetUp();
shared_handle_.Delete(kShmemSegmentName);
EXPECT_TRUE(shared_handle_.CreateNamed(kShmemSegmentName, false, 100));
// TODO(viettrungluu): Named shared memory is deprecated (crbug.com/345734).
// (I don't think we even need it here.)
EXPECT_TRUE(shared_handle_.CreateNamedDeprecated(kShmemSegmentName, false,
100));
}

virtual void TearDown() OVERRIDE {
Expand Down

0 comments on commit ff672b7

Please sign in to comment.