Merge pull request #11271 from Snuffleupagus/rm-DummyStatTimer

Re-factor `StatTimer` usage, in `src/display/api.js`, and remove `DummyStatTimer`

src/display/api.js

@@ -27,8 +27,8 @@ import {
   unreachable, warn
 } from '../shared/util';
 import {
-  DOMCanvasFactory, DOMCMapReaderFactory, DummyStatTimer, loadScript,
+  DOMCanvasFactory, DOMCMapReaderFactory, loadScript, PageViewport,
-  PageViewport, releaseImageResources, RenderingCancelledException, StatTimer
+  releaseImageResources, RenderingCancelledException, StatTimer
 } from './display_utils';
 import { FontFaceObject, FontLoader } from './font_loader';
 import { apiCompatibilityParams } from './api_compatibility';
@@ -905,7 +905,7 @@ class PDFPageProxy {
     this.pageIndex = pageIndex;
     this._pageInfo = pageInfo;
     this._transport = transport;
-    this._stats = (pdfBug ? new StatTimer() : DummyStatTimer);
+    this._stats = (pdfBug ? new StatTimer() : null);
     this._pdfBug = pdfBug;
     this.commonObjs = transport.commonObjs;
     this.objs = new PDFObjects();
@@ -995,8 +995,9 @@ class PDFPageProxy {
   render({ canvasContext, viewport, intent = 'display', enableWebGL = false,
            renderInteractiveForms = false, transform = null, imageLayer = null,
            canvasFactory = null, background = null, }) {
-    const stats = this._stats;
+    if (this._stats) {
-    stats.time('Overall');
+      this._stats.time('Overall');
+    }
 
     const renderingIntent = (intent === 'print' ? 'print' : 'display');
     // If there was a pending destroy, cancel it so no cleanup happens during
@@ -1029,7 +1030,9 @@ class PDFPageProxy {
         lastChunk: false,
       };
 
-      stats.time('Page Request');
+      if (this._stats) {
+        this._stats.time('Page Request');
+      }
       this._pumpOperatorList({
         pageIndex: this.pageNumber - 1,
         intent: renderingIntent,
@@ -1060,8 +1063,10 @@ class PDFPageProxy {
       } else {
         internalRenderTask.capability.resolve();
       }
-      stats.timeEnd('Rendering');
+      if (this._stats) {
-      stats.timeEnd('Overall');
+        this._stats.timeEnd('Rendering');
+        this._stats.timeEnd('Overall');
+      }
     };
 
     const internalRenderTask = new InternalRenderTask({
@@ -1094,7 +1099,9 @@ class PDFPageProxy {
         complete();
         return;
       }
-      stats.time('Rendering');
+      if (this._stats) {
+        this._stats.time('Rendering');
+      }
       internalRenderTask.initializeGraphics(transparency);
       internalRenderTask.operatorListChanged();
     }).catch(complete);
@@ -1137,7 +1144,9 @@ class PDFPageProxy {
         lastChunk: false,
       };
 
-      this._stats.time('Page Request');
+      if (this._stats) {
+        this._stats.time('Page Request');
+      }
       this._pumpOperatorList({
         pageIndex: this.pageIndex,
         intent: renderingIntent,
@@ -1259,7 +1268,7 @@ class PDFPageProxy {
     });
     this.objs.clear();
     this.annotationsPromise = null;
-    if (resetStats && this._stats instanceof StatTimer) {
+    if (resetStats && this._stats) {
       this._stats = new StatTimer();
     }
     this.pendingCleanup = false;
@@ -1273,7 +1282,9 @@ class PDFPageProxy {
     if (!intentState) {
       return; // Rendering was cancelled.
     }
-    this._stats.timeEnd('Page Request');
+    if (this._stats) {
+      this._stats.timeEnd('Page Request');
+    }
     // TODO Refactor RenderPageRequest to separate rendering
     // and operator list logic
     if (intentState.displayReadyCapability) {
@@ -1404,10 +1415,10 @@ class PDFPageProxy {
   }
 
   /**
-   * @type {Object} Returns page stats, if enabled.
+   * @type {Object} Returns page stats, if enabled; returns `null` otherwise.
    */
   get stats() {
-    return (this._stats instanceof StatTimer ? this._stats : null);
+    return this._stats;
   }
 }
 

src/display/display_utils.js

@@ -16,7 +16,7 @@
 
 import {
   assert, BaseException, CMapCompressionType, isString, removeNullCharacters,
-  stringToBytes, unreachable, Util, warn
+  stringToBytes, Util, warn
 } from '../shared/util';
 
 const DEFAULT_LINK_REL = 'noopener noreferrer nofollow';
@@ -380,28 +380,21 @@ function getFilenameFromUrl(url) {
 }
 
 class StatTimer {
-  constructor(enable = true) {
+  constructor() {
-    this.enabled = !!enable;
     this.started = Object.create(null);
     this.times = [];
   }
 
   time(name) {
-    if (!this.enabled) {
-      return;
-    }
     if (name in this.started) {
-      warn('Timer is already running for ' + name);
+      warn(`Timer is already running for ${name}`);
     }
     this.started[name] = Date.now();
   }
 
   timeEnd(name) {
-    if (!this.enabled) {
-      return;
-    }
     if (!(name in this.started)) {
-      warn('Timer has not been started for ' + name);
+      warn(`Timer has not been started for ${name}`);
     }
     this.times.push({
       'name': name,
@@ -414,7 +407,7 @@ class StatTimer {
 
   toString() {
     // Find the longest name for padding purposes.
-    let out = '', longest = 0;
+    let outBuf = [], longest = 0;
     for (const time of this.times) {
       const name = time.name;
       if (name.length > longest) {
@@ -423,31 +416,9 @@ class StatTimer {
     }
     for (const time of this.times) {
       const duration = time.end - time.start;
-      out += `${time.name.padEnd(longest)} ${duration}ms\n`;
+      outBuf.push(`${time.name.padEnd(longest)} ${duration}ms\n`);
     }
-    return out;
+    return outBuf.join('');
-  }
-}
-
-/**
- * Helps avoid having to initialize {StatTimer} instances, e.g. one for every
- * page, in cases where the collected stats are not actually being used.
- * This (dummy) class can thus, since all its methods are `static`, be directly
- * shared between multiple call-sites without the need to be initialized first.
- *
- * NOTE: This must implement the same interface as {StatTimer}.
- */
-class DummyStatTimer {
-  constructor() {
-    unreachable('Cannot initialize DummyStatTimer.');
-  }
-
-  static time(name) {}
-
-  static timeEnd(name) {}
-
-  static toString() {
-    return '';
   }
 }
 
@@ -593,7 +564,6 @@ export {
   DOMCMapReaderFactory,
   DOMSVGFactory,
   StatTimer,
-  DummyStatTimer,
   isFetchSupported,
   isValidFetchUrl,
   loadScript,