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

chrome/browser/resources/pdf/pdf.js

 // Copyright 2013 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.
 
 'use strict';
 
 <include src="../../../../ui/webui/resources/js/util.js">
 <include src="pdf_scripting_api.js">
 <include src="viewport.js">
 
@@ skipping 277 matching lines @@
       this.sendScriptingMessage_({
         type: 'documentLoaded'
       });
       if (this.lastViewportPosition_)
         this.viewport_.position = this.lastViewportPosition_;
     }
   },
 
   /**
    * @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.
    */
   onPasswordSubmitted_: function(event) {
     this.plugin_.postMessage({
       type: 'getPasswordComplete',
       password: event.detail.password
     });
   },
@@ skipping 57 matching lines @@
         this.viewport_.position = position;
         break;
       case 'setTranslatedStrings':
         this.passwordScreen_.text = message.data.getPasswordString;
         this.progressBar_.text = message.data.loadingString;
         this.errorScreen_.text = message.data.loadFailedString;
         break;
       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;
     }
   },
 
   /**
    * @private
    * A callback that's called before the zoom changes. Notify the plugin to stop
    * reacting to scroll events while zoom is taking place to avoid flickering.
    */
   beforeZoom_: function() {
     this.plugin_.postMessage({
@@ skipping 154 matching lines @@
 
   /**
    * @type {Viewport} the viewport of the PDF viewer.
    */
   get viewport() {
     return this.viewport_;
   }
 };
 
 var viewer = new PDFViewer();