Merge pull request #10554 from Snuffleupagus/rendering-cancelled

Simplify the `updatetextlayermatches` event handling in `TextLayerBuilder`
mozilla · Feb 16, 2019 · b6f5819 · b6f5819
2 parents 50afff9 + 599dcf5
commit b6f5819
Show file tree

Hide file tree

Showing 3 changed files with 26 additions and 49 deletions.
diff --git a/web/pdf_page_view.js b/web/pdf_page_view.js
@@ -154,6 +154,7 @@ class PDFPageView {
 
   reset(keepZoomLayer = false, keepAnnotations = false) {
     this.cancelRendering(keepAnnotations);
+    this.renderingState = RenderingStates.INITIAL;
 
     let div = this.div;
     div.style.width = Math.floor(this.viewport.width) + 'px';
@@ -258,14 +259,15 @@ class PDFPageView {
     this.reset(/* keepZoomLayer = */ true, /* keepAnnotations = */ true);
   }
 
+  /**
+   * PLEASE NOTE: Most likely you want to use the `this.reset()` method,
+   *              rather than calling this one directly.
+   */
   cancelRendering(keepAnnotations = false) {
-    const renderingState = this.renderingState;
-
     if (this.paintTask) {
       this.paintTask.cancel();
       this.paintTask = null;
     }
-    this.renderingState = RenderingStates.INITIAL;
     this.resume = null;
 
     if (this.textLayer) {
@@ -276,14 +278,6 @@ class PDFPageView {
       this.annotationLayer.cancel();
       this.annotationLayer = null;
     }
-
-    if (renderingState !== RenderingStates.INITIAL) {
-      this.eventBus.dispatch('pagecancelled', {
-        source: this,
-        pageNumber: this.id,
-        renderingState,
-      });
-    }
   }
 
   cssTransform(target, redrawAnnotations = false) {

diff --git a/web/pdf_thumbnail_view.js b/web/pdf_thumbnail_view.js
@@ -152,6 +152,7 @@ class PDFThumbnailView {
 
   reset() {
     this.cancelRendering();
+    this.renderingState = RenderingStates.INITIAL;
 
     this.pageWidth = this.viewport.width;
     this.pageHeight = this.viewport.height;
@@ -195,12 +196,15 @@ class PDFThumbnailView {
     this.reset();
   }
 
+  /**
+   * PLEASE NOTE: Most likely you want to use the `this.reset()` method,
+   *              rather than calling this one directly.
+   */
   cancelRendering() {
     if (this.renderTask) {
       this.renderTask.cancel();
       this.renderTask = null;
     }
-    this.renderingState = RenderingStates.INITIAL;
     this.resume = null;
   }
 

diff --git a/web/text_layer_builder.js b/web/text_layer_builder.js
@@ -53,9 +53,7 @@ class TextLayerBuilder {
     this.textLayerRenderTask = null;
     this.enhanceTextSelection = enhanceTextSelection;
 
-    this._boundEvents = Object.create(null);
-    this._bindEvents();
-
+    this._onUpdateTextLayerMatches = null;
     this._bindMouse();
   }
 
@@ -109,6 +107,16 @@ class TextLayerBuilder {
     }, function (reason) {
       // Cancelled or failed to render text layer; skipping errors.
     });
+
+    if (!this._onUpdateTextLayerMatches) {
+      this._onUpdateTextLayerMatches = (evt) => {
+        if (evt.pageIndex === this.pageIdx || evt.pageIndex === -1) {
+          this._updateMatches();
+        }
+      };
+      this.eventBus.on('updatetextlayermatches',
+                       this._onUpdateTextLayerMatches);
+    }
   }
 
   /**
@@ -119,6 +127,11 @@ class TextLayerBuilder {
       this.textLayerRenderTask.cancel();
       this.textLayerRenderTask = null;
     }
+    if (this._onUpdateTextLayerMatches) {
+      this.eventBus.off('updatetextlayermatches',
+                        this._onUpdateTextLayerMatches);
+      this._onUpdateTextLayerMatches = null;
+    }
   }
 
   setTextContentStream(readableStream) {
@@ -314,40 +327,6 @@ class TextLayerBuilder {
     this._renderMatches(this.matches);
   }
 
-  /**
-   * @private
-   */
-  _bindEvents() {
-    const { eventBus, _boundEvents, } = this;
-
-    _boundEvents.pageCancelled = (evt) => {
-      if (evt.pageNumber !== this.pageNumber) {
-        return;
-      }
-      if (this.textLayerRenderTask) {
-        console.error('TextLayerBuilder._bindEvents: `this.cancel()` should ' +
-          'have been called when the page was reset, or rendering cancelled.');
-        return;
-      }
-      // Ensure that all event listeners are cleaned up when the page is reset,
-      // since re-rendering will create new `TextLayerBuilder` instances and the
-      // number of (stale) event listeners would otherwise grow without bound.
-      for (const name in _boundEvents) {
-        eventBus.off(name.toLowerCase(), _boundEvents[name]);
-        delete _boundEvents[name];
-      }
-    };
-    _boundEvents.updateTextLayerMatches = (evt) => {
-      if (evt.pageIndex !== this.pageIdx && evt.pageIndex !== -1) {
-        return;
-      }
-      this._updateMatches();
-    };
-
-    eventBus.on('pagecancelled', _boundEvents.pageCancelled);
-    eventBus.on('updatetextlayermatches', _boundEvents.updateTextLayerMatches);
-  }
-
   /**
    * Improves text selection by adding an additional div where the mouse was
    * clicked. This reduces flickering of the content if the mouse is slowly