chore: use a less chatty BiDi+ (#10453)

2023-06-28 09:54:16 +02:00 · 2023-06-28 09:54:16 +02:00 · ceb6fbb365
commit ceb6fbb365
parent e452647739
7 changed files with 41 additions and 50 deletions
--- a/package-lock.json
+++ b/package-lock.json
@ -3020,9 +3020,9 @@
      }
    },
    "node_modules/chromium-bidi": {
-      "version": "0.4.13",
-      "resolved": "https://registry.npmjs.org/chromium-bidi/-/chromium-bidi-0.4.13.tgz",
-      "integrity": "sha512-9m2SY5DHI43OBQ7SMXjwp/iQaYo6iihqJ4IiD1OlrawGQTNveYYeJJt1yCqMxjp5y86m/uHxc9VooOWVlKFi4w==",
+      "version": "0.4.14",
+      "resolved": "https://registry.npmjs.org/chromium-bidi/-/chromium-bidi-0.4.14.tgz",
+      "integrity": "sha512-79S04G7lLmxbsjIkJaeIWKQQ31ZfzUsfoICWBdjqZnayWnCzBcFAqWhIGu7q85d5LhhauQK5x/gMqIJbV64+DQ==",
      "dependencies": {
        "mitt": "3.0.0"
      },
@ -10109,7 +10109,7 @@
      "license": "Apache-2.0",
      "dependencies": {
        "@puppeteer/browsers": "1.4.2",
-        "chromium-bidi": "0.4.13",
+        "chromium-bidi": "0.4.14",
        "cross-fetch": "3.1.6",
        "debug": "4.3.4",
        "devtools-protocol": "0.0.1135028",
@ -12230,9 +12230,9 @@
      }
    },
    "chromium-bidi": {
-      "version": "0.4.13",
-      "resolved": "https://registry.npmjs.org/chromium-bidi/-/chromium-bidi-0.4.13.tgz",
-      "integrity": "sha512-9m2SY5DHI43OBQ7SMXjwp/iQaYo6iihqJ4IiD1OlrawGQTNveYYeJJt1yCqMxjp5y86m/uHxc9VooOWVlKFi4w==",
+      "version": "0.4.14",
+      "resolved": "https://registry.npmjs.org/chromium-bidi/-/chromium-bidi-0.4.14.tgz",
+      "integrity": "sha512-79S04G7lLmxbsjIkJaeIWKQQ31ZfzUsfoICWBdjqZnayWnCzBcFAqWhIGu7q85d5LhhauQK5x/gMqIJbV64+DQ==",
      "requires": {
        "mitt": "3.0.0"
      }
@ -15653,7 +15653,7 @@
      "version": "file:packages/puppeteer-core",
      "requires": {
        "@puppeteer/browsers": "1.4.2",
-        "chromium-bidi": "0.4.13",
+        "chromium-bidi": "0.4.14",
        "cross-fetch": "3.1.6",
        "debug": "4.3.4",
        "devtools-protocol": "0.0.1135028",
--- a/packages/puppeteer-core/package.json
+++ b/packages/puppeteer-core/package.json
@ -141,7 +141,7 @@
  "author": "The Chromium Authors",
  "license": "Apache-2.0",
  "dependencies": {
-    "chromium-bidi": "0.4.13",
+    "chromium-bidi": "0.4.14",
    "cross-fetch": "3.1.6",
    "debug": "4.3.4",
    "devtools-protocol": "0.0.1135028",
--- a/packages/puppeteer-core/src/common/bidi/Browser.ts
+++ b/packages/puppeteer-core/src/common/bidi/Browser.ts
@ -36,11 +36,18 @@ import {debugError} from './utils.js';
 * @internal
 */
 export class Browser extends BrowserBase {
-  static readonly subscribeModules = [
+  static readonly subscribeModules: Bidi.Session.SubscriptionRequestEvent[] = [
    'browsingContext',
    'network',
    'log',
-    'cdp',
+  ];
+  static readonly subscribeCdpEvents: Bidi.CDP.EventNames[] = [
+    // Coverage
+    'cdp.Debugger.scriptParsed',
+    'cdp.CSS.styleSheetAdded',
+    'cdp.Runtime.executionContextsCleared',
+    // Tracing
+    'cdp.Tracing.tracingComplete',
  ];

  #browserName = '';
@ -67,11 +74,9 @@ export class Browser extends BrowserBase {
    }

    await opts.connection.send('session.subscribe', {
-      events: (browserName.toLocaleLowerCase().includes('firefox')
-        ? Browser.subscribeModules.filter(module => {
-            return !['cdp'].includes(module);
-          })
-        : Browser.subscribeModules) as Bidi.Message.EventNames[],
+      events: browserName.toLocaleLowerCase().includes('firefox')
+        ? Browser.subscribeModules
+        : [...Browser.subscribeModules, ...Browser.subscribeCdpEvents],
    });

    return new Browser({
--- a/packages/puppeteer-core/src/common/bidi/BrowsingContext.ts
+++ b/packages/puppeteer-core/src/common/bidi/BrowsingContext.ts
@ -51,7 +51,7 @@ export class CDPSessionWrapper extends EventEmitter implements CDPSession {
        context: context.id,
      })
      .then(session => {
-        this.#sessionId.resolve(session.result.cdpSession);
+        this.#sessionId.resolve(session.result.session!);
      })
      .catch(err => {
        this.#sessionId.reject(err);
@ -65,11 +65,11 @@ export class CDPSessionWrapper extends EventEmitter implements CDPSession {
    method: T,
    ...paramArgs: ProtocolMapping.Commands[T]['paramsType']
  ): Promise<ProtocolMapping.Commands[T]['returnType']> {
-    const cdpSession = await this.#sessionId.valueOrThrow();
+    const session = await this.#sessionId.valueOrThrow();
    const result = await this.#context.connection.send('cdp.sendCommand', {
-      cdpMethod: method,
-      cdpParams: paramArgs[0] || {},
-      cdpSession,
+      method: method,
+      params: paramArgs[0],
+      session,
    });
    return result.result;
  }
--- a/packages/puppeteer-core/src/common/bidi/Connection.ts
+++ b/packages/puppeteer-core/src/common/bidi/Connection.ts
@ -234,17 +234,14 @@ export class Connection extends EventEmitter {
      // `log.entryAdded` specific context
    } else if ('source' in event.params && event.params.source.context) {
      context = this.#browsingContexts.get(event.params.source.context);
-    } else if (event.method === 'cdp.eventReceived') {
+    } else if (isCDPEvent(event)) {
      // TODO: this is not a good solution and we need to find a better one.
      // Perhaps we need to have a dedicated CDP event emitter or emulate
      // the CDPSession interface with BiDi?.
-      const cdpSessionId = event.params.cdpSession;
+      const cdpSessionId = event.params.session;
      for (const context of this.#browsingContexts.values()) {
        if (context.cdpSession?.id() === cdpSessionId) {
-          context.cdpSession!.emit(
-            event.params.cdpMethod,
-            event.params.cdpParams
-          );
+          context.cdpSession!.emit(event.params.event, event.params.params);
        }
      }
    }
@ -285,3 +282,9 @@ function createProtocolError(object: Bidi.Message.ErrorResult): string {
  }
  return message;
 }
+
+function isCDPEvent(
+  event: Bidi.Message.EventMessage
+): event is Bidi.CDP.EventReceivedEvent {
+  return event.method.startsWith('cdp.');
+}
--- a/test/TestExpectations.json
+++ b/test/TestExpectations.json
@ -1739,18 +1739,6 @@
    "parameters": ["chrome", "webDriverBiDi"],
    "expectations": ["FAIL"]
  },
-  {
-    "testIdPattern": "[coverage.spec] Coverage specs JSCoverage includeRawScriptCoverage should include rawScriptCoverage field when enabled",
-    "platforms": ["darwin", "linux", "win32"],
-    "parameters": ["chrome", "webDriverBiDi"],
-    "expectations": ["FAIL"]
-  },
-  {
-    "testIdPattern": "[coverage.spec] Coverage specs JSCoverage includeRawScriptCoverage should not include rawScriptCoverage field when disabled",
-    "platforms": ["darwin", "linux", "win32"],
-    "parameters": ["chrome", "webDriverBiDi"],
-    "expectations": ["FAIL"]
-  },
  {
    "testIdPattern": "[coverage.spec] Coverage specs JSCoverage should ignore pptr internal scripts if reportAnonymousScripts is true",
    "platforms": ["darwin", "linux", "win32"],
@ -1758,13 +1746,7 @@
    "expectations": ["FAIL"]
  },
  {
-    "testIdPattern": "[coverage.spec] Coverage specs JSCoverage should work",
-    "platforms": ["darwin", "linux", "win32"],
-    "parameters": ["chrome", "webDriverBiDi"],
-    "expectations": ["FAIL"]
-  },
-  {
-    "testIdPattern": "[coverage.spec] Coverage specs JSCoverage shouldn't ignore eval() scripts if reportAnonymousScripts is true",
+    "testIdPattern": "[coverage.spec] Coverage specs JSCoverage should not ignore eval() scripts if reportAnonymousScripts is true",
    "platforms": ["darwin", "linux", "win32"],
    "parameters": ["chrome", "webDriverBiDi"],
    "expectations": ["FAIL", "PASS"]
--- a/test/src/coverage.spec.ts
+++ b/test/src/coverage.spec.ts
@ -24,7 +24,7 @@ describe('Coverage specs', function () {
      const {page, server} = await getTestState();
      await page.coverage.startJSCoverage();
      await page.goto(server.PREFIX + '/jscoverage/simple.html', {
-        waitUntil: 'networkidle0',
+        waitUntil: 'load',
      });
      const coverage = await page.coverage.stopJSCoverage();
      expect(coverage).toHaveLength(1);
@ -51,7 +51,7 @@ describe('Coverage specs', function () {
      const coverage = await page.coverage.stopJSCoverage();
      expect(coverage).toHaveLength(1);
    });
-    it("shouldn't ignore eval() scripts if reportAnonymousScripts is true", async () => {
+    it('should not ignore eval() scripts if reportAnonymousScripts is true', async () => {
      const {page, server} = await getTestState();

      await page.coverage.startJSCoverage({reportAnonymousScripts: true});
@ -183,7 +183,7 @@ describe('Coverage specs', function () {
        const {page, server} = await getTestState();
        await page.coverage.startJSCoverage();
        await page.goto(server.PREFIX + '/jscoverage/simple.html', {
-          waitUntil: 'networkidle0',
+          waitUntil: 'load',
        });
        const coverage = await page.coverage.stopJSCoverage();
        expect(coverage).toHaveLength(1);
@ -195,7 +195,7 @@ describe('Coverage specs', function () {
          includeRawScriptCoverage: true,
        });
        await page.goto(server.PREFIX + '/jscoverage/simple.html', {
-          waitUntil: 'networkidle0',
+          waitUntil: 'load',
        });
        const coverage = await page.coverage.stopJSCoverage();
        expect(coverage).toHaveLength(1);
@ -269,6 +269,7 @@ describe('Coverage specs', function () {
      await page.goto(server.PREFIX + '/csscoverage/media.html');
      const coverage = await page.coverage.stopCSSCoverage();
      expect(coverage).toHaveLength(1);
+      console.log(coverage);
      expect(coverage[0]!.url).toContain('/csscoverage/media.html');
      expect(coverage[0]!.ranges).toEqual([{start: 8, end: 40}]);
    });