Auto merge of #21048 - cbrewster:hash_change_history, r=asajeffrey

Track hash changes in session history

<!-- Please describe your changes on the following line: -->
Adds tracking of hash changes in the session history.

---
<!-- Thank you for contributing to Servo! Please replace each `[ ]` by `[X]` when the step is complete, and replace `__` with appropriate data: -->
- [X] `./mach build -d` does not report any errors
- [X] `./mach test-tidy` does not report any errors
- [X] These changes fix #14970 fix #13437 (github issue number if applicable).

<!-- Either: -->
- [X] There are tests for these changes OR
- [ ] These changes do not require tests because _____

<!-- Also, please make sure that "Allow edits from maintainers" checkbox is checked, so that we can help you if you get stuck somewhere along the way.-->

<!-- Pull requests that do not address these steps are welcome, but they will require additional verification as part of the review process. -->

<!-- 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/21048)
<!-- Reviewable:end -->
This commit is contained in:
bors-servo 2018-07-21 14:39:25 -04:00 committed by GitHub
commit a97d8b99ef
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
22 changed files with 110 additions and 87 deletions

View file

@ -1068,6 +1068,10 @@ impl<Message, LTF, STF> Constellation<Message, LTF, STF>
FromScriptMsg::LoadComplete => {
self.handle_load_complete_msg(source_top_ctx_id, source_pipeline_id)
}
// Handle navigating to a fragment
FromScriptMsg::NavigatedToFragment(new_url, replacement_enabled) => {
self.handle_navigated_to_fragment(source_pipeline_id, new_url, replacement_enabled);
}
// Handle a forward or back request
FromScriptMsg::TraverseHistory(direction) => {
self.handle_traverse_history_msg(source_top_ctx_id, direction);
@ -1858,6 +1862,26 @@ impl<Message, LTF, STF> Constellation<Message, LTF, STF>
self.handle_subframe_loaded(pipeline_id);
}
fn handle_navigated_to_fragment(&mut self, pipeline_id: PipelineId, new_url: ServoUrl, replacement_enabled: bool) {
let (top_level_browsing_context_id, old_url) = match self.pipelines.get_mut(&pipeline_id) {
Some(pipeline) => {
let old_url = replace(&mut pipeline.url, new_url.clone());
(pipeline.top_level_browsing_context_id, old_url)
}
None => return warn!("Pipeline {} navigated to fragment after closure", pipeline_id),
};
if !replacement_enabled {
let diff = SessionHistoryDiff::HashDiff {
pipeline_reloader: NeedsToReload::No(pipeline_id),
new_url,
old_url,
};
self.get_joint_session_history(top_level_browsing_context_id).push_diff(diff);
self.notify_history_changed(top_level_browsing_context_id);
}
}
fn handle_traverse_history_msg(&mut self,
top_level_browsing_context_id: TopLevelBrowsingContextId,
direction: TraversalDirection)
@ -1879,7 +1903,7 @@ impl<Message, LTF, STF> Constellation<Message, LTF, STF>
match diff {
SessionHistoryDiff::BrowsingContextDiff { browsing_context_id, ref new_reloader, .. } => {
browsing_context_changes.insert(browsing_context_id, new_reloader.clone());
}
},
SessionHistoryDiff::PipelineDiff {
ref pipeline_reloader,
new_history_state_id,
@ -1888,10 +1912,23 @@ impl<Message, LTF, STF> Constellation<Message, LTF, STF>
} => match *pipeline_reloader {
NeedsToReload::No(pipeline_id) => {
pipeline_changes.insert(pipeline_id, (Some(new_history_state_id), new_url.clone()));
}
},
NeedsToReload::Yes(pipeline_id, ..) => {
url_to_load.insert(pipeline_id, new_url.clone());
}
},
},
SessionHistoryDiff::HashDiff {
ref pipeline_reloader,
ref new_url,
..
} => match *pipeline_reloader {
NeedsToReload::No(pipeline_id) => {
let state = pipeline_changes.get(&pipeline_id).and_then(|change| change.0);
pipeline_changes.insert(pipeline_id, (state, new_url.clone()));
},
NeedsToReload::Yes(pipeline_id, ..) => {
url_to_load.insert(pipeline_id, new_url.clone());
},
},
}
session_history.past.push(diff);
@ -1917,10 +1954,23 @@ impl<Message, LTF, STF> Constellation<Message, LTF, STF>
} => match *pipeline_reloader {
NeedsToReload::No(pipeline_id) => {
pipeline_changes.insert(pipeline_id, (old_history_state_id, old_url.clone()));
}
},
NeedsToReload::Yes(pipeline_id, ..) => {
url_to_load.insert(pipeline_id, old_url.clone());
}
},
},
SessionHistoryDiff::HashDiff {
ref pipeline_reloader,
ref old_url,
..
} => match *pipeline_reloader {
NeedsToReload::No(pipeline_id) => {
let state = pipeline_changes.get(&pipeline_id).and_then(|change| change.0);
pipeline_changes.insert(pipeline_id, (state, old_url.clone()));
},
NeedsToReload::Yes(pipeline_id, ..) => {
url_to_load.insert(pipeline_id, old_url.clone());
},
},
}
session_history.future.push(diff);
@ -2059,7 +2109,6 @@ impl<Message, LTF, STF> Constellation<Message, LTF, STF>
None => return warn!("Push history state {} for closed pipeline {}", history_state_id, pipeline_id),
};
let session_history = self.get_joint_session_history(top_level_browsing_context_id);
let diff = SessionHistoryDiff::PipelineDiff {
pipeline_reloader: NeedsToReload::No(pipeline_id),
new_history_state_id: history_state_id,
@ -2067,7 +2116,8 @@ impl<Message, LTF, STF> Constellation<Message, LTF, STF>
old_history_state_id: old_state_id,
old_url: old_url,
};
session_history.push_diff(diff);
self.get_joint_session_history(top_level_browsing_context_id).push_diff(diff);
self.notify_history_changed(top_level_browsing_context_id);
}
fn handle_replace_history_state_msg(
@ -2372,7 +2422,7 @@ impl<Message, LTF, STF> Constellation<Message, LTF, STF>
Some(previous_load_data.clone())
}
},
SessionHistoryDiff::PipelineDiff { .. } => Some(previous_load_data.clone()),
_ => Some(previous_load_data.clone()),
}
};
@ -2393,7 +2443,7 @@ impl<Message, LTF, STF> Constellation<Message, LTF, STF>
Some(previous_load_data.clone())
}
},
SessionHistoryDiff::PipelineDiff { .. } => Some(previous_load_data.clone()),
_ => Some(previous_load_data.clone()),
}
};
@ -2496,13 +2546,14 @@ impl<Message, LTF, STF> Constellation<Message, LTF, STF>
if let Some(pipeline_id) = new_reloader.alive_pipeline_id() {
pipelines_to_close.push(pipeline_id);
}
}
},
SessionHistoryDiff::PipelineDiff { pipeline_reloader, new_history_state_id, .. } => {
if let Some(pipeline_id) = pipeline_reloader.alive_pipeline_id() {
let states = states_to_close.entry(pipeline_id).or_insert(Vec::new());
states.push(new_history_state_id);
}
}
},
_ => {},
}
}

