[unseasoned-pdf] Merge PDFEngine::DocumentFeatures into DocumentMetadata

The two structs served similar purposes, so removing the former is a
step in the right direction for removing the PDFEngine abstraction.

Additionally, the latter is a publicly accessible member of PDFEngine,
so there's no need to pass a struct to the PDFEngine::Client when
document loading is complete.

Bug: 1076554
Change-Id: I480533ad6618861a1a5bb5e191f795b547882745
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2676965
Commit-Queue: Daniel Hosseinian <dhoss@chromium.org>
Commit-Queue: Hui Yingst <nigi@chromium.org>
Auto-Submit: Daniel Hosseinian <dhoss@chromium.org>
Reviewed-by: Hui Yingst <nigi@chromium.org>
Cr-Commit-Position: refs/heads/master@{#850924}

pdf/document_metadata.h

@@ -28,23 +28,47 @@ enum class PdfVersion {
   kMaxValue = k2_0
 };
 
+// These values are persisted to logs. Entries should not be renumbered and
+// numeric values should never be reused.
+enum class FormType {
+  kNone = 0,
+  kAcroForm = 1,
+  kXFAFull = 2,
+  kXFAForeground = 3,
+  kMaxValue = kXFAForeground
+};
+
 // Document properties, including those specified in the document information
 // dictionary (see section 14.3.3 "Document Information Dictionary" of the ISO
-// 32000-1 standard), as well as other properties about the file.
-// TODO(crbug.com/93619): Finish adding fields like `size_bytes` and
-// `is_encrypted`.
+// 32000-1 standard).
+// TODO(crbug.com/93619): Add `size_bytes`.
 struct DocumentMetadata {
   DocumentMetadata();
   DocumentMetadata(const DocumentMetadata&) = delete;
   DocumentMetadata& operator=(const DocumentMetadata&) = delete;
   ~DocumentMetadata();
 
-  // Version of the document
+  // Version of the document.
   PdfVersion version = PdfVersion::kUnknown;
 
-  // Whether the document is optimized by linearization.
+  // Number of pages in the document.
+  size_t page_count = 0;
+
+  // Whether the document is optimized by linearization (see annex F "Linearized
+  // PDF" of the ISO 32000-1 standard).
   bool linearized = false;
 
+  // Whether the document contains file attachments (see section 12.5.6.15 "File
+  // Attachment Annotations" of the ISO 32000-1 standard).
+  bool has_attachments = false;
+
+  // Whether the document is tagged (see section 14.8 "Tagged PDF" of the ISO
+  // 32000-1 standard).
+  bool tagged = false;
+
+  // The type of form contained in the document.
+  FormType form_type = FormType::kNone;
+
   // The document's title.
   std::string title;
 
@@ -67,7 +91,7 @@ struct DocumentMetadata {
   // The date and time the document was created.
   base::Time creation_date;
 
-  // The date and time the document was most recently modified
+  // The date and time the document was most recently modified.
   base::Time mod_date;
 };
 

pdf/out_of_process_instance.cc

@@ -1484,14 +1484,14 @@ OutOfProcessInstance::SearchString(const base::char16* string,
   return results;
 }
 
-void OutOfProcessInstance::DocumentLoadComplete(
-    const PDFEngine::DocumentFeatures& document_features) {
+void OutOfProcessInstance::DocumentLoadComplete() {
   // Clear focus state for OSK.
   FormTextFieldFocusChange(false);
 
   DCHECK_EQ(LOAD_STATE_LOADING, document_load_state_);
   document_load_state_ = LOAD_STATE_COMPLETE;
   UserMetricsRecordAction("PDF.LoadSuccess");
+  RecordDocumentMetrics();
 
   // Note: If we are in print preview mode the scroll location is retained
   // across document loads so we don't want to scroll again and override it.
@@ -1533,16 +1533,6 @@ void OutOfProcessInstance::DocumentLoadComplete(
   }
 
   pp::PDF::SetContentRestriction(this, content_restrictions);
-  HistogramCustomCounts("PDF.PageCount", document_features.page_count, 1,
-                        1000000, 50);
-  HistogramEnumeration("PDF.HasAttachment", document_features.has_attachments
-                                                ? PdfHasAttachment::kYes
-                                                : PdfHasAttachment::kNo);
-  HistogramEnumeration("PDF.IsTagged", document_features.is_tagged
-                                           ? PdfIsTagged::kYes
-                                           : PdfIsTagged::kNo);
-  HistogramEnumeration("PDF.FormType", document_features.form_type);
-  HistogramEnumeration("PDF.Version", engine()->GetDocumentMetadata().version);
 }
 
 void OutOfProcessInstance::RotateClockwise() {
@@ -2277,6 +2267,20 @@ void OutOfProcessInstance::SendThumbnail(const std::string& message_id,
   PostMessage(reply);
 }
 
+void OutOfProcessInstance::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);
+}
+
 void OutOfProcessInstance::UserMetricsRecordAction(const std::string& action) {
   // TODO(raymes): Move this function to PPB_UMA_Private.
   pp::PDF::UserMetricsRecordAction(this, pp::Var(action));

pdf/out_of_process_instance.h

@@ -141,8 +141,7 @@ class OutOfProcessInstance : public PdfViewPluginBase,
   std::vector<SearchStringResult> SearchString(const base::char16* string,
                                                const base::char16* term,
                                                bool case_sensitive) override;
-  void DocumentLoadComplete(
-      const PDFEngine::DocumentFeatures& document_features) override;
+  void DocumentLoadComplete() override;
   void DocumentLoadFailed() override;
   pp::Instance* GetPluginInstance() override;
   void DocumentHasUnsupportedFeature(const std::string& feature) override;
@@ -220,6 +219,7 @@ class OutOfProcessInstance : public PdfViewPluginBase,
 
   void FormDidOpen(int32_t result);
 
+  void RecordDocumentMetrics();
   void UserMetricsRecordAction(const std::string& action);
 
   // Start loading accessibility information.

pdf/pdf_engine.h

@@ -83,16 +83,6 @@ class PDFEngine {
     PERMISSION_PRINT_HIGH_QUALITY,
   };
 
-  // These values are persisted to logs. Entries should not be renumbered and
-  // numeric values should never be reused.
-  enum class FormType {
-    kNone = 0,
-    kAcroForm = 1,
-    kXFAFull = 2,
-    kXFAForeground = 3,
-    kMaxValue = kXFAForeground
-  };
-
   // Maximum number of parameters a nameddest view can contain.
   static constexpr size_t kMaxViewParams = 4;
 
@@ -123,19 +113,6 @@ class PDFEngine {
     std::string xyz_params;
   };
 
-  // Features in a document that are relevant to measure.
-  struct DocumentFeatures {
-    // Number of pages in document.
-    size_t page_count = 0;
-    // Whether any files are attached to document (see "File Attachment
-    // Annotations" on page 637 of PDF Reference 1.7).
-    bool has_attachments = false;
-    // Whether the PDF is Tagged (see 10.7 "Tagged PDF" in PDF Reference 1.7).
-    bool is_tagged = false;
-    // What type of form the document contains.
-    FormType form_type = FormType::kNone;
-  };
-
   // The interface that's provided to the rendering engine.
   class Client {
    public:
@@ -246,8 +223,7 @@ class PDFEngine {
         bool case_sensitive) = 0;
 
     // Notifies the client that the document has finished loading.
-    virtual void DocumentLoadComplete(
-        const DocumentFeatures& document_features) {}
+    virtual void DocumentLoadComplete() {}
 
     // Notifies the client that the document has failed to load.
     virtual void DocumentLoadFailed() {}

pdf/pdf_view_web_plugin.cc

@@ -269,8 +269,7 @@ PdfViewWebPlugin::SearchString(const base::char16* string,
   return {};
 }
 
-void PdfViewWebPlugin::DocumentLoadComplete(
-    const PDFEngine::DocumentFeatures& document_features) {
+void PdfViewWebPlugin::DocumentLoadComplete() {
   NOTIMPLEMENTED();
 }
 

pdf/pdf_view_web_plugin.h

@@ -94,8 +94,7 @@ class PdfViewWebPlugin final : public PdfViewPluginBase,
   std::vector<SearchStringResult> SearchString(const base::char16* string,
                                                const base::char16* term,
                                                bool case_sensitive) override;
-  void DocumentLoadComplete(
-      const PDFEngine::DocumentFeatures& document_features) override;
+  void DocumentLoadComplete() override;
   void DocumentLoadFailed() override;
   pp::Instance* GetPluginInstance() override;
   void DocumentHasUnsupportedFeature(const std::string& feature) override;

pdf/pdfium/pdfium_assert_matching_enums.cc

@@ -4,7 +4,7 @@
 
 #include "build/build_config.h"
 #include "pdf/accessibility_structs.h"
-#include "pdf/pdf_engine.h"
+#include "pdf/document_metadata.h"
 #include "pdf/ppapi_migration/input_event_conversions.h"
 #include "ppapi/c/pp_input_event.h"
 #include "ppapi/c/private/ppb_pdf.h"
@@ -298,15 +298,12 @@ STATIC_ASSERT_ENUM(chrome_pdf::AccessibilityTextRenderMode::kClip,
 STATIC_ASSERT_ENUM(chrome_pdf::AccessibilityTextRenderMode::kMaxValue,
                    PP_TEXTRENDERINGMODE_LAST);
 
-STATIC_ASSERT_ENUM(chrome_pdf::PDFEngine::FormType::kNone, FORMTYPE_NONE);
-STATIC_ASSERT_ENUM(chrome_pdf::PDFEngine::FormType::kAcroForm,
-                   FORMTYPE_ACRO_FORM);
-STATIC_ASSERT_ENUM(chrome_pdf::PDFEngine::FormType::kXFAFull,
-                   FORMTYPE_XFA_FULL);
-STATIC_ASSERT_ENUM(chrome_pdf::PDFEngine::FormType::kXFAForeground,
+STATIC_ASSERT_ENUM(chrome_pdf::FormType::kNone, FORMTYPE_NONE);
+STATIC_ASSERT_ENUM(chrome_pdf::FormType::kAcroForm, FORMTYPE_ACRO_FORM);
+STATIC_ASSERT_ENUM(chrome_pdf::FormType::kXFAFull, FORMTYPE_XFA_FULL);
+STATIC_ASSERT_ENUM(chrome_pdf::FormType::kXFAForeground,
                    FORMTYPE_XFA_FOREGROUND);
-STATIC_ASSERT_ENUM(chrome_pdf::PDFEngine::FormType::kMaxValue,
-                   FORMTYPE_COUNT - 1);
+STATIC_ASSERT_ENUM(chrome_pdf::FormType::kMaxValue, FORMTYPE_COUNT - 1);
 
 STATIC_ASSERT_ENUM(PP_PRIVATEBUTTON_PUSHBUTTON, FPDF_FORMFIELD_PUSHBUTTON);
 STATIC_ASSERT_ENUM(PP_PRIVATEBUTTON_CHECKBOX, FPDF_FORMFIELD_CHECKBOX);

pdf/pdfium/pdfium_engine.cc

@@ -884,15 +884,7 @@ void PDFiumEngine::FinishLoadingDocument() {
     FORM_DoPageAAction(new_page, form(), FPDFPAGE_AACTION_OPEN);
   }
 
-  if (doc()) {
-    DocumentFeatures document_features;
-    document_features.page_count = pages_.size();
-    document_features.has_attachments = !doc_attachment_info_list_.empty();
-    document_features.is_tagged = FPDFCatalog_IsTagged(doc());
-    document_features.form_type =
-        static_cast<FormType>(FPDF_GetFormType(doc()));
-    client_->DocumentLoadComplete(document_features);
-  }
+  client_->DocumentLoadComplete();
 }
 
 void PDFiumEngine::UnsupportedFeature(const std::string& feature) {
@@ -4055,7 +4047,11 @@ void PDFiumEngine::LoadDocumentMetadata() {
   DCHECK(document_loaded_);
 
   doc_metadata_.version = GetDocumentVersion();
+  doc_metadata_.page_count = pages_.size();
   doc_metadata_.linearized = IsLinearized();
+  doc_metadata_.has_attachments = !doc_attachment_info_list_.empty();
+  doc_metadata_.tagged = FPDFCatalog_IsTagged(doc());
+  doc_metadata_.form_type = static_cast<FormType>(FPDF_GetFormType(doc()));
 
   // Document information dictionary entries
   doc_metadata_.title = GetTrimmedMetadataByField("Title");

pdf/preview_mode_client.cc

@@ -131,8 +131,7 @@ PreviewModeClient::SearchString(const base::char16* string,
   return std::vector<SearchStringResult>();
 }
 
-void PreviewModeClient::DocumentLoadComplete(
-    const PDFEngine::DocumentFeatures& document_features) {
+void PreviewModeClient::DocumentLoadComplete() {
   client_->PreviewDocumentLoadComplete();
 }
 

pdf/preview_mode_client.h

@@ -67,8 +67,7 @@ class PreviewModeClient : public PDFEngine::Client {
   std::vector<SearchStringResult> SearchString(const base::char16* string,
                                                const base::char16* term,
                                                bool case_sensitive) override;
-  void DocumentLoadComplete(
-      const PDFEngine::DocumentFeatures& document_features) override;
+  void DocumentLoadComplete() override;
   void DocumentLoadFailed() override;
   pp::Instance* GetPluginInstance() override;
   void DocumentHasUnsupportedFeature(const std::string& feature) override;