refactor: clean up some naming (#1796)

GoogleChromeLabs · Jan 31, 2024 · 119dd4c · 119dd4c
1 parent 38b574c
commit 119dd4c
Show file tree

Hide file tree

Showing 6 changed files with 53 additions and 75 deletions.
diff --git a/src/bidiMapper/domains/context/BrowsingContextImpl.ts b/src/bidiMapper/domains/context/BrowsingContextImpl.ts
@@ -58,17 +58,13 @@ export class BrowsingContextImpl {
 
   readonly #browsingContextStorage: BrowsingContextStorage;
 
-  readonly #deferreds = {
-    Page: {
-      navigatedWithinDocument:
-        new Deferred<Protocol.Page.NavigatedWithinDocumentEvent>(),
-      lifecycleEvent: {
-        DOMContentLoaded: new Deferred<Protocol.Page.LifecycleEventEvent>(),
-        load: new Deferred<Protocol.Page.LifecycleEventEvent>(),
-      },
-      frameStartedLoading:
-        new Deferred<Protocol.Page.FrameStartedLoadingEvent>(),
-    },
+  #lifecycle = {
+    DOMContentLoaded: new Deferred<Protocol.Page.LifecycleEventEvent>(),
+    load: new Deferred<Protocol.Page.LifecycleEventEvent>(),
+  };
+
+  #navigation = {
+    withinDocument: new Deferred<Protocol.Page.NavigatedWithinDocumentEvent>(),
   };
 
   #url = 'about:blank';
@@ -172,7 +168,7 @@ export class BrowsingContextImpl {
     }
 
     // Fail all ongoing navigations.
-    this.#failDeferredsIfNotFinished();
+    this.#failLifecycleIfNotFinished();
 
     this.#eventManager.registerEvent(
       {
@@ -265,11 +261,11 @@ export class BrowsingContextImpl {
   }
 
   async lifecycleLoaded() {
-    await this.#deferreds.Page.lifecycleEvent.load;
+    await this.#lifecycle.load;
   }
 
   async targetUnblockedOrThrow(): Promise<void> {
-    const result = await this.#cdpTarget.targetUnblocked;
+    const result = await this.#cdpTarget.unblocked;
     if (result.kind === 'error') {
       throw result.error;
     }
@@ -351,7 +347,7 @@ export class BrowsingContextImpl {
         }
         const timestamp = BrowsingContextImpl.getTimestamp();
         this.#url = params.url;
-        this.#deferreds.Page.navigatedWithinDocument.resolve(params);
+        this.#navigation.withinDocument.resolve(params);
 
         this.#eventManager.registerEvent(
           {
@@ -431,9 +427,7 @@ export class BrowsingContextImpl {
               },
               this.id
             );
-            this.#deferreds.Page.lifecycleEvent.DOMContentLoaded.resolve(
-              params
-            );
+            this.#lifecycle.DOMContentLoaded.resolve(params);
             break;
 
           case 'load':
@@ -450,7 +444,7 @@ export class BrowsingContextImpl {
               },
               this.id
             );
-            this.#deferreds.Page.lifecycleEvent.load.resolve(params);
+            this.#lifecycle.load.resolve(params);
             break;
         }
       }
