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

This reverts commit 6c9aa01. Reason for revert: breaks chromium.lkgr Win_SyzyASAN_LKG build Bug: 788176 Original change's description: > Count annotation subtypes that appear in PDF documents. > > Bug: chromium:768986 > Change-Id: I25fcf447501224d02be40565c8ec770a06edd2e6 > Reviewed-on: https://chromium-review.googlesource.com/740141 > Commit-Queue: Henrique Nakashima <hnakashima@chromium.org> > Reviewed-by: dsinclair <dsinclair@chromium.org> > Reviewed-by: Ilya Sherman <isherman@chromium.org> > Cr-Commit-Position: refs/heads/master@{#518771} TBR=dsinclair@chromium.org,isherman@chromium.org,hnakashima@chromium.org Change-Id: I05c59dcd9488428bf8182216bb81aac8c667ea4e No-Presubmit: true No-Tree-Checks: true No-Try: true Bug: chromium:768986 Reviewed-on: https://chromium-review.googlesource.com/788030 Reviewed-by: Sigurður Ásgeirsson <siggi@chromium.org> Commit-Queue: Sigurður Ásgeirsson <siggi@chromium.org> Cr-Commit-Position: refs/heads/master@{#518929}
marcosholgado · Nov 23, 2017 · 112d7b0 · 112d7b0
1 parent 57b2dc2
commit 112d7b0
Show file tree

Hide file tree

Showing 12 changed files with 5 additions and 167 deletions.
diff --git a/pdf/BUILD.gn b/pdf/BUILD.gn
@@ -42,7 +42,6 @@ if (enable_pdf) {
       "paint_manager.h",
       "pdf.cc",
       "pdf.h",
-      "pdf_engine.cc",
       "pdf_engine.h",
       "preview_mode_client.cc",
       "preview_mode_client.h",

diff --git a/pdf/out_of_process_instance.cc b/pdf/out_of_process_instance.cc
@@ -176,10 +176,6 @@ 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);
@@ -1178,11 +1174,8 @@ 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;
-  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) {
+  int num_pages = engine_->GetNumberOfPages();
+  for (int 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()));
@@ -1303,30 +1296,6 @@ 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_) {

diff --git a/pdf/out_of_process_instance.h b/pdf/out_of_process_instance.h
@@ -108,8 +108,6 @@ 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;
@@ -412,12 +410,6 @@ 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 {

diff --git a/pdf/pdf_engine.cc b/pdf/pdf_engine.cc
diff --git a/pdf/pdf_engine.h b/pdf/pdf_engine.h
@@ -14,7 +14,6 @@
 #endif
 
 #include <memory>
-#include <set>
 #include <string>
 #include <vector>
 
@@ -69,7 +68,6 @@ 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;
@@ -85,22 +83,6 @@ 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:
@@ -147,10 +129,6 @@ 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(

diff --git a/pdf/pdfium/pdfium_engine.cc b/pdf/pdfium/pdfium_engine.cc
@@ -1230,11 +1230,8 @@ void PDFiumEngine::OnPendingRequestComplete() {
   for (int pending_page : pending_pages_) {
     if (CheckPageAvailable(pending_page, &still_pending)) {
       update_pages = true;
-      if (IsPageVisible(pending_page)) {
-        client_->NotifyPageBecameVisible(
-            pages_[pending_page]->GetPageFeatures());
+      if (IsPageVisible(pending_page))
         client_->Invalidate(GetPageScreenRect(pending_page));
-      }
     }
   }
   pending_pages_.swap(still_pending);
@@ -3113,9 +3110,7 @@ void PDFiumEngine::CalculateVisiblePages() {
   pending_pages_.clear();
   doc_loader_->ClearPendingRequests();
 
-  std::vector<int> formerly_visible_pages;
-  std::swap(visible_pages_, formerly_visible_pages);
-
+  visible_pages_.clear();
   pp::Rect visible_rect(plugin_size_);
   for (size_t i = 0; i < pages_.size(); ++i) {
     // Check an entire PageScreenRect, since we might need to repaint side
@@ -3124,12 +3119,7 @@ void PDFiumEngine::CalculateVisiblePages() {
     // outside page area.
     if (visible_rect.Intersects(GetPageScreenRect(i))) {
       visible_pages_.push_back(i);
-      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());
-      }
+      CheckPageAvailable(i, &pending_pages_);
     } else {
       // Need to unload pages when we're not using them, since some PDFs use a
       // lot of memory.  See http://crbug.com/48791

diff --git a/pdf/pdfium/pdfium_page.cc b/pdf/pdfium/pdfium_page.cc
@@ -19,7 +19,6 @@
 #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;
@@ -573,27 +572,6 @@ 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_++;

diff --git a/pdf/pdfium/pdfium_page.h b/pdf/pdfium/pdfium_page.h
@@ -10,7 +10,6 @@
 
 #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"
@@ -108,8 +107,6 @@ 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; }
@@ -167,7 +164,6 @@ class PDFiumPage {
   bool calculated_links_;
   std::vector<Link> links_;
   bool available_;
-  PDFEngine::PageFeatures page_features_;
 };
 
 }  // namespace chrome_pdf

diff --git a/pdf/preview_mode_client.cc b/pdf/preview_mode_client.cc
@@ -59,9 +59,6 @@ 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);

diff --git a/pdf/preview_mode_client.h b/pdf/preview_mode_client.h
@@ -39,8 +39,6 @@ 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;

diff --git a/tools/metrics/histograms/enums.xml b/tools/metrics/histograms/enums.xml
@@ -4745,37 +4745,6 @@ 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"/>

diff --git a/tools/metrics/histograms/histograms.xml b/tools/metrics/histograms/histograms.xml
@@ -59534,14 +59534,6 @@ 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>