From 87dd93b7fc3cba0c3d4ebc92cbffbee45bafac73 Mon Sep 17 00:00:00 2001 From: Jonas Jenwald Date: Thu, 4 Mar 2021 14:49:02 +0100 Subject: [PATCH] Move handling of the PageOpen/PageClose events into the `PDFScriptingManager` (PR 13042 follow-up) By moving this code from the `BaseViewer` and into `PDFScriptingManager`, all of the scripting initialization/handling code is now limited to just one file/class which help overall readability (in my opinion). Also, this patch is a *net reduction* in number of lines of code which can never hurt. As part of these changes, the intermediary "pageopen"/"pageclose" events are now removed in favor of using the "regular" viewer events directly in `PDFScriptingManager`. Hence this removes some (strictly unnecessary) indirection in the current code, when handling PageOpen/PageClose events, which leads to overall fewer function calls in this part of the code. --- web/base_viewer.js | 81 +------------------------- web/pdf_scripting_manager.js | 106 +++++++++++++++++++++++++---------- 2 files changed, 77 insertions(+), 110 deletions(-) diff --git a/web/base_viewer.js b/web/base_viewer.js index 28c9fb0f2..bd8816740 100644 --- a/web/base_viewer.js +++ b/web/base_viewer.js @@ -470,7 +470,7 @@ class BaseViewer { this.findController.setDocument(null); } if (this._scriptingManager) { - // Defer this slightly, to allow the "pageclose" event to be handled. + // Defer this slightly, to allow the "PageClose" event to be handled. Promise.resolve().then(() => { this._scriptingManager.setDocument(null); }); @@ -670,8 +670,6 @@ class BaseViewer { this.eventBus._off("pagerendered", this._onAfterDraw); this._onAfterDraw = null; } - this._resetScriptingEvents(); - // Remove the pages from the DOM... this.viewer.textContent = ""; // ... and reset the Scroll mode CSS class(es) afterwards. @@ -1648,83 +1646,6 @@ class BaseViewer { this.currentPageNumber = Math.max(currentPageNumber - advance, 1); return true; } - - async initializeScriptingEvents() { - if (!this.enableScripting || this._pageOpenPendingSet) { - return; - } - const eventBus = this.eventBus, - pageOpenPendingSet = (this._pageOpenPendingSet = new Set()), - scriptingEvents = (this._scriptingEvents = Object.create(null)); - - const dispatchPageClose = pageNumber => { - if (pageOpenPendingSet.has(pageNumber)) { - return; // No "pageopen" event was dispatched for the previous page. - } - eventBus.dispatch("pageclose", { source: this, pageNumber }); - }; - const dispatchPageOpen = pageNumber => { - const pageView = this._pages[pageNumber - 1]; - if (pageView?.renderingState === RenderingStates.FINISHED) { - pageOpenPendingSet.delete(pageNumber); - - eventBus.dispatch("pageopen", { - source: this, - pageNumber, - actionsPromise: pageView.pdfPage?.getJSActions(), - }); - } else { - pageOpenPendingSet.add(pageNumber); - } - }; - - scriptingEvents.onPageChanging = ({ pageNumber, previous }) => { - if (pageNumber === previous) { - return; // The active page didn't change. - } - dispatchPageClose(previous); - dispatchPageOpen(pageNumber); - }; - eventBus._on("pagechanging", scriptingEvents.onPageChanging); - - scriptingEvents.onPageRendered = ({ pageNumber }) => { - if (!pageOpenPendingSet.has(pageNumber)) { - return; // No pending "pageopen" event for the newly rendered page. - } - if (pageNumber !== this._currentPageNumber) { - return; // The newly rendered page is no longer the current one. - } - dispatchPageOpen(pageNumber); - }; - eventBus._on("pagerendered", scriptingEvents.onPageRendered); - - scriptingEvents.onPagesDestroy = () => { - dispatchPageClose(this._currentPageNumber); - }; - eventBus._on("pagesdestroy", scriptingEvents.onPagesDestroy); - - // Ensure that a "pageopen" event is dispatched for the initial page. - dispatchPageOpen(this._currentPageNumber); - } - - /** - * @private - */ - _resetScriptingEvents() { - if (!this.enableScripting || !this._pageOpenPendingSet) { - return; - } - const eventBus = this.eventBus, - scriptingEvents = this._scriptingEvents; - - // Remove the event listeners. - eventBus._off("pagechanging", scriptingEvents.onPageChanging); - eventBus._off("pagerendered", scriptingEvents.onPageRendered); - eventBus._off("pagesdestroy", scriptingEvents.onPagesDestroy); - - this._pageOpenPendingSet = null; - this._scriptingEvents = null; - } } export { BaseViewer }; diff --git a/web/pdf_scripting_manager.js b/web/pdf_scripting_manager.js index dd24bd641..ac5b45935 100644 --- a/web/pdf_scripting_manager.js +++ b/web/pdf_scripting_manager.js @@ -15,6 +15,7 @@ import { createPromiseCapability, shadow } from "pdfjs-lib"; import { apiPageLayoutToSpreadMode } from "./ui_utils.js"; +import { RenderingStates } from "./pdf_rendering_queue.js"; /** * @typedef {Object} PDFScriptingManagerOptions @@ -44,6 +45,7 @@ class PDFScriptingManager { this._scripting = null; this._mouseState = Object.create(null); + this._pageEventsReady = false; this._ready = false; this._eventBus = eventBus; @@ -88,6 +90,7 @@ class PDFScriptingManager { if (!objects && !docActions) { // No FieldObjects or JavaScript actions were found in the document. + await this._destroyScripting(); return; } if (pdfDocument !== this._pdfDocument) { @@ -96,17 +99,33 @@ class PDFScriptingManager { this._scripting = this._createScripting(); this._internalEvents.set("updatefromsandbox", event => { + if (event?.source !== window) { + return; + } this._updateFromSandbox(event.detail); }); this._internalEvents.set("dispatcheventinsandbox", event => { this._scripting?.dispatchEventInSandbox(event.detail); }); - this._internalEvents.set("pageopen", event => { - this._pageOpen(event.pageNumber, event.actionsPromise); + this._internalEvents.set("pagechanging", ({ pageNumber, previous }) => { + if (pageNumber === previous) { + return; // The current page didn't change. + } + this._dispatchPageClose(previous); + this._dispatchPageOpen(pageNumber); }); - this._internalEvents.set("pageclose", event => { - this._pageClose(event.pageNumber); + this._internalEvents.set("pagerendered", ({ pageNumber }) => { + if (!this._pageOpenPending.has(pageNumber)) { + return; // No pending "PageOpen" event for the newly rendered page. + } + if (pageNumber !== this._pdfViewer.currentPageNumber) { + return; // The newly rendered page is no longer the current one. + } + this._dispatchPageOpen(pageNumber); + }); + this._internalEvents.set("pagesdestroy", event => { + this._dispatchPageClose(this._pdfViewer.currentPageNumber); }); this._domEvents.set("mousedown", event => { @@ -154,7 +173,10 @@ class PDFScriptingManager { id: "doc", name: "Open", }); - await this._pdfViewer.initializeScriptingEvents(); + await this._dispatchPageOpen( + this._pdfViewer.currentPageNumber, + /* initialize = */ true + ); // Defer this slightly, to ensure that scripting is *fully* initialized. Promise.resolve().then(() => { @@ -218,6 +240,13 @@ class PDFScriptingManager { return shadow(this, "_domEvents", new Map()); } + /** + * @private + */ + get _pageOpenPending() { + return shadow(this, "_pageOpenPending", new Set()); + } + /** * @private */ @@ -273,48 +302,63 @@ class PDFScriptingManager { /** * @private */ - async _pageOpen(pageNumber, actionsPromise) { + async _dispatchPageOpen(pageNumber, initialize = false) { const pdfDocument = this._pdfDocument, visitedPages = this._visitedPages; - visitedPages.set( - pageNumber, - (async () => { - // Avoid sending, and thus serializing, the `actions` data when the - // *same* page is opened several times. - let actions = null; - if (!visitedPages.has(pageNumber)) { - actions = await actionsPromise; - if (pdfDocument !== this._pdfDocument) { - return; // The document was closed while the actions resolved. - } - } + if (initialize) { + this._pageEventsReady = true; + } + if (!this._pageEventsReady) { + return; // Scripting isn't fully initialized yet. + } + const pageView = this._pdfViewer.getPageView(/* index = */ pageNumber - 1); - await this._scripting?.dispatchEventInSandbox({ - id: "page", - name: "PageOpen", - pageNumber, - actions, - }); - })() - ); + if (pageView?.renderingState !== RenderingStates.FINISHED) { + this._pageOpenPending.add(pageNumber); + return; // Wait for the page to finish rendering. + } + this._pageOpenPending.delete(pageNumber); + + const actionsPromise = (async () => { + // Avoid sending, and thus serializing, the `actions` data more than once. + const actions = await (!visitedPages.has(pageNumber) + ? pageView.pdfPage?.getJSActions() + : null); + if (pdfDocument !== this._pdfDocument) { + return; // The document was closed while the actions resolved. + } + + await this._scripting?.dispatchEventInSandbox({ + id: "page", + name: "PageOpen", + pageNumber, + actions, + }); + })(); + visitedPages.set(pageNumber, actionsPromise); } /** * @private */ - async _pageClose(pageNumber) { + async _dispatchPageClose(pageNumber) { const pdfDocument = this._pdfDocument, visitedPages = this._visitedPages; + if (!this._pageEventsReady) { + return; // Scripting isn't fully initialized yet. + } + if (this._pageOpenPending.has(pageNumber)) { + return; // The page is still rendering; no "PageOpen" event dispatched. + } const actionsPromise = visitedPages.get(pageNumber); if (!actionsPromise) { - // Ensure that the "pageclose" event was preceded by a "pageopen" event. - return; + return; // The "PageClose" event must be preceded by a "PageOpen" event. } visitedPages.set(pageNumber, null); - // Ensure that the "pageopen" event is handled first. + // Ensure that the "PageOpen" event is dispatched first. await actionsPromise; if (pdfDocument !== this._pdfDocument) { return; // The document was closed while the actions resolved. @@ -393,10 +437,12 @@ class PDFScriptingManager { } this._domEvents.clear(); + this._pageOpenPending.clear(); this._visitedPages.clear(); this._scripting = null; delete this._mouseState.isDown; + this._pageEventsReady = false; this._ready = false; this._destroyCapability?.resolve();