Print Preview: Finish removing global Javascript functions.

chrome/browser/resources/print_preview/preview_generator.js

 // Copyright (c) 2012 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.
 
 cr.define('print_preview', function() {
   'use strict';
 
   /**
    * Interface to the Chromium print preview generator.
    * @param {!print_preview.DestinationStore} destinationStore Used to get the
@@ skipping 117 matching lines @@
      * @private
      */
     this.isCssBackgroundEnabled_ = false;
 
     /**
      * Destination that was selected for the last preview.
      * @type {print_preview.Destination}
      * @private
      */
     this.selectedDestination_ = null;
-
-    /**
-     * Event tracker used to keep track of native layer events.
-     * @type {!EventTracker}
-     * @private
-     */
-    this.tracker_ = new EventTracker();
-
-    this.addEventListeners_();
   }
 
   /**
    * Event types dispatched by the preview generator.
    * @enum {string}
    */
   PreviewGenerator.EventType = {
     // Dispatched when the document can be printed.
     DOCUMENT_READY: 'print_preview.PreviewGenerator.DOCUMENT_READY',
 
     // Dispatched when a page preview is ready. The previewIndex field of the
     // event is the index of the page in the modified document, not the
     // original. So page 4 of the original document might be previewIndex = 0 of
     // the modified document.
     PAGE_READY: 'print_preview.PreviewGenerator.PAGE_READY',
 
     // Dispatched when the document preview starts to be generated.
     PREVIEW_START: 'print_preview.PreviewGenerator.PREVIEW_START',
 
     // Dispatched when the current print preview request fails.
     FAIL: 'print_preview.PreviewGenerator.FAIL'
   };
 
   PreviewGenerator.prototype = {
     __proto__: cr.EventTarget.prototype,
 
     /**
+     * Registers the preview generator for the WebUI events it responds to and
+     * adds the listeners to |listenerTracker| so that they can be
+     * automatically removed by this tracker later.
+     * @param {!WebUIListenerTracker} listenerTracker The tracker to add the
+     *     listeners to.
+     */
+    addWebUIEventListeners: function(listenerTracker) {

[dpapad, 2017/07/07 21:12:41]

Not for this CL: Just wondering, would it make sense if we take this approach
one step further and

1) pass the |listenerTracker| in the constructor
2) Call addWebUIEventListeners() within the constructor itself
3) Make addWebUIEventListeners private.

This is similar to what the previous code was doing with EventTracker, except
that it was not injected via the constructor. It also eliminates the misuse of
this API, since calling the public addWebUIEventListeners() could be
accidentally called twice, whereas the constructor can't be called twice.

[rbpotter, 2017/07/07 23:58:39]

I think that would make sense. Will change in follow-up CL.

[rbpotter, 2017/07/08 00:25:39]

Follow up for this and other issues at
https://codereview.chromium.org/2975663003/

