diff --git a/src/bidiMapper/modules/context/NavigationTracker.spec.ts b/src/bidiMapper/modules/context/NavigationTracker.spec.ts index 0868abf8c..86b6f217d 100644 --- a/src/bidiMapper/modules/context/NavigationTracker.spec.ts +++ b/src/bidiMapper/modules/context/NavigationTracker.spec.ts @@ -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); @@ -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); @@ -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(); @@ -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, ); @@ -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); @@ -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, ); diff --git a/src/bidiMapper/modules/context/NavigationTracker.ts b/src/bidiMapper/modules/context/NavigationTracker.ts index 286f94977..ed32579ba 100644 --- a/src/bidiMapper/modules/context/NavigationTracker.ts +++ b/src/bidiMapper/modules/context/NavigationTracker.ts @@ -56,6 +56,7 @@ class NavigationState { loaderId?: string; #isInitial: boolean; #eventManager: EventManager; + #navigated = false; get finished(): Promise { return this.#finished; @@ -97,7 +98,7 @@ class NavigationState { this.#started = true; } - finish(navigationResult: NavigationResult) { + #finish(navigationResult: NavigationResult) { this.#started = true; if ( @@ -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, + ), + ); + } } /** @@ -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, @@ -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. @@ -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', ); } @@ -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; @@ -365,9 +366,7 @@ 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(); } /** @@ -375,9 +374,7 @@ export class NavigationTracker { */ failNavigation(navigation: NavigationState, errorText: string) { this.#logger?.(LogType.debug, 'failCommandNavigation'); - navigation.finish( - new NavigationResult(NavigationEventName.NavigationFailed, errorText), - ); + navigation.fail(errorText); } /** @@ -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(); @@ -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); } } diff --git a/tests/browsing_context/__snapshots__/test_navigate_events.ambr b/tests/browsing_context/__snapshots__/test_navigate_events.ambr index 1b82f3354..4ca2f4db7 100644 --- a/tests/browsing_context/__snapshots__/test_navigate_events.ambr +++ b/tests/browsing_context/__snapshots__/test_navigate_events.ambr @@ -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({ diff --git a/tests/browsing_context/test_navigate_events.py b/tests/browsing_context/test_navigate_events.py index 940e92817..6d8dc3833 100644 --- a/tests/browsing_context/test_navigate_events.py +++ b/tests/browsing_context/test_navigate_events.py @@ -16,10 +16,10 @@ import pytest from syrupy.filters import props from test_helpers import (execute_command, goto_url, send_JSON_command, - subscribe) + subscribe, wait_for_event) -SNAPSHOT_EXCLUDE = props("timestamp", "message", "timings", "headers", - "stacktrace", "response", "initiator", "realm") +SNAPSHOT_EXCLUDE = props("timestamp", "timings", "headers", "stacktrace", + "response", "initiator", "realm") KEYS_TO_STABILIZE = ['context', 'navigation', 'id', 'url', 'request'] @@ -132,6 +132,48 @@ async def test_navigate_dataUrl_checkEvents(websocket, context_id, url_base, assert messages == snapshot(exclude=SNAPSHOT_EXCLUDE) +@pytest.mark.asyncio +async def test_navigate_hang_navigate_again_checkEvents( + websocket, context_id, url_base, url_hang_forever, + url_example_another_origin, read_messages, snapshot, + assert_no_more_messages): + # Use `url_example_another_origin`, as `url_example` will hang because of + # `url_hang_forever`. + await goto_url(websocket, context_id, url_base) + await set_beforeunload_handler(websocket, context_id) + await subscribe( + websocket, + ["browsingContext", "script.message", "network.beforeRequestSent"]) + + await send_JSON_command( + websocket, { + "method": "browsingContext.navigate", + "params": { + "url": url_hang_forever, + "wait": "complete", + "context": context_id + } + }) + + await wait_for_event(websocket, "browsingContext.navigationStarted") + + await send_JSON_command( + websocket, { + "method": "browsingContext.navigate", + "params": { + "url": url_example_another_origin, + "wait": "complete", + "context": context_id + } + }) + + messages = await read_messages(9, + keys_to_stabilize=KEYS_TO_STABILIZE, + check_no_other_messages=True, + sort=False) + assert messages == snapshot(exclude=SNAPSHOT_EXCLUDE) + + @pytest.mark.asyncio async def test_scriptNavigate_checkEvents(websocket, context_id, url_example, html, read_messages, snapshot): diff --git a/wpt-metadata/chromedriver/headful/webdriver/tests/bidi/browsing_context/navigation_failed/navigation_failed.py.ini b/wpt-metadata/chromedriver/headful/webdriver/tests/bidi/browsing_context/navigation_failed/navigation_failed.py.ini index 7d24e80a0..3e677dec0 100644 --- a/wpt-metadata/chromedriver/headful/webdriver/tests/bidi/browsing_context/navigation_failed/navigation_failed.py.ini +++ b/wpt-metadata/chromedriver/headful/webdriver/tests/bidi/browsing_context/navigation_failed/navigation_failed.py.ini @@ -4,3 +4,6 @@ [test_with_new_navigation] expected: [PASS, FAIL] + + [test_close_iframe] + expected: FAIL diff --git a/wpt-metadata/chromedriver/headless/webdriver/tests/bidi/browsing_context/navigation_failed/navigation_failed.py.ini b/wpt-metadata/chromedriver/headless/webdriver/tests/bidi/browsing_context/navigation_failed/navigation_failed.py.ini index 7d24e80a0..3e677dec0 100644 --- a/wpt-metadata/chromedriver/headless/webdriver/tests/bidi/browsing_context/navigation_failed/navigation_failed.py.ini +++ b/wpt-metadata/chromedriver/headless/webdriver/tests/bidi/browsing_context/navigation_failed/navigation_failed.py.ini @@ -4,3 +4,6 @@ [test_with_new_navigation] expected: [PASS, FAIL] + + [test_close_iframe] + expected: FAIL diff --git a/wpt-metadata/mapper/headless/webdriver/tests/bidi/browsing_context/navigation_failed/navigation_failed.py.ini b/wpt-metadata/mapper/headless/webdriver/tests/bidi/browsing_context/navigation_failed/navigation_failed.py.ini index 7d24e80a0..3e677dec0 100644 --- a/wpt-metadata/mapper/headless/webdriver/tests/bidi/browsing_context/navigation_failed/navigation_failed.py.ini +++ b/wpt-metadata/mapper/headless/webdriver/tests/bidi/browsing_context/navigation_failed/navigation_failed.py.ini @@ -4,3 +4,6 @@ [test_with_new_navigation] expected: [PASS, FAIL] + + [test_close_iframe] + expected: FAIL