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}

pdf/BUILD.gn

@@ -88,6 +88,8 @@ if (enable_pdf) {
       "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",

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

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_

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 @@ void PdfViewPluginBase::DocumentLoadComplete() {
   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 @@ void PdfViewPluginBase::DocumentLoadComplete() {
   SendBookmarks();
   SendMetadata();
 
+  OnDocumentLoadComplete();
+
   if (accessibility_state_ == AccessibilityState::kPending)
     LoadAccessibility();
 
@@ -1604,67 +1603,6 @@ void PdfViewPluginBase::LoadAccessibility() {
       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) {

pdf/pdf_view_plugin_base.h

@@ -224,6 +224,9 @@ class PdfViewPluginBase : public PDFEngine::Client,
   // 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 @@ class PdfViewPluginBase : public PDFEngine::Client,
   // 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);
 

pdf/pdf_view_plugin_base_unittest.cc

@@ -224,6 +224,8 @@ class FakePdfViewPluginBase : public PdfViewPluginBase {
               (),
               (override));
 
+  MOCK_METHOD(void, OnDocumentLoadComplete, (), (override));
+
   void SendMessage(base::Value message) override {
     sent_messages_.push_back(std::move(message));
   }

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 @@ bool PdfViewWebPlugin::InitializeCommon(
       /*has_edits=*/params->has_edits);
 
   SendSetSmoothScrolling();
+
+  if (!IsPrintPreview())
+    metrics_handler_ = std::make_unique<MetricsHandler>();
+
   return true;
 }
 
@@ -920,6 +925,10 @@ std::unique_ptr<UrlLoader> PdfViewWebPlugin::CreateUrlLoaderInternal() {
   return loader;
 }
 
+void PdfViewWebPlugin::OnDocumentLoadComplete() {
+  RecordDocumentMetrics();
+}
+
 void PdfViewWebPlugin::SendMessage(base::Value message) {
   post_message_sender_.Post(std::move(message));
 }
@@ -1156,4 +1165,13 @@ void PdfViewWebPlugin::ResetRecentlySentFindUpdate() {
   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

pdf/pdf_view_web_plugin.h

@@ -54,6 +54,7 @@ class MetafileSkia;
 
 namespace chrome_pdf {
 
+class MetricsHandler;
 class PDFiumEngine;
 class PdfAccessibilityDataHandler;
 
@@ -296,6 +297,7 @@ class PdfViewWebPlugin final : public PdfViewPluginBase,
   // 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 @@ class PdfViewWebPlugin final : public PdfViewPluginBase,
 
   void ResetRecentlySentFindUpdate();
 
+  // Records metrics about the document metadata.
+  void RecordDocumentMetrics();
+
   blink::WebString selected_text_;
 
   std::unique_ptr<Client> const client_;
@@ -442,6 +447,9 @@ class PdfViewWebPlugin final : public PdfViewPluginBase,
   // 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;