View file

@ -70,7 +70,7 @@ impl JointSessionHistory {
SessionHistoryDiff::BrowsingContextDiff { browsing_context_id, .. } => {
*browsing_context_id != context_id
},
SessionHistoryDiff::PipelineDiff { .. } => true,
_ => true,
}
});
self.future.retain(|diff| {
@ -78,7 +78,7 @@ impl JointSessionHistory {
SessionHistoryDiff::BrowsingContextDiff { browsing_context_id, .. } => {
*browsing_context_id != context_id
},
SessionHistoryDiff::PipelineDiff { .. } => true,
_ => true,
}
});
}
@ -174,6 +174,11 @@ pub enum SessionHistoryDiff {
/// The new url
new_url: ServoUrl,
},
HashDiff {
pipeline_reloader: NeedsToReload,
old_url: ServoUrl,
new_url: ServoUrl,
},
}
impl SessionHistoryDiff {
@ -186,7 +191,7 @@ impl SessionHistoryDiff {
NeedsToReload::Yes(..) => None,
}
},
SessionHistoryDiff::PipelineDiff { .. } => None,
_ => None,
}
}
@ -199,7 +204,7 @@ impl SessionHistoryDiff {
NeedsToReload::Yes(..) => None,
}
},
SessionHistoryDiff::PipelineDiff { .. } => None,
_ => None,
}
}
@ -213,12 +218,17 @@ impl SessionHistoryDiff {
if *new_reloader == *replaced_reloader {
*new_reloader = reloader.clone();
}
}
},
SessionHistoryDiff::PipelineDiff { ref mut pipeline_reloader, .. } => {
if *pipeline_reloader == *replaced_reloader {
*pipeline_reloader = reloader.clone();
}
}
},
SessionHistoryDiff::HashDiff { ref mut pipeline_reloader, .. } => {
if *pipeline_reloader == *replaced_reloader {
*pipeline_reloader = reloader.clone();
}
},
}
}
}

