Commit bc5323db authored by Lei Zhang's avatar Lei Zhang Committed by Commit Bot

Switch PdfCompositor UMAs to use kMaxValue enumerator value.

Enums in Mojo have |kMaxValue| automatically defined. Switch
PdfCompositor UMAs to use the 2-parameter UMA_HISTOGRAM_ENUMERATION
macro, which uses |kMaxValue|, and avoid the need for static_casts with
the 3-parameter UMA_HISTOGRAM_ENUMERATION variant.

Along the way, switch the PdfCompositor enum to kFoo style, and fix a
typo to stop referring to composting.

BUG=742517

Change-Id: Id9664371d3b2d7b0fe1203069d96c37b3e4b41de
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1661490Reviewed-by: default avatarDaniel Cheng <dcheng@chromium.org>
Commit-Queue: Lei Zhang <thestig@chromium.org>
Cr-Commit-Position: refs/heads/master@{#669957}
parent 3291e247
......@@ -315,7 +315,7 @@ void PrintPreviewMessageHandler::OnCompositePdfPageDone(
mojom::PdfCompositor::Status status,
base::ReadOnlySharedMemoryRegion region) {
DCHECK_CURRENTLY_ON(BrowserThread::UI);
if (status != mojom::PdfCompositor::Status::SUCCESS) {
if (status != mojom::PdfCompositor::Status::kSuccess) {
DLOG(ERROR) << "Compositing pdf failed with error " << status;
return;
}
......@@ -386,7 +386,7 @@ void PrintPreviewMessageHandler::OnCompositePdfDocumentDone(
mojom::PdfCompositor::Status status,
base::ReadOnlySharedMemoryRegion region) {
DCHECK_CURRENTLY_ON(BrowserThread::UI);
if (status != mojom::PdfCompositor::Status::SUCCESS) {
if (status != mojom::PdfCompositor::Status::kSuccess) {
DLOG(ERROR) << "Compositing pdf failed with error " << status;
return;
}
......
......@@ -285,7 +285,7 @@ void PrintViewManagerBase::OnComposePdfDone(
mojom::PdfCompositor::Status status,
base::ReadOnlySharedMemoryRegion region) {
DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
if (status != mojom::PdfCompositor::Status::SUCCESS) {
if (status != mojom::PdfCompositor::Status::kSuccess) {
DLOG(ERROR) << "Compositing pdf failed with error " << status;
return;
}
......
......@@ -206,12 +206,7 @@ void PrintCompositeClient::OnDidCompositePageToPdf(
mojom::PdfCompositor::CompositePageToPdfCallback callback,
mojom::PdfCompositor::Status status,
base::ReadOnlySharedMemoryRegion region) {
// Due to https://crbug.com/742517, we can not add and use COUNT for enums in
// mojo.
UMA_HISTOGRAM_ENUMERATION(
"CompositePageToPdf.Status", status,
static_cast<int32_t>(mojom::PdfCompositor::Status::COMPOSTING_FAILURE) +
1);
UMA_HISTOGRAM_ENUMERATION("CompositePageToPdf.Status", status);
std::move(callback).Run(status, std::move(region));
}
......@@ -224,12 +219,7 @@ void PrintCompositeClient::OnDidCompositeDocumentToPdf(
// Clear all stored printed subframes.
printed_subframes_.erase(document_cookie);
// Due to https://crbug.com/742517, we can not add and use COUNT for enums in
// mojo.
UMA_HISTOGRAM_ENUMERATION(
"CompositeDocToPdf.Status", status,
static_cast<int32_t>(mojom::PdfCompositor::Status::COMPOSTING_FAILURE) +
1);
UMA_HISTOGRAM_ENUMERATION("CompositeDocToPdf.Status", status);
std::move(callback).Run(status, std::move(region));
}
......
......@@ -178,7 +178,7 @@ void PdfCompositorImpl::HandleCompositionRequest(
base::ReadOnlySharedMemoryMapping mapping = serialized_content.Map();
if (!mapping.IsValid()) {
DLOG(ERROR) << "HandleCompositionRequest: Cannot map input.";
std::move(callback).Run(mojom::PdfCompositor::Status::HANDLE_MAP_ERROR,
std::move(callback).Run(mojom::PdfCompositor::Status::kHandleMapError,
base::ReadOnlySharedMemoryRegion());
return;
}
......@@ -208,7 +208,7 @@ mojom::PdfCompositor::Status PdfCompositorImpl::CompositeToPdf(
base::ReadOnlySharedMemoryRegion* region) {
if (!shared_mem.IsValid()) {
DLOG(ERROR) << "CompositeToPdf: Invalid input.";
return mojom::PdfCompositor::Status::HANDLE_MAP_ERROR;
return mojom::PdfCompositor::Status::kHandleMapError;
}
DeserializationContext subframes =
......@@ -219,14 +219,14 @@ mojom::PdfCompositor::Status PdfCompositorImpl::CompositeToPdf(
int page_count = SkMultiPictureDocumentReadPageCount(&stream);
if (!page_count) {
DLOG(ERROR) << "CompositeToPdf: No page is read.";
return mojom::PdfCompositor::Status::CONTENT_FORMAT_ERROR;
return mojom::PdfCompositor::Status::kContentFormatError;
}
std::vector<SkDocumentPage> pages(page_count);
SkDeserialProcs procs = DeserializationProcs(&subframes);
if (!SkMultiPictureDocumentRead(&stream, pages.data(), page_count, &procs)) {
DLOG(ERROR) << "CompositeToPdf: Page reading failed.";
return mojom::PdfCompositor::Status::CONTENT_FORMAT_ERROR;
return mojom::PdfCompositor::Status::kContentFormatError;
}
SkDynamicMemoryWStream wstream;
......@@ -243,12 +243,12 @@ mojom::PdfCompositor::Status PdfCompositorImpl::CompositeToPdf(
mojo::CreateReadOnlySharedMemoryRegion(wstream.bytesWritten());
if (!region_mapping.IsValid()) {
DLOG(ERROR) << "CompositeToPdf: Cannot create new shared memory region.";
return mojom::PdfCompositor::Status::HANDLE_MAP_ERROR;
return mojom::PdfCompositor::Status::kHandleMapError;
}
wstream.copyToAndReset(region_mapping.mapping.memory());
*region = std::move(region_mapping.region);
return mojom::PdfCompositor::Status::SUCCESS;
return mojom::PdfCompositor::Status::kSuccess;
}
void PdfCompositorImpl::CompositeSubframe(FrameInfo* frame_info) {
......
......@@ -44,7 +44,7 @@ class PdfCompositorServiceTest : public testing::Test {
void OnCompositeToPdfCallback(mojom::PdfCompositor::Status status,
base::ReadOnlySharedMemoryRegion region) {
if (status == mojom::PdfCompositor::Status::SUCCESS)
if (status == mojom::PdfCompositor::Status::kSuccess)
CallbackOnCompositeSuccess(region);
else
CallbackOnCompositeStatus(status);
......@@ -123,7 +123,7 @@ TEST_F(PdfCompositorServiceTest, InvokeCallbackOnContentError) {
auto serialized_content = base::ReadOnlySharedMemoryRegion::Create(10);
EXPECT_CALL(*this, CallbackOnCompositeStatus(
mojom::PdfCompositor::Status::CONTENT_FORMAT_ERROR))
mojom::PdfCompositor::Status::kContentFormatError))
.Times(1);
compositor_->CompositeDocumentToPdf(
5u, std::move(serialized_content.region), ContentToFrameMap(),
......
......@@ -14,10 +14,10 @@ interface PdfCompositor {
// These values are persisted to logs. Entries should not be renumbered and
// numeric values should never be reused.
enum Status {
SUCCESS = 0,
HANDLE_MAP_ERROR = 1,
CONTENT_FORMAT_ERROR = 2,
COMPOSTING_FAILURE = 3,
kSuccess = 0,
kHandleMapError = 1,
kContentFormatError = 2,
kCompositingFailure = 3,
};
// Notifies that a subframe is unavailable, such as the render frame process
......
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