Skip to content

Commit

Permalink
feat: abort navigation only after frame navigated (#2898)
Browse files Browse the repository at this point in the history 
WPT failures are due to the fact Chrome emits `frameNavigated` after
receiving headers.
web-platform-tests/wpt#49718
  • Loading branch information
@sadym-chromium
sadym-chromium authored Dec 17, 2024
1 parent 8dcb5ef commit 6c3d406
Show file tree
Hide file tree
Showing 7 changed files with 213 additions and 61 deletions.
36 changes: 29 additions & 7 deletions src/bidiMapper/modules/context/NavigationTracker.spec.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -160,7 +160,7 @@ describe('NavigationTracker', () => {
});
});

it('aborted by script-initiated navigation', async () => {
it('canceled by script-initiated navigation', async () => {
const navigation = navigationTracker.createPendingNavigation(SOME_URL);
navigationTracker.frameStartedNavigating(ANOTHER_URL, LOADER_ID);

Expand All @@ -173,19 +173,41 @@ describe('NavigationTracker', () => {
navigationTracker.frameRequestedNavigation(YET_ANOTHER_URL);

assertNavigationEvent(
ChromiumBidi.BrowsingContext.EventNames.NavigationAborted,
ChromiumBidi.BrowsingContext.EventNames.NavigationFailed,
navigation.navigationId,
ANOTHER_URL,
);

assert.equal(
(await navigation.finished).eventName,
NavigationEventName.NavigationAborted,
NavigationEventName.NavigationFailed,
);
assert.equal(navigationTracker.currentNavigationId, initialNavigationId);
assert.equal(navigationTracker.url, INITIAL_URL);
});

it('aborted by script-initiated navigation', async () => {
const navigation = navigationTracker.createPendingNavigation(SOME_URL);
navigationTracker.frameStartedNavigating(ANOTHER_URL, LOADER_ID);
navigationTracker.frameNavigated(ANOTHER_URL, LOADER_ID);

eventManager.registerEvent.reset();

navigationTracker.frameRequestedNavigation(YET_ANOTHER_URL);
navigationTracker.frameNavigated(YET_ANOTHER_URL, ANOTHER_LOADER_ID);

assertNavigationEvent(
ChromiumBidi.BrowsingContext.EventNames.NavigationAborted,
navigation.navigationId,
ANOTHER_URL,
);

assert.equal(
(await navigation.finished).eventName,
NavigationEventName.NavigationAborted,
);
});

it('failed command', async () => {
const navigation = navigationTracker.createPendingNavigation(SOME_URL);
navigationTracker.frameStartedNavigating(ANOTHER_URL, LOADER_ID);
Expand Down Expand Up @@ -296,7 +318,7 @@ describe('NavigationTracker', () => {
assertNoNavigationEvents();
});

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

assertNoNavigationEvents();
Expand All @@ -317,7 +339,7 @@ describe('NavigationTracker', () => {
navigationTracker.frameRequestedNavigation(YET_ANOTHER_URL);

assertNavigationEvent(
ChromiumBidi.BrowsingContext.EventNames.NavigationAborted,
ChromiumBidi.BrowsingContext.EventNames.NavigationFailed,
sinon.match.any,
ANOTHER_URL,
);
Expand All @@ -328,7 +350,7 @@ describe('NavigationTracker', () => {
assert.equal(navigationTracker.url, INITIAL_URL);
});

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

Expand All @@ -341,7 +363,7 @@ describe('NavigationTracker', () => {
navigationTracker.createPendingNavigation(YET_ANOTHER_URL);

assertNavigationEvent(
ChromiumBidi.BrowsingContext.EventNames.NavigationAborted,
ChromiumBidi.BrowsingContext.EventNames.NavigationFailed,
sinon.match.any,
ANOTHER_URL,
);
Expand Down
91 changes: 40 additions & 51 deletions src/bidiMapper/modules/context/NavigationTracker.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,7 @@ class NavigationState {
loaderId?: string;
#isInitial: boolean;
#eventManager: EventManager;
#navigated = false;

get finished(): Promise<NavigationResult> {
return this.#finished;
Expand Down Expand Up @@ -97,7 +98,7 @@ class NavigationState {
this.#started = true;
}

finish(navigationResult: NavigationResult) {
#finish(navigationResult: NavigationResult) {
this.#started = true;

if (
Expand All @@ -116,6 +117,30 @@ class NavigationState {
}
this.#finished.resolve(navigationResult);
}

frameNavigated() {
this.#navigated = true;
}

fragmentNavigated() {
this.#navigated = true;
this.#finish(new NavigationResult(NavigationEventName.FragmentNavigated));
}

load() {
this.#finish(new NavigationResult(NavigationEventName.Load));
}

fail(message: string) {
this.#finish(
new NavigationResult(
this.#navigated
? NavigationEventName.NavigationAborted
: NavigationEventName.NavigationFailed,
message,
),
);
}
}

/**
Expand Down Expand Up @@ -201,11 +226,8 @@ export class NavigationTracker {
this.#isInitialNavigation &&
urlMatchesAboutBlank(url);

this.#pendingNavigation?.finish(
new NavigationResult(
NavigationEventName.NavigationAborted,
'navigation canceled by concurrent navigation',
),
this.#pendingNavigation?.fail(
'navigation canceled by concurrent navigation',
);
const navigation = new NavigationState(
url,
Expand All @@ -218,20 +240,8 @@ export class NavigationTracker {
}

dispose() {
// TODO: check if it should be aborted or failed.
this.#pendingNavigation?.finish(
new NavigationResult(
NavigationEventName.NavigationFailed,
'navigation canceled by context disposal',
),
);
// TODO: check if it should be aborted or failed.
this.#currentNavigation.finish(
new NavigationResult(
NavigationEventName.NavigationFailed,
'navigation canceled by context disposal',
),
);
this.#pendingNavigation?.fail('navigation canceled by context disposal');
this.#currentNavigation.fail('navigation canceled by context disposal');
}

// Update the current url.
Expand Down Expand Up @@ -277,23 +287,16 @@ export class NavigationTracker {
this.createPendingNavigation(unreachableUrl, true);
navigation.url = unreachableUrl;
navigation.start();
navigation.finish(
new NavigationResult(
NavigationEventName.NavigationFailed,
'the requested url is unreachable',
),
);
navigation.fail('the requested url is unreachable');
return;
}

const navigation = this.#getNavigationForFrameNavigated(url, loaderId);
navigation.frameNavigated();

if (navigation !== this.#currentNavigation) {
this.#currentNavigation.finish(
new NavigationResult(
NavigationEventName.NavigationAborted,
'navigation canceled by concurrent navigation',
),
this.#currentNavigation.fail(
'navigation canceled by concurrent navigation',
);
}

Expand Down Expand Up @@ -340,9 +343,7 @@ export class NavigationTracker {
);

// Finish ongoing navigation.
fragmentNavigation.finish(
new NavigationResult(NavigationEventName.FragmentNavigated),
);
fragmentNavigation.fragmentNavigated();

if (fragmentNavigation === this.#pendingNavigation) {
this.#pendingNavigation = undefined;
Expand All @@ -365,19 +366,15 @@ export class NavigationTracker {
// Even if it was an initial navigation, it is finished.
this.#isInitialNavigation = false;

this.#loaderIdToNavigationsMap
.get(loaderId)
?.finish(new NavigationResult(NavigationEventName.Load));
this.#loaderIdToNavigationsMap.get(loaderId)?.load();
}

/**
* Fail navigation due to navigation command failed.
*/
failNavigation(navigation: NavigationState, errorText: string) {
this.#logger?.(LogType.debug, 'failCommandNavigation');
navigation.finish(
new NavigationResult(NavigationEventName.NavigationFailed, errorText),
);
navigation.fail(errorText);
}

/**
Expand All @@ -401,11 +398,8 @@ export class NavigationTracker {
return;
}

this.#currentNavigation.finish(
new NavigationResult(
NavigationEventName.NavigationAborted,
'navigation canceled by concurrent navigation',
),
this.#currentNavigation.fail(
'navigation canceled by concurrent navigation',
);

navigation.start();
Expand Down Expand Up @@ -462,11 +456,6 @@ export class NavigationTracker {
* that the navigation failed.
*/
networkLoadingFailed(loaderId: string, errorText: string) {
if (this.#loaderIdToNavigationsMap.has(loaderId)) {
const navigation = this.#loaderIdToNavigationsMap.get(loaderId)!;
navigation.finish(
new NavigationResult(NavigationEventName.NavigationFailed, errorText),
);
}
this.#loaderIdToNavigationsMap.get(loaderId)?.fail(errorText);
}
}
90 changes: 90 additions & 0 deletions tests/browsing_context/__snapshots__/test_navigate_events.ambr
View file
Original file line number Diff line number Diff line change
Expand Up @@ -180,6 +180,96 @@
}),
])
# ---
# name: test_navigate_hang_navigate_again_checkEvents
list([
dict({
'method': 'network.beforeRequestSent',
'params': dict({
'context': 'stable_0',
'isBlocked': False,
'navigation': 'stable_1',
'redirectCount': 0,
'request': 'stable_4',
}),
'type': 'event',
}),
dict({
'method': 'browsingContext.navigationFailed',
'params': dict({
'context': 'stable_0',
'navigation': 'stable_1',
'url': 'stable_3',
}),
'type': 'event',
}),
dict({
'error': 'unknown error',
'id': 'stable_5',
'message': 'navigation canceled by concurrent navigation',
'type': 'error',
}),
dict({
'method': 'script.message',
'params': dict({
'channel': 'beforeunload_channel',
'data': dict({
'type': 'string',
'value': 'beforeunload',
}),
'source': dict({
'context': 'stable_0',
}),
}),
'type': 'event',
}),
dict({
'method': 'browsingContext.navigationStarted',
'params': dict({
'context': 'stable_0',
'navigation': 'stable_6',
'url': 'stable_7',
}),
'type': 'event',
}),
dict({
'method': 'network.beforeRequestSent',
'params': dict({
'context': 'stable_0',
'isBlocked': False,
'navigation': 'stable_6',
'redirectCount': 0,
'request': 'stable_9',
}),
'type': 'event',
}),
dict({
'method': 'browsingContext.domContentLoaded',
'params': dict({
'context': 'stable_0',
'navigation': 'stable_6',
'url': 'stable_7',
}),
'type': 'event',
}),
dict({
'method': 'browsingContext.load',
'params': dict({
'context': 'stable_0',
'navigation': 'stable_6',
'url': 'stable_7',
}),
'type': 'event',
}),
dict({
'id': 'stable_10',
'result': dict({
'navigation': 'stable_6',
'url': 'stable_7',
}),
'type': 'success',
}),
])
# ---
# name: test_reload_aboutBlank_checkEvents
list([
dict({
Expand Down
Loading

0 comments on commit 6c3d406

Please sign in to comment.