Auto merge of #14312 - asajeffrey:script-discard-documents, r=<try>

Implement discarding Document objects to reclaim space.

<!-- Please describe your changes on the following line: -->

This PR implements document discarding. Active documents are kept alive strongly, but inactive documents are only kept alive weakly. When a document is GCd, it is marked as discarded, and if it is every reactivated, a reload of the URL is triggered.

Note that this PR is pretty aggressive about discarding, and can any inactive document (other than those being kept alive by other same-origin pipelines). We might want to damp it down a bit.

Also note that this interacts with browser.html in that the reloading triggered by reactivating a document triggers mozbrowser events.

To test this, I added a `-Zdiscard-inactive-documents` debug flag, which discards all inactive documents, even ones which are reachable through other same-origin pipelines.

---
<!-- 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 #14262.
- [X] These changes do not require tests because we should be able to use the existing tests with `-Zdiscard-inactive-documents`.

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

components/config/opts.rs

@@ -68,6 +68,9 @@ pub struct Opts {
 
     pub output_file: Option<String>,
 
+    /// How much session history to keep in each tab.
+    pub max_session_history: usize,
+
     /// Replace unpaires surrogates in DOM strings with U+FFFD.
     /// See https://github.com/servo/servo/issues/6564
     pub replace_surrogates: bool,
@@ -518,6 +521,7 @@ pub fn default_opts() -> Opts {
         userscripts: None,
         user_stylesheets: Vec::new(),
         output_file: None,
+        max_session_history: 16,
         replace_surrogates: false,
         gc_profile: false,
         load_webfonts_synchronously: false,
@@ -611,6 +615,7 @@ pub fn from_cmdline_args(args: &[String]) -> ArgumentParsingResult {
                 "Probability of randomly closing a pipeline (for testing constellation hardening).",
                 "0.0");
     opts.optopt("", "random-pipeline-closure-seed", "A fixed seed for repeatbility of random pipeline closure.", "");
+    opts.optopt("", "max-session-history", "Maximum amount of session history to store in each tab.", "16");
     opts.optmulti("Z", "debug",
                   "A comma-separated string of debug options. Pass help to show available options.", "");
     opts.optflag("h", "help", "Print this message");
@@ -779,6 +784,10 @@ pub fn from_cmdline_args(args: &[String]) -> ArgumentParsingResult {
         }
     };
 
+    let max_session_history = opt_match.opt_str("max-session-history").map(|max| {
+        max.parse().unwrap_or_else(|err| args_fail(&format!("Error parsing option: --max-session-history ({})", err)))
+    }).unwrap_or(16);
+
     if opt_match.opt_present("M") {
         MULTIPROCESS.store(true, Ordering::SeqCst)
     }
@@ -820,6 +829,7 @@ pub fn from_cmdline_args(args: &[String]) -> ArgumentParsingResult {
         userscripts: opt_match.opt_default("userscripts", ""),
         user_stylesheets: user_stylesheets,
         output_file: opt_match.opt_str("o"),
+        max_session_history: max_session_history,
         replace_surrogates: debug_options.replace_surrogates,
         gc_profile: debug_options.gc_profile,
         load_webfonts_synchronously: debug_options.load_webfonts_synchronously,

components/constellation/frame.rs

@@ -4,6 +4,7 @@
 
 use msg::constellation_msg::{FrameId, PipelineId};
 use pipeline::Pipeline;
+use servo_url::ServoUrl;
 use std::collections::HashMap;
 use std::iter::once;
 use std::mem::replace;
@@ -22,43 +23,59 @@ pub struct Frame {
     /// The frame id.
     pub id: FrameId,
 
+    /// The timestamp for the current session history entry
+    pub instant: Instant,
+
+    /// The pipeline for the current session history entry
+    pub pipeline_id: PipelineId,
+
+    /// The URL for the current session history entry
+    pub url: ServoUrl,
+
     /// The past session history, ordered chronologically.
     pub prev: Vec<FrameState>,
 
-    /// The currently active session history entry.
-    pub current: FrameState,
-
     /// The future session history, ordered reverse chronologically.
     pub next: Vec<FrameState>,
 }
 
 impl Frame {
     /// Create a new frame.
     /// Note this just creates the frame, it doesn't add it to the frame tree.
-    pub fn new(id: FrameId, pipeline_id: PipelineId) -> Frame {
+    pub fn new(id: FrameId, pipeline_id: PipelineId, url: ServoUrl) -> Frame {
         Frame {
             id: id,
+            pipeline_id: pipeline_id,
+            instant: Instant::now(),
+            url: url,
             prev: vec!(),
-            current: FrameState::new(pipeline_id, id),
             next: vec!(),
         }
     }
 
+    /// Get the current frame state.
+    pub fn current(&self) -> FrameState {
+        FrameState {
+            instant: self.instant,
+            frame_id: self.id,
+            pipeline_id: Some(self.pipeline_id),
+            url: self.url.clone(),
+        }
+    }
+
     /// Set the current frame entry, and push the current frame entry into the past.
-    pub fn load(&mut self, pipeline_id: PipelineId) {
-        self.prev.push(self.current.clone());
-        self.current = FrameState::new(pipeline_id, self.id);
+    pub fn load(&mut self, pipeline_id: PipelineId, url: ServoUrl) {
+        let current = self.current();
+        self.prev.push(current);
+        self.instant = Instant::now();
+        self.pipeline_id = pipeline_id;
+        self.url = url;
     }
 
     /// Set the future to be empty.
     pub fn remove_forward_entries(&mut self) -> Vec<FrameState> {
         replace(&mut self.next, vec!())
     }
-
-    /// Set the current frame entry, and drop the current frame entry.
-    pub fn replace_current(&mut self, pipeline_id: PipelineId) -> FrameState {
-        replace(&mut self.current, FrameState::new(pipeline_id, self.id))
-    }
 }
 
 /// An entry in a frame's session history.
@@ -70,23 +87,18 @@ impl Frame {
 pub struct FrameState {
     /// The timestamp for when the session history entry was created
     pub instant: Instant,
-    /// The pipeline for the document in the session history
-    pub pipeline_id: PipelineId,
+
+    /// The pipeline for the document in the session history,
+    /// None if the entry has been discarded
+    pub pipeline_id: Option<PipelineId>,
+
+    /// The URL for this entry, used to reload the pipeline if it has been discarded
+    pub url: ServoUrl,
+
     /// The frame that this session history entry is part of
     pub frame_id: FrameId,
 }
 
-impl FrameState {
-    /// Create a new session history entry.
-    fn new(pipeline_id: PipelineId, frame_id: FrameId) -> FrameState {
-        FrameState {
-            instant: Instant::now(),
-            pipeline_id: pipeline_id,
-            frame_id: frame_id,
-        }
-    }
-}
-
 /// Represents a pending change in the frame tree, that will be applied
 /// once the new pipeline has loaded and completed initial layout / paint.
 pub struct FrameChange {
@@ -100,9 +112,12 @@ pub struct FrameChange {
     /// The pipeline for the document being loaded.
     pub new_pipeline_id: PipelineId,
 
+    /// The URL for the document being loaded.
+    pub url: ServoUrl,
+
     /// Is the new document replacing the current document (e.g. a reload)
     /// or pushing it into the session history (e.g. a navigation)?
-    pub replace: bool,
+    pub replace: Option<FrameState>,
 }
 
 /// An iterator over a frame tree, returning the fully active frames in
@@ -137,14 +152,14 @@ impl<'a> Iterator for FrameTreeIterator<'a> {
                     continue;
                 },
             };
-            let pipeline = match self.pipelines.get(&frame.current.pipeline_id) {
+            let pipeline = match self.pipelines.get(&frame.pipeline_id) {
                 Some(pipeline) => pipeline,
                 None => {
-                    warn!("Pipeline {:?} iterated after closure.", frame.current.pipeline_id);
+                    warn!("Pipeline {:?} iterated after closure.", frame.pipeline_id);
                     continue;
                 },
             };
-            self.stack.extend(pipeline.children.iter().map(|&c| c));
+            self.stack.extend(pipeline.children.iter());
             return Some(frame)
         }
     }
@@ -169,6 +184,7 @@ pub struct FullFrameTreeIterator<'a> {
 impl<'a> Iterator for FullFrameTreeIterator<'a> {
     type Item = &'a Frame;
     fn next(&mut self) -> Option<&'a Frame> {
+        let pipelines = self.pipelines;
         loop {
             let frame_id = match self.stack.pop() {
                 Some(frame_id) => frame_id,
@@ -181,11 +197,12 @@ impl<'a> Iterator for FullFrameTreeIterator<'a> {
                     continue;
                 },
             };
-            for entry in frame.prev.iter().chain(frame.next.iter()).chain(once(&frame.current)) {
-                if let Some(pipeline) = self.pipelines.get(&entry.pipeline_id) {
-                    self.stack.extend(pipeline.children.iter().map(|&c| c));
-                }
-            }
+            let child_frame_ids = frame.prev.iter().chain(frame.next.iter())
+                .filter_map(|entry| entry.pipeline_id)
+                .chain(once(frame.pipeline_id))
+                .filter_map(|pipeline_id| pipelines.get(&pipeline_id))
+                .flat_map(|pipeline| pipeline.children.iter());
+            self.stack.extend(child_frame_ids);
             return Some(frame)
         }
     }

components/constellation/lib.rs

@@ -3,6 +3,7 @@
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
 #![feature(box_syntax)]
+#![feature(conservative_impl_trait)]
 #![feature(mpsc_select)]
 #![feature(plugin)]
 #![feature(proc_macro)]

components/script/dom/window.rs

@@ -901,18 +901,24 @@ impl Window {
     }
 
     pub fn clear_js_runtime(&self) {
+        // We tear down the active document, which causes all the attached
+        // nodes to dispose of their layout data. This messages the layout
+        // thread, informing it that it can safely free the memory.
         self.Document().upcast::<Node>().teardown();
 
-        // The above code may not catch all DOM objects
-        // (e.g. DOM objects removed from the tree that haven't
-        // been collected yet). Forcing a GC here means that
-        // those DOM objects will be able to call dispose()
-        // to free their layout data before the layout thread
-        // exits. Without this, those remaining objects try to
-        // send a message to free their layout data to the
-        // layout thread when the script thread is dropped,
-        // which causes a panic!
+        // The above code may not catch all DOM objects (e.g. DOM
+        // objects removed from the tree that haven't been collected
+        // yet). There should not be any such DOM nodes with layout
+        // data, but if there are, then when they are dropped, they
+        // will attempt to send a message to the closed layout thread.
+        // This causes memory safety issues, because the DOM node uses
+        // the layout channel from its window, and the window has
+        // already been GC'd.  For nodes which do not have a live
+        // pointer, we can avoid this by GCing now:
         self.Gc();
+        // but there may still be nodes being kept alive by user
+        // script.
+        // TODO: ensure that this doesn't happen!
 
         self.current_state.set(WindowState::Zombie);
         *self.js_runtime.borrow_mut() = None;
@@ -1445,6 +1451,15 @@ impl Window {
         None
     }
 
+    pub fn freeze(&self) {
+        self.upcast::<GlobalScope>().suspend();
+        // A hint to the JS runtime that now would be a good time to
+        // GC any unreachable objects generated by user script,
+        // or unattached DOM nodes. Attached DOM nodes can't be GCd yet,
+        // as the document might be thawed later.
+        self.Gc();
+    }
+
     pub fn thaw(&self) {
         self.upcast::<GlobalScope>().resume();
 

components/script/script_thread.rs

@@ -1346,7 +1346,7 @@ impl ScriptThread {
     fn handle_freeze_msg(&self, id: PipelineId) {
         let window = self.documents.borrow().find_window(id);
         if let Some(window) = window {
-            window.upcast::<GlobalScope>().suspend();
+            window.freeze();
             return;
         }
         let mut loads = self.incomplete_loads.borrow_mut();

components/script/timers.rs

@@ -228,18 +228,20 @@ impl OneshotTimers {
     }
 
     pub fn suspend(&self) {
-        assert!(self.suspended_since.get().is_none());
+        // Suspend is idempotent: do nothing if the timers are already suspended.
+        if self.suspended_since.get().is_some() {
+            return warn!("Suspending an already suspended timer.");
+        }
 
         self.suspended_since.set(Some(precise_time_ms()));
         self.invalidate_expected_event_id();
     }
 
     pub fn resume(&self) {
-        assert!(self.suspended_since.get().is_some());
-
+        // Suspend is idempotent: do nothing if the timers are already suspended.
         let additional_offset = match self.suspended_since.get() {
             Some(suspended_since) => precise_time_ms() - suspended_since,
-            None => panic!("Timers are not suspended.")
+            None => return warn!("Resuming an already resumed timer."),
         };
 
         self.suspension_offset.set(self.suspension_offset.get() + additional_offset);

components/url/lib.rs

@@ -24,7 +24,7 @@ use std::path::Path;
 use std::sync::Arc;
 use url::{Url, Origin, Position};
 
-#[derive(Debug, Clone, PartialEq, Eq, Hash)]
+#[derive(Debug, Clone, PartialEq, Eq, PartialOrd, Ord, Hash)]
 #[cfg_attr(feature = "servo", derive(HeapSizeOf, Serialize, Deserialize))]
 pub struct ServoUrl(Arc<Url>);