Skip to content

Commit

Permalink
Revert "chore: remove src/TaskQueue (puppeteer#5826)"
Browse files Browse the repository at this point in the history
This reverts commit 49ce659.
  • Loading branch information
Dmitry Steblyuk committed Apr 1, 2021
1 parent 30c7b3c commit 5568480
Show file tree
Hide file tree
Showing 5 changed files with 63 additions and 28 deletions.
6 changes: 5 additions & 1 deletion src/common/Browser.ts
Expand Up @@ -21,6 +21,7 @@ import { EventEmitter } from './EventEmitter.js';
import { Connection, ConnectionEmittedEvents } from './Connection.js';
import { Protocol } from 'devtools-protocol';
import { Page } from './Page.js';
import { TaskQueue } from './TaskQueue';
import { ChildProcess } from 'child_process';
import { Viewport } from './PuppeteerViewport.js';

Expand Down Expand Up @@ -206,6 +207,7 @@ export class Browser extends EventEmitter {
private _closeCallback: BrowserCloseCallback;
private _defaultContext: BrowserContext;
private _contexts: Map<string, BrowserContext>;
private _screenshotTaskQueue: TaskQueue;
/**
* @internal
* Used in Target.ts directly so cannot be marked private.
Expand All @@ -227,6 +229,7 @@ export class Browser extends EventEmitter {
this._ignoreHTTPSErrors = ignoreHTTPSErrors;
this._defaultViewport = defaultViewport;
this._process = process;
this._screenshotTaskQueue = new TaskQueue();
this._connection = connection;
this._closeCallback = closeCallback || function (): void {};

Expand Down Expand Up @@ -332,7 +335,8 @@ export class Browser extends EventEmitter {
context,
() => this._connection.createSession(targetInfo),
this._ignoreHTTPSErrors,
this._defaultViewport
this._defaultViewport,
this._screenshotTaskQueue
);
assert(
!this._targets.has(event.targetInfo.targetId),
Expand Down
2 changes: 1 addition & 1 deletion src/common/JSHandle.ts
Expand Up @@ -715,7 +715,7 @@ export class ElementHandle<
* {@link Page.screenshot} to take a screenshot of the element.
* If the element is detached from DOM, the method throws an error.
*/
async screenshot(options = {}): Promise<string | Buffer | void> {
async screenshot(options = {}): Promise<string | Buffer> {
let needsViewportReset = false;

let boundingBox = await this.boundingBox();
Expand Down
42 changes: 18 additions & 24 deletions src/common/Page.ts
Expand Up @@ -60,6 +60,7 @@ import {
} from './EvalTypes.js';
import { PDFOptions, paperFormats } from './PDFOptions.js';
import { isNode } from '../environment.js';
import { TaskQueue } from './TaskQueue';

/**
* @public
Expand Down Expand Up @@ -358,22 +359,6 @@ export interface PageEventObject {
workerdestroyed: WebWorker;
}

class ScreenshotTaskQueue {
_chain: Promise<Buffer | string | void>;

constructor() {
this._chain = Promise.resolve<Buffer | string | void>(undefined);
}

public postTask(
task: () => Promise<Buffer | string>
): Promise<Buffer | string | void> {
const result = this._chain.then(task);
this._chain = result.catch(() => {});
return result;
}
}

/**
* Page provides methods to interact with a single tab or
* {@link https://developer.chrome.com/extensions/background_pages | extension background page} in Chromium.
Expand Down Expand Up @@ -425,9 +410,15 @@ export class Page extends EventEmitter {
client: CDPSession,
target: Target,
ignoreHTTPSErrors: boolean,
defaultViewport: Viewport | null
defaultViewport: Viewport | null,
screenshotTaskQueue: TaskQueue
): Promise<Page> {
const page = new Page(client, target, ignoreHTTPSErrors);
const page = new Page(
client,
target,
ignoreHTTPSErrors,
screenshotTaskQueue
);
await page._initialize();
if (defaultViewport) await page.setViewport(defaultViewport);
return page;
Expand All @@ -448,7 +439,7 @@ export class Page extends EventEmitter {
private _coverage: Coverage;
private _javascriptEnabled = true;
private _viewport: Viewport | null;
private _screenshotTaskQueue: ScreenshotTaskQueue;
private _screenshotTaskQueue: TaskQueue;
private _workers = new Map<string, WebWorker>();
// TODO: improve this typedef - it's a function that takes a file chooser or
// something?
Expand All @@ -459,7 +450,12 @@ export class Page extends EventEmitter {
/**
* @internal
*/
constructor(client: CDPSession, target: Target, ignoreHTTPSErrors: boolean) {
constructor(
client: CDPSession,
target: Target,
ignoreHTTPSErrors: boolean,
screenshotTaskQueue: TaskQueue
) {
super();
this._client = client;
this._target = target;
Expand All @@ -476,7 +472,7 @@ export class Page extends EventEmitter {
this._emulationManager = new EmulationManager(client);
this._tracing = new Tracing(client);
this._coverage = new Coverage(client);
this._screenshotTaskQueue = new ScreenshotTaskQueue();
this._screenshotTaskQueue = screenshotTaskQueue;
this._viewport = null;

client.on('Target.attachedToTarget', (event) => {
Expand Down Expand Up @@ -1698,9 +1694,7 @@ export class Page extends EventEmitter {
await this._frameManager.networkManager().setCacheEnabled(enabled);
}

async screenshot(
options: ScreenshotOptions = {}
): Promise<Buffer | string | void> {
async screenshot(options: ScreenshotOptions = {}): Promise<Buffer | string> {
let screenshotType = null;
// options.type takes precedence over inferring the type from options.path
// because it may be a 0-length file with no extension created beforehand
Expand Down
9 changes: 7 additions & 2 deletions src/common/Target.ts
Expand Up @@ -20,6 +20,7 @@ import { CDPSession } from './Connection.js';
import { Browser, BrowserContext } from './Browser.js';
import { Viewport } from './PuppeteerViewport.js';
import { Protocol } from 'devtools-protocol';
import { TaskQueue } from './TaskQueue';

/**
* @public
Expand All @@ -33,6 +34,7 @@ export class Target {
private _defaultViewport?: Viewport;
private _pagePromise?: Promise<Page>;
private _workerPromise?: Promise<WebWorker>;
private _screenshotTaskQueue: TaskQueue;
/**
* @internal
*/
Expand Down Expand Up @@ -66,14 +68,16 @@ export class Target {
browserContext: BrowserContext,
sessionFactory: () => Promise<CDPSession>,
ignoreHTTPSErrors: boolean,
defaultViewport: Viewport | null
defaultViewport: Viewport | null,
screenshotTaskQueue: TaskQueue
) {
this._targetInfo = targetInfo;
this._browserContext = browserContext;
this._targetId = targetInfo.targetId;
this._sessionFactory = sessionFactory;
this._ignoreHTTPSErrors = ignoreHTTPSErrors;
this._defaultViewport = defaultViewport;
this._screenshotTaskQueue = screenshotTaskQueue;
/** @type {?Promise<!Puppeteer.Page>} */
this._pagePromise = null;
/** @type {?Promise<!WebWorker>} */
Expand Down Expand Up @@ -121,7 +125,8 @@ export class Target {
client,
this,
this._ignoreHTTPSErrors,
this._defaultViewport
this._defaultViewport,
this._screenshotTaskQueue
)
);
}
Expand Down
32 changes: 32 additions & 0 deletions src/common/TaskQueue.ts
@@ -0,0 +1,32 @@
/**
* Copyright 2020 Google Inc. All rights reserved.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/

export class TaskQueue {
private _chain: Promise<void>;

constructor() {
this._chain = Promise.resolve();
}

postTask<T>(task: () => Promise<T>): Promise<T> {
const result = this._chain.then(task);
this._chain = result.then(
() => undefined,
() => undefined
);
return result;
}
}

0 comments on commit 5568480

Please sign in to comment.