Commit 1ebf2c91 authored by Brian White's avatar Brian White Committed by Commit Bot

Remove unused CreateError histograms for shared memory.

Bug: 1053139
Change-Id: Ie9cec438ebfcfe99d1068fec9c89a969e958dd1e
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2108801Reviewed-by: default avatarErik Chen <erikchen@chromium.org>
Reviewed-by: default avatarAlex Ilin <alexilin@chromium.org>
Reviewed-by: default avatarNasko Oskov <nasko@chromium.org>
Reviewed-by: default avatarDaniel Cheng <dcheng@chromium.org>
Commit-Queue: Brian White <bcwhite@chromium.org>
Cr-Commit-Position: refs/heads/master@{#752014}
parent b860a604
...@@ -23,11 +23,6 @@ namespace subtle { ...@@ -23,11 +23,6 @@ namespace subtle {
namespace { namespace {
// Emits UMA metrics about encountered errors.
void LogCreateError(PlatformSharedMemoryRegion::CreateError error) {
UMA_HISTOGRAM_ENUMERATION("SharedMemory.CreateError", error);
}
int GetAshmemRegionProtectionMask(int fd) { int GetAshmemRegionProtectionMask(int fd) {
int prot = ashmem_get_prot_region(fd); int prot = ashmem_get_prot_region(fd);
if (prot < 0) { if (prot < 0) {
...@@ -145,7 +140,6 @@ bool PlatformSharedMemoryRegion::MapAtInternal(off_t offset, ...@@ -145,7 +140,6 @@ bool PlatformSharedMemoryRegion::MapAtInternal(off_t offset,
PlatformSharedMemoryRegion PlatformSharedMemoryRegion::Create(Mode mode, PlatformSharedMemoryRegion PlatformSharedMemoryRegion::Create(Mode mode,
size_t size) { size_t size) {
if (size == 0) { if (size == 0) {
LogCreateError(PlatformSharedMemoryRegion::CreateError::SIZE_ZERO);
return {}; return {};
} }
...@@ -154,7 +148,6 @@ PlatformSharedMemoryRegion PlatformSharedMemoryRegion::Create(Mode mode, ...@@ -154,7 +148,6 @@ PlatformSharedMemoryRegion PlatformSharedMemoryRegion::Create(Mode mode,
size_t rounded_size = bits::Align(size, GetPageSize()); size_t rounded_size = bits::Align(size, GetPageSize());
if (rounded_size < size || if (rounded_size < size ||
rounded_size > static_cast<size_t>(std::numeric_limits<int>::max())) { rounded_size > static_cast<size_t>(std::numeric_limits<int>::max())) {
LogCreateError(PlatformSharedMemoryRegion::CreateError::SIZE_TOO_LARGE);
return {}; return {};
} }
...@@ -166,8 +159,6 @@ PlatformSharedMemoryRegion PlatformSharedMemoryRegion::Create(Mode mode, ...@@ -166,8 +159,6 @@ PlatformSharedMemoryRegion PlatformSharedMemoryRegion::Create(Mode mode,
int fd = ashmem_create_region( int fd = ashmem_create_region(
SharedMemoryTracker::GetDumpNameForTracing(guid).c_str(), rounded_size); SharedMemoryTracker::GetDumpNameForTracing(guid).c_str(), rounded_size);
if (fd < 0) { if (fd < 0) {
LogCreateError(
PlatformSharedMemoryRegion::CreateError::CREATE_FILE_MAPPING_FAILURE);
DPLOG(ERROR) << "ashmem_create_region failed"; DPLOG(ERROR) << "ashmem_create_region failed";
return {}; return {};
} }
...@@ -175,13 +166,10 @@ PlatformSharedMemoryRegion PlatformSharedMemoryRegion::Create(Mode mode, ...@@ -175,13 +166,10 @@ PlatformSharedMemoryRegion PlatformSharedMemoryRegion::Create(Mode mode,
ScopedFD scoped_fd(fd); ScopedFD scoped_fd(fd);
int err = ashmem_set_prot_region(scoped_fd.get(), PROT_READ | PROT_WRITE); int err = ashmem_set_prot_region(scoped_fd.get(), PROT_READ | PROT_WRITE);
if (err < 0) { if (err < 0) {
LogCreateError(
PlatformSharedMemoryRegion::CreateError::REDUCE_PERMISSIONS_FAILURE);
DPLOG(ERROR) << "ashmem_set_prot_region failed"; DPLOG(ERROR) << "ashmem_set_prot_region failed";
return {}; return {};
} }
LogCreateError(PlatformSharedMemoryRegion::CreateError::SUCCESS);
return PlatformSharedMemoryRegion(std::move(scoped_fd), mode, size, guid); return PlatformSharedMemoryRegion(std::move(scoped_fd), mode, size, guid);
} }
......
...@@ -21,13 +21,6 @@ namespace subtle { ...@@ -21,13 +21,6 @@ namespace subtle {
namespace { namespace {
void LogCreateError(PlatformSharedMemoryRegion::CreateError error,
kern_return_t mac_error) {
UMA_HISTOGRAM_ENUMERATION("SharedMemory.CreateError", error);
if (mac_error != KERN_SUCCESS)
UmaHistogramSparse("SharedMemory.CreateMacError", mac_error);
}
} // namespace } // namespace
// static // static
...@@ -165,12 +158,10 @@ bool PlatformSharedMemoryRegion::MapAtInternal(off_t offset, ...@@ -165,12 +158,10 @@ bool PlatformSharedMemoryRegion::MapAtInternal(off_t offset,
PlatformSharedMemoryRegion PlatformSharedMemoryRegion::Create(Mode mode, PlatformSharedMemoryRegion PlatformSharedMemoryRegion::Create(Mode mode,
size_t size) { size_t size) {
if (size == 0) { if (size == 0) {
LogCreateError(CreateError::SIZE_ZERO, KERN_SUCCESS);
return {}; return {};
} }
if (size > static_cast<size_t>(std::numeric_limits<int>::max())) { if (size > static_cast<size_t>(std::numeric_limits<int>::max())) {
LogCreateError(CreateError::SIZE_TOO_LARGE, KERN_SUCCESS);
return {}; return {};
} }
...@@ -185,14 +176,11 @@ PlatformSharedMemoryRegion PlatformSharedMemoryRegion::Create(Mode mode, ...@@ -185,14 +176,11 @@ PlatformSharedMemoryRegion PlatformSharedMemoryRegion::Create(Mode mode,
MAP_MEM_NAMED_CREATE | VM_PROT_READ | VM_PROT_WRITE, MAP_MEM_NAMED_CREATE | VM_PROT_READ | VM_PROT_WRITE,
mac::ScopedMachSendRight::Receiver(named_right).get(), mac::ScopedMachSendRight::Receiver(named_right).get(),
MACH_PORT_NULL); // Parent handle. MACH_PORT_NULL); // Parent handle.
if (kr != KERN_SUCCESS)
LogCreateError(CreateError::CREATE_FILE_MAPPING_FAILURE, kr);
// Crash as soon as shm allocation fails to debug the issue // Crash as soon as shm allocation fails to debug the issue
// https://crbug.com/872237. // https://crbug.com/872237.
MACH_CHECK(kr == KERN_SUCCESS, kr) << "mach_make_memory_entry_64"; MACH_CHECK(kr == KERN_SUCCESS, kr) << "mach_make_memory_entry_64";
DCHECK_GE(vm_size, size); DCHECK_GE(vm_size, size);
LogCreateError(CreateError::SUCCESS, KERN_SUCCESS);
return PlatformSharedMemoryRegion(std::move(named_right), mode, size, return PlatformSharedMemoryRegion(std::move(named_right), mode, size,
UnguessableToken::Create()); UnguessableToken::Create());
} }
......
...@@ -18,12 +18,6 @@ namespace subtle { ...@@ -18,12 +18,6 @@ namespace subtle {
namespace { namespace {
#if !defined(OS_NACL)
void LogCreateError(PlatformSharedMemoryRegion::CreateError error) {
UMA_HISTOGRAM_ENUMERATION("SharedMemory.CreateError", error);
}
#endif
struct ScopedPathUnlinkerTraits { struct ScopedPathUnlinkerTraits {
static const FilePath* InvalidValue() { return nullptr; } static const FilePath* InvalidValue() { return nullptr; }
...@@ -219,12 +213,10 @@ PlatformSharedMemoryRegion PlatformSharedMemoryRegion::Create(Mode mode, ...@@ -219,12 +213,10 @@ PlatformSharedMemoryRegion PlatformSharedMemoryRegion::Create(Mode mode,
return {}; return {};
#else #else
if (size == 0) { if (size == 0) {
LogCreateError(PlatformSharedMemoryRegion::CreateError::SIZE_ZERO);
return {}; return {};
} }
if (size > static_cast<size_t>(std::numeric_limits<int>::max())) { if (size > static_cast<size_t>(std::numeric_limits<int>::max())) {
LogCreateError(PlatformSharedMemoryRegion::CreateError::SIZE_TOO_LARGE);
return {}; return {};
} }
...@@ -246,8 +238,6 @@ PlatformSharedMemoryRegion PlatformSharedMemoryRegion::Create(Mode mode, ...@@ -246,8 +238,6 @@ PlatformSharedMemoryRegion PlatformSharedMemoryRegion::Create(Mode mode,
false /* executable */, false /* executable */,
#endif #endif
&directory)) { &directory)) {
LogCreateError(
PlatformSharedMemoryRegion::CreateError::GET_SHMEM_TEMP_DIR_FAILURE);
return {}; return {};
} }
...@@ -256,8 +246,6 @@ PlatformSharedMemoryRegion PlatformSharedMemoryRegion::Create(Mode mode, ...@@ -256,8 +246,6 @@ PlatformSharedMemoryRegion PlatformSharedMemoryRegion::Create(Mode mode,
File shm_file(fd.release()); File shm_file(fd.release());
if (!shm_file.IsValid()) { if (!shm_file.IsValid()) {
LogCreateError(
PlatformSharedMemoryRegion::CreateError::CREATE_FILE_MAPPING_FAILURE);
PLOG(ERROR) << "Creating shared memory in " << path.value() << " failed"; PLOG(ERROR) << "Creating shared memory in " << path.value() << " failed";
FilePath dir = path.DirName(); FilePath dir = path.DirName();
if (access(dir.value().c_str(), W_OK | X_OK) < 0) { if (access(dir.value().c_str(), W_OK | X_OK) < 0) {
...@@ -280,43 +268,35 @@ PlatformSharedMemoryRegion PlatformSharedMemoryRegion::Create(Mode mode, ...@@ -280,43 +268,35 @@ PlatformSharedMemoryRegion PlatformSharedMemoryRegion::Create(Mode mode,
// Also open as readonly so that we can ConvertToReadOnly(). // Also open as readonly so that we can ConvertToReadOnly().
readonly_fd.reset(HANDLE_EINTR(open(path.value().c_str(), O_RDONLY))); readonly_fd.reset(HANDLE_EINTR(open(path.value().c_str(), O_RDONLY)));
if (!readonly_fd.is_valid()) { if (!readonly_fd.is_valid()) {
LogCreateError(
PlatformSharedMemoryRegion::CreateError::REDUCE_PERMISSIONS_FAILURE);
DPLOG(ERROR) << "open(\"" << path.value() << "\", O_RDONLY) failed"; DPLOG(ERROR) << "open(\"" << path.value() << "\", O_RDONLY) failed";
return {}; return {};
} }
} }
if (!AllocateFileRegion(&shm_file, 0, size)) { if (!AllocateFileRegion(&shm_file, 0, size)) {
LogCreateError(
PlatformSharedMemoryRegion::CreateError::ALLOCATE_FILE_REGION_FAILURE);
return {}; return {};
} }
if (readonly_fd.is_valid()) { if (readonly_fd.is_valid()) {
stat_wrapper_t shm_stat; stat_wrapper_t shm_stat;
if (File::Fstat(shm_file.GetPlatformFile(), &shm_stat) != 0) { if (File::Fstat(shm_file.GetPlatformFile(), &shm_stat) != 0) {
LogCreateError(PlatformSharedMemoryRegion::CreateError::FSTAT_FAILURE);
DPLOG(ERROR) << "fstat(fd) failed"; DPLOG(ERROR) << "fstat(fd) failed";
return {}; return {};
} }
stat_wrapper_t readonly_stat; stat_wrapper_t readonly_stat;
if (File::Fstat(readonly_fd.get(), &readonly_stat) != 0) { if (File::Fstat(readonly_fd.get(), &readonly_stat) != 0) {
LogCreateError(PlatformSharedMemoryRegion::CreateError::FSTAT_FAILURE);
DPLOG(ERROR) << "fstat(readonly_fd) failed"; DPLOG(ERROR) << "fstat(readonly_fd) failed";
return {}; return {};
} }
if (shm_stat.st_dev != readonly_stat.st_dev || if (shm_stat.st_dev != readonly_stat.st_dev ||
shm_stat.st_ino != readonly_stat.st_ino) { shm_stat.st_ino != readonly_stat.st_ino) {
LogCreateError(PlatformSharedMemoryRegion::CreateError::INODES_MISMATCH);
LOG(ERROR) << "Writable and read-only inodes don't match; bailing"; LOG(ERROR) << "Writable and read-only inodes don't match; bailing";
return {}; return {};
} }
} }
LogCreateError(PlatformSharedMemoryRegion::CreateError::SUCCESS);
return PlatformSharedMemoryRegion( return PlatformSharedMemoryRegion(
{ScopedFD(shm_file.TakePlatformFile()), std::move(readonly_fd)}, mode, {ScopedFD(shm_file.TakePlatformFile()), std::move(readonly_fd)}, mode,
size, UnguessableToken::Create()); size, UnguessableToken::Create());
......
...@@ -24,15 +24,6 @@ namespace subtle { ...@@ -24,15 +24,6 @@ namespace subtle {
namespace { namespace {
// Emits UMA metrics about encountered errors. Pass zero (0) for |winerror|
// if there is no associated Windows error.
void LogError(PlatformSharedMemoryRegion::CreateError error, DWORD winerror) {
UMA_HISTOGRAM_ENUMERATION("SharedMemory.CreateError", error);
static_assert(ERROR_SUCCESS == 0, "Windows error code changed!");
if (winerror != ERROR_SUCCESS)
UmaHistogramSparse("SharedMemory.CreateWinError", winerror);
}
typedef enum _SECTION_INFORMATION_CLASS { typedef enum _SECTION_INFORMATION_CLASS {
SectionBasicInformation, SectionBasicInformation,
} SECTION_INFORMATION_CLASS; } SECTION_INFORMATION_CLASS;
...@@ -96,9 +87,6 @@ HANDLE CreateFileMappingWithReducedPermissions(SECURITY_ATTRIBUTES* sa, ...@@ -96,9 +87,6 @@ HANDLE CreateFileMappingWithReducedPermissions(SECURITY_ATTRIBUTES* sa,
HANDLE h = CreateFileMapping(INVALID_HANDLE_VALUE, sa, PAGE_READWRITE, 0, HANDLE h = CreateFileMapping(INVALID_HANDLE_VALUE, sa, PAGE_READWRITE, 0,
static_cast<DWORD>(rounded_size), name); static_cast<DWORD>(rounded_size), name);
if (!h) { if (!h) {
LogError(
PlatformSharedMemoryRegion::CreateError::CREATE_FILE_MAPPING_FAILURE,
GetLastError());
return nullptr; return nullptr;
} }
...@@ -111,9 +99,6 @@ HANDLE CreateFileMappingWithReducedPermissions(SECURITY_ATTRIBUTES* sa, ...@@ -111,9 +99,6 @@ HANDLE CreateFileMappingWithReducedPermissions(SECURITY_ATTRIBUTES* sa,
DCHECK(rv); DCHECK(rv);
if (!success) { if (!success) {
LogError(
PlatformSharedMemoryRegion::CreateError::REDUCE_PERMISSIONS_FAILURE,
GetLastError());
return nullptr; return nullptr;
} }
...@@ -237,7 +222,6 @@ PlatformSharedMemoryRegion PlatformSharedMemoryRegion::Create(Mode mode, ...@@ -237,7 +222,6 @@ PlatformSharedMemoryRegion PlatformSharedMemoryRegion::Create(Mode mode,
// per mapping on average. // per mapping on average.
static const size_t kSectionSize = 65536; static const size_t kSectionSize = 65536;
if (size == 0) { if (size == 0) {
LogError(CreateError::SIZE_ZERO, 0);
return {}; return {};
} }
...@@ -245,7 +229,6 @@ PlatformSharedMemoryRegion PlatformSharedMemoryRegion::Create(Mode mode, ...@@ -245,7 +229,6 @@ PlatformSharedMemoryRegion PlatformSharedMemoryRegion::Create(Mode mode,
size_t rounded_size = bits::Align(size, kSectionSize); size_t rounded_size = bits::Align(size, kSectionSize);
if (rounded_size < size || if (rounded_size < size ||
rounded_size > static_cast<size_t>(std::numeric_limits<int>::max())) { rounded_size > static_cast<size_t>(std::numeric_limits<int>::max())) {
LogError(CreateError::SIZE_TOO_LARGE, 0);
return {}; return {};
} }
...@@ -256,15 +239,12 @@ PlatformSharedMemoryRegion PlatformSharedMemoryRegion::Create(Mode mode, ...@@ -256,15 +239,12 @@ PlatformSharedMemoryRegion PlatformSharedMemoryRegion::Create(Mode mode,
ACL dacl; ACL dacl;
SECURITY_DESCRIPTOR sd; SECURITY_DESCRIPTOR sd;
if (!InitializeAcl(&dacl, sizeof(dacl), ACL_REVISION)) { if (!InitializeAcl(&dacl, sizeof(dacl), ACL_REVISION)) {
LogError(CreateError::INITIALIZE_ACL_FAILURE, GetLastError());
return {}; return {};
} }
if (!InitializeSecurityDescriptor(&sd, SECURITY_DESCRIPTOR_REVISION)) { if (!InitializeSecurityDescriptor(&sd, SECURITY_DESCRIPTOR_REVISION)) {
LogError(CreateError::INITIALIZE_SECURITY_DESC_FAILURE, GetLastError());
return {}; return {};
} }
if (!SetSecurityDescriptorDacl(&sd, TRUE, &dacl, FALSE)) { if (!SetSecurityDescriptorDacl(&sd, TRUE, &dacl, FALSE)) {
LogError(CreateError::SET_SECURITY_DESC_FAILURE, GetLastError());
return {}; return {};
} }
...@@ -294,11 +274,9 @@ PlatformSharedMemoryRegion PlatformSharedMemoryRegion::Create(Mode mode, ...@@ -294,11 +274,9 @@ PlatformSharedMemoryRegion PlatformSharedMemoryRegion::Create(Mode mode,
win::ScopedHandle scoped_h(h); win::ScopedHandle scoped_h(h);
// Check if the shared memory pre-exists. // Check if the shared memory pre-exists.
if (GetLastError() == ERROR_ALREADY_EXISTS) { if (GetLastError() == ERROR_ALREADY_EXISTS) {
LogError(CreateError::ALREADY_EXISTS, ERROR_ALREADY_EXISTS);
return {}; return {};
} }
LogError(CreateError::SUCCESS, ERROR_SUCCESS);
return PlatformSharedMemoryRegion(std::move(scoped_h), mode, size, return PlatformSharedMemoryRegion(std::move(scoped_h), mode, size,
UnguessableToken::Create()); UnguessableToken::Create());
} }
......
...@@ -148674,6 +148674,10 @@ should be kept until we remove incident reporting. --> ...@@ -148674,6 +148674,10 @@ should be kept until we remove incident reporting. -->
<histogram name="SharedMemory.CreateError" enum="SharedMemoryCreateError" <histogram name="SharedMemory.CreateError" enum="SharedMemoryCreateError"
expires_after="M81"> expires_after="M81">
<obsolete>
Not accessed in months. Primary error was CREATE_FILE_MAPPING_FAILURE by
far. Removed 2020-03.
</obsolete>
<owner>erikchen@chromium.org</owner> <owner>erikchen@chromium.org</owner>
<owner>alexilin@chromium.org</owner> <owner>alexilin@chromium.org</owner>
<summary> <summary>
...@@ -148686,6 +148690,9 @@ should be kept until we remove incident reporting. --> ...@@ -148686,6 +148690,9 @@ should be kept until we remove incident reporting. -->
<histogram name="SharedMemory.CreateMacError" enum="MachKernReturn" <histogram name="SharedMemory.CreateMacError" enum="MachKernReturn"
expires_after="M85"> expires_after="M85">
<obsolete>
Not accessed in months and no data reported for it at all. Removed 2020-03.
</obsolete>
<owner>alexilin@chromium.org</owner> <owner>alexilin@chromium.org</owner>
<summary> <summary>
Emitted each time a shared memory region could not be created due to a Emitted each time a shared memory region could not be created due to a
...@@ -148696,6 +148703,9 @@ should be kept until we remove incident reporting. --> ...@@ -148696,6 +148703,9 @@ should be kept until we remove incident reporting. -->
<histogram name="SharedMemory.CreateWinError" enum="WinGetLastError" <histogram name="SharedMemory.CreateWinError" enum="WinGetLastError"
expires_after="M82"> expires_after="M82">
<obsolete>
Not accessed in months. Primary error was #1455 by far. Removed 2020-03.
</obsolete>
<owner>bcwhite@chromium.org</owner> <owner>bcwhite@chromium.org</owner>
<summary> <summary>
A histogram entry is emitted each time a shared memory object could not be A histogram entry is emitted each time a shared memory object could not be
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment