Skip to content

Commit

Permalink
Track hash changes in session history
Browse files Browse the repository at this point in the history
Notify history changed on pushState and scroll to frag
  • Loading branch information
cbrewster committed Jul 21, 2018
1 parent c1cc2aa commit 61442cc
Show file tree
Hide file tree
Showing 22 changed files with 110 additions and 87 deletions.
73 changes: 62 additions & 11 deletions components/constellation/constellation.rs
Expand Up @@ -1063,6 +1063,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);
Expand Down Expand Up @@ -1853,6 +1857,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)
Expand All @@ -1874,7 +1898,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,
Expand All @@ -1883,10 +1907,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);
Expand All @@ -1912,10 +1949,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);
Expand Down Expand Up @@ -2054,15 +2104,15 @@ 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,
new_url: url,
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(
Expand Down Expand Up @@ -2367,7 +2417,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()),
}
};

Expand All @@ -2388,7 +2438,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()),
}
};

Expand Down Expand Up @@ -2491,13 +2541,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);
}
}
},
_ => {},
}
}

Expand Down
22 changes: 16 additions & 6 deletions components/constellation/session_history.rs
Expand Up @@ -70,15 +70,15 @@ impl JointSessionHistory {
SessionHistoryDiff::BrowsingContextDiff { browsing_context_id, .. } => {
*browsing_context_id != context_id
},
SessionHistoryDiff::PipelineDiff { .. } => true,
_ => true,
}
});
self.future.retain(|diff| {
match diff {
SessionHistoryDiff::BrowsingContextDiff { browsing_context_id, .. } => {
*browsing_context_id != context_id
},
SessionHistoryDiff::PipelineDiff { .. } => true,
_ => true,
}
});
}
Expand Down Expand Up @@ -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 {
Expand All @@ -186,7 +191,7 @@ impl SessionHistoryDiff {
NeedsToReload::Yes(..) => None,
}
},
SessionHistoryDiff::PipelineDiff { .. } => None,
_ => None,
}
}

Expand All @@ -199,7 +204,7 @@ impl SessionHistoryDiff {
NeedsToReload::Yes(..) => None,
}
},
SessionHistoryDiff::PipelineDiff { .. } => None,
_ => None,
}
}

Expand All @@ -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();
}
},
}
}
}
5 changes: 4 additions & 1 deletion components/script/dom/history.rs
Expand Up @@ -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();
Expand Down
25 changes: 24 additions & 1 deletion components/script/dom/window.rs
Expand Up @@ -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};
Expand Down Expand Up @@ -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
}
Expand Down
3 changes: 3 additions & 0 deletions components/script_traits/script_msg.rs
Expand Up @@ -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.
Expand Down Expand Up @@ -184,6 +186,7 @@ impl fmt::Debug for ScriptMsg {
LoadUrl(..) => "LoadUrl",
AbortLoadUrl => "AbortLoadUrl",
PostMessage(..) => "PostMessage",
NavigatedToFragment(..) => "NavigatedToFragment",
TraverseHistory(..) => "TraverseHistory",
PushHistoryState(..) => "PushHistoryState",
ReplaceHistoryState(..) => "ReplaceHistoryState",
Expand Down
Expand Up @@ -4,8 +4,5 @@
expected: FAIL

[Instant scrolling while doing history navigation.]
expected: TIMEOUT

[Smooth scrolling while doing history navigation.]
expected: NOTRUN
expected: FAIL

2 changes: 0 additions & 2 deletions tests/wpt/metadata/encoding/single-byte-decoder.html.ini
Expand Up @@ -34,5 +34,3 @@
[windows-1254: iso_8859-9:1989 (XMLHttpRequest)]
expected: FAIL


[single-byte-decoder.html?TextDecoder]
Expand Up @@ -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
Expand Down

This file was deleted.

This file was deleted.

This file was deleted.

This file was deleted.

This file was deleted.

This file was deleted.

Expand Up @@ -98,8 +98,6 @@
expected: TIMEOUT


[parsing.html?81-90]

[parsing.html?91-100]
expected: TIMEOUT
[<meta>: "0; url=foo"]
Expand Down Expand Up @@ -247,8 +245,6 @@
expected: TIMEOUT


[parsing.html?131-last]

[parsing.html?1-10]
expected: TIMEOUT
[<meta>: "1"]
Expand Down
2 changes: 0 additions & 2 deletions tests/wpt/metadata/websockets/binary/001.html.ini
Expand Up @@ -4,5 +4,3 @@
[WebSockets: Send/Receive blob, blob size less than network array buffer]
expected: TIMEOUT


[001.html]
2 changes: 0 additions & 2 deletions tests/wpt/metadata/websockets/binary/002.html.ini
Expand Up @@ -3,5 +3,3 @@
[WebSockets: Send/Receive blob, blob size greater than network array buffer]
expected: TIMEOUT


[002.html]

0 comments on commit 61442cc

Please sign in to comment.