refactor: remove EventSubscription

puppeteer · Apr 16, 2024 · 2347dfe · 2347dfe
1 parent e0932e5
commit 2347dfe
Show file tree

Hide file tree

Showing 3 changed files with 49 additions and 89 deletions.
diff --git a/packages/puppeteer-core/src/cdp/Coverage.ts b/packages/puppeteer-core/src/cdp/Coverage.ts
@@ -7,7 +7,7 @@
 import type {Protocol} from 'devtools-protocol';
 
 import type {CDPSession} from '../api/CDPSession.js';
-import {EventSubscription} from '../common/EventEmitter.js';
+import {EventEmitter} from '../common/EventEmitter.js';
 import {debugError, PuppeteerURL} from '../common/util.js';
 import {assert} from '../util/assert.js';
 import {DisposableStack} from '../util/disposable.js';
@@ -229,19 +229,13 @@ export class JSCoverage {
     this.#scriptURLs.clear();
     this.#scriptSources.clear();
     this.#subscriptions = new DisposableStack();
-    this.#subscriptions.use(
-      new EventSubscription(
-        this.#client,
-        'Debugger.scriptParsed',
-        this.#onScriptParsed.bind(this)
-      )
+    const clientEmitter = this.#subscriptions.use(
+      new EventEmitter(this.#client)
     );
-    this.#subscriptions.use(
-      new EventSubscription(
-        this.#client,
-        'Runtime.executionContextsCleared',
-        this.#onExecutionContextsCleared.bind(this)
-      )
+    clientEmitter.on('Debugger.scriptParsed', this.#onScriptParsed.bind(this));
+    clientEmitter.on(
+      'Runtime.executionContextsCleared',
+      this.#onExecutionContextsCleared.bind(this)
     );
     await Promise.all([
       this.#client.send('Profiler.enable'),
@@ -355,20 +349,15 @@ export class CSSCoverage {
     this.#stylesheetURLs.clear();
     this.#stylesheetSources.clear();
     this.#eventListeners = new DisposableStack();
-    this.#eventListeners.use(
-      new EventSubscription(
-        this.#client,
-        'CSS.styleSheetAdded',
-        this.#onStyleSheet.bind(this)
-      )
+    const clientEmitter = this.#eventListeners.use(
+      new EventEmitter(this.#client)
     );
-    this.#eventListeners.use(
-      new EventSubscription(
-        this.#client,
-        'Runtime.executionContextsCleared',
-        this.#onExecutionContextsCleared.bind(this)
-      )
+    clientEmitter.on('CSS.styleSheetAdded', this.#onStyleSheet.bind(this));
+    clientEmitter.on(
+      'Runtime.executionContextsCleared',
+      this.#onExecutionContextsCleared.bind(this)
     );
+
     await Promise.all([
       this.#client.send('DOM.enable'),
       this.#client.send('CSS.enable'),

diff --git a/packages/puppeteer-core/src/cdp/LifecycleWatcher.ts b/packages/puppeteer-core/src/cdp/LifecycleWatcher.ts
@@ -10,7 +10,7 @@ import {type Frame, FrameEvent} from '../api/Frame.js';
 import type {HTTPRequest} from '../api/HTTPRequest.js';
 import type {HTTPResponse} from '../api/HTTPResponse.js';
 import type {TimeoutError} from '../common/Errors.js';
-import {EventSubscription} from '../common/EventEmitter.js';
+import {EventEmitter} from '../common/EventEmitter.js';
 import {NetworkManagerEvent} from '../common/NetworkManagerEvents.js';
 import {assert} from '../util/assert.js';
 import {Deferred} from '../util/Deferred.js';
@@ -103,70 +103,43 @@ export class LifecycleWatcher {
 
     this.#frame = frame;
     this.#timeout = timeout;
-    this.#subscriptions.use(
-      // Revert if TODO #1 is done
-      new EventSubscription(
-        frame._frameManager,
-        FrameManagerEvent.LifecycleEvent,
-        this.#checkLifecycleComplete.bind(this)
-      )
+    const frameManagerEmitter = this.#subscriptions.use(
+      new EventEmitter(frame._frameManager)
     );
-    this.#subscriptions.use(
-      new EventSubscription(
-        frame,
-        FrameEvent.FrameNavigatedWithinDocument,
-        this.#navigatedWithinDocument.bind(this)
-      )
+    frameManagerEmitter.on(
+      FrameManagerEvent.LifecycleEvent,
+      this.#checkLifecycleComplete.bind(this)
     );
-    this.#subscriptions.use(
-      new EventSubscription(
-        frame,
-        FrameEvent.FrameNavigated,
-        this.#navigated.bind(this)
-      )
-    );
-    this.#subscriptions.use(
-      new EventSubscription(
-        frame,
-        FrameEvent.FrameSwapped,
-        this.#frameSwapped.bind(this)
-      )
+
+    const frameEmitter = this.#subscriptions.use(new EventEmitter(frame));
+    frameEmitter.on(
+      FrameEvent.FrameNavigatedWithinDocument,
+      this.#navigatedWithinDocument.bind(this)
     );
-    this.#subscriptions.use(
-      new EventSubscription(
-        frame,
-        FrameEvent.FrameSwappedByActivation,
-        this.#frameSwapped.bind(this)
-      )
+    frameEmitter.on(FrameEvent.FrameNavigated, this.#navigated.bind(this));
+    frameEmitter.on(FrameEvent.FrameSwapped, this.#frameSwapped.bind(this));
+    frameEmitter.on(
+      FrameEvent.FrameSwappedByActivation,
+      this.#frameSwapped.bind(this)
     );
-    this.#subscriptions.use(
-      new EventSubscription(
-        frame,
-        FrameEvent.FrameDetached,
-        this.#onFrameDetached.bind(this)
-      )
+    frameEmitter.on(FrameEvent.FrameDetached, this.#onFrameDetached.bind(this));
+
+    const networkManagerEmitter = this.#subscriptions.use(
+      new EventEmitter(networkManager)
     );
-    this.#subscriptions.use(
-      new EventSubscription(
-        networkManager,
-        NetworkManagerEvent.Request,
-        this.#onRequest.bind(this)
-      )
+    networkManagerEmitter.on(
+      NetworkManagerEvent.Request,
+      this.#onRequest.bind(this)
     );
-    this.#subscriptions.use(
-      new EventSubscription(
-        networkManager,
-        NetworkManagerEvent.Response,
-        this.#onResponse.bind(this)
-      )
+    networkManagerEmitter.on(
+      NetworkManagerEvent.Response,
+      this.#onResponse.bind(this)
     );
-    this.#subscriptions.use(
-      new EventSubscription(
-        networkManager,
-        NetworkManagerEvent.RequestFailed,
-        this.#onRequestFailed.bind(this)
-      )
+    networkManagerEmitter.on(
+      NetworkManagerEvent.RequestFailed,
+      this.#onRequestFailed.bind(this)
     );
+
     this.#terminationDeferred = Deferred.create<Error>({
       timeout: this.#timeout,
       message: `Navigation timeout of ${this.#timeout} ms exceeded`,

diff --git a/packages/puppeteer-core/src/cdp/NetworkManager.ts b/packages/puppeteer-core/src/cdp/NetworkManager.ts
@@ -9,7 +9,7 @@ import type {Protocol} from 'devtools-protocol';
 import {CDPSessionEvent, type CDPSession} from '../api/CDPSession.js';
 import type {Frame} from '../api/Frame.js';
 import type {Credentials} from '../api/Page.js';
-import {EventEmitter, EventSubscription} from '../common/EventEmitter.js';
+import {EventEmitter} from '../common/EventEmitter.js';
 import {
   NetworkManagerEvent,
   type NetworkManagerEvents,
@@ -100,14 +100,12 @@ export class NetworkManager extends EventEmitter<NetworkManagerEvents> {
     }
     const subscriptions = new DisposableStack();
     this.#clients.set(client, subscriptions);
+    const clientEmitter = subscriptions.use(new EventEmitter(client));
+
     for (const [event, handler] of this.#handlers) {
-      subscriptions.use(
-        // TODO: Remove any here.
-        new EventSubscription(client, event, (arg: any) => {
-          return handler.bind(this)(client, arg);
-        })
-      );
+      clientEmitter.on(event, handler as any);
     }
+
     await Promise.all([
       this.#ignoreHTTPSErrors
         ? client.send('Security.setIgnoreCertificateErrors', {