@@ -566,8 +560,8 @@ export class BrowsingContextImpl {
   #documentChanged(loaderId?: Protocol.Network.LoaderId) {
     // Same document navigation.
     if (loaderId === undefined || this.#loaderId === loaderId) {
-      if (this.#deferreds.Page.navigatedWithinDocument.isFinished) {
-        this.#deferreds.Page.navigatedWithinDocument =
+      if (this.#navigation.withinDocument.isFinished) {
+        this.#navigation.withinDocument =
           new Deferred<Protocol.Page.NavigatedWithinDocumentEvent>();
       } else {
         this.#logger?.(
@@ -578,14 +572,14 @@ export class BrowsingContextImpl {
       return;
     }
 
-    this.#resetDeferredsIfFinished();
+    this.#resetLifecycleIfFinished();
 
     this.#loaderId = loaderId;
   }
 
-  #resetDeferredsIfFinished() {
-    if (this.#deferreds.Page.lifecycleEvent.DOMContentLoaded.isFinished) {
-      this.#deferreds.Page.lifecycleEvent.DOMContentLoaded =
+  #resetLifecycleIfFinished() {
+    if (this.#lifecycle.DOMContentLoaded.isFinished) {
+      this.#lifecycle.DOMContentLoaded =
         new Deferred<Protocol.Page.LifecycleEventEvent>();
     } else {
       this.#logger?.(
@@ -594,9 +588,8 @@ export class BrowsingContextImpl {
       );
     }
 
-    if (this.#deferreds.Page.lifecycleEvent.load.isFinished) {
-      this.#deferreds.Page.lifecycleEvent.load =
-        new Deferred<Protocol.Page.LifecycleEventEvent>();
+    if (this.#lifecycle.load.isFinished) {
+      this.#lifecycle.load = new Deferred<Protocol.Page.LifecycleEventEvent>();
     } else {
       this.#logger?.(
         BrowsingContextImpl.LOGGER_PREFIX,
@@ -605,15 +598,15 @@ export class BrowsingContextImpl {
     }
   }
 
-  #failDeferredsIfNotFinished() {
-    if (!this.#deferreds.Page.lifecycleEvent.DOMContentLoaded.isFinished) {
-      this.#deferreds.Page.lifecycleEvent.DOMContentLoaded.reject(
+  #failLifecycleIfNotFinished() {
+    if (!this.#lifecycle.DOMContentLoaded.isFinished) {
+      this.#lifecycle.DOMContentLoaded.reject(
         new UnknownErrorException('navigation canceled')
       );
     }
 
-    if (!this.#deferreds.Page.lifecycleEvent.load.isFinished) {
-      this.#deferreds.Page.lifecycleEvent.load.reject(
+    if (!this.#lifecycle.load.isFinished) {
+      this.#lifecycle.load.reject(
         new UnknownErrorException('navigation canceled')
       );
     }
@@ -652,17 +645,17 @@ export class BrowsingContextImpl {
       case BrowsingContext.ReadinessState.Interactive:
         // No `loaderId` means same-document navigation.
         if (cdpNavigateResult.loaderId === undefined) {
-          await this.#deferreds.Page.navigatedWithinDocument;
+          await this.#navigation.withinDocument;
         } else {
-          await this.#deferreds.Page.lifecycleEvent.DOMContentLoaded;
+          await this.#lifecycle.DOMContentLoaded;
         }
         break;
       case BrowsingContext.ReadinessState.Complete:
         // No `loaderId` means same-document navigation.
         if (cdpNavigateResult.loaderId === undefined) {
-          await this.#deferreds.Page.navigatedWithinDocument;
+          await this.#navigation.withinDocument;
         } else {
-          await this.lifecycleLoaded();
+          await this.#lifecycle.load;
         }
         break;
     }
@@ -684,16 +677,16 @@ export class BrowsingContextImpl {
       ignoreCache,
     });
 
-    this.#resetDeferredsIfFinished();
+    this.#resetLifecycleIfFinished();
 
     switch (wait) {
       case BrowsingContext.ReadinessState.None:
         break;
       case BrowsingContext.ReadinessState.Interactive:
-        await this.#deferreds.Page.lifecycleEvent.DOMContentLoaded;
+        await this.#lifecycle.DOMContentLoaded;
         break;
       case BrowsingContext.ReadinessState.Complete:
-        await this.lifecycleLoaded();
+        await this.#lifecycle.load;
         break;
     }
 

diff --git a/src/bidiMapper/domains/context/BrowsingContextProcessor.ts b/src/bidiMapper/domains/context/BrowsingContextProcessor.ts
@@ -387,11 +387,7 @@ export class BrowsingContextProcessor {
           break;
         }
 
-        const cdpTarget = this.#createCdpTarget(
-          targetCdpClient,
-          targetInfo,
-          sessionId
-        );
+        const cdpTarget = this.#createCdpTarget(targetCdpClient, targetInfo);
         const maybeContext = this.#browsingContextStorage.findContext(
           targetInfo.targetId
         );
@@ -428,11 +424,7 @@ export class BrowsingContextProcessor {
           break;
         }
 
-        const cdpTarget = this.#createCdpTarget(
-          targetCdpClient,
-          targetInfo,
-          sessionId
-        );
+        const cdpTarget = this.#createCdpTarget(targetCdpClient, targetInfo);
         this.#handleWorkerTarget(
           cdpTarget,
           this.#realmStorage.getRealm({
@@ -457,16 +449,14 @@ export class BrowsingContextProcessor {
 
   #createCdpTarget(
     targetCdpClient: CdpClient,
-    targetInfo: Protocol.Target.TargetInfo,
-    sessionId: string
+    targetInfo: Protocol.Target.TargetInfo
   ) {
     this.#setEventListeners(targetCdpClient);
 
     return CdpTarget.create(
       targetInfo.targetId,
       targetCdpClient,
       this.#browserCdpClient,
-      sessionId,
       this.#realmStorage,
       this.#eventManager,
       this.#preloadScriptStorage,

diff --git a/src/bidiMapper/domains/context/CdpTarget.ts b/src/bidiMapper/domains/context/CdpTarget.ts
@@ -30,10 +30,9 @@ import type {RealmStorage} from '../script/RealmStorage.js';
 import type {EventManager} from '../session/EventManager.js';
 
 export class CdpTarget {
-  readonly #targetId: Protocol.Target.TargetID;
+  readonly #id: Protocol.Target.TargetID;
   readonly #cdpClient: CdpClient;
   readonly #browserCdpClient: CdpClient;
-  readonly #cdpSessionId: Protocol.Target.SessionID;
   readonly #eventManager: EventManager;
 
   readonly #preloadScriptStorage: PreloadScriptStorage;
@@ -46,7 +45,6 @@ export class CdpTarget {
     targetId: Protocol.Target.TargetID,
     cdpClient: CdpClient,
     browserCdpClient: CdpClient,
-    cdpSessionId: Protocol.Target.SessionID,
     realmStorage: RealmStorage,
     eventManager: EventManager,
     preloadScriptStorage: PreloadScriptStorage,
@@ -58,7 +56,6 @@ export class CdpTarget {
       targetId,
       cdpClient,
       browserCdpClient,
-      cdpSessionId,
       eventManager,
       preloadScriptStorage,
       networkStorage,
@@ -81,29 +78,27 @@ export class CdpTarget {
     targetId: Protocol.Target.TargetID,
     cdpClient: CdpClient,
     browserCdpClient: CdpClient,
-    cdpSessionId: Protocol.Target.SessionID,
     eventManager: EventManager,
     preloadScriptStorage: PreloadScriptStorage,
     networkStorage: NetworkStorage,
     acceptInsecureCerts: boolean
   ) {
-    this.#targetId = targetId;
+    this.#id = targetId;
     this.#cdpClient = cdpClient;
-    this.#cdpSessionId = cdpSessionId;
     this.#eventManager = eventManager;
     this.#preloadScriptStorage = preloadScriptStorage;
     this.#networkStorage = networkStorage;
     this.#browserCdpClient = browserCdpClient;
     this.#acceptInsecureCerts = acceptInsecureCerts;
   }
 
-  /** Returns a promise that resolves when the target is unblocked. */
-  get targetUnblocked(): Deferred<Result<void>> {
+  /** Returns a deferred that resolves when the target is unblocked. */
+  get unblocked(): Deferred<Result<void>> {
     return this.#targetUnblocked;
   }
 
-  get targetId(): Protocol.Target.TargetID {
-    return this.#targetId;
+  get id(): Protocol.Target.TargetID {
+    return this.#id;
   }
 
   get cdpClient(): CdpClient {
@@ -116,7 +111,8 @@ export class CdpTarget {
 
   /** Needed for CDP escape path. */
   get cdpSessionId(): Protocol.Target.SessionID {
-    return this.#cdpSessionId;
+    // SAFETY we got the client by it's id for creating
+    return this.#cdpClient.sessionId!;
   }
 
   /** Calls `Fetch.enable` with the added network intercepts. */
@@ -191,7 +187,7 @@ export class CdpTarget {
           params: {
             event,
             params,
-            session: this.#cdpSessionId,
+            session: this.cdpSessionId,
           },
         },
         null

diff --git a/src/bidiMapper/domains/script/PreloadScript.ts b/src/bidiMapper/domains/script/PreloadScript.ts
@@ -132,7 +132,7 @@ export class PreloadScript {
       target: cdpTarget,
       preloadScriptId: addCdpPreloadScriptResult.identifier,
     });
-    this.#targetIds.add(cdpTarget.targetId);
+    this.#targetIds.add(cdpTarget.id);
   }
 
   /**
@@ -154,7 +154,7 @@ export class PreloadScript {
   /** Removes the provided cdp target from the list of cdp preload scripts. */
   dispose(cdpTargetId: Protocol.Target.TargetID) {
     this.#cdpPreloadScripts = this.#cdpPreloadScripts.filter(
-      (cdpPreloadScript) => cdpPreloadScript.target?.targetId !== cdpTargetId
+      (cdpPreloadScript) => cdpPreloadScript.target?.id !== cdpTargetId
     );
     this.#targetIds.delete(cdpTargetId);
   }

diff --git a/src/bidiMapper/domains/script/PreloadScriptStorage.spec.ts b/src/bidiMapper/domains/script/PreloadScriptStorage.spec.ts
@@ -34,7 +34,7 @@ describe('PreloadScriptStorage', () => {
   beforeEach(() => {
     preloadScriptStorage = new PreloadScriptStorage();
     cdpTarget = sinon.createStubInstance(CdpTarget);
-    sinon.stub(cdpTarget, 'targetId').get(() => CDP_TARGET_ID);
+    sinon.stub(cdpTarget, 'id').get(() => CDP_TARGET_ID);
   });
 
   it('initial state', () => {

diff --git a/src/bidiMapper/domains/script/PreloadScriptStorage.ts b/src/bidiMapper/domains/script/PreloadScriptStorage.ts
@@ -19,12 +19,11 @@ import type {CdpTarget} from '../context/CdpTarget.js';
 import type {PreloadScript} from './PreloadScript.js';
 
 /** PreloadScripts can be filtered by BiDi ID or target ID. */
-export type PreloadScriptFilter = Partial<
-  Pick<PreloadScript, 'id'> &
-    Pick<CdpTarget, 'targetId'> & {
-      global: boolean;
-    }
->;
+export type PreloadScriptFilter = Partial<{
+  id: PreloadScript['id'];
+  targetId: CdpTarget['id'];
+  global: boolean;
+}>;
 
 /**
  * Container class for preload scripts.