Skip to content

Commit

Permalink
fix: use requestWillBeSent for navigation started
Browse files Browse the repository at this point in the history
  • Loading branch information
jrandolf-2 committed Feb 13, 2024
1 parent 9c60d0a commit 697526a
Show file tree
Hide file tree
Showing 12 changed files with 57 additions and 114 deletions.
66 changes: 39 additions & 27 deletions src/bidiMapper/domains/context/BrowsingContextImpl.ts
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@ import {assert} from '../../../utils/assert.js';
import {Deferred} from '../../../utils/Deferred.js';
import {LogType, type LoggerFn} from '../../../utils/log.js';
import {inchesFromCm} from '../../../utils/unitConversions.js';
import {uuidv4} from '../../../utils/uuid.js';
import type {Realm} from '../script/Realm.js';
import type {RealmStorage} from '../script/RealmStorage.js';
import {WindowRealm} from '../script/WindowRealm.js';
Expand Down Expand Up @@ -72,6 +73,7 @@ export class BrowsingContextImpl {
readonly #eventManager: EventManager;
readonly #realmStorage: RealmStorage;
#loaderId?: Protocol.Network.LoaderId;
#fragmentIds = new Map<string, string>();
#cdpTarget: CdpTarget;
#maybeDefaultRealm?: Realm;
readonly #sharedIdWithFrame: boolean;
Expand Down Expand Up @@ -350,41 +352,32 @@ export class BrowsingContextImpl {
this.#url = params.url;
this.#navigation.withinDocument.resolve(params);

let url = this.#url;
let navigation = this.#fragmentIds.get(this.#url);
if (navigation === undefined) {
for (const [fragmentUrl, fragmentId] of [
...this.#fragmentIds.entries(),
].reverse()) {
url = fragmentUrl;
navigation = fragmentId;
break;
}
}

this.#eventManager.registerEvent(
{
type: 'event',
method: ChromiumBidi.BrowsingContext.EventNames.FragmentNavigated,
params: {
context: this.id,
navigation: null,
navigation: navigation ?? navigation ?? uuidv4(),
timestamp,
url: this.#url,
},
},
this.id
);
}
);

this.#cdpTarget.cdpClient.on(
'Page.frameStartedLoading',
(params: Protocol.Page.FrameStartedLoadingEvent) => {
if (this.id !== params.frameId) {
return;
}
this.#eventManager.registerEvent(
{
type: 'event',
method: ChromiumBidi.BrowsingContext.EventNames.NavigationStarted,
params: {
context: this.id,
navigation: null,
timestamp: BrowsingContextImpl.getTimestamp(),
url: '',
},
},
this.id
);
this.#fragmentIds.delete(url);
}
);

Expand All @@ -401,7 +394,7 @@ export class BrowsingContextImpl {
}

if (params.name === 'commit') {
this.#loaderId = params.loaderId;
this.#documentChanged(params.loaderId);
return;
}

