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="open_pdf_params_parser.js">
 <include src="pdf_scripting_api.js">
 <include src="viewport.js">
@@ skipping 129 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)
+      // We should not change zoom level when we are responsible for initiating
+      // the zoom. onZoomChange() is called before setZoomComplete() callback
+      // when we initiate the zoom.
+      if ((zoomDelta > MIN_ZOOM_DELTA) && !this.setZoomInProgress_)

[Nikhil, 2014/08/29 10:31:47]

Should this be a part of this CL or a separate one? Maybe associate it with OOP
PDF Issue - 303491.

[raymes, 2014/09/01 01:04:58]

It's fine to keep it here.

[Nikhil, 2014/09/01 09:11:44]

Acknowledged.

         this.viewport_.setZoom(zoomChangeInfo.newZoomFactor);
     }.bind(this));
   }
 
   // Parse open pdf parameters.
   var paramsParser = new OpenPDFParamsParser(this.streamDetails.originalUrl);
   this.urlParams_ = paramsParser.urlParams;
 }
 
 PDFViewer.prototype = {
@@ skipping 102 matching lines @@
    * Notify the plugin to print.
    */
   print_: function() {
     this.plugin_.postMessage({
       type: 'print',
     });
   },
 
   /**
    * @private
+   * Handle open pdf parameters. This function updates the viewport as per
+   * the parameters mentioned in the url while opening pdf. The order is
+   * important as later actions can override the effects of previous actions.
+   */
+  handleURLParams_: function() {
+    if (this.urlParams_.page)
+      this.viewport_.goToPage(this.urlParams_.page);
+    if (this.urlParams_.position) {
+      // Make sure we don't cancel effect of page parameter.
+      var currentPosition = this.viewport_.position;
+      var destinationPosition = {};
+      destinationPosition['x'] = currentPosition.x + this.urlParams_.position.x;
+      destinationPosition['y'] = currentPosition.y + this.urlParams_.position.y;

[raymes, 2014/09/01 01:04:58]

Is this the way that adobe reader works (does it add the position param to the
page param)? I would have though it would either choose one or the other.

Maybe you could just write this as:
this.viewport_.position = {
  x: this.viewport_.position.x + this.urlParams_.position.x;
  y: this.viewport_.position.y + this.urlParams_.position.y;
};

[Nikhil, 2014/09/01 09:11:44]

Please consider the following use case -
example.com/test.pdf#page=2&zoom=120,150,150 (or #zoom=120,150,150&page=2)

The above should zoom the position (150,150) of page 2 to 120% zoom level. Same
behavior is observed with Adobe Reader as pdf viewer. I think Firefox doesn't
support zoom parameter as of now.

To support above behavior, current implementation is in the following way -
[1] Go to page if mentioned
[2] Adjust position if mentioned (using current position)
[3] Set zoom level if mentioned

Hmm, maybe we should factor in current position in [2] only if page param is
passed? I think it could be possible for authors to specify a position and
hyperlink it to navigate up/down in a big document, in which case I think
current logic will fail. Since it will always use _current_ position and add to
it. But, I can't seem to find such a pdf to test this. 

Please let me know WDYT.

[raymes, 2014/09/02 03:27:36]

The way you have written it looks fine. But I think you can simplify the code as
I suggested in my previous comment.

+      this.viewport_.position = destinationPosition;
+    }
+    if (this.urlParams_.zoom)
+      this.viewport_.setZoom(this.urlParams_.zoom);
+  },
+
+  /**
+   * @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.handleURLParams_();
       this.loaded = true;
       var loadEvent = new Event('pdfload');
       window.dispatchEvent(loadEvent);
       this.sendScriptingMessage_({
         type: 'documentLoaded'
       });
-      if (this.lastViewportPosition_)
-        this.viewport_.position = this.lastViewportPosition_;
-
-      // Handle open pdf params. Order is important as later actions can
-      // override the effects of previous actions.
-      if (this.urlParams_.page)
-        this.viewport_.goToPage(this.urlParams_.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) {
@@ skipping 236 matching lines @@
 
   /**
    * @type {Viewport} the viewport of the PDF viewer.
    */
   get viewport() {
     return this.viewport_;
   }
 };
 
 var viewer = new PDFViewer();