Auto merge of #13167 - ConnorGBrewster:reload_replace_current, r=asajeffrey

Replace current session entry when reloading

<!-- Please describe your changes on the following line: -->
This PR adds a replacement option when navigating. It replaces the current session history entry after a new page has been loaded. This will prevent reloading from adding a new entry to 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 #13123 (github issue number if applicable).

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

<!-- 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/13167)
<!-- Reviewable:end -->

10 files changed, 89 insertions, 57 deletions

diff --git a/components/constellation/constellation.rs b/components/constellation/constellation.rs
index e501312e2d8..081f9c1eb4e 100644
--- a/components/constellation/constellation.rs
+++ b/components/constellation/constellation.rs
@@ -259,6 +259,10 @@ impl Frame {
     fn remove_forward_entries(&mut self) -> Vec<FrameState> {
         replace(&mut self.next, vec!())
     }
+
+    fn replace_current(&mut self, pipeline_id: PipelineId) -> FrameState {
+        replace(&mut self.current, FrameState::new(pipeline_id, self.id))
+    }
 }
 
 /// Represents a pending change in the frame tree, that will be applied
@@ -267,6 +271,7 @@ struct FrameChange {
     old_pipeline_id: Option<PipelineId>,
     new_pipeline_id: PipelineId,
     document_ready: bool,
+    replace: bool,
 }
 
 /// An iterator over a frame tree, returning nodes in depth-first order.
@@ -609,11 +614,13 @@ impl<Message, LTF, STF> Constellation<Message, LTF, STF>
 
     // Push a new (loading) pipeline to the list of pending frame changes
     fn push_pending_frame(&mut self, new_pipeline_id: PipelineId,
-                          old_pipeline_id: Option<PipelineId>) {
+                          old_pipeline_id: Option<PipelineId>,
+                          replace: bool) {
         self.pending_frames.push(FrameChange {
             old_pipeline_id: old_pipeline_id,
             new_pipeline_id: new_pipeline_id,
             document_ready: false,
+            replace: replace,
         });
     }
 
@@ -773,7 +780,7 @@ impl<Message, LTF, STF> Constellation<Message, LTF, STF>
             // However, if the id is not encompassed by another change, it will be.
             FromCompositorMsg::LoadUrl(source_id, load_data) => {
                 debug!("constellation got URL load message from compositor");
-                self.handle_load_url_msg(source_id, load_data);
+                self.handle_load_url_msg(source_id, load_data, false);
             }
             FromCompositorMsg::IsReadyToSaveImage(pipeline_states) => {
                 let is_ready = self.handle_is_ready_to_save_image(pipeline_states);
@@ -835,9 +842,9 @@ impl<Message, LTF, STF> Constellation<Message, LTF, STF>
             // Load a new page from a mouse click
             // If there is already a pending page (self.pending_frames), it will not be overridden;
             // However, if the id is not encompassed by another change, it will be.
-            FromScriptMsg::LoadUrl(source_id, load_data) => {
+            FromScriptMsg::LoadUrl(source_id, load_data, replace) => {
                 debug!("constellation got URL load message from script");
-                self.handle_load_url_msg(source_id, load_data);
+                self.handle_load_url_msg(source_id, load_data, replace);
             }
             // A page loaded has completed all parsing, script, and reflow messages have been sent.
             FromScriptMsg::LoadComplete(pipeline_id) => {
@@ -1151,8 +1158,7 @@ impl<Message, LTF, STF> Constellation<Message, LTF, STF>
             let load_data = LoadData::new(failure_url, None, None);
             self.new_pipeline(new_pipeline_id, parent_info, Some(pipeline_id), window_size, None, load_data, false);
 
-            self.push_pending_frame(new_pipeline_id, Some(pipeline_id));
-
+            self.push_pending_frame(new_pipeline_id, Some(pipeline_id), false);
         }
     }
 
@@ -1177,7 +1183,7 @@ impl<Message, LTF, STF> Constellation<Message, LTF, STF>
         self.new_pipeline(root_pipeline_id, None, None, Some(window_size), None,
                           LoadData::new(url.clone(), None, None), false);
         self.handle_load_start_msg(root_pipeline_id);
-        self.push_pending_frame(root_pipeline_id, None);
+        self.push_pending_frame(root_pipeline_id, None, false);
         self.compositor_proxy.send(ToCompositorMsg::ChangePageUrl(root_pipeline_id, url));
     }
 
