Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: provide proper network timings #2919

Merged
merged 5 commits into from
Dec 20, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions .github/workflows/e2e.yml
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,10 @@ jobs:
with:
node-version-file: '.nvmrc'
cache: npm
- name: Disable AppArmor
if: ${{ matrix.os == 'ubuntu-latest' }}
# https://chromium.googlesource.com/chromium/src/+/main/docs/security/apparmor-userns-restrictions.md
run: echo 0 | sudo tee /proc/sys/kernel/apparmor_restrict_unprivileged_userns
- uses: google/wireit@83d7f8bed70b7bcfc40f4b9f54f4b7485753991b # setup-github-actions-caching/v2.0.1
- name: Install and build npm dependencies
run: npm ci
Expand Down
3 changes: 3 additions & 0 deletions .github/workflows/selenium.yml
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,9 @@ jobs:
with:
node-version-file: '.nvmrc'
cache: npm
- name: Disable AppArmor
# https://chromium.googlesource.com/chromium/src/+/main/docs/security/apparmor-userns-restrictions.md
run: echo 0 | sudo tee /proc/sys/kernel/apparmor_restrict_unprivileged_userns
- uses: google/wireit@83d7f8bed70b7bcfc40f4b9f54f4b7485753991b # setup-github-actions-caching/v2.0.1
- name: Install and build npm dependencies
run: npm ci
Expand Down
3 changes: 3 additions & 0 deletions .github/workflows/webdriverio.yml
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,9 @@ jobs:
with:
node-version-file: '.nvmrc'
cache: npm
- name: Disable AppArmor
# https://chromium.googlesource.com/chromium/src/+/main/docs/security/apparmor-userns-restrictions.md
run: echo 0 | sudo tee /proc/sys/kernel/apparmor_restrict_unprivileged_userns
- uses: google/wireit@83d7f8bed70b7bcfc40f4b9f54f4b7485753991b # setup-github-actions-caching/v2.0.1
- name: Install and build npm dependencies
run: npm ci
Expand Down
2 changes: 2 additions & 0 deletions .github/workflows/wpt.yml
Original file line number Diff line number Diff line change
Expand Up @@ -82,6 +82,8 @@ jobs:
with:
node-version-file: '.nvmrc'
cache: npm
- name: Disable AppArmor
run: echo 0 | sudo tee /proc/sys/kernel/apparmor_restrict_unprivileged_userns
- uses: google/wireit@83d7f8bed70b7bcfc40f4b9f54f4b7485753991b # setup-github-actions-caching/v2.0.1
- name: Install and build npm dependencies
run: npm ci
Expand Down
4 changes: 2 additions & 2 deletions src/bidiMapper/modules/context/NavigationTracker.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -318,7 +318,7 @@ describe('NavigationTracker', () => {
assertNoNavigationEvents();
});