Expand All @@ -421,7 +414,7 @@ export class BrowsingContextImpl {
ChromiumBidi.BrowsingContext.EventNames.DomContentLoaded,
params: {
context: this.id,
navigation: this.#loaderId ?? null,
navigation: this.#loaderId,
timestamp,
url: this.#url,
},
Expand All @@ -438,7 +431,7 @@ export class BrowsingContextImpl {
method: ChromiumBidi.BrowsingContext.EventNames.Load,
params: {
context: this.id,
navigation: this.#loaderId ?? null,
navigation: this.#loaderId,
timestamp,
url: this.#url,
},
Expand Down Expand Up @@ -576,6 +569,21 @@ export class BrowsingContextImpl {
this.#resetLifecycleIfFinished();

this.#loaderId = loaderId;
if (this.#loaderId) {
this.#eventManager.registerEvent(
{
type: 'event',
method: ChromiumBidi.BrowsingContext.EventNames.NavigationStarted,
params: {
context: this.id,
navigation: this.#loaderId,
timestamp: BrowsingContextImpl.getTimestamp(),
url: '',
},
},
this.id
);
}
}

#resetLifecycleIfFinished() {
Expand Down Expand Up @@ -640,6 +648,10 @@ export class BrowsingContextImpl {

this.#documentChanged(cdpNavigateResult.loaderId);

if (cdpNavigateResult.loaderId === undefined) {
this.#fragmentIds.set(url, uuidv4());
}

switch (wait) {
case BrowsingContext.ReadinessState.None:
break;
Expand All @@ -662,7 +674,7 @@ export class BrowsingContextImpl {
}

return {
navigation: cdpNavigateResult.loaderId ?? null,
navigation: this.#loaderId ?? this.#fragmentIds.get(url) ?? null,
// Url can change due to redirect get the latest one.
url: wait === BrowsingContext.ReadinessState.None ? url : this.#url,
};
Expand Down
3 changes: 2 additions & 1 deletion tests/browsing_context/test_fragment_navigated.py
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@
# See the License for the specific language governing permissions and
# limitations under the License.
import pytest
from anys import ANY_STR
from test_helpers import (ANY_TIMESTAMP, goto_url, read_JSON_message,
send_JSON_command, subscribe)

Expand Down Expand Up @@ -43,7 +44,7 @@ async def test_browsingContext_fragmentNavigated_event(websocket, context_id):
"method": "browsingContext.fragmentNavigated",
"params": {
"context": context_id,
"navigation": None,
"navigation": ANY_STR,
"timestamp": ANY_TIMESTAMP,
"url": url + "#test",
}
Expand Down
18 changes: 10 additions & 8 deletions tests/browsing_context/test_navigate.py
Original file line number Diff line number Diff line change
Expand Up @@ -207,10 +207,10 @@ async def test_browsingContext_navigateSameDocumentNavigation_waitNone_navigated
await goto_url(websocket, context_id, url, "complete")

resp = await goto_url(websocket, context_id, url_with_hash_1, "none")
assert resp == {'navigation': None, 'url': url_with_hash_1}
assert resp == {'navigation': ANY_STR, 'url': url_with_hash_1}

resp = await goto_url(websocket, context_id, url_with_hash_2, "none")
assert resp == {'navigation': None, 'url': url_with_hash_2}
assert resp == {'navigation': ANY_STR, 'url': url_with_hash_2}


@pytest.mark.asyncio
Expand All @@ -225,7 +225,7 @@ async def test_browsingContext_navigateSameDocumentNavigation_waitInteractive_na

resp = await goto_url(websocket, context_id, url_with_hash_1,
"interactive")
assert resp == {'navigation': None, 'url': url_with_hash_1}
assert resp == {'navigation': ANY_STR, 'url': url_with_hash_1}

result = await get_tree(websocket, context_id)

Expand All @@ -241,7 +241,7 @@ async def test_browsingContext_navigateSameDocumentNavigation_waitInteractive_na

resp = await goto_url(websocket, context_id, url_with_hash_2,
"interactive")
assert resp == {'navigation': None, 'url': url_with_hash_2}
assert resp == {'navigation': ANY_STR, 'url': url_with_hash_2}

result = await get_tree(websocket, context_id)

Expand All @@ -267,7 +267,7 @@ async def test_browsingContext_navigateSameDocumentNavigation_waitComplete_navig
await goto_url(websocket, context_id, url, "complete")

resp = await goto_url(websocket, context_id, url_with_hash_1, "complete")
assert resp == {'navigation': None, 'url': url_with_hash_1}
assert resp == {'navigation': ANY_STR, 'url': url_with_hash_1}

result = await get_tree(websocket, context_id)

Expand All @@ -282,7 +282,7 @@ async def test_browsingContext_navigateSameDocumentNavigation_waitComplete_navig
} == result

resp = await goto_url(websocket, context_id, url_with_hash_2, "complete")
assert resp == {'navigation': None, 'url': url_with_hash_2}
assert resp == {'navigation': ANY_STR, 'url': url_with_hash_2}

result = await get_tree(websocket, context_id)

Expand Down Expand Up @@ -344,13 +344,15 @@ async def test_browsingContext_navigationStartedEvent_viaScript(
}
})

await read_JSON_message(websocket)