@@ -1299,7 +1305,7 @@ impl<Message, LTF, STF> Constellation<Message, LTF, STF>
                           load_data,
                           is_private);
 
-        self.push_pending_frame(load_info.new_pipeline_id, load_info.old_pipeline_id);
+        self.push_pending_frame(load_info.new_pipeline_id, load_info.old_pipeline_id, load_info.replace);
     }
 
     fn handle_set_cursor_msg(&mut self, cursor: Cursor) {
@@ -1376,22 +1382,24 @@ impl<Message, LTF, STF> Constellation<Message, LTF, STF>
         }
     }
 
-    fn handle_load_url_msg(&mut self, source_id: PipelineId, load_data: LoadData) {
-        self.load_url(source_id, load_data);
+    fn handle_load_url_msg(&mut self, source_id: PipelineId, load_data: LoadData, replace: bool) {
+        self.load_url(source_id, load_data, replace);
     }
 
-    fn load_url(&mut self, source_id: PipelineId, load_data: LoadData) -> Option<PipelineId> {
+    fn load_url(&mut self, source_id: PipelineId, load_data: LoadData, replace: bool) -> Option<PipelineId> {
         // 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
         // requested change so it can update its internal state.
+        //
+        // If replace is true, the current entry is replaced instead of a new entry being added.
         let parent_info = self.pipelines.get(&source_id).and_then(|source| source.parent_info);
         match parent_info {
             Some((parent_pipeline_id, _)) => {
                 self.handle_load_start_msg(source_id);
                 // Message the constellation to find the script thread for this iframe
                 // and issue an iframe load through there.
-                let msg = ConstellationControlMsg::Navigate(parent_pipeline_id, source_id, load_data);
+                let msg = ConstellationControlMsg::Navigate(parent_pipeline_id, source_id, load_data, replace);
                 let result = match self.pipelines.get(&parent_pipeline_id) {
                     Some(parent_pipeline) => parent_pipeline.script_chan.send(msg),
                     None => {
@@ -1429,7 +1437,7 @@ impl<Message, LTF, STF> Constellation<Message, LTF, STF>
                 let window_size = self.pipelines.get(&source_id).and_then(|source| source.size);
                 let new_pipeline_id = PipelineId::new();
                 self.new_pipeline(new_pipeline_id, None, None, window_size, None, load_data, false);
-                self.push_pending_frame(new_pipeline_id, Some(source_id));
+                self.push_pending_frame(new_pipeline_id, Some(source_id), replace);
 
                 // Send message to ScriptThread that will suspend all timers
                 match self.pipelines.get(&source_id) {
@@ -1744,14 +1752,14 @@ impl<Message, LTF, STF> Constellation<Message, LTF, STF>
                 self.compositor_proxy.send(ToCompositorMsg::ResizeTo(size));
             },
             WebDriverCommandMsg::LoadUrl(pipeline_id, load_data, reply) => {
-                self.load_url_for_webdriver(pipeline_id, load_data, reply);
+                self.load_url_for_webdriver(pipeline_id, load_data, reply, false);
             },
             WebDriverCommandMsg::Refresh(pipeline_id, reply) => {
                 let load_data = match self.pipelines.get(&pipeline_id) {
                     Some(pipeline) => LoadData::new(pipeline.url.clone(), None, None),
                     None => return warn!("Pipeline {:?} Refresh after closure.", pipeline_id),
                 };
-                self.load_url_for_webdriver(pipeline_id, load_data, reply);
+                self.load_url_for_webdriver(pipeline_id, load_data, reply, true);
             }
             WebDriverCommandMsg::ScriptCommand(pipeline_id, cmd) => {
                 let control_msg = ConstellationControlMsg::WebDriverScriptCommand(pipeline_id, cmd);
@@ -1897,8 +1905,9 @@ impl<Message, LTF, STF> Constellation<Message, LTF, STF>
     fn load_url_for_webdriver(&mut self,
                               pipeline_id: PipelineId,
                               load_data: LoadData,
-                              reply: IpcSender<webdriver_msg::LoadStatus>) {
-        let new_pipeline_id = self.load_url(pipeline_id, load_data);
+                              reply: IpcSender<webdriver_msg::LoadStatus>,
+                              replace: bool) {
+        let new_pipeline_id = self.load_url(pipeline_id, load_data, replace);
         if let Some(id) = new_pipeline_id {
             self.webdriver.load_channel = Some((id, reply));
         }
@@ -1931,8 +1940,17 @@ impl<Message, LTF, STF> Constellation<Message, LTF, STF>
                     pipeline.frame = Some(frame_id);
                 }
 
-                if let Some(ref mut frame) = self.frames.get_mut(&frame_id) {
-                    frame.load(frame_change.new_pipeline_id);
+                if frame_change.replace {
+                    let evicted = self.frames.get_mut(&frame_id).map(|frame| {
+                        frame.replace_current(frame_change.new_pipeline_id)
+                    });
+                    if let Some(evicted) = evicted {
+                        self.close_pipeline(evicted.pipeline_id, ExitPipelineMode::Normal);
+                    }
+                } else {
+                    if let Some(ref mut frame) = self.frames.get_mut(&frame_id) {
+                        frame.load(frame_change.new_pipeline_id);
+                    }
                 }
             }
             None => {
@@ -1956,19 +1974,20 @@ impl<Message, LTF, STF> Constellation<Message, LTF, STF>
             }
         }
 
-        // Build frame tree and send permission
-        self.send_frame_tree_and_grant_paint_permission();
-
-        // If this is an iframe, send a mozbrowser location change event.
-        // This is the result of a link being clicked and a navigation completing.
-        self.trigger_mozbrowserlocationchange(frame_change.new_pipeline_id);
+        if !frame_change.replace {
+            // If this is an iframe, send a mozbrowser location change event.
+            // This is the result of a link being clicked and a navigation completing.
+            self.trigger_mozbrowserlocationchange(frame_change.new_pipeline_id);
 
-        let frame_id = match self.get_top_level_frame_for_pipeline(Some(frame_change.new_pipeline_id)) {
-            Some(frame_id) => frame_id,
-            None => return warn!("Tried to remove forward history after root frame closure."),
-        };
+            let frame_id = match self.get_top_level_frame_for_pipeline(Some(frame_change.new_pipeline_id)) {
+                Some(frame_id) => frame_id,
+                None => return warn!("Tried to remove forward history after root frame closure."),
+            };
+            self.clear_joint_session_future(frame_id);
+        }
 
-        self.clear_joint_session_future(frame_id);
+        // Build frame tree and send permission
+        self.send_frame_tree_and_grant_paint_permission();
     }
 
     fn handle_activate_document_msg(&mut self, pipeline_id: PipelineId) {
diff --git a/components/script/dom/htmlanchorelement.rs b/components/script/dom/htmlanchorelement.rs
index 08e7e01b77a..bd77c0a1eba 100644
--- a/components/script/dom/htmlanchorelement.rs
+++ b/components/script/dom/htmlanchorelement.rs
@@ -588,5 +588,5 @@ fn follow_hyperlink(subject: &Element, hyperlink_suffix: Option<String>) {
 
     debug!("following hyperlink to {}", url);
     let window = document.window();
-    window.load_url(url);
+    window.load_url(url, false);
 }
diff --git a/components/script/dom/htmlformelement.rs b/components/script/dom/htmlformelement.rs
index d6a794621e5..268b04a159f 100644
--- a/components/script/dom/htmlformelement.rs
+++ b/components/script/dom/htmlformelement.rs
@@ -902,7 +902,7 @@ impl Runnable for PlannedNavigation {
     fn handler(self: Box<PlannedNavigation>) {
         if self.generation_id == self.form.root().generation_id.get() {
             let script_chan = self.script_chan.clone();
-            script_chan.send(MainThreadScriptMsg::Navigate(self.pipeline_id, self.load_data)).unwrap();
+            script_chan.send(MainThreadScriptMsg::Navigate(self.pipeline_id, self.load_data, false)).unwrap();
         }
     }
 }
diff --git a/components/script/dom/htmliframeelement.rs b/components/script/dom/htmliframeelement.rs
index 3b74d3e6e05..1503ff28e06 100644
--- a/components/script/dom/htmliframeelement.rs
+++ b/components/script/dom/htmliframeelement.rs
@@ -100,7 +100,7 @@ impl HTMLIFrameElement {
         (old_pipeline_id, new_pipeline_id)
     }
 
-    pub fn navigate_or_reload_child_browsing_context(&self, load_data: Option<LoadData>) {
+    pub fn navigate_or_reload_child_browsing_context(&self, load_data: Option<LoadData>, replace: bool) {
         let sandboxed = if self.is_sandboxed() {
             IFrameSandboxed
         } else {
@@ -134,6 +134,7 @@ impl HTMLIFrameElement {
             sandbox: sandboxed,
             is_private: private_iframe,
             frame_type: frame_type,
+            replace: replace,
         };
         window.constellation_chan()
               .send(ConstellationMsg::ScriptLoadedURLInIFrame(load_info))
@@ -150,7 +151,7 @@ impl HTMLIFrameElement {
 
         let document = document_from_node(self);
         self.navigate_or_reload_child_browsing_context(
-            Some(LoadData::new(url, document.get_referrer_policy(), Some(document.url().clone()))));
+            Some(LoadData::new(url, document.get_referrer_policy(), Some(document.url().clone()))), false);
     }
 
     #[allow(unsafe_code)]
@@ -491,7 +492,7 @@ impl HTMLIFrameElementMethods for HTMLIFrameElement {
     fn Reload(&self, _hard_reload: bool) -> ErrorResult {
         if self.Mozbrowser() {
             if self.upcast::<Node>().is_in_doc() {
-                self.navigate_or_reload_child_browsing_context(None);
+                self.navigate_or_reload_child_browsing_context(None, true);
             }
             Ok(())
         } else {
diff --git a/components/script/dom/location.rs b/components/script/dom/location.rs
index 72361193aa3..41783c877bc 100644
--- a/components/script/dom/location.rs
+++ b/components/script/dom/location.rs
@@ -40,7 +40,7 @@ impl Location {
                          setter: fn(&mut Url, USVString)) {
         let mut url = self.window.get_url();
         setter(&mut url, value);
-        self.window.load_url(url);
+        self.window.load_url(url, false);
     }
 }
 
@@ -51,13 +51,13 @@ impl LocationMethods for Location {
         //       _entry settings object_.
         let base_url = self.window.get_url();
         if let Ok(url) = base_url.join(&url.0) {
-            self.window.load_url(url);
+            self.window.load_url(url, false);
         }
     }
 
     // https://html.spec.whatwg.org/multipage/#dom-location-reload
     fn Reload(&self) {
-        self.window.load_url(self.get_url());
+        self.window.load_url(self.get_url(), true);
     }
 
     // https://html.spec.whatwg.org/multipage/#dom-location-hash
@@ -106,7 +106,7 @@ impl LocationMethods for Location {
     // https://html.spec.whatwg.org/multipage/#dom-location-href
     fn SetHref(&self, value: USVString) {
         if let Ok(url) = self.window.get_url().join(&value.0) {
-            self.window.load_url(url);
+            self.window.load_url(url, false);
         }
     }
 
diff --git a/components/script/dom/window.rs b/components/script/dom/window.rs
index f4efcd8dd76..069dd3fc2d6 100644
--- a/components/script/dom/window.rs
+++ b/components/script/dom/window.rs
@@ -1410,11 +1410,12 @@ impl Window {
     }
 
     /// Commence a new URL load which will either replace this window or scroll to a fragment.
-    pub fn load_url(&self, url: Url) {
+    pub fn load_url(&self, url: Url, replace: bool) {
         let doc = self.Document();
         self.main_thread_script_chan().send(
             MainThreadScriptMsg::Navigate(self.id,
-                LoadData::new(url, doc.get_referrer_policy(), Some(doc.url().clone())))).unwrap();
+                LoadData::new(url, doc.get_referrer_policy(), Some(doc.url().clone())),
+                replace)).unwrap();
     }
 
     pub fn handle_fire_timer(&self, timer_id: TimerEventId) {
diff --git a/components/script/script_thread.rs b/components/script/script_thread.rs
index 4a19547138c..298323929c1 100644
--- a/components/script/script_thread.rs
+++ b/components/script/script_thread.rs
@@ -228,8 +228,9 @@ pub enum MainThreadScriptMsg {
     /// should be closed (only dispatched to ScriptThread).
     ExitWindow(PipelineId),
     /// Begins a content-initiated load on the specified pipeline (only
-    /// dispatched to ScriptThread).
-    Navigate(PipelineId, LoadData),
+    /// dispatched to ScriptThread). Allows for a replace bool to be passed. If true,
+    /// the current entry will be replaced instead of a new entry being added.
+    Navigate(PipelineId, LoadData, bool),
     /// Tasks that originate from the DOM manipulation task source
     DOMManipulation(DOMManipulationTask),
     /// Tasks that originate from the user interaction task source
@@ -877,8 +878,8 @@ impl ScriptThread {
 
     fn handle_msg_from_constellation(&self, msg: ConstellationControlMsg) {
         match msg {
-            ConstellationControlMsg::Navigate(parent_pipeline_id, pipeline_id, load_data) =>
-                self.handle_navigate(parent_pipeline_id, Some(pipeline_id), load_data),
+            ConstellationControlMsg::Navigate(parent_pipeline_id, pipeline_id, load_data, replace) =>
+                self.handle_navigate(parent_pipeline_id, Some(pipeline_id), load_data, replace),
             ConstellationControlMsg::SendEvent(id, event) =>
                 self.handle_event(id, event),
             ConstellationControlMsg::ResizeInactive(id, new_size) =>
@@ -933,8 +934,8 @@ impl ScriptThread {
 
     fn handle_msg_from_script(&self, msg: MainThreadScriptMsg) {
         match msg {
-            MainThreadScriptMsg::Navigate(parent_pipeline_id, load_data) =>
-                self.handle_navigate(parent_pipeline_id, None, load_data),
+            MainThreadScriptMsg::Navigate(parent_pipeline_id, load_data, replace) =>
+                self.handle_navigate(parent_pipeline_id, None, load_data, replace),
             MainThreadScriptMsg::ExitWindow(id) =>
                 self.handle_exit_window_msg(id),
             MainThreadScriptMsg::DocumentLoadsComplete(id) =>
@@ -2000,7 +2001,10 @@ impl ScriptThread {
     /// https://html.spec.whatwg.org/multipage/#navigating-across-documents
     /// The entry point for content to notify that a new load has been requested
     /// for the given pipeline (specifically the "navigate" algorithm).
-    fn handle_navigate(&self, parent_pipeline_id: PipelineId, pipeline_id: Option<PipelineId>, load_data: LoadData) {
+    fn handle_navigate(&self, parent_pipeline_id: PipelineId,
+                              pipeline_id: Option<PipelineId>,
+                              load_data: LoadData,
+                              replace: bool) {
         // Step 7.
         {
             let nurl = &load_data.url;
@@ -2026,12 +2030,12 @@ impl ScriptThread {
                     doc.find_iframe(pipeline_id)
                 });
                 if let Some(iframe) = iframe.r() {
-                    iframe.navigate_or_reload_child_browsing_context(Some(load_data));
+                    iframe.navigate_or_reload_child_browsing_context(Some(load_data), replace);
                 }
             }
             None => {
                 self.constellation_chan
-                    .send(ConstellationMsg::LoadUrl(parent_pipeline_id, load_data))
+                    .send(ConstellationMsg::LoadUrl(parent_pipeline_id, load_data, replace))
                     .unwrap();
             }
         }
diff --git a/components/script_traits/lib.rs b/components/script_traits/lib.rs
index 19075c63a7d..c28942688e5 100644
--- a/components/script_traits/lib.rs
+++ b/components/script_traits/lib.rs
@@ -179,7 +179,7 @@ pub enum ConstellationControlMsg {
     NotifyVisibilityChange(PipelineId, PipelineId, bool),
     /// Notifies script thread that a url should be loaded in this iframe.
     /// First PipelineId is for the parent, second PipelineId is for the actual pipeline.
-    Navigate(PipelineId, PipelineId, LoadData),
+    Navigate(PipelineId, PipelineId, LoadData, bool),
     /// Requests the script thread forward a mozbrowser event to an iframe it owns,
     /// or to the window if no child pipeline id is provided.
     /// First PipelineId is for the parent, second PipelineId is for the actual pipeline.
@@ -459,6 +459,9 @@ pub struct IFrameLoadInfo {
     pub is_private: bool,
     /// Whether this iframe is a mozbrowser iframe
     pub frame_type: FrameType,
+    /// Wether this load should replace the current entry (reload). If true, the current
+    /// entry will be replaced instead of a new entry being added.
+    pub replace: bool,
 }
 
 // https://developer.mozilla.org/en-US/docs/Web/API/Using_the_Browser_API#Events
diff --git a/components/script_traits/script_msg.rs b/components/script_traits/script_msg.rs
index aaa22732937..c3c777bb9f7 100644
--- a/components/script_traits/script_msg.rs
+++ b/components/script_traits/script_msg.rs
@@ -83,8 +83,9 @@ pub enum ScriptMsg {
     /// All pending loads are complete, and the `load` event for this pipeline
     /// has been dispatched.
     LoadComplete(PipelineId),
-    /// A new load has been requested.
-    LoadUrl(PipelineId, LoadData),
+    /// A new load has been requested, with an option to replace the current entry once loaded
+    /// instead of adding a new entry.
+    LoadUrl(PipelineId, LoadData, bool),
     /// Dispatch a mozbrowser event to a given iframe,
     /// or to the window if no subpage id is provided.
     /// First PipelineId is for the parent, second PipelineId is for the actual pipeline.
diff --git a/tests/wpt/web-platform-tests/html/browsers/history/the-location-interface/location_reload.html b/tests/wpt/web-platform-tests/html/browsers/history/the-location-interface/location_reload.html
index 2972abf41a0..78b3cc35eb4 100644
--- a/tests/wpt/web-platform-tests/html/browsers/history/the-location-interface/location_reload.html
+++ b/tests/wpt/web-platform-tests/html/browsers/history/the-location-interface/location_reload.html
@@ -11,14 +11,19 @@
     <iframe></iframe>
 
     <script>
-
+    var history_length;
     async_test(function(t) {
 
       var url = new URL("./location_reload-iframe.html", window.location).href;
       var pingCount = 0;
 
       window._ping = t.step_func(function(innerURL) {
+          // Some browsers keep 'about:blank' in the session history
+          if (pingCount == 0) {
+            history_length = history.length;
+          }
           assert_equals(url, innerURL, "iframe url (" + pingCount + ")");
+          assert_equals(history_length, history.length, "history length (" + pingCount + ")");
           pingCount++;
           if (pingCount == 5) {
             iframe.src = 'about:blank';
@@ -28,10 +33,8 @@
 
       var iframe = document.querySelector("iframe");
       iframe.src = url;
-
+      history_length = history.length;
     });
-
-
     </script>
 
   </body>