chore: use devtools-protocol package (#6172)

* chore: Use devtools-protocol package

Rather than maintain our own protocol we can instead use the devtools-protocol package and pin it to the version of Chromium that Puppeteer is shipping with.

The only changes are naming changes between the bespoke protocol that Puppeteer created and the devtools-protocol one.

.eslintignore

@@ -6,8 +6,6 @@ node6/*
 node6-test/*
 experimental/
 lib/
-src/externs.d.ts
-src/protocol.d.ts
 /index.d.ts
 # We ignore this file because it uses ES imports which we don't yet use
 # in the Puppeteer src, so it trips up the ESLint-TypeScript parser.

.travis.yml

@@ -63,9 +63,9 @@ jobs:
       env:
         - CHROMIUM=true
       script:
-        - npm run compare-protocol-d-ts
         - npm run lint
         - npm run ensure-new-docs-up-to-date
+        - npm run ensure-correct-devtools-protocol-revision
 
     # This bot runs separately as it changes package.json to test puppeteer-core
     # and we don't want that leaking into other bots and causing issues.

docs/api.md

@@ -331,7 +331,7 @@
   * [target.worker()](#targetworker)
 - [class: CDPSession](#class-cdpsession)
   * [cdpSession.detach()](#cdpsessiondetach)
-  * [cdpSession.send(method[, params])](#cdpsessionsendmethod-params)
+  * [cdpSession.send(method[, ...paramArgs])](#cdpsessionsendmethod-paramargs)
 - [class: Coverage](#class-coverage)
   * [coverage.startCSSCoverage([options])](#coveragestartcsscoverageoptions)
   * [coverage.startJSCoverage([options])](#coveragestartjscoverageoptions)
@@ -3946,9 +3946,9 @@ await client.send('Animation.setPlaybackRate', {
 Detaches the cdpSession from the target. Once detached, the cdpSession object won't emit any events and can't be used
 to send messages.
 
-#### cdpSession.send(method[, params])
+#### cdpSession.send(method[, ...paramArgs])
 - `method` <[string]> protocol method name
-- `params` <[Object]> Optional method parameters
+- `...paramArgs` <[Object]> Optional method parameters
 - returns: <[Promise]<[Object]>>
 
 ### class: Coverage

new-docs/puppeteer.cdpsession.md

@@ -41,5 +41,5 @@ await client.send('Animation.setPlaybackRate', {
 |  Method | Modifiers | Description |
 |  --- | --- | --- |
 |  [detach()](./puppeteer.cdpsession.detach.md) |  | Detaches the cdpSession from the target. Once detached, the cdpSession object won't emit any events and can't be used to send messages. |
-|  [send(method, params)](./puppeteer.cdpsession.send.md) |  |  |
+|  [send(method, paramArgs)](./puppeteer.cdpsession.send.md) |  |  |
 

new-docs/puppeteer.cdpsession.send.md

@@ -7,17 +7,17 @@
 <b>Signature:</b>
 
 ```typescript
-send<T extends keyof Protocol.CommandParameters>(method: T, params?: Protocol.CommandParameters[T]): Promise<Protocol.CommandReturnValues[T]>;
+send<T extends keyof ProtocolMapping.Commands>(method: T, ...paramArgs: ProtocolMapping.Commands[T]['paramsType']): Promise<ProtocolMapping.Commands[T]['returnType']>;
 ```
 
 ## Parameters
 
 |  Parameter | Type | Description |
 |  --- | --- | --- |
 |  method | T |  |
-|  params | Protocol.CommandParameters\[T\] |  |
+|  paramArgs | ProtocolMapping.Commands\[T\]\['paramsType'\] |  |
 
 <b>Returns:</b>
 
-Promise&lt;Protocol.CommandReturnValues\[T\]&gt;
+Promise&lt;ProtocolMapping.Commands\[T\]\['returnType'\]&gt;
 

new-docs/puppeteer.page.deletecookie.md

@@ -7,14 +7,14 @@
 <b>Signature:</b>
 
 ```typescript
-deleteCookie(...cookies: Protocol.Network.deleteCookiesParameters[]): Promise<void>;
+deleteCookie(...cookies: Protocol.Network.DeleteCookiesRequest[]): Promise<void>;
 ```
 
 ## Parameters
 
 |  Parameter | Type | Description |
 |  --- | --- | --- |
-|  cookies | Protocol.Network.deleteCookiesParameters\[\] |  |
+|  cookies | Protocol.Network.DeleteCookiesRequest\[\] |  |
 
 <b>Returns:</b>
 

package.json

@@ -24,16 +24,15 @@
     "doc": "node utils/doclint/cli.js",
     "clean-lib": "rm -rf lib",
     "tsc": "npm run clean-lib && tsc --version && npm run tsc-cjs && npm run tsc-esm",
-    "tsc-cjs": "tsc -p . && cp src/protocol.d.ts lib/cjs",
-    "tsc-esm": "tsc --build tsconfig-esm.json && cp src/protocol.d.ts lib/esm",
+    "tsc-cjs": "tsc -p .",
+    "tsc-esm": "tsc --build tsconfig-esm.json",
     "typecheck": "tsc -p . --noEmit",
     "apply-next-version": "node utils/apply_next_version.js",
-    "update-protocol-d-ts": "node utils/protocol-types-generator update",
-    "compare-protocol-d-ts": "node utils/protocol-types-generator compare",
     "test-install": "scripts/test-install.sh",
     "generate-docs": "npm run tsc && api-extractor run --local --verbose && api-documenter markdown -i temp -o new-docs",
     "ensure-new-docs-up-to-date": "npm run generate-docs && exit `git status --porcelain | head -255 | wc -l`",
-    "generate-dependency-graph": "echo 'Requires graphviz installed locally!' && depcruise --exclude 'api.ts' --do-not-follow '^node_modules' --output-type dot src/index.ts | dot -T png > dependency-chart.png"
+    "generate-dependency-graph": "echo 'Requires graphviz installed locally!' && depcruise --exclude 'api.ts' --do-not-follow '^node_modules' --output-type dot src/index.ts | dot -T png > dependency-chart.png",
+    "ensure-correct-devtools-protocol-revision": "ts-node scripts/ensure-correct-devtools-protocol-package"
   },
   "files": [
     "lib/",
@@ -46,6 +45,7 @@
   "license": "Apache-2.0",
   "dependencies": {
     "debug": "^4.1.0",
+    "devtools-protocol": "0.0.754670",
     "extract-zip": "^2.0.0",
     "https-proxy-agent": "^4.0.0",
     "mime": "^2.0.3",

scripts/ensure-correct-devtools-protocol-package.ts

@@ -0,0 +1,83 @@
+/**
+ * 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.
+ */
+
+/**
+ * This script ensures that the pinned version of devtools-protocol in
+ * package.json is the right version for the current revision of Chromium that
+ * Puppeteer ships with.
+ *
+ * The devtools-protocol package publisher runs every hour and checks if there
+ * are protocol changes. If there are, it will be versioned with the revision
+ * number of the commit that last changed the .pdl files.
+ *
+ * Chromium branches/releases are figured out at a later point in time, so it's
+ * not true that each Chromium revision will have an exact matching revision
+ * version of devtools-protocol. To ensure we're using a devtools-protocol that
+ * is aligned with our revision, we want to find the largest package number
+ * that's <= the revision that Puppeteer is using.
+ *
+ * This script uses npm's `view` function to list all versions in a range and
+ * find the one closest to our Chromium revision.
+ */
+
+import { PUPPETEER_REVISIONS } from '../src/revisions';
+import { execSync } from 'child_process';
+
+import packageJson from '../package.json';
+
+const currentProtocolPackageInstalledVersion =
+  packageJson.dependencies['devtools-protocol'];
+
+/**
+ * Ensure that the devtools-protocol version is pinned.
+ */
+if (/^[^0-9]/.test(currentProtocolPackageInstalledVersion)) {
+  console.log(
+    `ERROR: devtools-protocol package is not pinned to a specific version.\n`
+  );
+  process.exit(1);
+}
+
+// find the right revision for our Chromium revision
+
+const command = `npm view "devtools-protocol@<=0.0.${PUPPETEER_REVISIONS.chromium}" version | tail -1`;
+
+console.log(
+  'Checking npm for devtools-protocol revisions:\n',
+  `'${command}'`,
+  '\n'
+);
+
+const output = execSync(command, {
+  encoding: 'utf8',
+});
+
+const bestRevisionFromNpm = output.split(' ')[1].replace(/'|\n/g, '');
+
+if (currentProtocolPackageInstalledVersion !== bestRevisionFromNpm) {
+  console.log(`ERROR: bad devtools-protocol revision detected:
+
+    Current Puppeteer Chromium revision: ${PUPPETEER_REVISIONS.chromium}
+    Current devtools-protocol version in package.json: ${currentProtocolPackageInstalledVersion}
+    Expected devtools-protocol version:                ${bestRevisionFromNpm}`);
+
+  process.exit(1);
+}
+
+console.log(
+  `Correct devtools-protocol version found (${bestRevisionFromNpm}).`
+);
+process.exit(0);

src/common/Accessibility.ts

@@ -16,7 +16,7 @@
 
 import { CDPSession } from './Connection';
 import { ElementHandle } from './JSHandle';
-import Protocol from '../protocol';
+import { Protocol } from 'devtools-protocol';
 
 /**
  * Represents a Node and the properties of it that are relevant to Accessibility.

src/common/Browser.ts

@@ -18,8 +18,8 @@ import { assert } from './assert';
 import { helper } from './helper';
 import { Target } from './Target';
 import { EventEmitter } from './EventEmitter';
-import Protocol from '../protocol';
 import { Connection, ConnectionEmittedEvents } from './Connection';
+import { Protocol } from 'devtools-protocol';
 import { Page } from './Page';
 import { ChildProcess } from 'child_process';
 import { Viewport } from './PuppeteerViewport';
@@ -272,7 +272,7 @@ export class Browser extends EventEmitter {
   }
 
   private async _targetCreated(
-    event: Protocol.Target.targetCreatedPayload
+    event: Protocol.Target.TargetCreatedEvent
   ): Promise<void> {
     const targetInfo = event.targetInfo;
     const { browserContextId } = targetInfo;
@@ -314,7 +314,7 @@ export class Browser extends EventEmitter {
   }
 
   private _targetInfoChanged(
-    event: Protocol.Target.targetInfoChangedPayload
+    event: Protocol.Target.TargetInfoChangedEvent
   ): void {
     const target = this._targets.get(event.targetInfo.targetId);
     assert(target, 'target should exist before targetInfoChanged');
@@ -500,7 +500,7 @@ export class Browser extends EventEmitter {
     return !this._connection._closed;
   }
 
-  private _getVersion(): Promise<Protocol.Browser.getVersionReturnValue> {
+  private _getVersion(): Promise<Protocol.Browser.GetVersionResponse> {
     return this._connection.send('Browser.getVersion');
   }
 }

src/common/Connection.ts

@@ -18,7 +18,8 @@ import { debug } from './Debug';
 const debugProtocolSend = debug('puppeteer:protocol:SEND ►');
 const debugProtocolReceive = debug('puppeteer:protocol:RECV ◀');
 
-import Protocol from '../protocol';
+import { Protocol } from 'devtools-protocol';
+import { ProtocolMapping } from 'devtools-protocol/types/protocol-mapping';
 import { ConnectionTransport } from './ConnectionTransport';
 import { EventEmitter } from './EventEmitter';
 
@@ -77,10 +78,17 @@ export class Connection extends EventEmitter {
     return this._url;
   }
 
-  send<T extends keyof Protocol.CommandParameters>(
+  send<T extends keyof ProtocolMapping.Commands>(
     method: T,
-    params?: Protocol.CommandParameters[T]
-  ): Promise<Protocol.CommandReturnValues[T]> {
+    ...paramArgs: ProtocolMapping.Commands[T]['paramsType']
+  ): Promise<ProtocolMapping.Commands[T]['returnType']> {
+    // There is only ever 1 param arg passed, but the Protocol defines it as an
+    // array of 0 or 1 items See this comment:
+    // https://github.com/ChromeDevTools/devtools-protocol/pull/113#issuecomment-412603285
+    // which explains why the protocol defines the params this way for better
+    // type-inference.
+    // So now we check if there are any params or not and deal with them accordingly.
+    const params = paramArgs.length ? paramArgs[0] : undefined;
     const id = this._rawSend({ method, params });
     return new Promise((resolve, reject) => {
       this._callbacks.set(id, { resolve, reject, error: new Error(), method });
@@ -232,17 +240,20 @@ export class CDPSession extends EventEmitter {
     this._sessionId = sessionId;
   }
 
-  send<T extends keyof Protocol.CommandParameters>(
+  send<T extends keyof ProtocolMapping.Commands>(
     method: T,
-    params?: Protocol.CommandParameters[T]
-  ): Promise<Protocol.CommandReturnValues[T]> {
+    ...paramArgs: ProtocolMapping.Commands[T]['paramsType']
+  ): Promise<ProtocolMapping.Commands[T]['returnType']> {
     if (!this._connection)
       return Promise.reject(
         new Error(
           `Protocol error (${method}): Session closed. Most likely the ${this._targetType} has been closed.`
         )
       );
 
+    // See the comment in Connection#send explaining why we do this.
+    const params = paramArgs.length ? paramArgs[0] : undefined;
+
     const id = this._connection._rawSend({
       sessionId: this._sessionId,
       method,

src/common/Coverage.ts

@@ -16,7 +16,7 @@
 
 import { assert } from './assert';
 import { helper, debugError, PuppeteerEventListener } from './helper';
-import Protocol from '../protocol';
+import { Protocol } from 'devtools-protocol';
 import { CDPSession } from './Connection';
 
 import { EVALUATION_SCRIPT_URL } from './ExecutionContext';
@@ -223,7 +223,7 @@ class JSCoverage {
   }
 
   async _onScriptParsed(
-    event: Protocol.Debugger.scriptParsedPayload
+    event: Protocol.Debugger.ScriptParsedEvent
   ): Promise<void> {
     // Ignore puppeteer-injected scripts
     if (event.url === EVALUATION_SCRIPT_URL) return;
@@ -246,10 +246,10 @@ class JSCoverage {
     this._enabled = false;
 
     const result = await Promise.all<
-      Protocol.Profiler.takePreciseCoverageReturnValue,
-      Protocol.Profiler.stopPreciseCoverageReturnValue,
-      Protocol.Profiler.disableReturnValue,
-      Protocol.Debugger.disableReturnValue
+      Protocol.Profiler.TakePreciseCoverageResponse,
+      void,
+      void,
+      void
     >([
       this._client.send('Profiler.takePreciseCoverage'),
       this._client.send('Profiler.stopPreciseCoverage'),
@@ -322,9 +322,7 @@ class CSSCoverage {
     this._stylesheetSources.clear();
   }
 
-  async _onStyleSheet(
-    event: Protocol.CSS.styleSheetAddedPayload
-  ): Promise<void> {
+  async _onStyleSheet(event: Protocol.CSS.StyleSheetAddedEvent): Promise<void> {
     const header = event.header;
     // Ignore anonymous scripts
     if (!header.sourceURL) return;

src/common/Dialog.ts

@@ -16,7 +16,7 @@
 
 import { assert } from './assert';
 import { CDPSession } from './Connection';
-import Protocol from '../protocol';
+import { Protocol } from 'devtools-protocol';
 
 /**
  * Dialog instances are dispatched by the {@link Page} via the `dialog` event.

src/common/EmulationManager.ts

@@ -15,7 +15,7 @@
  */
 import { CDPSession } from './Connection';
 import { Viewport } from './PuppeteerViewport';
-import Protocol from '../protocol';
+import { Protocol } from 'devtools-protocol';
 
 export class EmulationManager {
   _client: CDPSession;

src/common/ExecutionContext.ts

@@ -20,7 +20,7 @@ import { createJSHandle, JSHandle, ElementHandle } from './JSHandle';
 import { CDPSession } from './Connection';
 import { DOMWorld } from './DOMWorld';
 import { Frame } from './FrameManager';
-import Protocol from '../protocol';
+import { Protocol } from 'devtools-protocol';
 import { EvaluateHandleFn, SerializableOrJSHandle } from './EvalTypes';
 
 export const EVALUATION_SCRIPT_URL = '__puppeteer_evaluation_script__';
@@ -301,7 +301,7 @@ export class ExecutionContext {
       return { value: arg };
     }
 
-    function rewriteError(error: Error): Protocol.Runtime.evaluateReturnValue {
+    function rewriteError(error: Error): Protocol.Runtime.EvaluateResponse {
       if (error.message.includes('Object reference chain is too long'))
         return { result: { type: 'undefined' } };
       if (error.message.includes("Object couldn't be returned by value"))

src/common/FileChooser.ts

@@ -15,7 +15,7 @@
  */
 
 import { ElementHandle } from './JSHandle';
-import Protocol from '../protocol';
+import { Protocol } from 'devtools-protocol';
 import { assert } from './assert';
 
 /**
@@ -45,7 +45,7 @@ export class FileChooser {
    */
   constructor(
     element: ElementHandle,
-    event: Protocol.Page.fileChooserOpenedPayload
+    event: Protocol.Page.FileChooserOpenedEvent
   ) {
     this._element = element;
     this._multiple = event.mode !== 'selectSingle';