View file

@ -85,7 +85,10 @@ impl History {
// Step 6
let hash_changed = old_url.fragment() != url.fragment();
// TODO: Step 8 - scroll restoration
// Step 8
if let Some(fragment) = url.fragment() {
document.check_and_scroll_fragment(fragment);
}
// Step 11
let state_changed = state_id != self.state_id.get();

View file

@ -33,7 +33,10 @@ use dom::cssstyledeclaration::{CSSModificationAccess, CSSStyleDeclaration, CSSSt
use dom::customelementregistry::CustomElementRegistry;
use dom::document::{AnimationFrameCallback, Document};
use dom::element::Element;
use dom::event::Event;
use dom::eventtarget::EventTarget;
use dom::globalscope::GlobalScope;
use dom::hashchangeevent::HashChangeEvent;
use dom::history::History;
use dom::location::Location;
use dom::mediaquerylist::{MediaQueryList, WeakMediaQueryListVec};
@ -1582,13 +1585,33 @@ impl Window {
referrer_policy: Option<ReferrerPolicy>) {
let doc = self.Document();
let referrer_policy = referrer_policy.or(doc.get_referrer_policy());
// https://html.spec.whatwg.org/multipage/#navigating-across-documents
if !force_reload && url.as_url()[..Position::AfterQuery] ==
doc.url().as_url()[..Position::AfterQuery] {
// Step 6
if let Some(fragment) = url.fragment() {
self.send_to_constellation(ScriptMsg::NavigatedToFragment(url.clone(), replace));
doc.check_and_scroll_fragment(fragment);
let this = Trusted::new(self);
let old_url = doc.url().into_string();
let new_url = url.clone().into_string();
let task = task!(hashchange_event: move || {
let this = this.root();
let event = HashChangeEvent::new(
&this,
atom!("hashchange"),
false,
false,
old_url,
new_url);
event.upcast::<Event>().fire(this.upcast::<EventTarget>());
});
// FIXME(nox): Why are errors silenced here?
let _ = self.script_chan.send(CommonScriptMsg::Task(
ScriptThreadEventCategory::DomEvent,
Box::new(self.task_canceller(TaskSourceName::DOMManipulation).wrap_task(task)),
self.pipeline_id()
));
doc.set_url(url.clone());
return
}

View file

@ -116,6 +116,8 @@ pub enum ScriptMsg {
AbortLoadUrl,
/// Post a message to the currently active window of a given browsing context.
PostMessage(BrowsingContextId, Option<ImmutableOrigin>, Vec<u8>),
/// Inform the constellation that a fragment was navigated to and whether or not it was a replacement navigation.
NavigatedToFragment(ServoUrl, bool),
/// HTMLIFrameElement Forward or Back traversal.
TraverseHistory(TraversalDirection),
/// Inform the constellation of a pushed history state.
@ -184,6 +186,7 @@ impl fmt::Debug for ScriptMsg {
LoadUrl(..) => "LoadUrl",
AbortLoadUrl => "AbortLoadUrl",
PostMessage(..) => "PostMessage",
NavigatedToFragment(..) => "NavigatedToFragment",
TraverseHistory(..) => "TraverseHistory",
PushHistoryState(..) => "PushHistoryState",
ReplaceHistoryState(..) => "ReplaceHistoryState",

View file

@ -4,8 +4,5 @@
expected: FAIL
[Instant scrolling while doing history navigation.]
expected: TIMEOUT
[Smooth scrolling while doing history navigation.]
expected: NOTRUN
expected: FAIL

View file

@ -34,5 +34,3 @@
[windows-1254: iso_8859-9:1989 (XMLHttpRequest)]
expected: FAIL
[single-byte-decoder.html?TextDecoder]

View file

@ -15,8 +15,6 @@
expected: FAIL
[request-keepalive-quota.html?include=slow-1]
[request-keepalive-quota.html?include=slow-2]
[A Keep-Alive fetch() should return only its allocated Quota upon promise resolution.]
expected: FAIL

View file

@ -1,6 +0,0 @@
[hashchange_event.html]
type: testharness
expected: TIMEOUT
[Queue a task to fire hashchange event]
expected: TIMEOUT

View file

@ -1,6 +0,0 @@
[004.html]
type: testharness
expected: TIMEOUT
[Fragment Navigation: hashchange event]
expected: TIMEOUT

View file

@ -1,6 +0,0 @@
[005.html]
type: testharness
expected: TIMEOUT
[Fragment Navigation: hashchange event]
expected: TIMEOUT

View file

@ -1,6 +0,0 @@
[006.html]
type: testharness
expected: TIMEOUT
[Fragment Navigation: hashchange event multiple changes old/newURL]
expected: TIMEOUT

View file

@ -1,6 +0,0 @@
[007.html]
type: testharness
expected: TIMEOUT
[Fragment Navigation: hashchange event multiple changes old/newURL]
expected: TIMEOUT

View file

@ -1,10 +0,0 @@
[004.html]
[browser needs to support hashchange events for this testcase]
expected: FAIL
[queued .go commands should all be executed when the queue is processed]
expected: FAIL
[history position should be calculated when executing, not when calling the .go command]
expected: FAIL

View file

@ -98,8 +98,6 @@
expected: TIMEOUT
[parsing.html?81-90]
[parsing.html?91-100]
expected: TIMEOUT
[<meta>: "0; url=foo"]
@ -247,8 +245,6 @@
expected: TIMEOUT
[parsing.html?131-last]
[parsing.html?1-10]
expected: TIMEOUT
[<meta>: "1"]

View file

@ -4,5 +4,3 @@
[WebSockets: Send/Receive blob, blob size less than network array buffer]
expected: TIMEOUT
[001.html]

View file

@ -3,5 +3,3 @@
[WebSockets: Send/Receive blob, blob size greater than network array buffer]
expected: TIMEOUT
[002.html]

View file

@ -3,5 +3,3 @@
[WebSockets: Send/Receive ArrayBuffer, size greater than network array buffer]
expected: TIMEOUT
[004.html]

View file

@ -4,5 +4,3 @@
[WebSockets: Send/Receive ArrayBuffer, size less than network array buffer]
expected: TIMEOUT
[005.html]

View file

@ -4,5 +4,3 @@
[WebSockets: bufferedAmount for ArrayBuffer]
expected: TIMEOUT
[bufferedAmount-arraybuffer.html]

View file

@ -4,5 +4,3 @@
[WebSockets: bufferedAmount for blob]
expected: TIMEOUT
[bufferedAmount-blob.html]

View file

@ -5,7 +5,3 @@
[sync-no-timeout]
expected: FAIL
[sync-no-timeout.any.worker.html]
[sync-no-timeout.any.html]