servo: Merge #13580 - Removed the session history from BrowsingContext (from asajeffrey:script-browsingcontext-without-session-history); r=ConnorGBrewster.
authorAlan Jeffrey <ajeffrey@mozilla.com>
Wed, 05 Oct 2016 13:19:37 -0500
changeset 339841 3cce2d176a7fe234364c429b7d9918797a0792b6
parent 339840 87fc57c8457def5fa65210f7e957998e14dc5ad6
child 339842 c9f2b5e830951cae701ce57cfdeea65d5d03bdac
push id31307
push usergszorc@mozilla.com
push dateSat, 04 Feb 2017 00:59:06 +0000
treeherdermozilla-central@94079d43835f [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersConnorGBrewster
servo: Merge #13580 - Removed the session history from BrowsingContext (from asajeffrey:script-browsingcontext-without-session-history); r=ConnorGBrewster. <!-- Please describe your changes on the following line: --> The session history is stored in the constellation and accessed via the `History` object, so `BrowsingContext` doesn't need to track it. cc @jdm @ConnorGBrewster @Ms2ger --- <!-- 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 do not require tests because it's a refactoring. <!-- Pull requests that do not address these steps are welcome, but they will require additional verification as part of the review process. --> Source-Repo: https://github.com/servo/servo Source-Revision: 896be066dd34e4d5f5f2c6425c093f5fa091401b
servo/components/script/dom/browsingcontext.rs
servo/components/script/script_thread.rs
--- a/servo/components/script/dom/browsingcontext.rs
+++ b/servo/components/script/dom/browsingcontext.rs
@@ -1,19 +1,17 @@
 /* This Source Code Form is subject to the terms of the Mozilla Public
  * License, v. 2.0. If a copy of the MPL was not distributed with this
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
 use dom::bindings::cell::DOMRefCell;
-use dom::bindings::codegen::Bindings::DocumentBinding::DocumentMethods;
 use dom::bindings::conversions::{ToJSValConvertible, root_from_handleobject};
-use dom::bindings::js::{JS, Root, RootedReference};
+use dom::bindings::js::{JS, MutNullableHeap, Root, RootedReference};
 use dom::bindings::proxyhandler::{fill_property_descriptor, get_property_descriptor};
 use dom::bindings::reflector::{Reflectable, MutReflectable, Reflector};
-use dom::bindings::str::DOMString;
 use dom::bindings::trace::JSTraceable;
 use dom::bindings::utils::WindowProxyHandler;
 use dom::bindings::utils::get_array_index_from_id;
 use dom::document::Document;
 use dom::element::Element;
 use dom::window::Window;
 use js::JSCLASS_IS_GLOBAL;
 use js::glue::{CreateWrapperProxyHandler, ProxyTraps, NewWindowProxy};
@@ -23,49 +21,51 @@ use js::jsapi::{JSAutoCompartment, JSCon
 use js::jsapi::{JSPROP_READONLY, JSTracer, JS_DefinePropertyById};
 use js::jsapi::{JS_ForwardGetPropertyTo, JS_ForwardSetPropertyTo, JS_GetClass};
 use js::jsapi::{JS_GetOwnPropertyDescriptorById, JS_HasPropertyById};
 use js::jsapi::{MutableHandle, MutableHandleObject, MutableHandleValue};
 use js::jsapi::{ObjectOpResult, PropertyDescriptor};
 use js::jsval::{UndefinedValue, PrivateValue};
 use msg::constellation_msg::PipelineId;
 use std::cell::Cell;
-use url::Url;
 
 #[dom_struct]
+// NOTE: the browsing context for a window is managed in two places:
+// here, in script, but also in the constellation. The constellation
+// manages the session history, which in script is accessed through
+// History objects, messaging the constellation.
 pub struct BrowsingContext {
     reflector: Reflector,
 
     /// Pipeline id associated with this context.
     id: PipelineId,
 
     /// Indicates if reflow is required when reloading.
     needs_reflow: Cell<bool>,
 
-    /// Stores this context's session history
-    history: DOMRefCell<Vec<SessionHistoryEntry>>,
-
-    /// The index of the active session history entry
-    active_index: Cell<usize>,
-
     /// Stores the child browsing contexts (ex. iframe browsing context)
     children: DOMRefCell<Vec<JS<BrowsingContext>>>,
 
+    /// The current active document.
+    /// Note that the session history is stored in the constellation,
+    /// in the script thread we just track the current active document.
+    active_document: MutNullableHeap<JS<Document>>,
+
+    /// The containing iframe element, if this is a same-origin iframe
     frame_element: Option<JS<Element>>,
 }
 
 impl BrowsingContext {
     pub fn new_inherited(frame_element: Option<&Element>, id: PipelineId) -> BrowsingContext {
         BrowsingContext {
             reflector: Reflector::new(),
             id: id,
             needs_reflow: Cell::new(true),
-            history: DOMRefCell::new(vec![]),
-            active_index: Cell::new(0),
             children: DOMRefCell::new(vec![]),
+            active_document: Default::default(),
             frame_element: frame_element.map(JS::from_ref),
         }
     }
 
     #[allow(unsafe_code)]
     pub fn new(window: &Window, frame_element: Option<&Element>, id: PipelineId) -> Root<BrowsingContext> {
         unsafe {
             let WindowProxyHandler(handler) = window.windowproxy_handler();
@@ -85,39 +85,26 @@ impl BrowsingContext {
             SetProxyExtra(window_proxy.get(), 0, &PrivateValue(raw as *const _));
 
             (*raw).init_reflector(window_proxy.get());
 
             Root::from_ref(&*raw)
         }
     }
 
-    pub fn init(&self, document: &Document) {
-        assert!(self.history.borrow().is_empty());
-        assert_eq!(self.active_index.get(), 0);
-        self.history.borrow_mut().push(SessionHistoryEntry::new(document, document.url().clone(), document.Title()));
-    }
-
-    pub fn push_history(&self, document: &Document) {
-        let mut history = self.history.borrow_mut();
-        // Clear all session history entries after the active index
-        history.drain((self.active_index.get() + 1)..);
-        history.push(SessionHistoryEntry::new(document, document.url().clone(), document.Title()));
-        self.active_index.set(self.active_index.get() + 1);
-        assert_eq!(self.active_index.get(), history.len() - 1);
+    pub fn set_active_document(&self, document: &Document) {
+        self.active_document.set(Some(document))
     }
 
     pub fn active_document(&self) -> Root<Document> {
-        Root::from_ref(&self.history.borrow()[self.active_index.get()].document)
+        self.active_document.get().expect("No active document.")
     }
 
     pub fn maybe_active_document(&self) -> Option<Root<Document>> {
-        self.history.borrow().get(self.active_index.get()).map(|entry| {
-            Root::from_ref(&*entry.document)
-        })
+        self.active_document.get()
     }
 
     pub fn active_window(&self) -> Root<Window> {
         Root::from_ref(self.active_document().window())
     }
 
     pub fn frame_element(&self) -> Option<&Element> {
         self.frame_element.r()
@@ -162,19 +149,18 @@ impl BrowsingContext {
 
     pub fn find_child_by_id(&self, pipeline_id: PipelineId) -> Option<Root<Window>> {
         self.children.borrow().iter().find(|context| {
             let window = context.active_window();
             window.pipeline_id() == pipeline_id
         }).map(|context| context.active_window())
     }
 
-    pub fn clear_session_history(&self) {
-        self.active_index.set(0);
-        self.history.borrow_mut().clear();
+    pub fn unset_active_document(&self) {
+        self.active_document.set(None)
     }
 
     pub fn iter(&self) -> ContextIterator {
         ContextIterator {
             stack: vec!(Root::from_ref(self)),
         }
     }
 
@@ -203,37 +189,16 @@ impl Iterator for ContextIterator {
             self.stack.extend(context.children.borrow()
                                               .iter()
                                               .map(|c| Root::from_ref(&**c)));
         }
         popped
     }
 }
 
-// This isn't a DOM struct, just a convenience struct
-// without a reflector, so we don't mark this as #[dom_struct]
-#[must_root]
-#[privatize]
-#[derive(JSTraceable, HeapSizeOf)]
-pub struct SessionHistoryEntry {
-    document: JS<Document>,
-    url: Url,
-    title: DOMString,
-}
-
-impl SessionHistoryEntry {
-    fn new(document: &Document, url: Url, title: DOMString) -> SessionHistoryEntry {
-        SessionHistoryEntry {
-            document: JS::from_ref(document),
-            url: url,
-            title: title,
-        }
-    }
-}
-
 #[allow(unsafe_code)]
 unsafe fn GetSubframeWindow(cx: *mut JSContext,
                             proxy: HandleObject,
                             id: HandleId)
                             -> Option<Root<Window>> {
     let index = get_array_index_from_id(cx, id);
     if let Some(index) = index {
         rooted!(in(cx) let target = GetProxyPrivate(*proxy.ptr).to_object());
--- a/servo/components/script/script_thread.rs
+++ b/servo/components/script/script_thread.rs
@@ -1650,18 +1650,16 @@ impl ScriptThread {
                             self.script_thread.root_browsing_context().remove(id).unwrap();
                         },
                         ContextToRemove::None => {},
                     }
                 }
             }
         }
 
-        let mut using_new_context = true;
-
         let (browsing_context, context_to_remove) = if !self.root_browsing_context_exists() {
             // Create a new context tree entry. This will become the root context.
             let new_context = BrowsingContext::new(&window, frame_element, incomplete.pipeline_id);
             // We have a new root frame tree.
             self.browsing_context.set(Some(&new_context));
             (new_context, ContextToRemove::Root)
         } else if let Some((parent, _)) = incomplete.parent_info {
             // Create a new context tree entry. This will be a child context.
@@ -1670,17 +1668,16 @@ impl ScriptThread {
             let root_context = self.root_browsing_context();
             // TODO(gw): This find will fail when we are sharing script threads
             // between cross origin iframes in the same TLD.
             let parent_context = root_context.find(parent)
                                              .expect("received load for child context with missing parent");
             parent_context.push_child_context(&*new_context);
             (new_context, ContextToRemove::Child(incomplete.pipeline_id))
         } else {
-            using_new_context = false;
             (self.root_browsing_context(), ContextToRemove::None)
         };
 
         window.init_browsing_context(&browsing_context);
         let mut context_remover = AutoContextRemover::new(self, context_to_remove);
 
         let last_modified = metadata.headers.as_ref().and_then(|headers| {
             headers.get().map(|&LastModified(HttpDate(ref tm))| dom_last_modified(tm))
@@ -1736,21 +1733,17 @@ impl ScriptThread {
                                      Some(final_url.clone()),
                                      is_html_document,
                                      content_type,
                                      last_modified,
                                      DocumentSource::FromParser,
                                      loader,
                                      referrer,
                                      referrer_policy);
-        if using_new_context {
-            browsing_context.init(&document);
-        } else {
-            browsing_context.push_history(&document);
-        }
+        browsing_context.set_active_document(&document);
         document.set_ready_state(DocumentReadyState::Loading);
 
         self.constellation_chan
             .send(ConstellationMsg::ActivateDocument(incomplete.pipeline_id))
             .unwrap();
 
         // Notify devtools that a new script global exists.
         self.notify_devtools(document.Title(), final_url.clone(), (browsing_context.pipeline_id(), None));
@@ -2233,17 +2226,17 @@ fn shut_down_layout(context_tree: &Brows
     }
 
     // Drop our references to the JSContext and DOM objects.
     for context in context_tree.iter() {
         let window = context.active_window();
         window.clear_js_runtime();
 
         // Sever the connection between the global and the DOM tree
-        context.clear_session_history();
+        context.unset_active_document();
     }
 
     // Destroy the layout thread. If there were node leaks, layout will now crash safely.
     for chan in channels {
         chan.send(message::Msg::ExitNow).ok();
     }
 }