Documentation and cleanup.

components/script/dom/bindings/utils.rs

@@ -45,8 +45,6 @@ use js::JSFUN_CONSTRUCTOR;
 use js;
 
 /// Proxy handler for a WindowProxy.
-#[allow(raw_pointer_derive)]
-#[derive(Copy)]
 pub struct WindowProxyHandler(pub *const libc::c_void);
 
 #[allow(raw_pointer_derive)]

components/script/dom/cssstyledeclaration.rs

@@ -221,9 +221,8 @@ impl<'a> CSSStyleDeclarationMethods for JSRef<'a, CSSStyleDeclaration> {
 
         let owner = self.owner.root();
         let window = window_from_node(owner.r()).root();
-        let window = window.r();
         let decl_block = parse_style_attribute(synthesized_declaration.as_slice(),
-                                               &window.get_url());
+                                               &window.r().get_url());
 
         // Step 7
         if decl_block.normal.len() == 0 {
@@ -269,9 +268,8 @@ impl<'a> CSSStyleDeclarationMethods for JSRef<'a, CSSStyleDeclaration> {
 
         let owner = self.owner.root();
         let window = window_from_node(owner.r()).root();
-        let window = window.r();
         let decl_block = parse_style_attribute(property.as_slice(),
-                                               &window.get_url());
+                                               &window.r().get_url());
         let element: JSRef<Element> = ElementCast::from_ref(owner.r());
 
         // Step 5

components/script/dom/document.rs

@@ -386,7 +386,7 @@ impl<'a> DocumentHelpers<'a> for JSRef<'a, Document> {
             Some(root) => root,
             None => return None,
         };
