Move PDF metrics code into a separate MetricsHandler class.
The PDF UMA metrics code is self-contained and can be its own class.
Separate it out from the much larger PdfViewPluginBase class.
Along the way, instantiate MetricsHandler in PdfViewWebPlugin, as
PdfViewPluginBase will merge into PdfViewWebPlugin.
Bug: 1302059
Change-Id: I72fcf27f461ffaa71e03c8b61084a258711ee11b
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3534391
Reviewed-by: Nigi <nigi@chromium.org>
Commit-Queue: Lei Zhang <thestig@chromium.org>
Cr-Commit-Position: refs/heads/main@{#986679}
diff --git a/pdf/BUILD.gn b/pdf/BUILD.gn
index fa98d89..2b0aa2f 100644
--- a/pdf/BUILD.gn
+++ b/pdf/BUILD.gn
@@ -88,6 +88,8 @@
"draw_utils/shadow.h",
"file_extension.cc",
"file_extension.h",
+ "metrics_handler.cc",
+ "metrics_handler.h",
"page_orientation.cc",
"page_orientation.h",
"paint_aggregator.cc",
diff --git a/pdf/metrics_handler.cc b/pdf/metrics_handler.cc
new file mode 100644
index 0000000..6e191d5
--- /dev/null
+++ b/pdf/metrics_handler.cc
@@ -0,0 +1,59 @@
+// Copyright 2022 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 "pdf/metrics_handler.h"
+
+#include <vector>
+
+#include "base/metrics/histogram_functions.h"
+#include "pdf/document_metadata.h"
+#include "pdf/file_extension.h"
+
+namespace chrome_pdf {
+
+namespace {
+
+// These values are persisted to logs. Entries should not be renumbered and
+// numeric values should never be reused.
+enum class PdfHasAttachment {
+ kNo = 0,
+ kYes = 1,
+ kMaxValue = kYes,
+};
+
+// These values are persisted to logs. Entries should not be renumbered and
+// numeric values should never be reused.
+enum class PdfIsTagged {
+ kNo = 0,
+ kYes = 1,
+ kMaxValue = kYes,
+};
+
+} // namespace
+
+MetricsHandler::MetricsHandler() = default;
+
+MetricsHandler::~MetricsHandler() = default;
+
+void MetricsHandler::RecordAttachmentTypes(
+ const std::vector<DocumentAttachmentInfo>& attachments) {
+ for (const auto& info : attachments) {
+ base::UmaHistogramEnumeration("PDF.AttachmentType",
+ FileNameToExtensionIndex(info.name));
+ }
+}
+
+void MetricsHandler::RecordDocumentMetrics(const DocumentMetadata& metadata) {
+ base::UmaHistogramEnumeration("PDF.Version", metadata.version);
+ base::UmaHistogramCustomCounts("PDF.PageCount", metadata.page_count, 1,
+ 1000000, 50);
+ base::UmaHistogramEnumeration(
+ "PDF.HasAttachment", metadata.has_attachments ? PdfHasAttachment::kYes
+ : PdfHasAttachment::kNo);
+ base::UmaHistogramEnumeration(
+ "PDF.IsTagged", metadata.tagged ? PdfIsTagged::kYes : PdfIsTagged::kNo);
+ base::UmaHistogramEnumeration("PDF.FormType", metadata.form_type);
+}
+
+} // namespace chrome_pdf
diff --git a/pdf/metrics_handler.h b/pdf/metrics_handler.h
new file mode 100644
index 0000000..6d6a794
--- /dev/null
+++ b/pdf/metrics_handler.h
@@ -0,0 +1,32 @@
+// Copyright 2022 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.
+
+#ifndef PDF_METRICS_HANDLER_H_
+#define PDF_METRICS_HANDLER_H_
+
+#include <vector>
+
+#include "pdf/document_attachment_info.h"
+
+namespace chrome_pdf {
+
+struct DocumentMetadata;
+
+// Handles various UMA metrics. Note that action metrics are handled separately.
+class MetricsHandler {
+ public:
+ MetricsHandler();
+ MetricsHandler(const MetricsHandler& other) = delete;
+ MetricsHandler& operator=(const MetricsHandler& other) = delete;
+ ~MetricsHandler();
+
+ void RecordAttachmentTypes(
+ const std::vector<DocumentAttachmentInfo>& attachments);
+
+ void RecordDocumentMetrics(const DocumentMetadata& metadata);
+};
+
+} // namespace chrome_pdf
+
+#endif // PDF_METRICS_HANDLER_H_
diff --git a/pdf/pdf_view_plugin_base.cc b/pdf/pdf_view_plugin_base.cc
index 2de8461..8340901 100644
--- a/pdf/pdf_view_plugin_base.cc
+++ b/pdf/pdf_view_plugin_base.cc
@@ -28,7 +28,6 @@
#include "base/location.h"
#include "base/logging.h"
#include "base/memory/weak_ptr.h"
-#include "base/metrics/histogram_functions.h"
#include "base/notreached.h"
#include "base/numerics/safe_conversions.h"
#include "base/strings/string_number_conversions.h"
@@ -45,7 +44,6 @@
#include "pdf/content_restriction.h"
#include "pdf/document_layout.h"
#include "pdf/document_metadata.h"
-#include "pdf/file_extension.h"
#include "pdf/paint_ready_rect.h"
#include "pdf/pdf_engine.h"
#include "pdf/pdf_features.h"
@@ -385,7 +383,6 @@
document_load_state_ = DocumentLoadState::kComplete;
UserMetricsRecordAction("PDF.LoadSuccess");
- RecordDocumentMetrics();
// Clear the focus state for on-screen keyboards.
FormFieldFocusChange(PDFEngine::FocusFieldType::kNoFocus);
@@ -397,6 +394,8 @@
SendBookmarks();
SendMetadata();
+ OnDocumentLoadComplete();
+
if (accessibility_state_ == AccessibilityState::kPending)
LoadAccessibility();
@@ -1604,67 +1603,6 @@
kAccessibilityPageDelay);
}
-namespace {
-
-// These values are persisted to logs. Entries should not be renumbered and
-// numeric values should never be reused.
-enum class PdfHasAttachment {
- kNo = 0,
- kYes = 1,
- kMaxValue = kYes,
-};
-
-// These values are persisted to logs. Entries should not be renumbered and
-// numeric values should never be reused.
-enum class PdfIsTagged {
- kNo = 0,
- kYes = 1,
- kMaxValue = kYes,
-};
-
-} // namespace
-
-void PdfViewPluginBase::RecordAttachmentTypes() {
- const std::vector<DocumentAttachmentInfo>& list =
- engine()->GetDocumentAttachmentInfoList();
- for (const auto& info : list) {
- HistogramEnumeration("PDF.AttachmentType",
- FileNameToExtensionIndex(info.name));
- }
-}
-
-void PdfViewPluginBase::RecordDocumentMetrics() {
- const DocumentMetadata& document_metadata = engine()->GetDocumentMetadata();
- HistogramEnumeration("PDF.Version", document_metadata.version);
- HistogramCustomCounts("PDF.PageCount", document_metadata.page_count, 1,
- 1000000, 50);
- HistogramEnumeration("PDF.HasAttachment", document_metadata.has_attachments
- ? PdfHasAttachment::kYes
- : PdfHasAttachment::kNo);
- HistogramEnumeration("PDF.IsTagged", document_metadata.tagged
- ? PdfIsTagged::kYes
- : PdfIsTagged::kNo);
- HistogramEnumeration("PDF.FormType", document_metadata.form_type);
- RecordAttachmentTypes();
-}
-
-template <typename T>
-void PdfViewPluginBase::HistogramEnumeration(const char* name, T sample) {
- if (IsPrintPreview())
- return;
- base::UmaHistogramEnumeration(name, sample);
-}
-
-void PdfViewPluginBase::HistogramCustomCounts(const char* name,
- int32_t sample,
- int32_t min,
- int32_t max,
- uint32_t bucket_count) {
- if (IsPrintPreview())
- return;
- base::UmaHistogramCustomCounts(name, sample, min, max, bucket_count);
-}
-
void PdfViewPluginBase::DidOpen(std::unique_ptr<UrlLoader> loader,
int32_t result) {
if (result == kSuccess) {
diff --git a/pdf/pdf_view_plugin_base.h b/pdf/pdf_view_plugin_base.h
index 2dae3da..c98ddac9 100644
--- a/pdf/pdf_view_plugin_base.h
+++ b/pdf/pdf_view_plugin_base.h
@@ -224,6 +224,9 @@
// frame's origin.
virtual std::unique_ptr<UrlLoader> CreateUrlLoaderInternal() = 0;
+ // Runs when document load completes.
+ virtual void OnDocumentLoadComplete() = 0;
+
bool HandleInputEvent(const blink::WebInputEvent& event);
// Handles `postMessage()` calls from the embedder.
@@ -468,25 +471,6 @@
// Starts loading accessibility information.
void LoadAccessibility();
- // Records metrics about the attachment types.
- void RecordAttachmentTypes();
-
- // Records metrics about the document metadata.
- void RecordDocumentMetrics();
-
- // Adds a sample to an enumerated histogram and filters out print preview
- // usage.
- template <typename T>
- void HistogramEnumeration(const char* name, T sample);
-
- // Adds a sample to a custom counts histogram and filters out print preview
- // usage.
- void HistogramCustomCounts(const char* name,
- int32_t sample,
- int32_t min,
- int32_t max,
- uint32_t bucket_count);
-
// Handles `LoadUrl()` result.
void DidOpen(std::unique_ptr<UrlLoader> loader, int32_t result);
diff --git a/pdf/pdf_view_plugin_base_unittest.cc b/pdf/pdf_view_plugin_base_unittest.cc
index 3853f3d..7559503a 100644
--- a/pdf/pdf_view_plugin_base_unittest.cc
+++ b/pdf/pdf_view_plugin_base_unittest.cc
@@ -224,6 +224,8 @@
(),
(override));
+ MOCK_METHOD(void, OnDocumentLoadComplete, (), (override));
+
void SendMessage(base::Value message) override {
sent_messages_.push_back(std::move(message));
}
diff --git a/pdf/pdf_view_web_plugin.cc b/pdf/pdf_view_web_plugin.cc
index 813278e..0bedd50b 100644
--- a/pdf/pdf_view_web_plugin.cc
+++ b/pdf/pdf_view_web_plugin.cc
@@ -32,6 +32,7 @@
#include "cc/paint/paint_image_builder.h"
#include "net/cookies/site_for_cookies.h"
#include "pdf/accessibility_structs.h"
+#include "pdf/metrics_handler.h"
#include "pdf/mojom/pdf.mojom.h"
#include "pdf/parsed_params.h"
#include "pdf/pdf_accessibility_data_handler.h"
@@ -363,6 +364,10 @@
/*has_edits=*/params->has_edits);
SendSetSmoothScrolling();
+
+ if (!IsPrintPreview())
+ metrics_handler_ = std::make_unique<MetricsHandler>();
+
return true;
}
@@ -920,6 +925,10 @@
return loader;
}
+void PdfViewWebPlugin::OnDocumentLoadComplete() {
+ RecordDocumentMetrics();
+}
+
void PdfViewWebPlugin::SendMessage(base::Value message) {
post_message_sender_.Post(std::move(message));
}
@@ -1156,4 +1165,13 @@
recently_sent_find_update_ = false;
}
+void PdfViewWebPlugin::RecordDocumentMetrics() {
+ if (!metrics_handler_)
+ return;
+
+ metrics_handler_->RecordDocumentMetrics(engine()->GetDocumentMetadata());
+ metrics_handler_->RecordAttachmentTypes(
+ engine()->GetDocumentAttachmentInfoList());
+}
+
} // namespace chrome_pdf
diff --git a/pdf/pdf_view_web_plugin.h b/pdf/pdf_view_web_plugin.h
index 70917d2..75b5e49a 100644
--- a/pdf/pdf_view_web_plugin.h
+++ b/pdf/pdf_view_web_plugin.h
@@ -54,6 +54,7 @@
namespace chrome_pdf {
+class MetricsHandler;
class PDFiumEngine;
class PdfAccessibilityDataHandler;
@@ -296,6 +297,7 @@
// PdfViewPluginBase:
base::WeakPtr<PdfViewPluginBase> GetWeakPtr() override;
std::unique_ptr<UrlLoader> CreateUrlLoaderInternal() override;
+ void OnDocumentLoadComplete() override;
void SendMessage(base::Value message) override;
void SaveAs() override;
void InitImageData(const gfx::Size& size) override;
@@ -379,6 +381,9 @@
void ResetRecentlySentFindUpdate();
+ // Records metrics about the document metadata.
+ void RecordDocumentMetrics();
+
blink::WebString selected_text_;
std::unique_ptr<Client> const client_;
@@ -442,6 +447,9 @@
// Stores the tickmarks to be shown for the current find results.
std::vector<gfx::Rect> tickmarks_;
+ // Only instantiated when not print previewing.
+ std::unique_ptr<MetricsHandler> metrics_handler_;
+
// The metafile in which to save the printed output. Assigned a value only
// between `PrintBegin()` and `PrintEnd()` calls.
raw_ptr<printing::MetafileSkia> printing_metafile_ = nullptr;