Skip to content

Commit

Permalink
Merge pull request #18193 from calixteman/rm_observer
Browse files Browse the repository at this point in the history
Disconnect the resize observer and remove scroll listener when unbinding window events
  • Loading branch information
calixteman committed May 30, 2024
2 parents ea34e5c + 4430b6b commit 4d9c25a
Show file tree
Hide file tree
Showing 4 changed files with 35 additions and 4 deletions.
7 changes: 7 additions & 0 deletions web/app.js
Original file line number Diff line number Diff line change
Expand Up @@ -159,6 +159,7 @@ const PDFViewerApplication = {
_downloadUrl: "",
_eventBusAbortController: null,
_windowAbortController: null,
_globalAbortController: new AbortController(),
documentInfo: null,
metadata: null,
_contentDispositionFilename: null,
Expand Down Expand Up @@ -463,6 +464,7 @@ const PDFViewerApplication = {
enablePermissions: AppOptions.get("enablePermissions"),
pageColors,
mlManager: this.mlManager,
abortSignal: this._globalAbortController.signal,
});
this.pdfViewer = pdfViewer;

Expand All @@ -477,6 +479,7 @@ const PDFViewerApplication = {
renderingQueue: pdfRenderingQueue,
linkService: pdfLinkService,
pageColors,
abortSignal: this._globalAbortController.signal,
});
pdfRenderingQueue.setThumbnailViewer(this.pdfThumbnailViewer);
}
Expand Down Expand Up @@ -2092,6 +2095,10 @@ const PDFViewerApplication = {
unbindWindowEvents() {
this._windowAbortController?.abort();
this._windowAbortController = null;
if (AppOptions.get("isInAutomation")) {
this._globalAbortController?.abort();
this._globalAbortController = null;
}
},

_accumulateTicks(ticks, prop) {
Expand Down
9 changes: 8 additions & 1 deletion web/pdf_thumbnail_viewer.js
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,8 @@ const THUMBNAIL_SELECTED_CLASS = "selected";
* @property {Object} [pageColors] - Overwrites background and foreground colors
* with user defined ones in order to improve readability in high contrast
* mode.
* @property {AbortSignal} [abortSignal] - The AbortSignal for the window
* events.
*/

/**
Expand All @@ -57,14 +59,19 @@ class PDFThumbnailViewer {
linkService,
renderingQueue,
pageColors,
abortSignal,
}) {
this.container = container;
this.eventBus = eventBus;
this.linkService = linkService;
this.renderingQueue = renderingQueue;
this.pageColors = pageColors || null;

this.scroll = watchScroll(this.container, this.#scrollUpdated.bind(this));
this.scroll = watchScroll(
this.container,
this.#scrollUpdated.bind(this),
abortSignal
);
this.#resetView();
}

Expand Down
16 changes: 15 additions & 1 deletion web/pdf_viewer.js
Original file line number Diff line number Diff line change
Expand Up @@ -309,7 +309,21 @@ class PDFViewer {
this.renderingQueue = options.renderingQueue;
}

this.scroll = watchScroll(this.container, this._scrollUpdate.bind(this));
const { abortSignal } = options;
abortSignal?.addEventListener(
"abort",
() => {
this.#resizeObserver.disconnect();
this.#resizeObserver = null;
},
{ once: true }
);

this.scroll = watchScroll(
this.container,
this._scrollUpdate.bind(this),
abortSignal
);
this.presentationModeState = PresentationModeState.UNKNOWN;
this._resetView();

Expand Down
7 changes: 5 additions & 2 deletions web/ui_utils.js
Original file line number Diff line number Diff line change
Expand Up @@ -155,7 +155,7 @@ function scrollIntoView(element, spot, scrollMatches = false) {
* Helper function to start monitoring the scroll event and converting them into
* PDF.js friendly one: with scroll debounce and scroll direction.
*/
function watchScroll(viewAreaElement, callback) {
function watchScroll(viewAreaElement, callback, abortSignal = undefined) {
const debounceScroll = function (evt) {
if (rAF) {
return;
Expand Down Expand Up @@ -189,7 +189,10 @@ function watchScroll(viewAreaElement, callback) {
};

let rAF = null;
viewAreaElement.addEventListener("scroll", debounceScroll, true);
viewAreaElement.addEventListener("scroll", debounceScroll, {
useCapture: true,
signal: abortSignal,
});
return state;
}

Expand Down

0 comments on commit 4d9c25a

Please sign in to comment.