it('canceled by script-initiated navigation', async () => {
it('canceled by script-initiated navigation', () => {
navigationTracker.frameRequestedNavigation(SOME_URL);

assertNoNavigationEvents();
Expand Down Expand Up @@ -350,7 +350,7 @@ describe('NavigationTracker', () => {
assert.equal(navigationTracker.url, INITIAL_URL);
});

it('canceled by command navigation', async () => {
it('canceled by command navigation', () => {
navigationTracker.frameRequestedNavigation(SOME_URL);
navigationTracker.frameStartedNavigating(ANOTHER_URL, LOADER_ID);

Expand Down
74 changes: 57 additions & 17 deletions src/bidiMapper/modules/network/NetworkRequest.ts
Original file line number Diff line number Diff line change
Expand Up @@ -23,27 +23,27 @@
import type {Protocol} from 'devtools-protocol';

import {
Network,
type BrowsingContext,
ChromiumBidi,
Network,
type NetworkEvent,
} from '../../../protocol/protocol.js';
import {assert} from '../../../utils/assert.js';
import {Deferred} from '../../../utils/Deferred.js';
import {LogType, type LoggerFn} from '../../../utils/log.js';
import {type LoggerFn, LogType} from '../../../utils/log.js';
import type {CdpTarget} from '../cdp/CdpTarget.js';
import type {EventManager} from '../session/EventManager.js';

import type {NetworkStorage} from './NetworkStorage.js';
import {
computeHeadersSize,
bidiBodySizeFromCdpPostDataEntries,
bidiNetworkHeadersFromCdpNetworkHeaders,
cdpToBiDiCookie,
cdpFetchHeadersFromBidiNetworkHeaders,
cdpAuthChallengeResponseFromBidiAuthContinueWithAuthAction,
bidiBodySizeFromCdpPostDataEntries,
networkHeaderFromCookieHeaders,
cdpFetchHeadersFromBidiNetworkHeaders,
cdpToBiDiCookie,
computeHeadersSize,
getTiming,
networkHeaderFromCookieHeaders,
} from './NetworkUtils.js';

const REALM_REGEX = /(?<=realm=").*(?=")/;
Expand Down Expand Up @@ -304,26 +304,66 @@ export class NetworkRequest {
}

get #timings(): Network.FetchTimingInfo {
// The timing in the CDP events are provided relative to the event's baseline.
// However, the baseline can be different for different events, and the events have to
// be normalized throughout resource events. Normalize events timestamps by the
// request.
// TODO: Verify this is correct.
const responseTimeOffset = getTiming(
getTiming(this.#response.info?.timing?.requestTime) -
getTiming(this.#request.info?.timestamp),
);

return {
// TODO: Verify this is correct
timeOrigin: getTiming(this.#response.info?.timing?.requestTime),
requestTime: getTiming(this.#response.info?.timing?.requestTime),
timeOrigin: Math.round(getTiming(this.#request.info?.wallTime) * 1000),
sadym-chromium marked this conversation as resolved.
Show resolved Hide resolved
// Timing baseline.
// TODO: Verify this is correct.
requestTime: 0,
// TODO: set if redirect detected.
redirectStart: 0,
// TODO: set if redirect detected.
redirectEnd: 0,
// TODO: Verify this is correct
// https://source.chromium.org/chromium/chromium/src/+/main:net/base/load_timing_info.h;l=145
fetchStart: getTiming(this.#response.info?.timing?.requestTime),
dnsStart: getTiming(this.#response.info?.timing?.dnsStart),
dnsEnd: getTiming(this.#response.info?.timing?.dnsEnd),
connectStart: getTiming(this.#response.info?.timing?.connectStart),
connectEnd: getTiming(this.#response.info?.timing?.connectEnd),
tlsStart: getTiming(this.#response.info?.timing?.sslStart),
requestStart: getTiming(this.#response.info?.timing?.sendStart),
fetchStart: getTiming(
sadym-chromium marked this conversation as resolved.
Show resolved Hide resolved
this.#response.info?.timing?.workerFetchStart,
responseTimeOffset,
),
// fetchStart: 0,
dnsStart: getTiming(
this.#response.info?.timing?.dnsStart,
responseTimeOffset,
),
dnsEnd: getTiming(
this.#response.info?.timing?.dnsEnd,
responseTimeOffset,
),
connectStart: getTiming(
this.#response.info?.timing?.connectStart,
responseTimeOffset,
),
connectEnd: getTiming(
this.#response.info?.timing?.connectEnd,
responseTimeOffset,
),
tlsStart: getTiming(
this.#response.info?.timing?.sslStart,
responseTimeOffset,
),
requestStart: getTiming(
this.#response.info?.timing?.sendStart,
responseTimeOffset,
),
// https://source.chromium.org/chromium/chromium/src/+/main:net/base/load_timing_info.h;l=196
responseStart: getTiming(
this.#response.info?.timing?.receiveHeadersStart,
responseTimeOffset,
),
responseEnd: getTiming(
this.#response.info?.timing?.receiveHeadersEnd,
responseTimeOffset,
),
responseEnd: getTiming(this.#response.info?.timing?.receiveHeadersEnd),
};
}

Expand Down
9 changes: 6 additions & 3 deletions src/bidiMapper/modules/network/NetworkUtils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -366,13 +366,16 @@ export function bidiBodySizeFromCdpPostDataEntries(
return size;
}

export function getTiming(timing: number | undefined): number {
export function getTiming(
timing: number | undefined,
offset: number = 0,
): number {
if (!timing) {
return 0;
}
if (timing < 0) {
if (timing <= 0 || timing + offset <= 0) {
return 0;
}

return timing;
return timing + offset;
}
13 changes: 9 additions & 4 deletions tests/network/test_network.py
Original file line number Diff line number Diff line change
Expand Up @@ -422,10 +422,15 @@ async def test_network_before_request_sent_event_with_data_url_emitted(


@pytest.mark.asyncio
@pytest.mark.parametrize('capabilities', [{
'acceptInsecureCerts': True
}],
indirect=True)
async def test_network_sends_only_included_cookies(websocket, context_id,
url_base):
url_example_another_origin,
url_bad_ssl):

await goto_url(websocket, context_id, "https://example.com")
await goto_url(websocket, context_id, url_bad_ssl)

await execute_command(
websocket, {
Expand All @@ -446,7 +451,7 @@ async def test_network_sends_only_included_cookies(websocket, context_id,
websocket, {
"method": "browsingContext.navigate",
"params": {
"url": url_base,
"url": url_example_another_origin,
"wait": "complete",
"context": context_id
}
Expand All @@ -463,7 +468,7 @@ async def test_network_sends_only_included_cookies(websocket, context_id,
"redirectCount": 0,
"request": {
"request": ANY_STR,
"url": url_base,
"url": url_example_another_origin,
"method": "GET",
"headers": ANY_LIST,
"cookies": [],
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,3 @@
[before_request_sent.py]
[test_serviceworker_request]
expected: FAIL

[test_destination_initiator]
expected: FAIL
Original file line number Diff line number Diff line change
Expand Up @@ -14,14 +14,5 @@
[test_response_status[407-Proxy Authentication Required\]]
expected: FAIL

[test_request_timing_info]
expected: FAIL

[test_serviceworker_request]
expected: FAIL

[test_url_with_fragment]
expected: FAIL

[test_destination_initiator]
expected: FAIL
Original file line number Diff line number Diff line change
Expand Up @@ -8,14 +8,5 @@
[test_www_authenticate]
expected: FAIL

[test_request_timing_info]
expected: FAIL

[test_serviceworker_request]
expected: FAIL

[test_url_with_fragment]
expected: FAIL

[test_destination_initiator]
expected: FAIL
Original file line number Diff line number Diff line change
@@ -1,6 +1,3 @@
[before_request_sent.py]
[test_serviceworker_request]
expected: FAIL

[test_destination_initiator]
expected: FAIL
Original file line number Diff line number Diff line change
Expand Up @@ -14,14 +14,5 @@
[test_response_status[407-Proxy Authentication Required\]]
expected: FAIL

[test_request_timing_info]
expected: FAIL

[test_serviceworker_request]
expected: FAIL

[test_url_with_fragment]
expected: FAIL

[test_destination_initiator]
expected: FAIL
Original file line number Diff line number Diff line change
Expand Up @@ -8,14 +8,5 @@
[test_www_authenticate]
expected: FAIL

[test_request_timing_info]
expected: FAIL

[test_serviceworker_request]
expected: FAIL

[test_url_with_fragment]
expected: FAIL

[test_destination_initiator]
expected: FAIL
Original file line number Diff line number Diff line change
@@ -1,6 +1,3 @@
[before_request_sent.py]
[test_serviceworker_request]
expected: FAIL

[test_destination_initiator]
expected: FAIL
Original file line number Diff line number Diff line change
Expand Up @@ -14,14 +14,5 @@
[test_response_status[407-Proxy Authentication Required\]]
expected: FAIL

[test_request_timing_info]
expected: FAIL

[test_serviceworker_request]
expected: FAIL

[test_url_with_fragment]
expected: FAIL

[test_destination_initiator]
expected: FAIL
Original file line number Diff line number Diff line change
Expand Up @@ -5,14 +5,5 @@
[test_response_status[407-Proxy Authentication Required\]]
expected: FAIL

[test_request_timing_info]
expected: FAIL

[test_serviceworker_request]
expected: FAIL

[test_url_with_fragment]
expected: FAIL

[test_destination_initiator]
expected: FAIL
Loading