+      listenerTracker.add(
+          'page-count-ready', this.onPageCountReady_.bind(this));
+      listenerTracker.add(
+          'page-layout-ready', this.onPageLayoutReady_.bind(this));
+      listenerTracker.add(
+          'page-preview-ready', this.onPagePreviewReady_.bind(this));
+    },
+
+    /**
      * Request that new preview be generated. A preview request will not be
      * generated if the print ticket has not changed sufficiently.
      * @return {{id: number,
      *           request: Promise}} The preview request id, or -1 if no preview
      *     was requested, and a promise that will resolve when the preview is
      *     complete (null if no preview was actually requested).
      */
     requestPreview: function() {
       if (!this.printTicketStore_.isTicketValidForPreview() ||
           !this.printTicketStore_.isInitialized ||
@@ skipping 25 matching lines @@
       this.inFlightRequestId_++;
       this.generateDraft_ = this.documentInfo_.isModifiable && previewChanged;
       return {
         id: this.inFlightRequestId_,
         request: this.nativeLayer_.getPreview(
             this.destinationStore_.selectedDestination, this.printTicketStore_,
             this.documentInfo_, this.generateDraft_, this.inFlightRequestId_),
       };
     },
 
-    /** Removes all event listeners that the preview generator has attached. */
-    removeEventListeners: function() {
-      this.tracker_.removeAll();
-    },
-
-    /**
-     * Adds event listeners to the relevant native layer events.
-     * @private
-     */
-    addEventListeners_: function() {
-      var nativeLayerEventTarget = this.nativeLayer_.getEventTarget();
-      this.tracker_.add(
-          nativeLayerEventTarget,
-          print_preview.NativeLayer.EventType.PAGE_LAYOUT_READY,
-          this.onPageLayoutReady_.bind(this));
-      this.tracker_.add(
-          nativeLayerEventTarget,
-          print_preview.NativeLayer.EventType.PAGE_COUNT_READY,
-          this.onPageCountReady_.bind(this));
-      this.tracker_.add(
-          nativeLayerEventTarget,
-          print_preview.NativeLayer.EventType.PAGE_PREVIEW_READY,
-          this.onPagePreviewReady_.bind(this));
-    },
-
     /**
      * Dispatches a PAGE_READY event to signal that a page preview is ready.
      * @param {number} previewIndex Index of the page with respect to the pages
      *     shown in the preview. E.g an index of 0 is the first displayed page,
      *     but not necessarily the first original document page.
      * @param {number} pageNumber Number of the page with respect to the
      *     document. A value of 3 means it's the third page of the original
      *     document.
      * @param {number} previewUid Unique identifier of the preview.
      * @private
@@ skipping 61 matching lines @@
     hasPreviewPageRangeChanged_: function() {
       return this.pageRanges_ == null ||
           !areRangesEqual(
                  this.printTicketStore_.pageRange.getPageRanges(),
                  this.pageRanges_);
     },
 
     /**
      * Called when the page layout of the document is ready. Always occurs
      * as a result of a preview request.
-     * @param {Event} event Contains layout info about the document.
+     * @param {{marginTop: number,
+     *          marginLeft: number,
+     *          marginBottom: number,
+     *          marginRight: number,
+     *          contentWidth: number,
+     *          contentHeight: number,
+     *          printableAreaX: number,
+     *          printableAreaY: number,
+     *          printableAreaWidth: number,
+     *          printableAreaHeight: number,
+     *        }} pageLayout Layout information about the document.
+     * @param {boolean} hasCustomPageSizeStyle Whether this document has a
+     *     custom page size or style to use.
      * @private
      */
-    onPageLayoutReady_: function(event) {
+    onPageLayoutReady_: function(pageLayout, hasCustomPageSizeStyle) {
       // NOTE: A request ID is not specified, so assuming its for the current
       // in-flight request.
 
       var origin = new print_preview.Coordinate2d(
-          event.pageLayout.printableAreaX, event.pageLayout.printableAreaY);
+          pageLayout.printableAreaX, pageLayout.printableAreaY);
       var size = new print_preview.Size(
-          event.pageLayout.printableAreaWidth,
-          event.pageLayout.printableAreaHeight);
+          pageLayout.printableAreaWidth, pageLayout.printableAreaHeight);
 
       var margins = new print_preview.Margins(
-          Math.round(event.pageLayout.marginTop),
-          Math.round(event.pageLayout.marginRight),
-          Math.round(event.pageLayout.marginBottom),
-          Math.round(event.pageLayout.marginLeft));
+          Math.round(pageLayout.marginTop), Math.round(pageLayout.marginRight),
+          Math.round(pageLayout.marginBottom),
+          Math.round(pageLayout.marginLeft));
 
       var o = print_preview.ticket_items.CustomMarginsOrientation;
       var pageSize = new print_preview.Size(
-          event.pageLayout.contentWidth + margins.get(o.LEFT) +
-              margins.get(o.RIGHT),
-          event.pageLayout.contentHeight + margins.get(o.TOP) +
+          pageLayout.contentWidth + margins.get(o.LEFT) + margins.get(o.RIGHT),
+          pageLayout.contentHeight + margins.get(o.TOP) +
               margins.get(o.BOTTOM));
 
       this.documentInfo_.updatePageInfo(
           new print_preview.PrintableArea(origin, size), pageSize,
-          event.hasCustomPageSizeStyle, margins);
+          hasCustomPageSizeStyle, margins);
     },
 
     /**
      * Called when the document page count is received from the native layer.
      * Always occurs as a result of a preview request.
-     * @param {Event} event Contains the document's page count.
+     * @param {number} pageCount The document's page count.
+     * @param {number} previewResponseId The request ID that corresponds to this
+     *     page count.
+     * @param {number} fitToPageScaling The scaling required to fit the document
+     *     to page (unused).
      * @private
      */
-    onPageCountReady_: function(event) {
-      if (this.inFlightRequestId_ != event.previewResponseId) {
+    onPageCountReady_: function(
+        pageCount, previewResponseId, fitToPageScaling) {
+      if (this.inFlightRequestId_ != previewResponseId) {
         return;  // Ignore old response.
       }
-      this.documentInfo_.updatePageCount(event.pageCount);
+      this.documentInfo_.updatePageCount(pageCount);
       this.pageRanges_ = this.printTicketStore_.pageRange.getPageRanges();
     },
 
     /**
      * Called when a page's preview has been generated. Dispatches a
      * PAGE_READY event.
-     * @param {Event} event Contains the page index and preview UID.
+     * @param {number} pageIndex The index of the page whose preview is ready.
+     * @param {number} previewUid The unique ID of the print preview UI.
+     * @param {number} previewResponseId The preview request ID that this page
+     *     preview is a response to.
      * @private
      */
-    onPagePreviewReady_: function(event) {
-      if (this.inFlightRequestId_ != event.previewResponseId) {
+    onPagePreviewReady_: function(pageIndex, previewUid, previewResponseId) {
+      if (this.inFlightRequestId_ != previewResponseId) {
         return;  // Ignore old response.
       }
-      var pageNumber = event.pageIndex + 1;
+      var pageNumber = pageIndex + 1;
       var pageNumberSet = this.printTicketStore_.pageRange.getPageNumberSet();
       if (pageNumberSet.hasPageNumber(pageNumber)) {
         var previewIndex = pageNumberSet.getPageNumberIndex(pageNumber);
         if (previewIndex == 0) {
-          this.dispatchPreviewStartEvent_(event.previewUid, event.pageIndex);
+          this.dispatchPreviewStartEvent_(previewUid, pageIndex);
         }
-        this.dispatchPageReadyEvent_(
-            previewIndex, pageNumber, event.previewUid);
+        this.dispatchPageReadyEvent_(previewIndex, pageNumber, previewUid);
       }
     },
 
     /**
      * Called when the preview generation is complete. Dispatches a
      * DOCUMENT_READY event.
      * @param {number} previewResponseId
      * @param {number} previewUid
      */
     onPreviewGenerationDone: function(previewResponseId, previewUid) {
@@ skipping 16 matching lines @@
     onPreviewGenerationFail_: function() {
       // NOTE: No request ID is returned from Chromium so its assumed its the
       // current one.
       cr.dispatchSimpleEvent(this, PreviewGenerator.EventType.FAIL);
     }
   };
 
   // Export
   return {PreviewGenerator: PreviewGenerator};
 });