PDF: Use PDF metadata for the title instead of the last path element.

Review URL: https://codereview.chromium.org/1303103003

Cr-Commit-Position: refs/heads/master@{#345838}

chrome/browser/pdf/pdf_extension_test.cc

@@ -326,6 +326,10 @@ IN_PROC_BROWSER_TEST_F(PDFExtensionTest, ZoomManager) {
   RunTestsInFile("zoom_manager_test.js", "test.pdf");
 }
 
+IN_PROC_BROWSER_TEST_F(PDFExtensionTest, Title) {
+  RunTestsInFile("title_test.js", "test-title.pdf");
+}
+
 // Ensure that the internal PDF plugin application/x-google-chrome-pdf won't be
 // loaded if it's not loaded in the chrome extension page.
 IN_PROC_BROWSER_TEST_F(PDFExtensionTest, EnsureInternalPluginDisabled) {
@@ -412,3 +416,7 @@ IN_PROC_BROWSER_TEST_F(MaterialPDFExtensionTest, Elements) {
 IN_PROC_BROWSER_TEST_F(MaterialPDFExtensionTest, ToolbarManager) {
   RunTestsInFile("toolbar_manager_test.js", "test.pdf");
 }
+
+IN_PROC_BROWSER_TEST_F(MaterialPDFExtensionTest, Title) {
+  RunTestsInFile("title_test.js", "test-title.pdf");
+}

chrome/browser/resources/pdf/elements/viewer-pdf-toolbar/viewer-pdf-toolbar.html

@@ -33,7 +33,7 @@
         <div id="buttons" class="invisible flex-5">
           <!-- TODO(tsergeant): "Bookmarks" should be localized. -->
           <viewer-toolbar-dropdown id="bookmarks"
-                                   hidden$="[[!bookmarks]]"
+                                   hidden$="[[!bookmarks.length]]"
                                    header="Bookmarks"
                                    open-icon="bookmark"
                                    closed-icon="bookmark-border">

chrome/browser/resources/pdf/pdf.js

@@ -149,8 +149,6 @@ function PDFViewer(browserApi) {
   window.addEventListener('message', this.handleScriptingMessage.bind(this),
                           false);
 
-  document.title = decodeURIComponent(
-      getFilenameFromURL(this.browserApi_.getStreamInfo().originalUrl));
   this.plugin_.setAttribute('src',
                             this.browserApi_.getStreamInfo().originalUrl);
   this.plugin_.setAttribute('stream-url',
@@ -198,7 +196,6 @@ function PDFViewer(browserApi) {
         this.viewport_.zoomOut.bind(this.viewport_));
 
     this.materialToolbar_ = $('material-toolbar');
-    this.materialToolbar_.docTitle = document.title;
     this.materialToolbar_.addEventListener('save', this.save_.bind(this));
     this.materialToolbar_.addEventListener('print', this.print_.bind(this));
     this.materialToolbar_.addEventListener('rotate-right',
@@ -607,10 +604,18 @@ PDFViewer.prototype = {
       case 'cancelStreamUrl':
         chrome.mimeHandlerPrivate.abortStream();
         break;
-      case 'bookmarks':
+      case 'metadata':
+        if (message.data.title) {
+          document.title = message.data.title;
+        } else {
+          document.title = decodeURIComponent(
+              getFilenameFromURL(this.browserApi_.getStreamInfo().originalUrl));
+        }
         this.bookmarks_ = message.data.bookmarks;
-        if (this.isMaterial_ && this.bookmarks_.length !== 0)
+        if (this.isMaterial_) {
+          this.materialToolbar_.docTitle = document.title;
           this.materialToolbar_.bookmarks = this.bookmarks;
+        }
         break;
       case 'setIsSelecting':
         this.viewportScroller_.setEnableScrolling(message.data.isSelecting);

chrome/test/data/pdf/basic_plugin_test.js

@@ -55,6 +55,15 @@ var tests = [
       chrome.test.assertEq('this is some text\nsome more text', selectedText);
     }));
   },
+
+  /**
+   * Test that the filename is used as the title.pdf.
+   */
+  function testHasCorrectTitle() {
+    chrome.test.assertEq('test.pdf', document.title);
+
+    chrome.test.succeed();
+  },
 ];
 
 var scriptingAPI = new PDFScriptingAPI(window, window);

chrome/test/data/pdf/title_test.js

@@ -0,0 +1,19 @@
+// Copyright 2015 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.
+
+var tests = [
+  /**
+   * Test that the correct title is displayed for test-title.pdf.
+   */
+  function testHasCorrectTitle() {
+    chrome.test.assertEq('PDF title test', document.title);
+
+    chrome.test.succeed();
+  }
+];
+
+var scriptingAPI = new PDFScriptingAPI(window, window);
+scriptingAPI.setLoadCallback(function() {
+  chrome.test.runTests(tests);
+});

pdf/out_of_process_instance.cc

@@ -77,9 +77,10 @@ const char kJSPageHeight[] = "height";
 // Document load progress arguments (Plugin -> Page)
 const char kJSLoadProgressType[] = "loadProgress";
 const char kJSProgressPercentage[] = "progress";
-// Bookmarks
-const char kJSBookmarksType[] = "bookmarks";
+// Metadata
+const char kJSMetadataType[] = "metadata";
 const char kJSBookmarks[] = "bookmarks";
+const char kJSTitle[] = "title";
 // Get password arguments (Plugin -> Page)
 const char kJSGetPasswordType[] = "getPassword";
 // Get password complete arguments (Page -> Plugin)
@@ -1126,10 +1127,14 @@ void OutOfProcessInstance::DocumentLoadComplete(int page_count) {
     OnGeometryChanged(0, 0);
   }
 
-  pp::VarDictionary bookmarks_message;
-  bookmarks_message.Set(pp::Var(kType), pp::Var(kJSBookmarksType));
-  bookmarks_message.Set(pp::Var(kJSBookmarks), engine_->GetBookmarks());
-  PostMessage(bookmarks_message);
+  pp::VarDictionary metadata_message;
+  metadata_message.Set(pp::Var(kType), pp::Var(kJSMetadataType));
+  std::string title = engine_->GetMetadata("Title");
+  if (!title.empty())
+    metadata_message.Set(pp::Var(kJSTitle), pp::Var(title));
+
+  metadata_message.Set(pp::Var(kJSBookmarks), engine_->GetBookmarks());
+  PostMessage(metadata_message);
 
   pp::VarDictionary progress_message;
   progress_message.Set(pp::Var(kType), pp::Var(kJSLoadProgressType));

pdf/pdf_engine.h

@@ -279,6 +279,8 @@ class PDFEngine {
   virtual void SetScrollPosition(const pp::Point& position) = 0;
 
   virtual bool IsProgressiveLoad() = 0;
+
+  virtual std::string GetMetadata(const std::string& key) = 0;
 };
 
 // Interface for exports that wrap the PDF engine.

pdf/pdfium/pdfium_api_string_buffer_adapter.cc

@@ -7,7 +7,6 @@
 #include <string>
 
 #include "base/logging.h"
-#include "base/numerics/safe_math.h"
 #include "base/strings/string16.h"
 #include "base/strings/string_util.h"
 
@@ -36,12 +35,6 @@ void* PDFiumAPIStringBufferAdapter<StringType>::GetData() {
   return data_;
 }
 
-template <class StringType>
-void PDFiumAPIStringBufferAdapter<StringType>::Close(int actual_size) {
-  base::CheckedNumeric<size_t> unsigned_size = actual_size;
-  Close(unsigned_size.ValueOrDie());
-}
-
 template <class StringType>
 void PDFiumAPIStringBufferAdapter<StringType>::Close(size_t actual_size) {
   DCHECK(!is_closed_);
@@ -58,8 +51,36 @@ void PDFiumAPIStringBufferAdapter<StringType>::Close(size_t actual_size) {
   }
 }
 
+template <class StringType>
+PDFiumAPIStringBufferSizeInBytesAdapter<StringType>::
+    PDFiumAPIStringBufferSizeInBytesAdapter(StringType* str,
+                                            size_t expected_size,
+                                            bool check_expected_size)
+    : adapter_(str,
+               expected_size / sizeof(typename StringType::value_type),
+               check_expected_size) {
+  DCHECK(expected_size % sizeof(typename StringType::value_type) == 0);
+}
+
+template <class StringType>
+PDFiumAPIStringBufferSizeInBytesAdapter<
+    StringType>::~PDFiumAPIStringBufferSizeInBytesAdapter() = default;
+
+template <class StringType>
+void* PDFiumAPIStringBufferSizeInBytesAdapter<StringType>::GetData() {
+  return adapter_.GetData();
+}
+
+template <class StringType>
+void PDFiumAPIStringBufferSizeInBytesAdapter<StringType>::Close(
+    size_t actual_size) {
+  DCHECK(actual_size % sizeof(typename StringType::value_type) == 0);
+  adapter_.Close(actual_size / sizeof(typename StringType::value_type));
+}
+
 // explicit instantiations
 template class PDFiumAPIStringBufferAdapter<std::string>;
 template class PDFiumAPIStringBufferAdapter<base::string16>;
+template class PDFiumAPIStringBufferSizeInBytesAdapter<base::string16>;
 
 }  // namespace chrome_pdf

pdf/pdfium/pdfium_api_string_buffer_adapter.h

@@ -6,6 +6,7 @@
 #define PDF_PDFIUM_PDFIUM_API_STRING_BUFFER_ADAPTER_H_
 
 #include "base/basictypes.h"
+#include "base/numerics/safe_math.h"
 
 namespace chrome_pdf {
 
@@ -35,9 +36,14 @@ class PDFiumAPIStringBufferAdapter {
   // Resizes |str_| to |actual_size| - 1 characters, thereby removing the extra
   // null-terminator. This must be called prior to the adapter's destruction.
   // The pointer returned by GetData() should be considered invalid.
-  void Close(int actual_size);
   void Close(size_t actual_size);
 
+  template <typename IntType>
+  void Close(IntType actual_size) {
+    base::CheckedNumeric<size_t> unsigned_size = actual_size;
+    Close(unsigned_size.ValueOrDie());
+  }
+
  private:
   StringType* const str_;
   void* const data_;
@@ -48,6 +54,46 @@ class PDFiumAPIStringBufferAdapter {
   DISALLOW_COPY_AND_ASSIGN(PDFiumAPIStringBufferAdapter);
 };
 
+// Helper to deal with the fact that many PDFium APIs write the null-terminator
+// into string buffers that are passed to them, but the PDF plugin likes to pass
+// in std::strings / base::string16s, where one should not count on the internal
+// string buffers to be null-terminated. This version is suitable for APIs that
+// work in terms of number of bytes instead of the number of characters.
+template <class StringType>
+class PDFiumAPIStringBufferSizeInBytesAdapter {
+ public:
+  // |str| is the string to write into.
+  // |expected_size| is the number of bytes the PDFium API will write,
+  // including the null-terminator. It should be at least the size of a
+  // character in bytes.
+  // |check_expected_size| whether to check the actual number of bytes
+  // written into |str| against |expected_size| when calling Close().
+  PDFiumAPIStringBufferSizeInBytesAdapter(StringType* str,
+                                          size_t expected_size,
+                                          bool check_expected_size);
+  ~PDFiumAPIStringBufferSizeInBytesAdapter();
+
+  // Returns a pointer to |str_|'s buffer. The buffer's size is large enough to
+  // hold |expected_size_| + sizeof(StringType::value_type) bytes, so the PDFium
+  // API that uses the pointer has space to write a null-terminator.
+  void* GetData();
+
+  // Resizes |str_| to |actual_size| - sizeof(StringType::value_type) bytes,
+  // thereby removing the extra null-terminator. This must be called prior to
+  // the adapter's destruction. The pointer returned by GetData() should be
+  // considered invalid.
+  void Close(size_t actual_size);
+
+  template <typename IntType>
+  void Close(IntType actual_size) {
+    base::CheckedNumeric<size_t> unsigned_size = actual_size;
+    Close(unsigned_size.ValueOrDie());
+  }
+
+ private:
+  PDFiumAPIStringBufferAdapter<StringType> adapter_;
+};
+
 }  // namespace chrome_pdf
 
 #endif  // PDF_PDFIUM_PDFIUM_API_STRING_BUFFER_ADAPTER_H_

pdf/pdfium/pdfium_engine.cc

@@ -525,14 +525,11 @@ pp::VarDictionary TraverseBookmarks(FPDF_DOCUMENT doc, FPDF_BOOKMARK bookmark) {
   pp::VarDictionary dict;
   base::string16 title;
   unsigned long buffer_size = FPDFBookmark_GetTitle(bookmark, NULL, 0);
-  size_t title_length = base::checked_cast<size_t>(buffer_size) /
-      sizeof(base::string16::value_type);
-  if (title_length > 0) {
-    PDFiumAPIStringBufferAdapter<base::string16> api_string_adapter(
-        &title, title_length, true);
-    void* data = api_string_adapter.GetData();
-    FPDFBookmark_GetTitle(bookmark, data, buffer_size);
-    api_string_adapter.Close(title_length);
+  if (buffer_size > 0) {
+    PDFiumAPIStringBufferSizeInBytesAdapter<base::string16> api_string_adapter(
+        &title, buffer_size, true);
+    api_string_adapter.Close(FPDFBookmark_GetTitle(
+        bookmark, api_string_adapter.GetData(), buffer_size));
   }
   dict.Set(pp::Var("title"), pp::Var(base::UTF16ToUTF8(title)));
 
@@ -555,6 +552,19 @@ pp::VarDictionary TraverseBookmarks(FPDF_DOCUMENT doc, FPDF_BOOKMARK bookmark) {
   return dict;
 }
 
+std::string GetDocumentMetadata(FPDF_DOCUMENT doc, const std::string& key) {
+  size_t size = FPDF_GetMetaText(doc, key.c_str(), nullptr, 0);
+  if (size == 0)
+    return std::string();
+
+  base::string16 value;
+  PDFiumAPIStringBufferSizeInBytesAdapter<base::string16> string_adapter(
+      &value, size, false);
+  string_adapter.Close(
+      FPDF_GetMetaText(doc, key.c_str(), string_adapter.GetData(), size));
+  return base::UTF16ToUTF8(value);
+}
+
 }  // namespace
 
 bool InitializeSDK() {
@@ -1176,6 +1186,10 @@ bool PDFiumEngine::IsProgressiveLoad() {
   return doc_loader_.is_partial_document();
 }
 
+std::string PDFiumEngine::GetMetadata(const std::string& key) {
+  return GetDocumentMetadata(doc(), key);
+}
+
 void PDFiumEngine::OnPartialDocumentLoaded() {
   file_access_.m_FileLen = doc_loader_.document_size();
   fpdf_availability_ = FPDFAvail_Create(&file_availability_, &file_access_);
@@ -3931,15 +3945,11 @@ bool PDFiumEngineExports::RenderPDFPageToDC(const void* pdf_buffer,
   // bitmap. Note that this code does not kick in for PDFs printed from Chrome
   // because in that case we create a temp PDF first before printing and this
   // temp PDF does not have a creator string that starts with "cairo".
-  base::string16 creator;
-  size_t buffer_bytes = FPDF_GetMetaText(doc, "Creator", NULL, 0);
-  if (buffer_bytes > 1) {
-    FPDF_GetMetaText(doc, "Creator",
-                     base::WriteInto(&creator, buffer_bytes + 1), buffer_bytes);
-  }
   bool use_bitmap = false;
-  if (base::StartsWith(creator, L"cairo", base::CompareCase::INSENSITIVE_ASCII))
+  if (base::StartsWith(GetDocumentMetadata(doc, "Creator"), "cairo",
+                       base::CompareCase::INSENSITIVE_ASCII)) {
     use_bitmap = true;
+  }
 
   // Another temporary hack. Some PDFs seems to render very slowly if
   // FPDF_RenderPage is directly used on a printer DC. I suspect it is

pdf/pdfium/pdfium_engine.h

@@ -99,6 +99,7 @@ class PDFiumEngine : public PDFEngine,
   virtual pp::Point GetScrollPosition();
   virtual void SetScrollPosition(const pp::Point& position);
   virtual bool IsProgressiveLoad();
+  virtual std::string GetMetadata(const std::string& key);
 
   // DocumentLoader::Client implementation.
   virtual pp::Instance* GetPluginInstance();