Propagate browser zoom changes to embedded PDFs.

chrome/browser/resources/pdf/zoom_manager.js

Index: chrome/browser/resources/pdf/zoom_manager.js
diff --git a/chrome/browser/resources/pdf/zoom_manager.js b/chrome/browser/resources/pdf/zoom_manager.js
index 2b52c54aba365b95ecfc84ddfbc38ced89971912..31de9381629d3687aca267d4260e90bf093c344c 100644
--- a/chrome/browser/resources/pdf/zoom_manager.js
+++ b/chrome/browser/resources/pdf/zoom_manager.js
@@ -5,20 +5,89 @@
 'use strict';
 
 /**
- * A class that manages updating the browser with zoom changes.
+ * Abstract parent of classes that manage updating the browser
+ * with zoom changes and/or updating the viewer's zoom when
+ * the browser zoom changes.
  */
 class ZoomManager {
   /**
-   * Constructs a ZoomManager
+   * Constructs a ZoomManager.
+   * @param {!Viewport} viewport A Viewport for which to manage zoom.
+   * @param {number} initialZoom The initial browser zoom level.
+   */
+  constructor(viewport, initialZoom) {
+    if (this.constructor === ZoomManager) {
+      throw new TypeError('Instantiated abstract class: ZoomManager');
+    }
+    this.viewport_ = viewport;
+    this.browserZoom_ = initialZoom;
+  }
+
+  /**
+   * Creates the appropriate kind of zoom manager given the zoom behaviour.
+   * @param {BrowserApi.ZoomBehaviour} zoomBehaviour How to manage zoom.
+   * @param {!Viewport} viewport A Viewport for which to manage zoom.
+   * @param {Function} setBrowserZoomFunction A function that sets the browser
+   *     zoom to the provided value.
+   * @param {number} initialZoom The initial browser zoom level.
+   */
+  static create(zoomBehaviour, viewport, setBrowserZoomFunction, initialZoom) {
+    switch (zoomBehaviour) {
+      case BrowserApi.ZoomBehaviour.MANAGE:
+        return new ActiveZoomManager(
+            viewport, setBrowserZoomFunction, initialZoom);
+      case BrowserApi.ZoomBehaviour.PROPAGATE_PARENT:
+        return new EmbeddedZoomManager(viewport, initialZoom);
+      default:
+        return new InactiveZoomManager(viewport, initialZoom);
+    }
+  }
+
+  /**
+   * Invoked when a browser-initiated zoom-level change occurs.
+   * @param {number} newZoom the zoom level to zoom to.
+   */
+  onBrowserZoomChange(newZoom) {}
+
+  /**
+   * Invoked when an extension-initiated zoom-level change occurs.
+   */
+  onPdfZoomChange() {}
+
+  /**
+   * Returns whether two numbers are approximately equal.
+   * @param {number} a The first number.
+   * @param {number} b The second number.
+   */
+  floatingPointEquals(a, b) {
+    let MIN_ZOOM_DELTA = 0.01;
+    // 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.
+    return Math.abs(a - b) <= MIN_ZOOM_DELTA;
+  }
+};
+
+/**
+ * InactiveZoomManager has no control over the browser's zoom
+ * and does not respond to browser zoom changes.
+ */
+class InactiveZoomManager extends ZoomManager {};
+
+/**
+ * ActiveZoomManager controls the browser's zoom.
+ */
+class ActiveZoomManager extends ZoomManager {
+  /**
+   * Constructs a ActiveZoomManager.
    * @param {!Viewport} viewport A Viewport for which to manage zoom.
    * @param {Function} setBrowserZoomFunction A function that sets the browser
    *     zoom to the provided value.
    * @param {number} initialZoom The initial browser zoom level.
    */
   constructor(viewport, setBrowserZoomFunction, initialZoom) {
-    this.viewport_ = viewport;
+    super(viewport, initialZoom);
     this.setBrowserZoomFunction_ = setBrowserZoomFunction;
-    this.browserZoom_ = initialZoom;
     this.changingBrowserZoom_ = null;
   }
 
@@ -65,19 +134,32 @@ class ZoomManager {
       // change was in progress. We call back into onPdfZoomChange to ensure the
       // browser zoom is up to date.
       this.onPdfZoomChange();
+    }.bind(this), function() {
+      this.changingBrowserZoom_ = null;
     }.bind(this));
   }
+};
 
+/**
+ * This EmbeddedZoomManager responds to changes in the browser zoom,
+ * but does not control the browser zoom.
+ */
+class EmbeddedZoomManager extends ZoomManager {
   /**
-   * Returns whether two numbers are approximately equal.
-   * @param {number} a The first number.
-   * @param {number} b The second number.
+   * Invoked when a browser-initiated zoom-level change occurs.
+   * @param {number} newZoom the new browser zoom level.
    */
-  floatingPointEquals(a, b) {
-    let MIN_ZOOM_DELTA = 0.01;
-    // 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.
-    return Math.abs(a - b) <= MIN_ZOOM_DELTA;
+  onBrowserZoomChange(newZoom) {
+    // The viewer may have been zoomed independently of the browser,
+    // so we reapply the viewer's difference in zoom.
+    let internalZoomRatio = this.viewport_.zoom / this.browserZoom_;

[raymes, 2016/11/17 00:59:57]

I'm still not a big fan of the fact that we're working backward to determine the
internal zoom - which means that in essence we're just scaling relatively with
each zoom which is prone to rounding error and will affect the behavior of
zooming in the plugin. It seems like it would be better to keep track of browser
zoom separately from the internal zoom and apply them on top of each other.

[Sam McNally, 2016/11/18 04:59:23]

+1

[Kevin McNee, 2016/11/22 23:17:32]

Done.

+    let newViewportZoom = newZoom * internalZoomRatio;
+
+    this.browserZoom_ = newZoom;
+
+    if (this.floatingPointEquals(this.viewport_.zoom, newViewportZoom))
+      return;
+
+    this.viewport_.setZoom(newViewportZoom);
   }
 };