-        let root: JSRef<Node> = NodeCast::from_ref(root);
+        let root = NodeCast::from_ref(root);
         let win = self.window.root();
         let address = match win.r().layout().hit_test(root.to_trusted_node_address(), *point) {
             Ok(HitTestResponse(node_address)) => {

components/script/dom/htmliframeelement.rs

@@ -172,8 +172,8 @@ impl<'a> HTMLIFrameElementMethods for JSRef<'a, HTMLIFrameElement> {
             let window = window_from_node(self).root();
             let window = window.r();
             let children = window.page().children.borrow();
-            children.iter().find(|child| {
+            children.iter().find(|page| {
-                let window = child.window().root();
+                let window = page.window().root();
                 window.r().subpage() == Some(subpage_id)
             }).map(|page| page.window())
         })

components/script/dom/window.rs

@@ -56,7 +56,7 @@ use rustc_serialize::base64::{FromBase64, ToBase64, STANDARD};
 use std::cell::{Cell, Ref, RefMut};
 use std::default::Default;
 use std::ffi::CString;
-use std::mem::replace;
+use std::mem;
 use std::num::Float;
 use std::rc::Rc;
 use std::sync::mpsc::{channel, Receiver};
@@ -543,7 +543,7 @@ impl<'a> WindowHelpers for JSRef<'a, Window> {
     /// layout task has finished any pending request messages.
     fn join_layout(self) {
         let mut layout_join_port = self.layout_join_port.borrow_mut();
-        if let Some(join_port) = replace(&mut *layout_join_port, None) {
+        if let Some(join_port) = mem::replace(&mut *layout_join_port, None) {
             match join_port.try_recv() {
                 Err(Empty) => {
                     info!("script: waiting on layout");
@@ -652,7 +652,7 @@ impl<'a> WindowHelpers for JSRef<'a, Window> {
     }
 
     fn windowproxy_handler(self) -> WindowProxyHandler {
-        self.dom_static.windowproxy_handler
+        WindowProxyHandler(self.dom_static.windowproxy_handler.0)
     }
 
     fn get_next_subpage_id(self) -> SubpageId {

components/script/page.rs

@@ -10,10 +10,11 @@ use dom::window::Window;
 
 use msg::constellation_msg::{PipelineId, SubpageId};
 use util::smallvec::SmallVec;
+use std::cell::Cell;
 use std::rc::Rc;
 use url::Url;
 
-/// Encapsulates a handle to a frame and its associated layout information.
+/// Encapsulates a handle to a frame in a frame tree.
 #[jstraceable]
 pub struct Page {
     /// Pipeline id associated with this page.
@@ -27,9 +28,11 @@ pub struct Page {
 
     /// Cached copy of the most recent url loaded by the script, after all redirections.
     /// TODO(tkuehn): this currently does not follow any particular caching policy
-    /// and simply caches pages forever (!). The bool indicates if reflow is required
+    /// and simply caches pages forever (!).
-    /// when reloading.
+    url: Url,
-    url: DOMRefCell<(Url, bool)>,
+
+    /// Indicates if reflow is required when reloading.
+    needs_reflow: Cell<bool>,
 
     // Child Pages.
     pub children: DOMRefCell<Vec<Rc<Page>>>,
@@ -67,7 +70,8 @@ impl Page {
             id: id,
             subpage_id: subpage_id,
             frame: DOMRefCell::new(None),
-            url: DOMRefCell::new((url, true)),
+            url: url,
+            needs_reflow: Cell::new(true),
             children: DOMRefCell::new(vec!()),
         }
     }
@@ -123,8 +127,8 @@ impl Iterator for PageIterator {
 
 impl Page {
     pub fn set_reflow_status(&self, status: bool) -> bool {
-        let old = (*self.url.borrow()).1;
+        let old = self.needs_reflow.get();
-        (*self.url.borrow_mut()).1 = status;
+        self.needs_reflow.set(status);
         old
     }
 

components/script/script_task.rs

@@ -3,7 +3,19 @@
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
 //! The script task is the task that owns the DOM in memory, runs JavaScript, and spawns parsing
-//! and layout tasks.
+//! and layout tasks. It's in charge of processing events for all same-origin pages in a frame
+//! tree, and manages the entire lifetime of pages in the frame tree from initial request to
+//! teardown.
+//!
+//! Page loads follow a two-step process. When a request for a new page load is received, the
+//! network request is initiated and the relevant data pertaining to the new page is stashed.
+//! While the non-blocking request is ongoing, the script task is free to process further events,
+//! noting when they pertain to ongoing loads (such as resizes/viewport adjustments). When the
+//! initial response is received for an ongoing load, the second phase starts - the frame tree
+//! entry is created, along with the Window and Document objects, and the appropriate parser
+//! takes over the response body. Once parsing is complete, the document lifecycle for loading
+//! a page runs its course and the script task returns to processing events in the main event
+//! loop.
 
 #![allow(unsafe_blocks)]
 
@@ -83,16 +95,27 @@ use time::Tm;
 
 thread_local!(pub static STACK_ROOTS: Cell<Option<RootCollectionPtr>> = Cell::new(None));
 
+/// A document load that is in the process of fetching the requested resource. Contains
+/// data that will need to be present when the document and frame tree entry are created,
+/// but is only easily available at initiation of the load and on a push basis (so some
+/// data will be updated according to future resize events, viewport changes, etc.)
 struct InProgressLoad {
+    /// The pipeline which requested this load.
     pipeline_id: PipelineId,
+    /// The parent pipeline and child subpage associated with this load, if any.
     subpage_id: Option<(PipelineId, SubpageId)>,
+    /// The current window size associated with this pipeline.
     window_size: WindowSizeData,
+    /// Channel to the layout task associated with this pipeline.
     layout_chan: LayoutChan,
+    /// The current viewport clipping rectangle applying to this pipelie, if any.
     clip_rect: Option<Rect<f32>>,
+    /// The requested URL of the load.
     url: Url,
 }
 
 impl InProgressLoad {
+    /// Create a new InProgressLoad object.
     fn new(id: PipelineId,
            subpage_id: Option<(PipelineId, SubpageId)>,
            layout_chan: LayoutChan,
@@ -445,6 +468,7 @@ impl ScriptTask {
         (js_runtime, js_context)
     }
 
+    // Return the root page in the frame tree. Panics if it doesn't exist.
     fn root_page(&self) -> Rc<Page> {
         self.page.borrow().as_ref().unwrap().clone()
     }
@@ -472,7 +496,7 @@ impl ScriptTask {
 
         {
             let page = self.page.borrow();
-            if let Some(ref page) = page.as_ref() {
+            if let Some(page) = page.as_ref() {
                 for page in page.iter() {
                     // Only process a resize if layout is idle.
                     let window = page.window().root();
@@ -686,6 +710,7 @@ impl ScriptTask {
         panic!("Page rect message sent to nonexistent pipeline");
      }
 
+    /// Handle a request to load a page in a new child frame of an existing page.
     fn handle_new_layout(&self, new_layout_info: NewLayoutInfo) {
         let NewLayoutInfo {
             old_pipeline_id,
@@ -703,6 +728,7 @@ impl ScriptTask {
         let parent_window = parent_page.window().root();
         let chan = layout_chan.downcast_ref::<Sender<layout_interface::Msg>>().unwrap();
         let layout_chan = LayoutChan(chan.clone());
+        // Kick off the fetch for the new resource.
         let new_load = InProgressLoad::new(new_pipeline_id, Some((old_pipeline_id, subpage_id)),
                                            layout_chan, parent_window.r().window_size(),
                                            load_data.url.clone());
@@ -736,6 +762,8 @@ impl ScriptTask {
         window.r().thaw();
     }
 
+    /// Handle a request to make a previously-created pipeline active.
+    //TODO: unsuspend JS and timers, etc. when we support such things.
     fn handle_activate(&self, pipeline_id: PipelineId) {
         // We should only get this message when moving in history, so all pages requested
         // should exist.
@@ -790,8 +818,11 @@ impl ScriptTask {
         self.compositor.borrow_mut().close();
     }
 
+    /// We have received notification that the response associated with a load has completed.
+    /// Kick off the document and frame tree creation process using the result.
     fn handle_page_fetch_complete(&self, id: PipelineId, subpage: Option<SubpageId>,
                                   response: LoadResponse) {
+        // Any notification received should refer to an existing, in-progress load that is tracked.
         let idx = self.incomplete_loads.borrow().iter().position(|&:load| {
             load.pipeline_id == id && load.subpage_id.map(|sub| sub.1) == subpage
         }).unwrap();
@@ -832,7 +863,9 @@ impl ScriptTask {
         let final_url = response.metadata.final_url.clone();
         debug!("ScriptTask: loading {} on page {:?}", incomplete.url.serialize(), incomplete.pipeline_id);
 
-        let root_page_exists = self.page.borrow().is_none();
+        // We should either be initializing a root page or loading a child page of an
+        // existing one.
+        let root_page_exists = self.page.borrow().is_some();
         assert!(incomplete.subpage_id.is_none() || root_page_exists);
 
         let frame_element = incomplete.subpage_id.and_then(|(parent_id, subpage_id)| {
@@ -858,18 +891,17 @@ impl ScriptTask {
 
         self.compositor.borrow_mut().set_ready_state(incomplete.pipeline_id, Loading);
 
-        let last_modified = response.metadata.headers.as_ref().and_then(|headers| {
-            headers.get().map(|&LastModified(ref tm)| tm.clone())
-        });
-
         let cx = self.js_context.borrow();
         let cx = cx.as_ref().unwrap();
 
+        // Create a new frame tree entry.
         let page = Rc::new(Page::new(incomplete.pipeline_id, incomplete.subpage_id.map(|p| p.1),
                                      final_url.clone()));
-        if root_page_exists {
+        if !root_page_exists {
+            // We have a new root frame tree.
             *self.page.borrow_mut() = Some(page.clone());
         } else if let Some((parent, _)) = incomplete.subpage_id {
+            // We have a new child frame.
             let parent_page = self.root_page();
             parent_page.find(parent).expect("received load for subpage with missing parent");
             parent_page.children.borrow_mut().push(page.clone());
@@ -894,11 +926,15 @@ impl ScriptTask {
         let document = Document::new(window.r(), Some(final_url.clone()),
                                      IsHTMLDocument::HTMLDocument, None,
                                      DocumentSource::FromParser).root();
+
+        window.r().init_browser_context(document.r(), frame_element.r());
+
+        let last_modified = response.metadata.headers.as_ref().and_then(|headers| {
+            headers.get().map(|&LastModified(ref tm)| tm.clone())
+        });
         if let Some(tm) = last_modified {
             document.r().set_last_modified(dom_last_modified(&tm));
         }
-        window.r().init_browser_context(document.r(), frame_element.r());
-
 
         // Create the root frame
         page.set_frame(Some(Frame {
@@ -1097,6 +1133,8 @@ impl ScriptTask {
         self.force_reflow(&*page);
     }
 
+    /// Initiate a non-blocking fetch for a specified resource. Stores the InProgressLoad
+    /// argument until a notification is received that the fetch is complete.
     fn start_page_load(&self, incomplete: InProgressLoad, mut load_data: LoadData) {
         let id = incomplete.pipeline_id.clone();
         let subpage = incomplete.subpage_id.clone().map(|p| p.1);