Mirrors/servo
1
0
Fork 0
mirror of https://github.com/servo/servo.git synced 2025-08-03 20:50:07 +01:00
Code Issues Projects Releases Packages Wiki Activity

Auto merge of #22999 - jdm:iframe-replace, r=asajeffrey,cbrewster

Browse source 
Fix replacement logic when navigating nested browsing contexts

These changes also fix a bug where traversing the session history in a nested browsing context did not update the iframe's contentWindow appropriately.

- [x] `./mach build -d` does not report any errors
- [x] `./mach test-tidy` does not report any errors
- [x] These changes fix #22996
- [x] There are tests for these changes

<!-- Reviewable:start -->
---
This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/22999)
<!-- Reviewable:end -->
This commit is contained in:
bors-servo 2019-03-07 21:20:14 -05:00 • committed by GitHub
commit ac3c002138
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
10 changed files with 128 additions and 8 deletions
Download patch file Download diff file Expand all files Collapse all files

49
components/constellation/constellation.rs
View file

@ -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<PipelineId> {
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
@ -2640,12 +2675,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 +2701,7 @@ where
let msg = ConstellationControlMsg::UpdatePipelineId(
parent_pipeline_id,
browsing_context_id,
top_level_id,
new_pipeline_id,
UpdatePipelineIdReason::Traversal,
);
@ -3581,6 +3621,7 @@ where
let msg = ConstellationControlMsg::UpdatePipelineId(
parent_pipeline_id,
change.browsing_context_id,
change.top_level_browsing_context_id,
pipeline_id,
UpdatePipelineIdReason::Navigation,
);

4
components/constellation/pipeline.rs
View file

@ -89,6 +89,9 @@ pub struct Pipeline {
/// The history states owned by this pipeline.
pub history_states: HashSet<HistoryStateId>,
/// 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);

2
components/constellation/session_history.rs
View file

@ -37,6 +37,7 @@ impl JointSessionHistory {
}
pub fn push_diff(&mut self, diff: SessionHistoryDiff) -> Vec<SessionHistoryDiff> {
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,

4
components/script/dom/htmliframeelement.rs
View file

@ -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,

21
components/script/script_thread.rs
View file

@ -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,
);

1
components/script_traits/lib.rs
View file

@ -303,6 +303,7 @@ pub enum ConstellationControlMsg {
UpdatePipelineId(
PipelineId,
BrowsingContextId,
TopLevelBrowsingContextId,
PipelineId,
UpdatePipelineIdReason,
),

28
tests/wpt/mozilla/meta/MANIFEST.json
View file

@ -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"

25
tests/wpt/mozilla/tests/mozilla/history.html Normal file
View file

@ -0,0 +1,25 @@
<!doctype html>
<meta charset="utf-8">
<title>Traversing history causes iframe contentWindow to update</title>
<script src="/resources/testharness.js"></script>
<script src="/resources/testharnessreport.js"></script>
<iframe src="resources/first.html"></iframe>
<script>
var iframe = document.querySelector('iframe');
var t = async_test();
onload = t.step_func(function() {
iframe.onload = t.step_func(function() {
assert_true(iframe.contentWindow.location.href.endsWith('second.html'));
iframe.contentWindow.history.back();
t.step_timeout(function() {
assert_true(iframe.contentWindow.location.href.endsWith('first.html'));
iframe.contentWindow.history.forward();
t.step_timeout(function() {
assert_true(iframe.contentWindow.location.href.endsWith('second.html'));
t.done();
}, 1000);
}, 1000);
});
iframe.src = "resources/second.html";
});
</script>

1
tests/wpt/mozilla/tests/mozilla/resources/first.html Normal file
View file

@ -0,0 +1 @@
<body>hello world</body>

1
tests/wpt/mozilla/tests/mozilla/resources/second.html Normal file
View file

@ -0,0 +1 @@
<body>goodbye world</body>