From edfd15c36f9c050512ebdfab3553cd3a7f6ff06f Mon Sep 17 00:00:00 2001 From: Josh Matthews Date: Thu, 7 Mar 2019 18:58:53 -0500 Subject: [PATCH 1/2] Ensure iframe's contentWindow is updated when traversing the session history. --- components/constellation/constellation.rs | 10 ++++++++-- components/script/script_thread.rs | 21 ++++++++++++++++++++- components/script_traits/lib.rs | 1 + 3 files changed, 29 insertions(+), 3 deletions(-) diff --git a/components/constellation/constellation.rs b/components/constellation/constellation.rs index 2b6df7d456a..f8804c4af7c 100644 --- a/components/constellation/constellation.rs +++ b/components/constellation/constellation.rs @@ -2640,12 +2640,16 @@ where }, }; - let (old_pipeline_id, parent_pipeline_id) = + let (old_pipeline_id, parent_pipeline_id, top_level_id) = match self.browsing_contexts.get_mut(&browsing_context_id) { Some(browsing_context) => { let old_pipeline_id = browsing_context.pipeline_id; browsing_context.update_current_entry(new_pipeline_id); - (old_pipeline_id, browsing_context.parent_pipeline_id) + ( + old_pipeline_id, + browsing_context.parent_pipeline_id, + browsing_context.top_level_id, + ) }, None => { return warn!( @@ -2662,6 +2666,7 @@ where let msg = ConstellationControlMsg::UpdatePipelineId( parent_pipeline_id, browsing_context_id, + top_level_id, new_pipeline_id, UpdatePipelineIdReason::Traversal, ); @@ -3581,6 +3586,7 @@ where let msg = ConstellationControlMsg::UpdatePipelineId( parent_pipeline_id, change.browsing_context_id, + change.top_level_browsing_context_id, pipeline_id, UpdatePipelineIdReason::Navigation, ); diff --git a/components/script/script_thread.rs b/components/script/script_thread.rs index 4076a56f9ca..448acd23790 100644 --- a/components/script/script_thread.rs +++ b/components/script/script_thread.rs @@ -1429,7 +1429,7 @@ impl ScriptThread { NotifyVisibilityChange(id, ..) => Some(id), Navigate(id, ..) => Some(id), PostMessage { target: id, .. } => Some(id), - UpdatePipelineId(_, _, id, _) => Some(id), + UpdatePipelineId(_, _, _, id, _) => Some(id), UpdateHistoryState(id, ..) => Some(id), RemoveHistoryStates(id, ..) => Some(id), FocusIFrame(id, ..) => Some(id), @@ -1617,11 +1617,13 @@ impl ScriptThread { ConstellationControlMsg::UpdatePipelineId( parent_pipeline_id, browsing_context_id, + top_level_browsing_context_id, new_pipeline_id, reason, ) => self.handle_update_pipeline_id( parent_pipeline_id, browsing_context_id, + top_level_browsing_context_id, new_pipeline_id, reason, ), @@ -2153,6 +2155,7 @@ impl ScriptThread { &self, parent_pipeline_id: PipelineId, browsing_context_id: BrowsingContextId, + top_level_browsing_context_id: TopLevelBrowsingContextId, new_pipeline_id: PipelineId, reason: UpdatePipelineIdReason, ) { @@ -2163,6 +2166,21 @@ impl ScriptThread { if let Some(frame_element) = frame_element { frame_element.update_pipeline_id(new_pipeline_id, reason); } + + if let Some(window) = self.documents.borrow().find_window(new_pipeline_id) { + // Ensure that the state of any local window proxies accurately reflects + // the new pipeline. + let _ = self.local_window_proxy( + &*window, + browsing_context_id, + top_level_browsing_context_id, + Some(parent_pipeline_id), + // Any local window proxy has already been created, so there + // is no need to pass along existing opener information that + // will be discarded. + None, + ); + } } fn handle_update_history_state_msg( @@ -2870,6 +2888,7 @@ impl ScriptThread { self.handle_update_pipeline_id( parent_pipeline, window_proxy.browsing_context_id(), + window_proxy.top_level_browsing_context_id(), incomplete.pipeline_id, UpdatePipelineIdReason::Navigation, ); diff --git a/components/script_traits/lib.rs b/components/script_traits/lib.rs index e2bc5c4d1ed..724d76daede 100644 --- a/components/script_traits/lib.rs +++ b/components/script_traits/lib.rs @@ -303,6 +303,7 @@ pub enum ConstellationControlMsg { UpdatePipelineId( PipelineId, BrowsingContextId, + TopLevelBrowsingContextId, PipelineId, UpdatePipelineIdReason, ), From c2ce7d72a12f814db3feec6c622e2c1bfe7f2691 Mon Sep 17 00:00:00 2001 From: Josh Matthews Date: Thu, 7 Mar 2019 18:59:57 -0500 Subject: [PATCH 2/2] Make nested browsing context navigations check the loaded status of the active document of the nested browsing context. --- components/constellation/constellation.rs | 39 ++++++++++++++++++- components/constellation/pipeline.rs | 4 ++ components/constellation/session_history.rs | 2 + components/script/dom/htmliframeelement.rs | 4 +- tests/wpt/mozilla/meta/MANIFEST.json | 28 +++++++++++++ tests/wpt/mozilla/tests/mozilla/history.html | 25 ++++++++++++ .../tests/mozilla/resources/first.html | 1 + .../tests/mozilla/resources/second.html | 1 + 8 files changed, 99 insertions(+), 5 deletions(-) create mode 100644 tests/wpt/mozilla/tests/mozilla/history.html create mode 100644 tests/wpt/mozilla/tests/mozilla/resources/first.html create mode 100644 tests/wpt/mozilla/tests/mozilla/resources/second.html diff --git a/components/constellation/constellation.rs b/components/constellation/constellation.rs index f8804c4af7c..02c4f757875 100644 --- a/components/constellation/constellation.rs +++ b/components/constellation/constellation.rs @@ -922,6 +922,14 @@ where } fn add_pending_change(&mut self, change: SessionHistoryChange) { + debug!( + "adding pending session history change with {}", + if change.replace.is_some() { + "replacement" + } else { + "no replacement" + }, + ); self.handle_load_start_msg( change.top_level_browsing_context_id, change.browsing_context_id, @@ -1915,13 +1923,29 @@ where top_level_browsing_context_id, new_pipeline_id, is_private, - replace, + mut replace, } = load_info.info; // If no url is specified, reload. let old_pipeline = load_info .old_pipeline_id .and_then(|id| self.pipelines.get(&id)); + + // Replacement enabled also takes into account whether the document is "completely loaded", + // see https://html.spec.whatwg.org/multipage/#the-iframe-element:completely-loaded + debug!("checking old pipeline? {:?}", load_info.old_pipeline_id); + if let Some(old_pipeline) = old_pipeline { + replace |= !old_pipeline.completely_loaded; + debug!( + "old pipeline is {}completely loaded", + if old_pipeline.completely_loaded { + "" + } else { + "not " + } + ); + } + let load_data = load_info.load_data.unwrap_or_else(|| { let url = match old_pipeline { Some(old_pipeline) => old_pipeline.url.clone(), @@ -1964,6 +1988,7 @@ where ); }, }; + let replace = if replace { Some(NeedsToReload::No(browsing_context.pipeline_id)) } else { @@ -2214,7 +2239,12 @@ where load_data: LoadData, replace: bool, ) -> Option { - debug!("Loading {} in pipeline {}.", load_data.url, source_id); + debug!( + "Loading {} in pipeline {}, {}replacing.", + load_data.url, + source_id, + if replace { "" } else { "not " } + ); // If this load targets an iframe, its framing element may exist // in a separate script thread than the framed document that initiated // the new load. The framing element must be notified about the @@ -2376,6 +2406,11 @@ where self.webdriver.load_channel = None; } + if let Some(pipeline) = self.pipelines.get_mut(&pipeline_id) { + debug!("marking pipeline {:?} as loaded", pipeline_id); + pipeline.completely_loaded = true; + } + // Notify the embedder that the TopLevelBrowsingContext current document // has finished loading. // We need to make sure the pipeline that has finished loading is the current diff --git a/components/constellation/pipeline.rs b/components/constellation/pipeline.rs index c8b5f4fc73c..be369c0c08e 100644 --- a/components/constellation/pipeline.rs +++ b/components/constellation/pipeline.rs @@ -89,6 +89,9 @@ pub struct Pipeline { /// The history states owned by this pipeline. pub history_states: HashSet, + + /// Has this pipeline received a notification that it is completely loaded? + pub completely_loaded: bool, } /// Initial setup data needed to construct a pipeline. @@ -355,6 +358,7 @@ impl Pipeline { load_data: load_data, history_state_id: None, history_states: HashSet::new(), + completely_loaded: false, }; pipeline.notify_visibility(is_visible); diff --git a/components/constellation/session_history.rs b/components/constellation/session_history.rs index 142c4bbc4f3..0fdcd129baa 100644 --- a/components/constellation/session_history.rs +++ b/components/constellation/session_history.rs @@ -37,6 +37,7 @@ impl JointSessionHistory { } pub fn push_diff(&mut self, diff: SessionHistoryDiff) -> Vec { + debug!("pushing a past entry; removing future"); self.past.push(diff); mem::replace(&mut self.future, vec![]) } @@ -85,6 +86,7 @@ impl JointSessionHistory { } pub fn remove_entries_for_browsing_context(&mut self, context_id: BrowsingContextId) { + debug!("removing entries for context {}", context_id); self.past.retain(|diff| match diff { SessionHistoryDiff::BrowsingContextDiff { browsing_context_id, diff --git a/components/script/dom/htmliframeelement.rs b/components/script/dom/htmliframeelement.rs index f9e249ca85a..3a01d36beeb 100644 --- a/components/script/dom/htmliframeelement.rs +++ b/components/script/dom/htmliframeelement.rs @@ -278,9 +278,7 @@ impl HTMLIFrameElement { // see https://html.spec.whatwg.org/multipage/#the-iframe-element:about:blank-3 let is_about_blank = pipeline_id.is_some() && pipeline_id == self.about_blank_pipeline_id.get(); - // Replacement enabled also takes into account whether the document is "completely loaded", - // see https://html.spec.whatwg.org/multipage/#the-iframe-element:completely-loaded - let replace = is_about_blank || !document.is_completely_loaded(); + let replace = is_about_blank; self.navigate_or_reload_child_browsing_context( Some(load_data), NavigationType::Regular, diff --git a/tests/wpt/mozilla/meta/MANIFEST.json b/tests/wpt/mozilla/meta/MANIFEST.json index df20041a903..214ce079f44 100644 --- a/tests/wpt/mozilla/meta/MANIFEST.json +++ b/tests/wpt/mozilla/meta/MANIFEST.json @@ -10459,6 +10459,11 @@ {} ] ], + "mozilla/resources/first.html": [ + [ + {} + ] + ], "mozilla/resources/http-cache.js": [ [ {} @@ -10499,6 +10504,11 @@ {} ] ], + "mozilla/resources/second.html": [ + [ + {} + ] + ], "mozilla/resources/ssl.https.html": [ [ {} @@ -12659,6 +12669,12 @@ {} ] ], + "mozilla/history.html": [ + [ + "/_mozilla/mozilla/history.html", + {} + ] + ], "mozilla/hit-test-background.html": [ [ "/_mozilla/mozilla/hit-test-background.html", @@ -19424,6 +19440,10 @@ "9baa0cdcd5abad00b321e8b9351a1bc162783ed5", "support" ], + "mozilla/history.html": [ + "130307f1e9c8bc4c5ee6fff4d5fef8fda89a1564", + "testharness" + ], "mozilla/hit-test-background.html": [ "5212954e4ee6ecb684212e7373e24a2268434b1c", "testharness" @@ -19796,6 +19816,10 @@ "5f0242874cfa47b84af35325ad651690cd9fb790", "support" ], + "mozilla/resources/first.html": [ + "b4359ad2855339999cfeda0c2681a51da6fdd940", + "support" + ], "mozilla/resources/http-cache.js": [ "34aaacf536f31e4d9ae003cb0891ede965201f08", "support" @@ -19828,6 +19852,10 @@ "b0883f382e1a80609b7b2c7904e701fbe6760b14", "support" ], + "mozilla/resources/second.html": [ + "c4fbe534ed193e1d192c0338997a8d9da8eb6406", + "support" + ], "mozilla/resources/ssl.https.html": [ "8faa57c0c47c4fdf27c052d059b28ee1088235e9", "support" diff --git a/tests/wpt/mozilla/tests/mozilla/history.html b/tests/wpt/mozilla/tests/mozilla/history.html new file mode 100644 index 00000000000..130307f1e9c --- /dev/null +++ b/tests/wpt/mozilla/tests/mozilla/history.html @@ -0,0 +1,25 @@ + + +Traversing history causes iframe contentWindow to update + + + + diff --git a/tests/wpt/mozilla/tests/mozilla/resources/first.html b/tests/wpt/mozilla/tests/mozilla/resources/first.html new file mode 100644 index 00000000000..b4359ad2855 --- /dev/null +++ b/tests/wpt/mozilla/tests/mozilla/resources/first.html @@ -0,0 +1 @@ +hello world diff --git a/tests/wpt/mozilla/tests/mozilla/resources/second.html b/tests/wpt/mozilla/tests/mozilla/resources/second.html new file mode 100644 index 00000000000..c4fbe534ed1 --- /dev/null +++ b/tests/wpt/mozilla/tests/mozilla/resources/second.html @@ -0,0 +1 @@ +goodbye world