response = await read_JSON_message(websocket)
assert response == {
'type': 'event',
"method": "browsingContext.navigationStarted",
"params": {
"context": context_id,
"navigation": None,
"navigation": ANY_STR,
"timestamp": ANY_TIMESTAMP,
# TODO: Should report correct string
"url": ANY_STR,
Expand Down Expand Up @@ -381,7 +383,7 @@ async def test_browsingContext_navigationStartedEvent_viaCommand(
"method": "browsingContext.navigationStarted",
"params": {
"context": context_id,
"navigation": None,
"navigation": ANY_STR,
"timestamp": ANY_TIMESTAMP,
# TODO: Should report correct string
"url": ANY_STR,
Expand Down
12 changes: 6 additions & 6 deletions tests/browsing_context/test_nested_browsing_context.py
Original file line number Diff line number Diff line change
Expand Up @@ -167,10 +167,10 @@ async def test_nestedBrowsingContext_navigateSameDocumentNavigation_waitNone_nav
await goto_url(websocket, iframe_id, url, "complete")

resp = await goto_url(websocket, iframe_id, url_with_hash_1, "none")
assert resp == {'navigation': None, 'url': url_with_hash_1}
assert resp == {'navigation': ANY_STR, 'url': url_with_hash_1}

resp = await goto_url(websocket, iframe_id, url_with_hash_2, "none")
assert resp == {'navigation': None, 'url': url_with_hash_2}
assert resp == {'navigation': ANY_STR, 'url': url_with_hash_2}


@pytest.mark.asyncio
Expand All @@ -184,7 +184,7 @@ async def test_nestedBrowsingContext_navigateSameDocumentNavigation_waitInteract
await goto_url(websocket, iframe_id, url, "complete")

resp = await goto_url(websocket, iframe_id, url_with_hash_1, "interactive")
assert resp == {'navigation': None, 'url': url_with_hash_1}
assert resp == {'navigation': ANY_STR, 'url': url_with_hash_1}

result = await get_tree(websocket, iframe_id)

Expand All @@ -199,7 +199,7 @@ async def test_nestedBrowsingContext_navigateSameDocumentNavigation_waitInteract
} == result

resp = await goto_url(websocket, iframe_id, url_with_hash_2, "interactive")
assert resp == {'navigation': None, 'url': url_with_hash_2}
assert resp == {'navigation': ANY_STR, 'url': url_with_hash_2}

result = await get_tree(websocket, iframe_id)

Expand All @@ -225,7 +225,7 @@ async def test_nestedBrowsingContext_navigateSameDocumentNavigation_waitComplete
await goto_url(websocket, iframe_id, url, "complete")

resp = await goto_url(websocket, iframe_id, url_with_hash_1, "complete")
assert resp == {'navigation': None, 'url': url_with_hash_1}
assert resp == {'navigation': ANY_STR, 'url': url_with_hash_1}

result = await get_tree(websocket, iframe_id)

Expand All @@ -240,7 +240,7 @@ async def test_nestedBrowsingContext_navigateSameDocumentNavigation_waitComplete
} == result

resp = await goto_url(websocket, iframe_id, url_with_hash_2, "complete")
assert resp == {'navigation': None, 'url': url_with_hash_2}
assert resp == {'navigation': ANY_STR, 'url': url_with_hash_2}

result = await get_tree(websocket, iframe_id)

Expand Down
Original file line number Diff line number Diff line change
@@ -1,15 +1,3 @@
[hash.py]
[test_navigate_in_the_same_document[without hash to with hash\]]
expected: FAIL

[test_navigate_in_the_same_document[with different hashes\]]
expected: FAIL

[test_navigate_in_iframe]
expected: FAIL

[test_navigate_unique_navigation_id]
expected: FAIL

[test_navigate_in_the_same_document[with identical hashes\]]
expected: FAIL
Original file line number Diff line number Diff line change
Expand Up @@ -2,9 +2,6 @@
[test_subscribe]
expected: FAIL

[test_timestamp]
expected: FAIL

[test_iframe]
expected: FAIL

Expand Down Expand Up @@ -34,6 +31,3 @@

[test_redirect_navigation]
expected: FAIL

[test_navigate_history_pushstate]
expected: FAIL
Original file line number Diff line number Diff line change
@@ -1,15 +1,3 @@
[hash.py]
[test_navigate_in_the_same_document[without hash to with hash\]]
expected: FAIL

[test_navigate_in_the_same_document[with different hashes\]]
expected: FAIL

[test_navigate_in_iframe]
expected: FAIL

[test_navigate_unique_navigation_id]
expected: FAIL

[test_navigate_in_the_same_document[with identical hashes\]]
expected: FAIL
Original file line number Diff line number Diff line change
Expand Up @@ -2,9 +2,6 @@
[test_subscribe]
expected: FAIL

[test_timestamp]
expected: FAIL

[test_iframe]
expected: FAIL

Expand Down Expand Up @@ -34,6 +31,3 @@

[test_redirect_navigation]
expected: FAIL

[test_navigate_history_pushstate]
expected: FAIL
Original file line number Diff line number Diff line change
@@ -1,15 +1,3 @@
[hash.py]
[test_navigate_in_the_same_document[without hash to with hash\]]
expected: FAIL

[test_navigate_in_the_same_document[with different hashes\]]
expected: FAIL

[test_navigate_in_iframe]
expected: FAIL

[test_navigate_unique_navigation_id]
expected: FAIL

[test_navigate_in_the_same_document[with identical hashes\]]
expected: FAIL
Original file line number Diff line number Diff line change
Expand Up @@ -2,9 +2,6 @@
[test_subscribe]
expected: FAIL

[test_timestamp]
expected: FAIL

[test_iframe]
expected: FAIL

Expand Down Expand Up @@ -34,6 +31,3 @@

[test_redirect_navigation]
expected: FAIL

[test_navigate_history_pushstate]
expected: FAIL
Original file line number Diff line number Diff line change
@@ -1,15 +1,3 @@
[hash.py]
[test_navigate_in_the_same_document[without hash to with hash\]]
expected: FAIL

[test_navigate_in_the_same_document[with different hashes\]]
expected: FAIL

[test_navigate_in_iframe]
expected: FAIL

[test_navigate_unique_navigation_id]
expected: FAIL

[test_navigate_in_the_same_document[with identical hashes\]]
expected: FAIL
Loading

0 comments on commit 697526a

Please sign in to comment.