diff --git a/moli-page-types/src/lib.rs b/moli-page-types/src/lib.rs index 7128c7a466..45c402c15f 100644 --- a/moli-page-types/src/lib.rs +++ b/moli-page-types/src/lib.rs @@ -187,9 +187,9 @@ pub use inspector_state::{ renderer_inspector_protocol_configuration_command_from_method, }; pub use navigation_history::{ - NavigationActivationSeed, NavigationHistoryDocumentId, NavigationHistoryEntrySeed, - NavigationHistoryMutation, NavigationHistorySerializedEntry, NavigationTraversalSeedCandidate, - SameDocumentHistoryUpdate, + NavigationActivationSeed, NavigationHistoryDocumentId, NavigationHistoryEntryId, + NavigationHistoryEntryKey, NavigationHistoryEntrySeed, NavigationHistoryMutation, + NavigationHistorySerializedEntry, NavigationTraversalSeedCandidate, SameDocumentHistoryUpdate, apply_child_browsing_context_javascript_url_navigation_to_entry_seed, apply_child_browsing_context_navigation_to_entry_seed, child_browsing_context_single_entry_seed, cross_document_navigation_seed, diff --git a/moli-page-types/src/navigation_history.rs b/moli-page-types/src/navigation_history.rs index 31f69760af..e63ae91714 100644 --- a/moli-page-types/src/navigation_history.rs +++ b/moli-page-types/src/navigation_history.rs @@ -2,6 +2,8 @@ use std::sync::atomic::{AtomicU64, Ordering}; use url::Url; static NEXT_NAVIGATION_HISTORY_DOCUMENT_ID: AtomicU64 = AtomicU64::new(1); +static NEXT_NAVIGATION_HISTORY_ENTRY_ID: AtomicU64 = AtomicU64::new(1); +static NEXT_NAVIGATION_HISTORY_ENTRY_KEY: AtomicU64 = AtomicU64::new(1); /// Opaque identity shared by session-history entries that belong to the same /// `Document`. @@ -37,6 +39,89 @@ fn allocate_navigation_history_document_id(counter: &AtomicU64) -> NavigationHis NavigationHistoryDocumentId(format!("document-{raw}")) } +/// Opaque identity for one Navigation API entry incarnation. +/// +/// A replacement allocates a new id even when it retains the same session +/// history slot and therefore the same [`NavigationHistoryEntryKey`]. This is +/// deliberately independent from both the Document identity and history +/// index. +#[derive(Debug, Clone, PartialEq, Eq, Hash)] +pub struct NavigationHistoryEntryId(String); + +impl NavigationHistoryEntryId { + pub fn allocate() -> Self { + allocate_navigation_history_entry_id(&NEXT_NAVIGATION_HISTORY_ENTRY_ID) + } + + pub fn from_serialized(token: String) -> Self { + Self(token) + } + + pub fn as_str(&self) -> &str { + &self.0 + } +} + +impl std::ops::Deref for NavigationHistoryEntryId { + type Target = str; + + fn deref(&self) -> &Self::Target { + self.as_str() + } +} + +fn allocate_navigation_history_entry_id(counter: &AtomicU64) -> NavigationHistoryEntryId { + let raw = counter + .fetch_update(Ordering::Relaxed, Ordering::Relaxed, |current| { + current.checked_add(1) + }) + .expect("Navigation History entry id allocator exhausted"); + NavigationHistoryEntryId(format!("entry-{raw}")) +} + +/// Opaque identity for one session-history slot exposed to Navigation API. +/// +/// Same-origin replacement retains the key; push and cross-origin +/// replacement allocate a fresh key. The token is never derived from a URL, +/// history index, or Document id. +#[derive(Debug, Clone, PartialEq, Eq, Hash)] +pub struct NavigationHistoryEntryKey(String); + +impl NavigationHistoryEntryKey { + pub fn allocate() -> Self { + allocate_navigation_history_entry_key(&NEXT_NAVIGATION_HISTORY_ENTRY_KEY) + } + + pub fn from_serialized(token: String) -> Self { + Self(token) + } + + pub fn as_str(&self) -> &str { + &self.0 + } + + fn is_empty(&self) -> bool { + self.0.is_empty() + } +} + +impl std::ops::Deref for NavigationHistoryEntryKey { + type Target = str; + + fn deref(&self) -> &Self::Target { + self.as_str() + } +} + +fn allocate_navigation_history_entry_key(counter: &AtomicU64) -> NavigationHistoryEntryKey { + let raw = counter + .fetch_update(Ordering::Relaxed, Ordering::Relaxed, |current| { + current.checked_add(1) + }) + .expect("Navigation History entry key allocator exhausted"); + NavigationHistoryEntryKey(format!("key-{raw}")) +} + #[derive(Debug, Clone, PartialEq, Eq)] pub struct NavigationHistorySerializedEntry { pub url: String, @@ -46,8 +131,8 @@ pub struct NavigationHistorySerializedEntry { pub document_id: NavigationHistoryDocumentId, pub history_index: u32, pub index: u32, - pub id: String, - pub key: String, + pub id: NavigationHistoryEntryId, + pub key: NavigationHistoryEntryKey, } #[derive(Debug, Clone, PartialEq, Eq)] @@ -112,8 +197,8 @@ pub fn initial_navigation_history_seed( 0, 0, initial_document_id, - "entry-0", - "key-0", + NavigationHistoryEntryId::allocate(), + NavigationHistoryEntryKey::allocate(), None, None, ), @@ -122,8 +207,8 @@ pub fn initial_navigation_history_seed( 1, 0, current_document_id, - "entry-1", - "key-1", + NavigationHistoryEntryId::allocate(), + NavigationHistoryEntryKey::allocate(), None, None, ), @@ -143,8 +228,8 @@ pub fn initial_navigation_history_seed( 0, 0, NavigationHistoryDocumentId::allocate(), - "entry-0", - "key-0", + NavigationHistoryEntryId::allocate(), + NavigationHistoryEntryKey::allocate(), None, None, )]; @@ -170,8 +255,8 @@ pub fn child_browsing_context_single_entry_seed(url: Option<&Url>) -> Navigation 0, 0, NavigationHistoryDocumentId::allocate(), - "entry-0", - "key-0", + NavigationHistoryEntryId::allocate(), + NavigationHistoryEntryKey::allocate(), None, None, ), @@ -180,8 +265,8 @@ pub fn child_browsing_context_single_entry_seed(url: Option<&Url>) -> Navigation 1, 0, NavigationHistoryDocumentId::allocate(), - "entry-1", - "key-1", + NavigationHistoryEntryId::allocate(), + NavigationHistoryEntryKey::allocate(), None, None, ), @@ -201,8 +286,8 @@ pub fn child_browsing_context_single_entry_seed(url: Option<&Url>) -> Navigation 0, 0, NavigationHistoryDocumentId::allocate(), - "entry-0", - "key-0", + NavigationHistoryEntryId::allocate(), + NavigationHistoryEntryKey::allocate(), None, None, )]; @@ -233,8 +318,8 @@ pub fn apply_child_browsing_context_navigation_to_entry_seed( next_index, current_navigation_index + 1, NavigationHistoryDocumentId::allocate(), - &format!("entry-{next_index}"), - &format!("key-{next_index}"), + NavigationHistoryEntryId::allocate(), + NavigationHistoryEntryKey::allocate(), history_state_json, navigation_state_json, )); @@ -289,15 +374,14 @@ pub fn replace_child_browsing_context_navigation_in_entry_seed( .find(|entry| entry.history_index == current_index) .cloned(); let next_document_id = NavigationHistoryDocumentId::allocate(); - let next_key = replacement_entry_key(current_index, previous_entry.as_ref(), url); - let next_entry_id = next_document_id.as_str().to_owned(); + let next_key = replacement_entry_key(previous_entry.as_ref(), url); let next_entry = navigation_history_entry( url.as_str(), current_index, current_navigation_index, next_document_id, - &next_entry_id, - &next_key, + NavigationHistoryEntryId::allocate(), + next_key, history_state_json, navigation_state_json, ); @@ -331,8 +415,8 @@ pub fn apply_child_browsing_context_javascript_url_navigation_to_entry_seed( }; let next_document_id = NavigationHistoryDocumentId::allocate(); let mut next_entry = previous_entry.clone(); - next_entry.document_id = next_document_id.clone(); - next_entry.id = next_document_id.as_str().to_owned(); + next_entry.document_id = next_document_id; + next_entry.id = NavigationHistoryEntryId::allocate(); if let Some(existing) = seed .entries .iter_mut() @@ -368,8 +452,8 @@ pub fn cross_document_navigation_seed( next_index, current_navigation_index + 1, NavigationHistoryDocumentId::allocate(), - &format!("entry-{next_index}"), - &format!("key-{next_index}"), + NavigationHistoryEntryId::allocate(), + NavigationHistoryEntryKey::allocate(), None, None, ); @@ -378,19 +462,14 @@ pub fn cross_document_navigation_seed( } NavigationHistoryMutation::Replace => { let next_document_id = NavigationHistoryDocumentId::allocate(); - let next_key = replacement_entry_key( - current_index, - current_entry_snapshot.as_ref(), - destination_url, - ); - let next_entry_id = next_document_id.as_str().to_owned(); + let next_key = replacement_entry_key(current_entry_snapshot.as_ref(), destination_url); let entry = navigation_history_entry( destination_url.as_str(), current_index, current_navigation_index, next_document_id, - &next_entry_id, - &next_key, + NavigationHistoryEntryId::allocate(), + next_key, None, None, ); @@ -475,8 +554,8 @@ fn navigation_history_entry( history_index: u32, index: u32, document_id: NavigationHistoryDocumentId, - id: &str, - key: &str, + id: NavigationHistoryEntryId, + key: NavigationHistoryEntryKey, history_state_json: Option, navigation_state_json: Option, ) -> NavigationHistorySerializedEntry { @@ -488,16 +567,15 @@ fn navigation_history_entry( document_id, history_index, index, - id: id.to_owned(), - key: key.to_owned(), + id, + key, } } fn replacement_entry_key( - history_index: u32, previous_entry: Option<&NavigationHistorySerializedEntry>, url: &Url, -) -> String { +) -> NavigationHistoryEntryKey { if previous_entry .and_then(|entry| Url::parse(&entry.url).ok()) .is_some_and(|previous_url| same_origin(&previous_url, url)) @@ -505,9 +583,9 @@ fn replacement_entry_key( return previous_entry .map(|entry| entry.key.clone()) .filter(|key| !key.is_empty()) - .unwrap_or_else(|| format!("key-{history_index}")); + .unwrap_or_else(NavigationHistoryEntryKey::allocate); } - format!("key-{history_index}-{}", url.as_str()) + NavigationHistoryEntryKey::allocate() } fn same_origin(left: &Url, right: &Url) -> bool { @@ -524,6 +602,14 @@ mod tests { NavigationHistoryDocumentId::from_serialized(token.to_owned()) } + fn entry_id(token: &str) -> NavigationHistoryEntryId { + NavigationHistoryEntryId::from_serialized(token.to_owned()) + } + + fn entry_key(token: &str) -> NavigationHistoryEntryKey { + NavigationHistoryEntryKey::from_serialized(token.to_owned()) + } + #[test] fn initial_navigation_history_seed_preserves_global_about_blank_predecessor() { let seed = initial_navigation_history_seed(true, "https://example.test/page"); @@ -592,8 +678,8 @@ mod tests { 4, 2, document_id("opaque-existing-document"), - "entry-4", - "key-4", + entry_id("entry-4"), + entry_key("key-4"), None, None, )]; @@ -633,6 +719,62 @@ mod tests { assert_eq!(counter.load(Ordering::Relaxed), u64::MAX); } + #[test] + fn entry_identity_allocators_reject_exhaustion_without_wrapping() { + let id_counter = AtomicU64::new(u64::MAX); + let id_exhausted = + std::panic::catch_unwind(|| allocate_navigation_history_entry_id(&id_counter)); + assert!(id_exhausted.is_err()); + assert_eq!(id_counter.load(Ordering::Relaxed), u64::MAX); + + let key_counter = AtomicU64::new(u64::MAX); + let key_exhausted = + std::panic::catch_unwind(|| allocate_navigation_history_entry_key(&key_counter)); + assert!(key_exhausted.is_err()); + assert_eq!(key_counter.load(Ordering::Relaxed), u64::MAX); + } + + #[test] + fn push_after_back_allocates_fresh_identity_for_reused_history_index() { + let first = Url::parse("https://example.test/first").unwrap(); + let second = Url::parse("https://example.test/second").unwrap(); + let replacement = Url::parse("https://example.test/replacement").unwrap(); + let mut seed = child_browsing_context_single_entry_seed(None); + apply_child_browsing_context_navigation_to_entry_seed(&mut seed, &first, None, None); + apply_child_browsing_context_navigation_to_entry_seed(&mut seed, &second, None, None); + let retired_forward_entry = seed.entries[2].clone(); + + seed.current_index = 1; + apply_child_browsing_context_navigation_to_entry_seed(&mut seed, &replacement, None, None); + + let replacement_entry = &seed.entries[2]; + assert_eq!( + replacement_entry.history_index, + retired_forward_entry.history_index + ); + assert_ne!(replacement_entry.id, retired_forward_entry.id); + assert_ne!(replacement_entry.key, retired_forward_entry.key); + assert_ne!( + replacement_entry.id.as_str(), + replacement_entry.document_id.as_str(), + "entry incarnation identity must not be projected from Document identity" + ); + } + + #[test] + fn cross_origin_replace_allocates_a_fresh_entry_key() { + let first = Url::parse("https://first.example/page").unwrap(); + let second = Url::parse("https://second.example/page").unwrap(); + let mut seed = child_browsing_context_single_entry_seed(Some(&first)); + let previous = seed.entries[1].clone(); + + replace_child_browsing_context_navigation_in_entry_seed(&mut seed, &second, None, None); + + assert_eq!(seed.entries[1].history_index, previous.history_index); + assert_ne!(seed.entries[1].id, previous.id); + assert_ne!(seed.entries[1].key, previous.key); + } + #[test] fn child_activation_omits_from_for_cross_origin_navigation() { let first = Url::parse("http://127.0.0.1:1111/common/blank.html").unwrap(); @@ -712,8 +854,8 @@ mod tests { 0, 0, document_id("document-0"), - "entry-0", - "key-0", + entry_id("entry-0"), + entry_key("key-0"), None, None, ), @@ -722,8 +864,8 @@ mod tests { 1, 1, document_id("document-1"), - "entry-1", - "key-1", + entry_id("entry-1"), + entry_key("key-1"), None, None, ), @@ -732,8 +874,8 @@ mod tests { 2, 2, document_id("document-2"), - "entry-2", - "key-2", + entry_id("entry-2"), + entry_key("key-2"), None, None, ), @@ -768,8 +910,8 @@ mod tests { 4, 2, document_id("document-4"), - "entry-4", - "key-4", + entry_id("entry-4"), + entry_key("key-4"), None, None, )]; @@ -802,8 +944,8 @@ mod tests { 7, 3, document_id("document-7"), - "entry-7", - "key-7", + entry_id("entry-7"), + entry_key("key-7"), None, None, )]; @@ -827,8 +969,8 @@ mod tests { 0, 0, document_id("document-1"), - "entry-0", - "key-0", + entry_id("entry-0"), + entry_key("key-0"), None, None, ), @@ -837,8 +979,8 @@ mod tests { 1, 1, document_id("document-1"), - "entry-1", - "key-1", + entry_id("entry-1"), + entry_key("key-1"), None, None, ), @@ -855,8 +997,8 @@ mod tests { 0, 0, document_id("document-0"), - "entry-0", - "key-0", + entry_id("entry-0"), + entry_key("key-0"), None, None, ), @@ -865,8 +1007,8 @@ mod tests { 1, 1, document_id("document-1"), - "entry-1", - "key-1", + entry_id("entry-1"), + entry_key("key-1"), None, None, ), diff --git a/moli-renderer-v8/src/context_bootstrap/history_mutation.rs b/moli-renderer-v8/src/context_bootstrap/history_mutation.rs index 7b33d96e70..5c062e9868 100644 --- a/moli-renderer-v8/src/context_bootstrap/history_mutation.rs +++ b/moli-renderer-v8/src/context_bootstrap/history_mutation.rs @@ -4,8 +4,8 @@ use super::navigation_callbacks::cancel_active_intercepted_same_document_navigat use super::navigation_entry::{ copy_navigation_entry_document_id, create_navigation_entry, history_entries, history_index, navigation_current_entry, navigation_current_entry_index, navigation_entry_key_value, - new_navigation_entry_token, set_history_entries, set_history_index, set_history_state, - stringify_history_state, sync_navigation_current_entry_from_history_entry, + new_navigation_entry_id, new_navigation_entry_key, set_history_entries, set_history_index, + set_history_state, stringify_history_state, sync_navigation_current_entry_from_history_entry, }; use super::navigation_entry_state::{clone_history_entry_state, set_history_entry_state}; use super::navigation_events::{ @@ -199,8 +199,8 @@ fn mutate_history_object<'s>( None, None, next_navigation_index, - &new_navigation_entry_token(), - &new_navigation_entry_token(), + &new_navigation_entry_id(), + &new_navigation_entry_key(), ); if let Some(previous_entry) = previous_entry { copy_navigation_entry_document_id(scope, previous_entry, entry); @@ -215,7 +215,7 @@ fn mutate_history_object<'s>( HistoryMutationKind::Replace => { let key = previous_entry .and_then(|entry| navigation_entry_key_value(scope, entry)) - .unwrap_or_else(new_navigation_entry_token); + .unwrap_or_else(|| new_navigation_entry_key().as_str().to_owned()); let entry = create_navigation_entry( scope, url.as_str(), @@ -223,7 +223,7 @@ fn mutate_history_object<'s>( None, None, current_navigation_index, - &new_navigation_entry_token(), + &new_navigation_entry_id(), &key, ); if let Some(previous_entry) = previous_entry { diff --git a/moli-renderer-v8/src/context_bootstrap/navigation_entry.rs b/moli-renderer-v8/src/context_bootstrap/navigation_entry.rs index 03e63765a0..c615850905 100644 --- a/moli-renderer-v8/src/context_bootstrap/navigation_entry.rs +++ b/moli-renderer-v8/src/context_bootstrap/navigation_entry.rs @@ -14,8 +14,8 @@ use super::navigation_window::{ }; use super::*; use crate::util::{get_private_value, set_private_value}; +use moli_page_types::{NavigationHistoryEntryId, NavigationHistoryEntryKey}; use moli_webapi_declare::WebApiObject; -use std::sync::atomic::{AtomicU64, Ordering}; const NAVIGATION_ENTRY_INITIAL_INDEX_SLOT: &str = "__lmNavigationEntryInitialIndex"; const NAVIGATION_ENTRY_JOINT_TOP_INDEX_SLOT: &str = "__lmNavigationEntryJointTopIndex"; @@ -25,7 +25,6 @@ const NAVIGATION_ENTRY_ID_SLOT: &str = "__lmNavigationEntryId"; const NAVIGATION_ENTRY_KEY_SLOT: &str = "__lmNavigationEntryKey"; const NAVIGATION_ENTRY_SCROLL_X_SLOT: &str = "__lmNavigationEntryScrollX"; const NAVIGATION_ENTRY_SCROLL_Y_SLOT: &str = "__lmNavigationEntryScrollY"; -static NEXT_NAVIGATION_ENTRY_TOKEN: AtomicU64 = AtomicU64::new(1); #[derive(WebApiObject)] #[webapi( @@ -211,9 +210,12 @@ pub(super) fn create_navigation_entry<'s>( entry } -pub(super) fn new_navigation_entry_token() -> String { - let sequence = NEXT_NAVIGATION_ENTRY_TOKEN.fetch_add(1, Ordering::Relaxed); - navigation_token_uuid_from_seed(&format!("generated-navigation-entry-{sequence}")) +pub(super) fn new_navigation_entry_id() -> NavigationHistoryEntryId { + NavigationHistoryEntryId::allocate() +} + +pub(super) fn new_navigation_entry_key() -> NavigationHistoryEntryKey { + NavigationHistoryEntryKey::allocate() } pub(super) fn navigation_entry_key_value<'s>( diff --git a/moli-renderer-v8/src/context_bootstrap/navigation_mutation.rs b/moli-renderer-v8/src/context_bootstrap/navigation_mutation.rs index e7c2ef7e91..3ecf9e8e59 100644 --- a/moli-renderer-v8/src/context_bootstrap/navigation_mutation.rs +++ b/moli-renderer-v8/src/context_bootstrap/navigation_mutation.rs @@ -7,9 +7,10 @@ use super::navigation_activation::{ use super::navigation_entry::{ copy_navigation_entry_document_id, create_navigation_entry, history_entries, history_index, navigation_current_entry, navigation_current_entry_index, navigation_entry_key_value, - navigation_entry_url_value, new_navigation_entry_token, set_history_entries, set_history_index, - set_history_state, set_navigation_entry_document_id, set_navigation_entry_joint_top_index, - stringify_history_state, sync_navigation_current_entry_from_history_entry, + navigation_entry_url_value, new_navigation_entry_id, new_navigation_entry_key, + set_history_entries, set_history_index, set_history_state, set_navigation_entry_document_id, + set_navigation_entry_joint_top_index, stringify_history_state, + sync_navigation_current_entry_from_history_entry, }; use super::navigation_entry_state::{ clone_history_entry_state, clone_navigation_entry_state, set_navigation_entry_state, diff --git a/moli-renderer-v8/src/context_bootstrap/navigation_mutation/local.rs b/moli-renderer-v8/src/context_bootstrap/navigation_mutation/local.rs index 2eae871e3f..91c8ddd0c4 100644 --- a/moli-renderer-v8/src/context_bootstrap/navigation_mutation/local.rs +++ b/moli-renderer-v8/src/context_bootstrap/navigation_mutation/local.rs @@ -35,14 +35,11 @@ pub(crate) fn apply_local_window_location_navigation<'s>( None, None, next_navigation_index, - &new_navigation_entry_token(), - &new_navigation_entry_token(), - ); - set_navigation_entry_document_id( - scope, - next_entry, - &format!("document-{next_index}-{}", resolved.as_str()), + &new_navigation_entry_id(), + &new_navigation_entry_key(), ); + let document_id = moli_page_types::NavigationHistoryDocumentId::allocate(); + set_navigation_entry_document_id(scope, next_entry, document_id.as_str()); bind_navigation_entry_runtime_owner(scope, next_entry, owner); let _ = next_entries.set_index(scope, next_index, next_entry.into()); set_history_entries(scope, history, next_entries); @@ -57,7 +54,7 @@ pub(crate) fn apply_local_window_location_navigation<'s>( let key = previous_entry .filter(|entry| replacement_keeps_navigation_key(scope, *entry, resolved)) .and_then(|entry| navigation_entry_key_value(scope, entry)) - .unwrap_or_else(new_navigation_entry_token); + .unwrap_or_else(|| new_navigation_entry_key().as_str().to_owned()); let entry = create_navigation_entry( scope, resolved.as_str(), @@ -65,14 +62,11 @@ pub(crate) fn apply_local_window_location_navigation<'s>( None, None, current_navigation_index, - &new_navigation_entry_token(), + &new_navigation_entry_id(), &key, ); - set_navigation_entry_document_id( - scope, - entry, - &format!("document-{current_index}-{}", resolved.as_str()), - ); + let document_id = moli_page_types::NavigationHistoryDocumentId::allocate(); + set_navigation_entry_document_id(scope, entry, document_id.as_str()); bind_navigation_entry_runtime_owner(scope, entry, owner); let _ = entries.set_index(scope, current_index, entry.into()); set_history_entries(scope, history, entries); diff --git a/moli-renderer-v8/src/context_bootstrap/navigation_mutation/same_document.rs b/moli-renderer-v8/src/context_bootstrap/navigation_mutation/same_document.rs index 662d3c2da1..2fb2fc86d8 100644 --- a/moli-renderer-v8/src/context_bootstrap/navigation_mutation/same_document.rs +++ b/moli-renderer-v8/src/context_bootstrap/navigation_mutation/same_document.rs @@ -43,7 +43,7 @@ pub(in crate::context_bootstrap) fn update_navigation_current_entry_for_same_doc LocationNavigationKind::Assign => { if should_replace_initial_child_entry { let key = navigation_entry_key_value(scope, current_entry) - .unwrap_or_else(new_navigation_entry_token); + .unwrap_or_else(|| new_navigation_entry_key().as_str().to_owned()); let entry = create_navigation_entry( scope, href, @@ -51,7 +51,7 @@ pub(in crate::context_bootstrap) fn update_navigation_current_entry_for_same_doc navigation_state_json.as_deref(), None, current_navigation_index, - &new_navigation_entry_token(), + &new_navigation_entry_id(), &key, ); copy_navigation_entry_document_id(scope, current_entry, entry); @@ -71,8 +71,8 @@ pub(in crate::context_bootstrap) fn update_navigation_current_entry_for_same_doc navigation_state_json.as_deref(), None, next_navigation_index, - &new_navigation_entry_token(), - &new_navigation_entry_token(), + &new_navigation_entry_id(), + &new_navigation_entry_key(), ); copy_navigation_entry_document_id(scope, current_entry, next_entry); set_child_joint_top_index_for_entry(scope, owner, Some(next_entry)); @@ -92,7 +92,7 @@ pub(in crate::context_bootstrap) fn update_navigation_current_entry_for_same_doc } LocationNavigationKind::Replace => { let key = navigation_entry_key_value(scope, current_entry) - .unwrap_or_else(new_navigation_entry_token); + .unwrap_or_else(|| new_navigation_entry_key().as_str().to_owned()); let entry = create_navigation_entry( scope, href, @@ -100,7 +100,7 @@ pub(in crate::context_bootstrap) fn update_navigation_current_entry_for_same_doc navigation_state_json.as_deref(), None, current_navigation_index, - &new_navigation_entry_token(), + &new_navigation_entry_id(), &key, ); copy_navigation_entry_document_id(scope, current_entry, entry); @@ -164,8 +164,8 @@ pub(in crate::context_bootstrap) fn apply_navigation_navigate_same_document<'s>( None, None, next_navigation_index, - &new_navigation_entry_token(), - &new_navigation_entry_token(), + &new_navigation_entry_id(), + &new_navigation_entry_key(), ); if let Some(previous_entry) = previous_entry { copy_navigation_entry_document_id(scope, previous_entry, next_entry); @@ -191,7 +191,7 @@ pub(in crate::context_bootstrap) fn apply_navigation_navigate_same_document<'s>( LocationNavigationKind::Replace => { let key = previous_entry .and_then(|entry| navigation_entry_key_value(scope, entry)) - .unwrap_or_else(new_navigation_entry_token); + .unwrap_or_else(|| new_navigation_entry_key().as_str().to_owned()); let entry = create_navigation_entry( scope, href, @@ -199,7 +199,7 @@ pub(in crate::context_bootstrap) fn apply_navigation_navigate_same_document<'s>( None, None, current_navigation_index, - &new_navigation_entry_token(), + &new_navigation_entry_id(), &key, ); if let Some(previous_entry) = previous_entry { diff --git a/moli-renderer-v8/src/context_bootstrap/navigation_restore.rs b/moli-renderer-v8/src/context_bootstrap/navigation_restore.rs index d4ea83e62d..0461e1d9a4 100644 --- a/moli-renderer-v8/src/context_bootstrap/navigation_restore.rs +++ b/moli-renderer-v8/src/context_bootstrap/navigation_restore.rs @@ -11,6 +11,9 @@ use super::navigation_projection::set_history_length_from_visible_entries; use super::navigation_result::clear_active_cross_document_navigation_if_matches; use super::navigation_window::{window_history_for_holder, window_navigation_for_holder}; use crate::native_bridge::NavigationHistoryEntrySeed; +use moli_page_types::{ + NavigationHistoryDocumentId, NavigationHistoryEntryId, NavigationHistoryEntryKey, +}; pub(crate) fn install_navigation_bootstrap_entry( scope: &mut v8::PinScope<'_, '_>, @@ -57,8 +60,19 @@ pub(crate) fn install_navigation_bootstrap_entry_for_holder<'s>( } let current_entry = current_entry.unwrap_or_else(|| { current_state = Some(v8::null(scope).into()); - let entry = create_navigation_entry(scope, "about:blank", None, None, None, 0, "", ""); - let document_id = moli_page_types::NavigationHistoryDocumentId::allocate(); + let entry_id = NavigationHistoryEntryId::allocate(); + let entry_key = NavigationHistoryEntryKey::allocate(); + let entry = create_navigation_entry( + scope, + "about:blank", + None, + None, + None, + 0, + entry_id.as_str(), + entry_key.as_str(), + ); + let document_id = NavigationHistoryDocumentId::allocate(); set_navigation_entry_document_id(scope, entry, document_id.as_str()); bind_navigation_entry_runtime_owner(scope, entry, owner); entry diff --git a/moli-renderer-v8/src/context_bootstrap/navigation_seed.rs b/moli-renderer-v8/src/context_bootstrap/navigation_seed.rs index c0a090bd9d..97cb9edd6f 100644 --- a/moli-renderer-v8/src/context_bootstrap/navigation_seed.rs +++ b/moli-renderer-v8/src/context_bootstrap/navigation_seed.rs @@ -12,6 +12,7 @@ use super::navigation_window::{ }; use crate::native_bridge::NavigationHistoryEntrySeed; use moli_page_types::{ + NavigationHistoryDocumentId, NavigationHistoryEntryId, NavigationHistoryEntryKey, initial_navigation_history_seed as page_initial_navigation_history_seed, reload_navigation_seed, traversal_navigation_seed_candidate, }; @@ -63,6 +64,8 @@ pub(super) fn build_current_navigation_entry_from_seed<'s>( .find(|entry| entry.history_index == seed.current_index) else { let fallback_state_json = stringify_history_state(scope, fallback_state); + let entry_id = NavigationHistoryEntryId::allocate(); + let entry_key = NavigationHistoryEntryKey::allocate(); let entry = create_navigation_entry( scope, "about:blank", @@ -70,10 +73,10 @@ pub(super) fn build_current_navigation_entry_from_seed<'s>( fallback_state_json.as_deref(), None, 0, - "", - "", + entry_id.as_str(), + entry_key.as_str(), ); - let document_id = moli_page_types::NavigationHistoryDocumentId::allocate(); + let document_id = NavigationHistoryDocumentId::allocate(); set_navigation_entry_document_id(scope, entry, document_id.as_str()); bind_navigation_entry_runtime_owner(scope, entry, owner); return entry; diff --git a/moli-renderer-v8/src/context_bootstrap/navigation_serialize.rs b/moli-renderer-v8/src/context_bootstrap/navigation_serialize.rs index 593ec63803..bffc8c533c 100644 --- a/moli-renderer-v8/src/context_bootstrap/navigation_serialize.rs +++ b/moli-renderer-v8/src/context_bootstrap/navigation_serialize.rs @@ -12,8 +12,8 @@ use super::navigation_window::{ }; use super::*; use crate::native_bridge::{ - NavigationActivationSeed, NavigationHistoryDocumentId, NavigationHistoryEntrySeed, - NavigationHistorySerializedEntry, + NavigationActivationSeed, NavigationHistoryDocumentId, NavigationHistoryEntryId, + NavigationHistoryEntryKey, NavigationHistoryEntrySeed, NavigationHistorySerializedEntry, }; use crate::{ document_runtime::DomHandle, native_bridge::node_runtime_and_handle_from_object, @@ -94,11 +94,15 @@ pub(super) fn serialize_navigation_entry_object<'s>( let id = get_own_static_property(scope, entry, "id") .and_then(|value| value.to_string(scope)) .map(|value| value.to_rust_string_lossy(scope)) - .unwrap_or_default(); + .filter(|value| !value.is_empty()) + .map(NavigationHistoryEntryId::from_serialized) + .unwrap_or_else(NavigationHistoryEntryId::allocate); let key = get_own_static_property(scope, entry, "key") .and_then(|value| value.to_string(scope)) .map(|value| value.to_rust_string_lossy(scope)) - .unwrap_or_default(); + .filter(|value| !value.is_empty()) + .map(NavigationHistoryEntryKey::from_serialized) + .unwrap_or_else(NavigationHistoryEntryKey::allocate); if let Some(snapshot) = history_entries .iter() .find(|snapshot| snapshot.id == id && snapshot.key == key) @@ -119,13 +123,15 @@ pub(super) fn serialize_navigation_entry_object<'s>( .filter(|value| *value >= 0) .map(|value| value as u32) .unwrap_or(0); - let document_id = navigation_entry_document_id(scope, entry).unwrap_or_else(|| id.clone()); + let document_id = navigation_entry_document_id(scope, entry) + .map(NavigationHistoryDocumentId::from_serialized) + .unwrap_or_else(NavigationHistoryDocumentId::allocate); NavigationHistorySerializedEntry { url, history_state_json, navigation_state_json, referrer_policy: navigation_entry_referrer_policy_value(scope, entry), - document_id: NavigationHistoryDocumentId::from_serialized(document_id), + document_id, history_index: entry_index, index: entry_index, id, @@ -218,18 +224,24 @@ pub(super) fn serialize_history_entries<'s>( let id = get_own_static_property(scope, entry, "id") .and_then(|value| value.to_string(scope)) .map(|value| value.to_rust_string_lossy(scope)) - .unwrap_or_else(|| format!("entry-{entry_index}")); + .filter(|value| !value.is_empty()) + .map(NavigationHistoryEntryId::from_serialized) + .unwrap_or_else(NavigationHistoryEntryId::allocate); let key = get_own_static_property(scope, entry, "key") .and_then(|value| value.to_string(scope)) .map(|value| value.to_rust_string_lossy(scope)) - .unwrap_or_else(|| format!("key-{entry_index}")); - let document_id = navigation_entry_document_id(scope, entry).unwrap_or_else(|| id.clone()); + .filter(|value| !value.is_empty()) + .map(NavigationHistoryEntryKey::from_serialized) + .unwrap_or_else(NavigationHistoryEntryKey::allocate); + let document_id = navigation_entry_document_id(scope, entry) + .map(NavigationHistoryDocumentId::from_serialized) + .unwrap_or_else(NavigationHistoryDocumentId::allocate); snapshots.push(NavigationHistorySerializedEntry { url, history_state_json, navigation_state_json, referrer_policy: navigation_entry_referrer_policy_value(scope, entry), - document_id: NavigationHistoryDocumentId::from_serialized(document_id), + document_id, history_index: index, index: entry_index, id, diff --git a/moli-renderer-v8/src/native_bridge/context_host/child_frames.rs b/moli-renderer-v8/src/native_bridge/context_host/child_frames.rs index 0a5c679e77..634a371496 100644 --- a/moli-renderer-v8/src/native_bridge/context_host/child_frames.rs +++ b/moli-renderer-v8/src/native_bridge/context_host/child_frames.rs @@ -1,7 +1,7 @@ use super::{ ChildBrowsingContextBootstrap, ChildBrowsingContextSnapshot, ChildFrameAttachmentSnapshot, - JsContextHost, NavigationActivationSeed, NavigationHistoryDocumentId, - NavigationHistoryEntrySeed, NavigationHistorySerializedEntry, + JsContextHost, NavigationActivationSeed, NavigationHistoryDocumentId, NavigationHistoryEntryId, + NavigationHistoryEntryKey, NavigationHistoryEntrySeed, NavigationHistorySerializedEntry, child_documents::CompletedFrameOwnerResourceTiming, }; use crate::{ @@ -749,8 +749,8 @@ impl ChildBrowsingContextEntry { document_id: NavigationHistoryDocumentId::allocate(), history_index: 1, index: 0, - id: "entry-1".to_owned(), - key: "key-1".to_owned(), + id: NavigationHistoryEntryId::allocate(), + key: NavigationHistoryEntryKey::allocate(), }); self.navigation_entry_seed.current_index = 1; self.mark_initial_attribute_target_navigation_activation(); diff --git a/moli-renderer-v8/src/native_bridge/context_host/mod.rs b/moli-renderer-v8/src/native_bridge/context_host/mod.rs index d34a009c37..7516aff756 100644 --- a/moli-renderer-v8/src/native_bridge/context_host/mod.rs +++ b/moli-renderer-v8/src/native_bridge/context_host/mod.rs @@ -196,8 +196,8 @@ pub(crate) use messages::{ PendingWindowMessage, PendingWindowMessageEndpoint, PendingWindowMessageSource, }; pub(crate) use moli_page_types::{ - NavigationActivationSeed, NavigationHistoryDocumentId, NavigationHistoryEntrySeed, - NavigationHistorySerializedEntry, + NavigationActivationSeed, NavigationHistoryDocumentId, NavigationHistoryEntryId, + NavigationHistoryEntryKey, NavigationHistoryEntrySeed, NavigationHistorySerializedEntry, }; pub(crate) use navigation::{PendingLocationNavigation, PendingTopLevelNavigation}; pub(crate) use popups::{