OOP PDF - Add support for "zoom" 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 128 matching lines @@
   if (chrome.tabs) {
     chrome.tabs.setZoomSettings({mode: 'manual', scope: 'per-tab'},
                                 this.afterZoom_.bind(this));
     chrome.tabs.onZoomChange.addListener(function(zoomChangeInfo) {
       // If the zoom level is close enough to the current zoom level, don't
       // change it. This avoids us getting into an infinite loop of zoom changes
       // due to floating point error.
       var MIN_ZOOM_DELTA = 0.01;
       var zoomDelta = Math.abs(this.viewport_.zoom -
                                zoomChangeInfo.newZoomFactor);
-      if (zoomDelta > MIN_ZOOM_DELTA)
+      if ((zoomDelta > MIN_ZOOM_DELTA) && !this.setZoomInProgress_)

[Nikhil, 2014/08/21 11:36:50]

This was again calling setZoom() with older zoom factor even though we are in
between of zoom triggered by #zoom parameter.
 
For test.pdf#zoom=120
this.viewport_.zoom = 1.2
zoomChangeInfo.newZoomFactor = 1

Since the delta was greater than MIN_ZOOM_DELTA, setZoom() was again called
setting viewport_.zoom to 1 and canceling the effect of #zoom parameter.

Please let me know if this fix is acceptable. If so, I'd really like to move it
to separate CL to keep this CL only about 'zoom' param implementation :) 

[raymes, 2014/08/22 03:52:21]

Hmm I'm not sure if this is completely correct. Basically it means the order of
zoom events isn't preserved. For example if I do the following:
1) Initiate a zoom through the plugin
2) Initiate a zoom from the browser

Then the affect of (2) might be ignored, which isn't what we want. 

The problem here is that you want to set an "initial zoom". Have a look in
Viewport.setDocumentDimensions(). This is where we set the initial zoom (look
inside the block which has 
if (initialDimensions) {...

So I think we should add a member to Viewport (Viewport.initialViewport) which
is used in setDocumentDimensions (rather than the current way it computes the
initial zoom). initialViewport could be a dictionary which contains the initial
position/page to scroll to as well, which could also be set in that method.

Then we could actually just call handleOpenPDFParams_ from the PDFViewer
constructor.

         this.viewport_.setZoom(zoomChangeInfo.newZoomFactor);
     }.bind(this));
   }
 }
 
 PDFViewer.prototype = {
   /**
    * @private
    * Handle key events. These may come from the user directly or via the
    * scripting API.
@@ skipping 97 matching lines @@
    * @private
    * Notify the plugin to print.
    */
   print_: function() {
     this.plugin_.postMessage({
       type: 'print',
     });
   },
 
   /**
+   * Handle zoom parameter of open PDF parameters. If this
+   * parameter is passed while opening PDF then PDF should be opened
+   * at the specified zoom level.
+   * @param {number} zoom value.
+   */
+  handleZoomParam_: function(paramValue) {
+    var paramValueSplit = paramValue.split(',');
+    if ((paramValueSplit.length != 1) && (paramValueSplit.length != 3))
+      return;
+
+    // User scale of 100 means zoom value of 100% i.e. zoom factor of 1.0.
+    var zoomFactor = parseFloat(paramValueSplit[0]) / 100;
+
+    // Handle #zoom=scale.
+    if (paramValueSplit.length == 1) {
+      this.viewport_.setZoom(zoomFactor);
+      return;
+    }
+
+    // Handle #zoom=scale,left,top.
+    var position = {x: parseFloat(paramValueSplit[1]),
+                    y: parseFloat(paramValueSplit[2])};
+    this.viewport_.position = position;
+    this.viewport_.setZoom(zoomFactor);
+  },
+
+  /**
    * @private
    * Handle open PDF parameters. These parameters are mentioned in the URL
    * and specify actions to be performed when opening PDF files.
-   * See http://crbug.com/64309 for details.
+   * See http://www.adobe.com/content/dam/Adobe/en/devnet/acrobat/pdfs/pdf_open_parameters.pdf

[raymes, 2014/08/22 03:52:21]

you'll have to split up the URL so it doesn't extend past 80chars :(

+   * for details.
    */
   handleOpenPDFParams_: function() {
     var originalUrl = this.streamDetails.originalUrl;
     var paramIndex = originalUrl.search('#');
     if (paramIndex == -1)
       return;
 
     var paramTokens = originalUrl.substring(paramIndex + 1).split('&');
     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) {
       // |pageNumber| is 1-based, but goToPage() take a zero-based page number.
       var pageNumber = parseInt(paramsDictionary['page']);
       if (!isNaN(pageNumber))
         this.viewport_.goToPage(pageNumber - 1);
     }
+    if ('zoom' in paramsDictionary)
+      this.handleZoomParam_(paramsDictionary['zoom']);
   },

[raymes, 2014/08/22 03:52:21]

This class is already quite large and I'd like to start pulling stuff out of it
for maintainability. Would you be able to pull out these two functions into a
separate class/file called OpenPDFParamsParser (or something to that effect).
Then you could have some code in the PDFViewer constructor which looks like
this:

var params_parser = new OpenPDFParamsParser(url);
viewport_.initialViewport = params_parser.viewport;  // this is just dictionary
with a few fields

And then in Viewport.setDocumentDimensions:

if (this.initialViewport.position)
  // scroll to position
else if (this.initialViewport.page)
  // scroll to page
else 
  // scroll to (0, 0)

if (this.initialViewport.zoom)
  // zoom to zoom
else
  ...

Does this make sense?

 
   /**
    * @private
    * Update the loading progress of the document in response to a progress
    * message being received from the plugin.
    * @param {number} progress the progress as a percentage.
    */
   updateProgress_: function(progress) {
     this.progressBar_.progress = progress;
     if (progress == -1) {
       // Document load failed.
       this.errorScreen_.style.visibility = 'visible';
       this.sizer_.style.display = 'none';
       this.toolbar_.style.visibility = 'hidden';
       if (this.passwordScreen_.active) {
         this.passwordScreen_.deny();
         this.passwordScreen_.active = false;
       }
     } else if (progress == 100) {
       // Document load complete.
+      if (this.lastViewportPosition_)
+        this.viewport_.position = this.lastViewportPosition_;
       this.handleOpenPDFParams_();
       this.loaded = true;
       var loadEvent = new Event('pdfload');
       window.dispatchEvent(loadEvent);
       this.sendScriptingMessage_({
         type: 'documentLoaded'
       });
-      if (this.lastViewportPosition_)
-        this.viewport_.position = this.lastViewportPosition_;
     }
   },
 
   /**
    * @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) {
@@ skipping 236 matching lines @@
 
   /**
    * @type {Viewport} the viewport of the PDF viewer.
    */
   get viewport() {
     return this.viewport_;
   }
 };
 
 var viewer = new PDFViewer();