Reland "Count annotation subtypes that appear in PDF documents."

Original: https://chromium-review.googlesource.com/c/chromium/src/+/740141
Revert: https://chromium-review.googlesource.com/c/chromium/src/+/788030

This reverts commit 112d7b0.

Bug: 768986,788176
TBR: isherman@chromium.org
Change-Id: Ie5c8889f73443b9eefccd803f15cd0c0b1cd1ce9
Reviewed-on: https://chromium-review.googlesource.com/788330
Reviewed-by: dsinclair <dsinclair@chromium.org>
Commit-Queue: Henrique Nakashima <hnakashima@chromium.org>
Cr-Commit-Position: refs/heads/master@{#520262}

pdf/BUILD.gn

@@ -42,6 +42,7 @@ if (enable_pdf) {
       "paint_manager.h",
       "pdf.cc",
       "pdf.h",
+      "pdf_engine.cc",
       "pdf_engine.h",
       "preview_mode_client.cc",
       "preview_mode_client.h",

pdf/out_of_process_instance.cc

@@ -176,6 +176,10 @@ enum PDFFeatures {
   FEATURES_COUNT
 };
 
+// Used for UMA. Do not delete entries, and keep in sync with histograms.xml
+// and pdfium/public/fpdf_annot.h.
+const int kAnnotationTypesCount = 28;
+
 PP_Var GetLinkAtPosition(PP_Instance instance, PP_Point point) {
   pp::Var var;
   void* object = pp::Instance::GetPerInstanceObject(instance, kPPPPdfInterface);
@@ -1174,8 +1178,11 @@ void OutOfProcessInstance::DocumentSizeUpdated(const pp::Size& size) {
   dimensions.Set(kJSDocumentWidth, pp::Var(document_size_.width()));
   dimensions.Set(kJSDocumentHeight, pp::Var(document_size_.height()));
   pp::VarArray page_dimensions_array;
-  int num_pages = engine_->GetNumberOfPages();
-  for (int i = 0; i < num_pages; ++i) {
+  size_t num_pages = engine_->GetNumberOfPages();
+  if (page_is_processed_.size() < num_pages)
+    page_is_processed_.resize(num_pages);
+
+  for (size_t i = 0; i < num_pages; ++i) {
     pp::Rect page_rect = engine_->GetPageRect(i);
     pp::VarDictionary page_dimensions;
     page_dimensions.Set(kJSPageX, pp::Var(page_rect.x()));
@@ -1298,6 +1305,30 @@ void OutOfProcessInstance::NotifySelectedFindResultChanged(
   SelectedFindResultChanged(current_find_index);
 }
 
+void OutOfProcessInstance::NotifyPageBecameVisible(
+    const PDFEngine::PageFeatures* page_features) {
+  if (!page_features || !page_features->IsInitialized() ||
+      page_features->index >= static_cast<int>(page_is_processed_.size()) ||
+      page_is_processed_[page_features->index]) {
+    return;
+  }
+
+  for (const int annotation_type : page_features->annotation_types) {
+    DCHECK_GE(annotation_type, 0);
+    DCHECK_LT(annotation_type, kAnnotationTypesCount);
+    if (annotation_type < 0 || annotation_type >= kAnnotationTypesCount)
+      continue;
+
+    if (annotation_types_counted_.find(annotation_type) ==
+        annotation_types_counted_.end()) {
+      HistogramEnumeration("PDF.AnnotationType", annotation_type,
+                           kAnnotationTypesCount);
+      annotation_types_counted_.insert(annotation_type);
+    }
+  }
+  page_is_processed_[page_features->index] = true;
+}
+
 void OutOfProcessInstance::GetDocumentPassword(
     pp::CompletionCallbackWithOutput<pp::Var> callback) {
   if (password_callback_) {

pdf/out_of_process_instance.h

@@ -108,6 +108,8 @@ class OutOfProcessInstance : public pp::Instance,
   void UpdateTickMarks(const std::vector<pp::Rect>& tickmarks) override;
   void NotifyNumberOfFindResultsChanged(int total, bool final_result) override;
   void NotifySelectedFindResultChanged(int current_find_index) override;
+  void NotifyPageBecameVisible(
+      const PDFEngine::PageFeatures* page_features) override;
   void GetDocumentPassword(
       pp::CompletionCallbackWithOutput<pp::Var> callback) override;
   void Alert(const std::string& message) override;
@@ -410,6 +412,12 @@ class OutOfProcessInstance : public pp::Instance,
   // toolbar.
   int top_toolbar_height_in_viewport_coords_;
 
+  // Whether each page had its features processed.
+  std::vector<bool> page_is_processed_;
+
+  // Annotation types that were already counted for this document.
+  std::set<int> annotation_types_counted_;
+
   // The current state of accessibility: either off, enabled but waiting
   // for the document to load, or fully loaded.
   enum AccessibilityState {

pdf/pdf_engine.cc

@@ -0,0 +1,20 @@
+// Copyright 2017 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/pdf_engine.h"
+
+namespace chrome_pdf {
+
+PDFEngine::PageFeatures::PageFeatures(){};
+
+PDFEngine::PageFeatures::PageFeatures(const PageFeatures& other)
+    : index(other.index), annotation_types(other.annotation_types) {}
+
+PDFEngine::PageFeatures::~PageFeatures(){};
+
+bool PDFEngine::PageFeatures::IsInitialized() const {
+  return index >= 0;
+}
+
+}  // namespace chrome_pdf

pdf/pdf_engine.h

@@ -14,6 +14,7 @@
 #endif
 
 #include <memory>
+#include <set>
 #include <string>
 #include <vector>
 
@@ -68,6 +69,7 @@ class PDFEngine {
     kCount = 4,
   };
 
+  // Features in a document that are relevant to measure.
   struct DocumentFeatures {
     // Number of pages in document.
     size_t page_count = 0;
@@ -83,6 +85,22 @@ class PDFEngine {
     FormType form_type = FormType::kNone;
   };
 
+  // Features in a page that are relevant to measure.
+  struct PageFeatures {
+    PageFeatures();
+    PageFeatures(const PageFeatures& other);
+    ~PageFeatures();
+
+    // Whether the instance has been initialized and filled.
+    bool IsInitialized() const;
+
+    // 0-based page index in the document. < 0 when uninitialized.
+    int index = -1;
+
+    // Set of annotation types found in page.
+    std::set<int> annotation_types;
+  };
+
   // The interface that's provided to the rendering engine.
   class Client {
    public:
@@ -129,6 +147,10 @@ class PDFEngine {
     // Updates the index of the currently selected search item.
     virtual void NotifySelectedFindResultChanged(int current_find_index) = 0;
 
+    // Notifies a page became visible.
+    virtual void NotifyPageBecameVisible(
+        const PDFEngine::PageFeatures* page_features) = 0;
+
     // Prompts the user for a password to open this document. The callback is
     // called when the password is retrieved.
     virtual void GetDocumentPassword(

pdf/pdfium/pdfium_engine.cc

@@ -1228,8 +1228,11 @@ void PDFiumEngine::OnPendingRequestComplete() {
   for (int pending_page : pending_pages_) {
     if (CheckPageAvailable(pending_page, &still_pending)) {
       update_pages = true;
-      if (IsPageVisible(pending_page))
+      if (IsPageVisible(pending_page)) {
+        client_->NotifyPageBecameVisible(
+            pages_[pending_page]->GetPageFeatures());
         client_->Invalidate(GetPageScreenRect(pending_page));
+      }
     }
   }
   pending_pages_.swap(still_pending);
@@ -3106,16 +3109,23 @@ void PDFiumEngine::CalculateVisiblePages() {
   pending_pages_.clear();
   doc_loader_->ClearPendingRequests();
 
-  visible_pages_.clear();
+  std::vector<int> formerly_visible_pages;
+  std::swap(visible_pages_, formerly_visible_pages);
+
   pp::Rect visible_rect(plugin_size_);
-  for (size_t i = 0; i < pages_.size(); ++i) {
+  for (int i = 0; i < static_cast<int>(pages_.size()); ++i) {
     // Check an entire PageScreenRect, since we might need to repaint side
     // borders and shadows even if the page itself is not visible.
     // For example, when user use pdf with different page sizes and zoomed in
     // outside page area.
     if (visible_rect.Intersects(GetPageScreenRect(i))) {
       visible_pages_.push_back(i);
-      CheckPageAvailable(i, &pending_pages_);
+      if (CheckPageAvailable(i, &pending_pages_)) {
+        auto it = std::find(formerly_visible_pages.begin(),
+                            formerly_visible_pages.end(), i);
+        if (it == formerly_visible_pages.end())
+          client_->NotifyPageBecameVisible(pages_[i]->GetPageFeatures());
+      }
     } else {
       // Need to unload pages when we're not using them, since some PDFs use a
       // lot of memory.  See http://crbug.com/48791

pdf/pdfium/pdfium_page.cc

@@ -19,6 +19,7 @@
 #include "pdf/pdfium/pdfium_api_string_buffer_adapter.h"
 #include "pdf/pdfium/pdfium_engine.h"
 #include "printing/units.h"
+#include "third_party/pdfium/public/fpdf_annot.h"
 
 using printing::ConvertUnitDouble;
 using printing::kPointsPerInch;
@@ -572,6 +573,27 @@ pp::Rect PDFiumPage::PageToScreen(const pp::Point& offset,
                   new_size_y.ValueOrDie());
 }
 
+const PDFEngine::PageFeatures* PDFiumPage::GetPageFeatures() {
+  // If page_features_ is cached, return the cached features.
+  if (page_features_.IsInitialized())
+    return &page_features_;
+
+  FPDF_PAGE page = GetPage();
+  if (!page)
+    return nullptr;
+
+  // Initialize and cache page_features_.
+  page_features_.index = index_;
+  int annotation_count = FPDFPage_GetAnnotCount(page);
+  for (int i = 0; i < annotation_count; ++i) {
+    FPDF_ANNOTATION annotation = FPDFPage_GetAnnot(page, i);
+    FPDF_ANNOTATION_SUBTYPE subtype = FPDFAnnot_GetSubtype(annotation);
+    page_features_.annotation_types.insert(subtype);
+  }
+
+  return &page_features_;
+}
+
 PDFiumPage::ScopedLoadCounter::ScopedLoadCounter(PDFiumPage* page)
     : page_(page) {
   page_->loading_count_++;

pdf/pdfium/pdfium_page.h

@@ -10,6 +10,7 @@
 
 #include "base/optional.h"
 #include "base/strings/string16.h"
+#include "pdf/pdf_engine.h"
 #include "ppapi/cpp/rect.h"
 #include "third_party/pdfium/public/fpdf_doc.h"
 #include "third_party/pdfium/public/fpdf_formfill.h"
@@ -107,6 +108,8 @@ class PDFiumPage {
                         double bottom,
                         int rotation) const;
 
+  const PDFEngine::PageFeatures* GetPageFeatures();
+
   int index() const { return index_; }
   pp::Rect rect() const { return rect_; }
   void set_rect(const pp::Rect& r) { rect_ = r; }
@@ -164,6 +167,7 @@ class PDFiumPage {
   bool calculated_links_;
   std::vector<Link> links_;
   bool available_;
+  PDFEngine::PageFeatures page_features_;
 };
 
 }  // namespace chrome_pdf

pdf/preview_mode_client.cc

@@ -59,6 +59,9 @@ void PreviewModeClient::NotifySelectedFindResultChanged(
   NOTREACHED();
 }
 
+void PreviewModeClient::NotifyPageBecameVisible(
+    const PDFEngine::PageFeatures* page_features) {}
+
 void PreviewModeClient::GetDocumentPassword(
     pp::CompletionCallbackWithOutput<pp::Var> callback) {
   callback.Run(PP_ERROR_FAILED);

pdf/preview_mode_client.h

@@ -39,6 +39,8 @@ class PreviewModeClient : public PDFEngine::Client {
   void UpdateTickMarks(const std::vector<pp::Rect>& tickmarks) override;
   void NotifyNumberOfFindResultsChanged(int total, bool final_result) override;
   void NotifySelectedFindResultChanged(int current_find_index) override;
+  void NotifyPageBecameVisible(
+      const PDFEngine::PageFeatures* page_features) override;
   void GetDocumentPassword(
       pp::CompletionCallbackWithOutput<pp::Var> callback) override;
   void Alert(const std::string& message) override;

tools/metrics/histograms/enums.xml

@@ -4776,6 +4776,37 @@ uploading your change for review.  These are checked by presubmit scripts.
   <int value="22" label="Profile image"/>
 </enum>
 
+<enum name="ChromePDFViewerAnnotationType">
+  <int value="0" label="Unknown"/>
+  <int value="1" label="Text"/>
+  <int value="2" label="Link"/>
+  <int value="3" label="FreeText"/>
+  <int value="4" label="Line"/>
+  <int value="5" label="Square"/>
+  <int value="6" label="Circle"/>
+  <int value="7" label="Polygon"/>
+  <int value="8" label="Polyline"/>
+  <int value="9" label="Highlight"/>
+  <int value="10" label="Underline"/>
+  <int value="11" label="Squiggly"/>
+  <int value="12" label="StrikeOut"/>
+  <int value="13" label="Stamp"/>
+  <int value="14" label="Caret"/>
+  <int value="15" label="Ink"/>
+  <int value="16" label="Popup"/>
+  <int value="17" label="FileAttachment"/>
+  <int value="18" label="Sound"/>
+  <int value="19" label="Movie"/>
+  <int value="20" label="Widget"/>
+  <int value="21" label="Screen"/>
+  <int value="22" label="PrinterMark"/>
+  <int value="23" label="TrapNet"/>
+  <int value="24" label="Watermark"/>
+  <int value="25" label="3D"/>
+  <int value="26" label="RichMedia"/>
+  <int value="27" label="XFAWidget"/>
+</enum>
+
 <enum name="ChromePDFViewerLoadStatus">
   <int value="0" label="Loaded a full-page PDF with Chrome PDF Viewer"/>
   <int value="1" label="Loaded an embedded PDF with Chrome PDF Viewer"/>

tools/metrics/histograms/histograms.xml

@@ -59768,6 +59768,14 @@ http://cs/file:chrome/histograms.xml - but prefer this file for new entries.
   </summary>
 </histogram>
 
+<histogram name="PDF.AnnotationType" enum="ChromePDFViewerAnnotationType">
+  <owner>hnakashima@chromium.org</owner>
+  <summary>
+    Tracks documents opened in the PDF viewer that displayed one or more pages
+    with the given annotation type.
+  </summary>
+</histogram>
+
 <histogram name="PDF.DocumentFeature" enum="PDFFeatures">
   <owner>tsergeant@chromium.org</owner>
   <summary>