diff options
author | bors-servo <lbergstrom+bors@mozilla.com> | 2017-01-04 13:58:57 -0800 |
---|---|---|
committer | GitHub <noreply@github.com> | 2017-01-04 13:58:57 -0800 |
commit | 16b0da5004fd730de87883daa35a78b6af01f042 (patch) | |
tree | 3390d897e9d9cb6826f603cbac89006e82ddbc43 /components/script | |
parent | 2fe914e2fa68f44db903bc3de55d9823a44cdf0d (diff) | |
parent | c14c431d0a9ec448f5dc7e65afc4457ede519cce (diff) | |
download | servo-16b0da5004fd730de87883daa35a78b6af01f042.tar.gz servo-16b0da5004fd730de87883daa35a78b6af01f042.zip |
Auto merge of #14312 - asajeffrey:script-discard-documents, r=cbrewster
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 -->
Diffstat (limited to 'components/script')
-rw-r--r-- | components/script/dom/window.rs | 33 | ||||
-rw-r--r-- | components/script/script_thread.rs | 2 | ||||
-rw-r--r-- | components/script/timers.rs | 10 |
3 files changed, 31 insertions, 14 deletions
diff --git a/components/script/dom/window.rs b/components/script/dom/window.rs index 7dd8d71e9c8..37bf817ed7a 100644 --- a/components/script/dom/window.rs +++ b/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(); diff --git a/components/script/script_thread.rs b/components/script/script_thread.rs index 065d214a323..563e4c52529 100644 --- a/components/script/script_thread.rs +++ b/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(); diff --git a/components/script/timers.rs b/components/script/timers.rs index 14c4eb89c13..b86bdc17aae 100644 --- a/components/script/timers.rs +++ b/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); |