OOP PDF - Add support for "page" open pdf parameter

chrome/browser/resources/pdf/pdf.js

Index: chrome/browser/resources/pdf/pdf.js
diff --git a/chrome/browser/resources/pdf/pdf.js b/chrome/browser/resources/pdf/pdf.js
index 7c3d906bf68dc831ca37c47fc447c616c5c2fa25..a2ef12eaa1908f1d92199032c3e403b0b93ed4a7 100644
--- a/chrome/browser/resources/pdf/pdf.js
+++ b/chrome/browser/resources/pdf/pdf.js
@@ -295,6 +295,50 @@ PDFViewer.prototype = {
 
   /**
    * @private
+   * Handle page param of open PDF parameters.
+   * @param {number} page value.
+   */
+  handlePageParam_: function(value) {
+    if (value > 0) {
+      // value is 1-based.
+      this.viewport_.goToPage(value - 1);
+    }
+  },

[raymes, 2014/08/15 01:07:18]

I would suggest just inlining this function. viewport_.goToPage should already
check value > 0 so you don't need that check.

[Nikhil, 2014/08/16 10:24:01]

Done.

+
+  /**
+   * @private
+   * Handle open PDF parameters.
+   */
+  handleOpenPDFParams_: function() {
+    var originalUrl = this.streamDetails.originalUrl;
+    var hasParams = originalUrl.search('#');
+    if (hasParams == -1)
+      return;
+
+    var paramTokens = originalUrl.substring(hasParams + 1).split('&');
+    // Handle the case of http://foo.com/bar#NAMEDDEST. This is not explicitly
+    // mentioned except by example in the Adobe "PDF Open Parameters" document.
+    if ((paramTokens.length == 1) && (paramTokens[0].search('=') == -1)) {
+      // FIXME: Send message for GetNamedDestinationPage();

[raymes, 2014/08/15 01:07:18]

Are you going to add support for this soon? If not we might as well just remove
these lines in this CL.

Also, when you do go to implement this, I would suggest sending a single message
from the plugin to the JS when the document is loaded which contains all of the
named pages, and then storing it as a member variable in this object.

[Nikhil, 2014/08/16 10:24:01]

Yes, I'm planning to add support for this soon in next CL. I've removed these
lines for now.

Based on your suggestion, I think we also need to add an interface in pdfium
engine that provides all named destinations. Currently, we only have
GetNamedDestinationPage() which returns page index for a particular destination.

Please let me know if it makes sense, and I'll check how to support this.

[raymes, 2014/08/18 00:39:34]

I had a look at the API that pdfium exposes and it doesn't look like we can get
a list of named destinations :(. So we might need to do what you were proposing
and add a new message which has a reply with the page number.

[Nikhil, 2014/08/18 04:53:32]

I'll check it further.

+      return;
+    }
+
+    var paramsDictionary = {};
+    for (var i = 0; i < paramTokens.length; ++i) {
+      var keyValueSplit = paramTokens[i].split('=');
+      if (keyValueSplit.length != 2)
+        continue;
+      paramsDictionary[keyValueSplit[0]] = keyValueSplit[1];
+    }
+
+    // Order is important as later actions can override the effects
+    // of previous actions.
+    if ('page' in paramsDictionary)
+      this.handlePageParam_(paramsDictionary['page']);
+  },
+
+  /**
+   * @private
    * An event handler for handling password-submitted events. These are fired
    * when an event is entered into the password screen.
    * @param {Object} event a password-submitted event.
@@ -372,6 +416,9 @@ PDFViewer.prototype = {
       case 'cancelStreamUrl':
         chrome.streamsPrivate.abort(this.streamDetails.streamUrl);
         break;
+      case 'openPDFParams':
+        this.handleOpenPDFParams_();

[raymes, 2014/08/15 01:07:17]

Rather than calling this here, we can get rid of the new message you added and
just call this function inside updateProgress() (if progress == 100%)

[Nikhil, 2014/08/16 10:24:01]

Done.

+        break;
     }
   },