feat(network intercept): populate "intercepts" in base event params (#…

…1500)

Bug: #644

src/bidiMapper/CommandProcessor.ts

@@ -82,7 +82,7 @@ export class CommandProcessor extends EventEmitter<CommandProcessorEventsMap> {
     this.#parser = parser;
     this.#logger = logger;
 
-    const networkStorage = new NetworkStorage(eventManager);
+    const networkStorage = new NetworkStorage();
     const preloadScriptStorage = new PreloadScriptStorage();
 
     // keep-sorted start block=yes

src/bidiMapper/domains/context/CdpTarget.ts

@@ -61,7 +61,7 @@ export class CdpTarget {
     );
 
     LogManager.create(cdpTarget, realmStorage, eventManager);
-    NetworkManager.create(cdpTarget, networkStorage);
+    NetworkManager.create(cdpTarget, eventManager, networkStorage);
 
     cdpTarget.#setEventListeners();
 

src/bidiMapper/domains/network/NetworkManager.ts

@@ -24,17 +24,24 @@ import type {Protocol} from 'devtools-protocol';
 
 import type {Network} from '../../../protocol/protocol.js';
 import type {CdpTarget} from '../context/CdpTarget.js';
+import type {EventManager} from '../events/EventManager.js';
 
 import {NetworkRequest} from './NetworkRequest.js';
 import type {NetworkStorage} from './NetworkStorage.js';
 
 /** Maps 1:1 to CdpTarget. */
 export class NetworkManager {
   readonly #cdpTarget: CdpTarget;
+  readonly #eventManager: EventManager;
   readonly #networkStorage: NetworkStorage;
 
-  private constructor(cdpTarget: CdpTarget, networkStorage: NetworkStorage) {
+  private constructor(
+    cdpTarget: CdpTarget,
+    eventManager: EventManager,
+    networkStorage: NetworkStorage
+  ) {
     this.#cdpTarget = cdpTarget;
+    this.#eventManager = eventManager;
     this.#networkStorage = networkStorage;
   }
 
@@ -58,7 +65,8 @@ export class NetworkManager {
 
     request = new NetworkRequest(
       id,
-      this.#networkStorage.eventManager,
+      this.#eventManager,
+      this.#networkStorage,
       this.#cdpTarget,
       redirectCount
     );
@@ -70,9 +78,14 @@ export class NetworkManager {
 
   static create(
     cdpTarget: CdpTarget,
+    eventManager: EventManager,
     networkStorage: NetworkStorage
   ): NetworkManager {
-    const networkManager = new NetworkManager(cdpTarget, networkStorage);
+    const networkManager = new NetworkManager(
+      cdpTarget,
+      eventManager,
+      networkStorage
+    );
 
     cdpTarget.browserCdpClient.on(
       'Target.detachedFromTarget',

src/bidiMapper/domains/network/NetworkRequest.ts

@@ -52,7 +52,7 @@ export class NetworkRequest {
    * The identifier for a request resulting from a redirect matches that of the
    * request that initiated it.
    */
-  readonly requestId: Network.Request;
+  #requestId: Network.Request;
 
   // TODO: Handle auth required?
   /**
@@ -66,6 +66,7 @@ export class NetworkRequest {
   #redirectCount: number;
 
   #eventManager: EventManager;
+  #networkStorage: NetworkStorage;
 
   #request: {
     info?: Protocol.Network.RequestWillBeSentEvent;
@@ -87,15 +88,21 @@ export class NetworkRequest {
   constructor(
     requestId: Network.Request,
     eventManager: EventManager,
+    networkStorage: NetworkStorage,
     cdpTarget: CdpTarget,
     redirectCount = 0
   ) {
-    this.requestId = requestId;
+    this.#requestId = requestId;
     this.#eventManager = eventManager;
+    this.#networkStorage = networkStorage;
     this.#cdpTarget = cdpTarget;
     this.#redirectCount = redirectCount;
   }
 
+  get requestId(): string {
+    return this.#requestId;
+  }
+
   get url(): string | undefined {
     return this.#response.info?.url ?? this.#request.info?.request.url;
   }
@@ -394,14 +401,22 @@ export class NetworkRequest {
   }
 
   #getBaseEventParams(phase?: Network.InterceptPhase): Network.BaseParameters {
+    // TODO: Set this in terms of intercepts?
+    const isBlocked = phase !== undefined && phase === this.#interceptPhase;
+    const intercepts = this.#networkStorage.getNetworkIntercepts(
+      this.#requestId,
+      phase
+    );
+
     return {
-      isBlocked: phase !== undefined && phase === this.#interceptPhase,
+      isBlocked,
       context: this.#context,
       navigation: this.#getNavigationId(),
       redirectCount: this.#redirectCount,
       request: this.#getRequestData(),
       // Timestamp should be in milliseconds, while CDP provides it in seconds.
       timestamp: Math.round((this.#request.info?.wallTime ?? 0) * 1000),
+      intercepts: isBlocked ? intercepts : undefined,
     };
   }
 

src/bidiMapper/domains/network/NetworkStorage.spec.ts

@@ -15,26 +15,19 @@
  * limitations under the License.
  */
 import {expect} from 'chai';
-import sinon from 'sinon';
 
 import {Network} from '../../../protocol/protocol.js';
-import {EventManager} from '../events/EventManager.js';
 
-import {NetworkRequest} from './NetworkRequest.js';
 import {NetworkStorage} from './NetworkStorage.js';
 
 const UUID_REGEX =
   /^[0-9a-fA-F]{8}\b-[0-9a-fA-F]{4}\b-[0-9a-fA-F]{4}\b-[0-9a-fA-F]{4}\b-[0-9a-fA-F]{12}$/i;
 
 describe('NetworkStorage', () => {
-  let eventManager: sinon.SinonStubbedInstance<EventManager>;
-  let networkRequest: sinon.SinonStubbedInstance<NetworkRequest>;
   let networkStorage: NetworkStorage;
 
   beforeEach(() => {
-    eventManager = sinon.createStubInstance(EventManager);
-    networkRequest = sinon.createStubInstance(NetworkRequest);
-    networkStorage = new NetworkStorage(eventManager);
+    networkStorage = new NetworkStorage();
   });
 
   it('requestStageFromPhase', () => {
@@ -214,14 +207,6 @@ describe('NetworkStorage', () => {
     expect(networkStorage.hasBlockedRequests()).to.be.true;
   });
 
-  it('has network requests', () => {
-    expect(networkStorage.hasNetworkRequests()).to.be.false;
-
-    networkStorage.addRequest(networkRequest);
-
-    expect(networkStorage.hasNetworkRequests()).to.be.true;
-  });
-
   describe('getFetchEnableParams', () => {
     it('no intercepts', () => {
       expect(networkStorage.getFetchEnableParams()).to.deep.equal({

src/bidiMapper/domains/network/NetworkStorage.ts

@@ -16,20 +16,14 @@
  */
 import type {Protocol} from 'devtools-protocol';
 
-import {
-  Network,
-  NoSuchInterceptException,
-  ChromiumBidi,
-} from '../../../protocol/protocol.js';
+import {Network, NoSuchInterceptException} from '../../../protocol/protocol.js';
 import {URLPattern} from '../../../utils/UrlPattern.js';
 import {uuidv4} from '../../../utils/uuid.js';
-import type {EventManager} from '../events/EventManager.js';
 
 import type {NetworkRequest} from './NetworkRequest.js';
 
 /** Stores network and intercept maps. */
 export class NetworkStorage {
-  #eventManager: EventManager;
   /**
    * A map from network request ID to Network Request objects.
    * Needed as long as information about requests comes from different events.
@@ -56,14 +50,6 @@ export class NetworkStorage {
     }
   >();
 
-  constructor(eventManager: EventManager) {
-    this.#eventManager = eventManager;
-  }
-
-  get eventManager() {
-    return this.#eventManager;
-  }
-
   disposeRequestMap() {
     for (const request of this.#requestMap.values()) {
       request.dispose();
@@ -291,32 +277,14 @@ export class NetworkStorage {
 
   /** #@see https://w3c.github.io/webdriver-bidi/#get-the-network-intercepts */
   getNetworkIntercepts(
-    event: Exclude<
-      ChromiumBidi.Network.EventNames,
-      ChromiumBidi.Network.EventNames.FetchError
-    >,
-    requestId: Network.Request
+    requestId: Network.Request,
+    phase?: Network.InterceptPhase
   ): Network.Intercept[] {
     const request = this.#requestMap.get(requestId);
     if (!request) {
       return [];
     }
 
-    let phase: Network.InterceptPhase | undefined = undefined;
-    switch (event) {
-      case ChromiumBidi.Network.EventNames.BeforeRequestSent:
-        phase = Network.InterceptPhase.BeforeRequestSent;
-        break;
-      case ChromiumBidi.Network.EventNames.ResponseStarted:
-        phase = Network.InterceptPhase.ResponseStarted;
-        break;
-      case ChromiumBidi.Network.EventNames.AuthRequired:
-        phase = Network.InterceptPhase.AuthRequired;
-        break;
-      case ChromiumBidi.Network.EventNames.ResponseCompleted:
-        return [];
-    }
-
     const interceptIds: Network.Intercept[] = [];
 
     for (const [

tests/network/test_add_intercept.py

@@ -322,6 +322,7 @@ async def test_add_intercept_blocks(websocket, context_id, url_patterns):
     assert event_response == {
         "method": "network.beforeRequestSent",
         "params": {
+            "intercepts": [result["intercept"]],
             "isBlocked": True,
             "initiator": {
                 "type": "other",

tests/network/test_continue_request.py

@@ -164,7 +164,7 @@ async def test_continue_request_completes(websocket, context_id, example_url):
                     ["network.beforeRequestSent", "network.responseCompleted"],
                     [context_id])
 
-    await execute_command(
+    result = await execute_command(
         websocket, {
             "method": "network.addIntercept",
             "params": {
@@ -195,6 +195,7 @@ async def test_continue_request_completes(websocket, context_id, example_url):
             "initiator": {
                 "type": "other",
             },
+            "intercepts": [result["intercept"]],
             "isBlocked": True,
             "navigation": ANY_STR,
             "redirectCount": 0,
@@ -246,7 +247,7 @@ async def test_continue_request_twice(websocket, context_id, example_url):
                     ["network.beforeRequestSent", "network.responseCompleted"],
                     [context_id])
 
-    await execute_command(
+    result = await execute_command(
         websocket, {
             "method": "network.addIntercept",
             "params": {
@@ -277,6 +278,7 @@ async def test_continue_request_twice(websocket, context_id, example_url):
             "initiator": {
                 "type": "other",
             },
+            "intercepts": [result["intercept"]],
             "isBlocked": True,
             "navigation": ANY_STR,
             "redirectCount": 0,
@@ -367,6 +369,7 @@ async def test_continue_request_remove_intercept_inflight_request(
             "initiator": {
                 "type": "other",
             },
+            "intercepts": [intercept_id],
             "isBlocked": True,
             "navigation": ANY_STR,
             "redirectCount": 0,

tests/network/test_fail_request.py

@@ -119,6 +119,7 @@ async def test_fail_request_twice(websocket, context_id, example_url):
             "initiator": {
                 "type": "other",
             },
+            "intercepts": [result["intercept"]],
             "isBlocked": True,
             "navigation": ANY_STR,
             "redirectCount": 0,
@@ -322,6 +323,7 @@ async def test_fail_request_completes(websocket, context_id, example_url):
             "initiator": {
                 "type": "other",
             },
+            "intercepts": [result["intercept"]],
             "isBlocked": True,
             "navigation": ANY_STR,
             "redirectCount": 0,
@@ -395,6 +397,7 @@ async def test_fail_request_completes_new_request_still_blocks(
     assert result == {
         "intercept": ANY_UUID,
     }
+    intercept_id = result["intercept"]
 
     await send_JSON_command(
         websocket, {
@@ -415,6 +418,7 @@ async def test_fail_request_completes_new_request_still_blocks(
             "initiator": {
                 "type": "other",
             },
+            "intercepts": [intercept_id],
             "isBlocked": True,
             "navigation": ANY_STR,
             "redirectCount": 0,
@@ -487,6 +491,7 @@ async def test_fail_request_completes_new_request_still_blocks(
             "initiator": {
                 "type": "other",
             },
+            "intercepts": [intercept_id],
             "isBlocked": True,
             "navigation": ANY_STR,
             "redirectCount": 0,
@@ -553,6 +558,7 @@ async def test_fail_request_multiple_contexts(websocket, context_id,
             "initiator": {
                 "type": "other",
             },
+            "intercepts": [result["intercept"]],
             "isBlocked": True,
             "navigation": ANY_STR,
             "redirectCount": 0,
@@ -592,6 +598,7 @@ async def test_fail_request_multiple_contexts(websocket, context_id,
             "initiator": {
                 "type": "other",
             },
+            "intercepts": [result["intercept"]],
             "isBlocked": True,
             "navigation": ANY_STR,
             "redirectCount": 0,
@@ -721,6 +728,7 @@ async def test_fail_request_remove_intercept_inflight_request(
             "initiator": {
                 "type": "other",
             },
+            "intercepts": [intercept_id],
             "isBlocked": True,
             "navigation": ANY_STR,
             "redirectCount": 0,

tests/network/test_remove_intercept.py

@@ -151,6 +151,7 @@ async def test_remove_intercept_unblocks(websocket, context_id,
             "initiator": {
                 "type": "other",
             },
+            "intercepts": [intercept_id],
             "isBlocked": True,
             "navigation": ANY_STR,
             "redirectCount": 0,
@@ -249,6 +250,7 @@ async def test_remove_intercept_does_not_affect_another_intercept(
                 }, ]
             },
         })
+    intercept_id_2 = result["intercept"]
 
     await send_JSON_command(
         websocket, {
@@ -268,6 +270,7 @@ async def test_remove_intercept_does_not_affect_another_intercept(
             "initiator": {
                 "type": "other",
             },
+            "intercepts": [intercept_id_1],
             "isBlocked": True,
             "navigation": ANY_STR,
             "redirectCount": 0,
@@ -304,6 +307,7 @@ async def test_remove_intercept_does_not_affect_another_intercept(
             "initiator": {
                 "type": "other",
             },
+            "intercepts": [intercept_id_2],
             "isBlocked": True,
             "navigation": ANY_STR,
             "redirectCount": 0,