feat: unblock event queue when network events are blocked

Furthermore, update some E2E tests so that they do not rely on CDP events. Bug #644 Co-authored-by: Maksim Sadym <sadym@chromium.org>
GoogleChromeLabs · Oct 10, 2023 · 9aa5b47 · 9aa5b47
1 parent 382f8b5
commit 9aa5b47
Show file tree

Hide file tree

Showing 4 changed files with 434 additions and 130 deletions.
diff --git a/src/bidiMapper/domains/network/NetworkRequest.ts b/src/bidiMapper/domains/network/NetworkRequest.ts
@@ -53,8 +53,12 @@ export class NetworkRequest {
    */
   readonly requestId: Network.Request;
 
-  /** Indicates whether the request is blocked by a network intercept. */
-  #isBlocked = false;
+  // TODO: Handle auth required?
+  /**
+   * Indicates the network intercept phase, if the request is currently blocked.
+   * Undefined necessarily implies that the request is not blocked.
+   */
+  #interceptPhase: Network.InterceptPhase | undefined = undefined;
 
   #servedFromCache = false;
 
@@ -126,7 +130,8 @@ export class NetworkRequest {
       this.#servedFromCache ||
       // Sometimes there is no extra info and the response
       // is the only place we can find out
-      Boolean(this.#response.info && !this.#response.info.hasExtraInfo);
+      Boolean(this.#response.info && !this.#response.info.hasExtraInfo) ||
+      this.#interceptPhase === Network.InterceptPhase.BeforeRequestSent;
 
     if (this.#request.info && requestExtraInfoCompleted) {
       this.#beforeRequestSentDeferred.resolve({
@@ -140,7 +145,8 @@ export class NetworkRequest {
       // Response from cache don't have extra info
       this.#servedFromCache ||
       // Don't expect extra info if the flag is false
-      Boolean(this.#response.info && !this.#response.info.hasExtraInfo);
+      Boolean(this.#response.info && !this.#response.info.hasExtraInfo) ||
+      this.#interceptPhase === Network.InterceptPhase.ResponseStarted;
 
     if (this.#response.info && responseExtraInfoCompleted) {
       this.#responseCompletedDeferred.resolve({
@@ -264,31 +270,32 @@ export class NetworkRequest {
       },
     });
 
-    this.#isBlocked = true;
+    this.#interceptPhase = phase;
+    this.#emitEventsIfReady();
   }
 
   /** @see https://chromedevtools.github.io/devtools-protocol/tot/Fetch/#method-failRequest */
   async failRequest(
-    fetchRequestId: Protocol.Fetch.RequestId,
+    networkId: Network.Request,
     errorReason: Protocol.Network.ErrorReason
   ) {
     await this.#cdpTarget.cdpClient.sendCommand('Fetch.failRequest', {
-      requestId: fetchRequestId,
+      requestId: networkId,
       errorReason,
     });
 
-    this.#isBlocked = false;
+    this.#interceptPhase = undefined;
   }
 
   /** @see https://chromedevtools.github.io/devtools-protocol/tot/Fetch/#method-continueRequest */
   async continueRequest(
-    fetchRequestId: Protocol.Fetch.RequestId,
+    networkId: Protocol.Fetch.RequestId,
     url?: string,
     method?: string
   ) {
     // TODO: Expand.
     await this.#cdpTarget.cdpClient.sendCommand('Fetch.continueRequest', {
-      requestId: fetchRequestId,
+      requestId: networkId,
       url,
       method,
       // TODO: Set?
@@ -297,7 +304,7 @@ export class NetworkRequest {
       // interceptResponse:,
     });
 
-    this.#isBlocked = false;
+    this.#interceptPhase = undefined;
   }
 
   dispose() {
@@ -313,9 +320,9 @@ export class NetworkRequest {
     return this.#request.info?.frameId ?? null;
   }
 
-  #getBaseEventParams(): Network.BaseParameters {
+  #getBaseEventParams(phase?: Network.InterceptPhase): Network.BaseParameters {
     return {
-      isBlocked: this.#isBlocked,
+      isBlocked: phase !== undefined && phase === this.#interceptPhase,
       context: this.#context,
       navigation: this.#getNavigationId(),
       redirectCount: this.#redirectCount,
@@ -407,7 +414,7 @@ export class NetworkRequest {
     return {
       method: ChromiumBidi.Network.EventNames.BeforeRequestSent,
       params: {
-        ...this.#getBaseEventParams(),
+        ...this.#getBaseEventParams(Network.InterceptPhase.BeforeRequestSent),
         initiator: {
           type: NetworkRequest.#getInitiatorType(
             this.#request.info.initiator.type

diff --git a/tests/network/test_add_intercept.py b/tests/network/test_add_intercept.py
@@ -13,9 +13,10 @@
 #  See the License for the specific language governing permissions and
 #  limitations under the License.
 import pytest
-from anys import ANY_DICT, ANY_STR
-from test_helpers import (ANY_UUID, AnyExtending, execute_command,
-                          send_JSON_command, subscribe, wait_for_event)
+from anys import ANY_DICT, ANY_LIST, ANY_STR
+from test_helpers import (ANY_TIMESTAMP, ANY_UUID, AnyExtending,
+                          execute_command, send_JSON_command, subscribe,
+                          wait_for_event)
 
 
 @pytest.mark.asyncio
@@ -289,8 +290,8 @@ async def test_add_intercept_type_pattern_port_empty_invalid(websocket):
                              "pattern",
                              "string and pattern",
                          ])
-async def test_add_intercept_blocks(websocket, context_id, url_patterns,
-                                    example_url):
+async def test_add_intercept_blocks_use_cdp_events(websocket, context_id,
+                                                   url_patterns, example_url):
     await subscribe(websocket, ["cdp.Fetch.requestPaused"])
 
     result = await execute_command(
@@ -334,3 +335,91 @@ async def test_add_intercept_blocks(websocket, context_id, url_patterns,
         },
         "type": "event",
     }
+
+
+@pytest.mark.asyncio
+@pytest.mark.parametrize("url_patterns", [
+    [
+        {
+            "type": "string",
+            "pattern": "https://www.example.com/",
+        },
+    ],
+    [
+        {
+            "type": "pattern",
+            "protocol": "https",
+            "hostname": "www.example.com",
+            "pathname": "/",
+        },
+    ],
+    [
+        {
+            "type": "string",
+            "pattern": "https://www.example.com/",
+        },
+        {
+            "type": "pattern",
+            "protocol": "https",
+            "hostname": "www.example.com",
+            "pathname": "/",
+        },
+    ],
+],
+                         ids=[
+                             "string",
+                             "pattern",
+                             "string and pattern",
+                         ])
+async def test_add_intercept_blocks_use_bidi_events(websocket, context_id,
+                                                    url_patterns, example_url):
+    await subscribe(websocket, ["network.beforeRequestSent"])
+
+    result = await execute_command(
+        websocket, {
+            "method": "network.addIntercept",
+            "params": {
+                "phases": ["beforeRequestSent"],
+                "urlPatterns": url_patterns,
+            },
+        })
+
+    assert result == {
+        "intercept": ANY_UUID,
+    }
+
+    await send_JSON_command(
+        websocket, {
+            "method": "browsingContext.navigate",
+            "params": {
+                "url": example_url,
+                "context": context_id,
+            }
+        })
+
+    event_response = await wait_for_event(websocket,
+                                          "network.beforeRequestSent")
+    assert event_response == {
+        "method": "network.beforeRequestSent",
+        "params": {
+            "isBlocked": True,
+            "initiator": {
+                "type": "other",
+            },
+            "context": context_id,
+            "navigation": ANY_STR,
+            "redirectCount": 0,
+            "request": {
+                "request": ANY_STR,
+                "url": example_url,
+                "method": "GET",
+                "headers": ANY_LIST,
+                "cookies": [],
+                "headersSize": -1,
+                "bodySize": 0,
+                "timings": ANY_DICT
+            },
+            "timestamp": ANY_TIMESTAMP,
+        },
+        "type": "event",
+    }