Bug fixes, mainly about document loading.

components/compositing/compositor.rs

@@ -739,6 +739,7 @@ impl<Window: WindowMethods> IOCompositor<Window> {
     fn set_frame_tree(&mut self,
                       frame_tree: &SendableFrameTree,
                       response_chan: IpcSender<()>) {
+        debug!("Setting the frame tree for pipeline {}", frame_tree.pipeline.id);
         if let Err(e) = response_chan.send(()) {
             warn!("Sending reponse to set frame tree failed ({}).", e);
         }

components/constellation/constellation.rs

@@ -610,6 +610,10 @@ impl<Message, LTF, STF> Constellation<Message, LTF, STF>
 
         assert!(!self.pipelines.contains_key(&pipeline_id));
         self.pipelines.insert(pipeline_id, pipeline);
+
+        if !self.frames.contains_key(&frame_id) {
+            self.new_frame(frame_id, pipeline_id);
+        }
     }
 
     // Get an iterator for the current frame tree. Specify self.root_frame_id to
@@ -668,13 +672,16 @@ impl<Message, LTF, STF> Constellation<Message, LTF, STF>
     // Create a new frame and update the internal bookkeeping.
     fn new_frame(&mut self, frame_id: FrameId, pipeline_id: PipelineId) {
         let frame = Frame::new(frame_id, pipeline_id);
-
-        match self.pipelines.get_mut(&pipeline_id) {
-            Some(pipeline) => pipeline.is_mature = true,
-            None => return warn!("Pipeline {} matured after closure.", pipeline_id),
-        };
-
         self.frames.insert(frame_id, frame);
+
+        // If a child frame, add it to the parent pipeline.
+        let parent_info = self.pipelines.get(&pipeline_id)
+            .and_then(|pipeline| pipeline.parent_info);
+        if let Some((parent_id, _)) = parent_info {
+            if let Some(parent) = self.pipelines.get_mut(&parent_id) {
+                parent.add_child(frame_id);
+            }
+        }
     }
 
     /// Handles loading pages, navigation, and granting access to the compositor
@@ -777,6 +784,7 @@ impl<Message, LTF, STF> Constellation<Message, LTF, STF>
             }
             FromCompositorMsg::IsReadyToSaveImage(pipeline_states) => {
                 let is_ready = self.handle_is_ready_to_save_image(pipeline_states);
+                debug!("Ready to save image {:?}.", is_ready);
                 if opts::get().is_running_problem_test {
                     println!("got ready to save image query, result is {:?}", is_ready);
                 }
@@ -1022,8 +1030,31 @@ impl<Message, LTF, STF> Constellation<Message, LTF, STF>
         self.shutting_down = true;
 
         // TODO: exit before the root frame is initialized?
+        debug!("Removing root frame.");
         let root_frame_id = self.root_frame_id;
         self.close_frame(root_frame_id, ExitPipelineMode::Normal);
+
+        // Close any pending frames and pipelines
+        while let Some(pending) = self.pending_frames.pop() {
+            debug!("Removing pending frame {}.", pending.frame_id);
+            self.close_frame(pending.frame_id, ExitPipelineMode::Normal);
+            debug!("Removing pending pipeline {}.", pending.new_pipeline_id);
+            self.close_pipeline(pending.new_pipeline_id, ExitPipelineMode::Normal);
+        }
+
+        // In case there are frames which weren't attached to the frame tree, we close them.
+        let frame_ids: Vec<FrameId> = self.frames.keys().cloned().collect();
+        for frame_id in frame_ids {
+            debug!("Removing detached frame {}.", frame_id);
+            self.close_frame(frame_id, ExitPipelineMode::Normal);
+        }
+
+        // In case there are pipelines which weren't attached to the pipeline tree, we close them.
+        let pipeline_ids: Vec<PipelineId> = self.pipelines.keys().cloned().collect();
+        for pipeline_id in pipeline_ids {
+            debug!("Removing detached pipeline {}.", pipeline_id);
+            self.close_pipeline(pipeline_id, ExitPipelineMode::Normal);
+        }
     }
 
     fn handle_shutdown(&mut self) {
@@ -1219,6 +1250,7 @@ impl<Message, LTF, STF> Constellation<Message, LTF, STF>
         let msg = ConstellationControlMsg::DispatchFrameLoadEvent {
             target: frame_id,
             parent: subframe_parent_id,
+            child: pipeline_id,
         };
         let result = match self.pipelines.get(&subframe_parent_id) {
             Some(pipeline) => pipeline.script_chan.send(msg),
@@ -1592,13 +1624,10 @@ impl<Message, LTF, STF> Constellation<Message, LTF, STF>
 
     fn handle_mozbrowser_event_msg(&mut self,
                                    parent_pipeline_id: PipelineId,
-                                   pipeline_id: Option<PipelineId>,
+                                   pipeline_id: PipelineId,
                                    event: MozBrowserEvent) {
         assert!(PREFS.is_mozbrowser_enabled());
-
+        let frame_id = self.pipelines.get(&pipeline_id).map(|pipeline| pipeline.frame_id);
-        let frame_id = pipeline_id
-            .and_then(|pipeline_id| self.pipelines.get(&pipeline_id))
-            .map(|pipeline| pipeline.frame_id);
 
         // Find the script channel for the given parent pipeline,
         // and pass the event to that script thread.
@@ -1922,11 +1951,6 @@ impl<Message, LTF, STF> Constellation<Message, LTF, STF>
         }
 
         if self.frames.contains_key(&frame_change.frame_id) {
-            // Mature the new pipeline, and return frames evicted from history.
-            if let Some(ref mut pipeline) = self.pipelines.get_mut(&frame_change.new_pipeline_id) {
-                pipeline.is_mature = true;
-            }
-
             if frame_change.replace {
                 let evicted = self.frames.get_mut(&frame_change.frame_id).map(|frame| {
                     frame.replace_current(frame_change.new_pipeline_id)
@@ -1942,16 +1966,6 @@ impl<Message, LTF, STF> Constellation<Message, LTF, STF>
         } else {
             // The new pipeline is in a new frame with no history
             self.new_frame(frame_change.frame_id, frame_change.new_pipeline_id);
-
-            // If a child frame, add it to the parent pipeline. Otherwise
-            // it must surely be the root frame being created!
-            let parent_info = self.pipelines.get(&frame_change.new_pipeline_id)
-                .and_then(|pipeline| pipeline.parent_info);
-            if let Some((parent_id, _)) = parent_info {
-                if let Some(parent) = self.pipelines.get_mut(&parent_id) {
-                    parent.add_child(frame_change.frame_id);
-                }
-            }
         }
 
         if !frame_change.replace {
@@ -1970,49 +1984,25 @@ impl<Message, LTF, STF> Constellation<Message, LTF, STF>
     fn handle_activate_document_msg(&mut self, pipeline_id: PipelineId) {
         debug!("Document ready to activate {:?}", pipeline_id);
 
-        if let Some(ref child_pipeline) = self.pipelines.get(&pipeline_id) {
+        // Notify the parent (if there is one).
-            if let Some(ref parent_info) = child_pipeline.parent_info {
+        if let Some(pipeline) = self.pipelines.get(&pipeline_id) {
-                if let Some(parent_pipeline) = self.pipelines.get(&parent_info.0) {
+            if let Some((parent_pipeline_id, _)) = pipeline.parent_info {
-                    let _ = parent_pipeline.script_chan
+                if let Some(parent_pipeline) = self.pipelines.get(&parent_pipeline_id) {
-                                           .send(ConstellationControlMsg::FramedContentChanged(
+                    let msg = ConstellationControlMsg::FramedContentChanged(parent_pipeline_id, pipeline.frame_id);
-                                               parent_info.0,
+                    let _ = parent_pipeline.script_chan.send(msg);
-                                               child_pipeline.frame_id));
                 }
             }
         }
 
-        // If this pipeline is already part of the current frame tree,
-        // we don't need to do anything.
-        if self.pipeline_is_in_current_frame(pipeline_id) {
-            return;
-        }
-
         // Find the pending frame change whose new pipeline id is pipeline_id.
-        // If it is found, mark this pending frame as ready to be enabled.
         let pending_index = self.pending_frames.iter().rposition(|frame_change| {
             frame_change.new_pipeline_id == pipeline_id
         });
-        if let Some(pending_index) = pending_index {
-            self.pending_frames[pending_index].document_ready = true;
-        }
 
-        // This is a bit complex. We need to loop through pending frames and find
+        // If it is found, remove it from the pending frames, and make it
-        // ones that can be swapped. A frame can be swapped (enabled) once it is
+        // the active document of its frame.
-        // ready to layout (has document_ready set), and also is mature
+        if let Some(pending_index) = pending_index {
-        // (i.e. the pipeline it is replacing has been enabled and now has a frame).
+            let frame_change = self.pending_frames.swap_remove(pending_index);
-        // The outer loop is required because any time a pipeline is enabled, that
-        // may affect whether other pending frames are now able to be enabled. On the
-        // other hand, if no frames can be enabled after looping through all pending
-        // frames, we can safely exit the loop, knowing that we will need to wait on
-        // a dependent pipeline to be ready to paint.
-        while let Some(valid_frame_change) = self.pending_frames.iter().rposition(|frame_change| {
-            let frame_is_mature = frame_change.old_pipeline_id
-                .and_then(|old_pipeline_id| self.pipelines.get(&old_pipeline_id))
-                .map(|old_pipeline| old_pipeline.is_mature)
-                .unwrap_or(true);
-            frame_change.document_ready && frame_is_mature
-        }) {
-            let frame_change = self.pending_frames.swap_remove(valid_frame_change);
             self.add_or_replace_pipeline_in_frame_tree(frame_change);
         }
     }
@@ -2112,6 +2102,7 @@ impl<Message, LTF, STF> Constellation<Message, LTF, STF>
         // screenshot can safely be written.
         for frame in self.current_frame_tree_iter(self.root_frame_id) {
             let pipeline_id = frame.current.pipeline_id;
+            debug!("Checking readiness of frame {}, pipeline {}.", frame.id, pipeline_id);
 
             let pipeline = match self.pipelines.get(&pipeline_id) {
                 None => {
@@ -2277,7 +2268,9 @@ impl<Message, LTF, STF> Constellation<Message, LTF, STF>
             self.close_frame(*child_frame, exit_mode);
         }
 
-        let pipeline = match self.pipelines.get_mut(&pipeline_id) {
+        // Note, we don't remove the pipeline now, we wait for the message to come back from
+        // the pipeline.
+        let pipeline = match self.pipelines.get(&pipeline_id) {
             Some(pipeline) => pipeline,
             None => return warn!("Closing pipeline {:?} twice.", pipeline_id),
         };
@@ -2349,6 +2342,7 @@ impl<Message, LTF, STF> Constellation<Message, LTF, STF>
         // Note that this function can panic, due to ipc-channel creation failure.
         // avoiding this panic would require a mechanism for dealing
         // with low-resource scenarios.
+        debug!("Sending frame tree for frame {}.", self.root_frame_id);
         if let Some(frame_tree) = self.frame_to_sendable(self.root_frame_id) {
             let (chan, port) = ipc::channel().expect("Failed to create IPC channel!");
             self.compositor_proxy.send(ToCompositorMsg::SetFrameTree(frame_tree,

components/constellation/pipeline.rs

@@ -69,9 +69,6 @@ pub struct Pipeline {
     /// Whether this pipeline should be treated as visible for the purposes of scheduling and
     /// resource management.
     pub visible: bool,
-    /// Whether this pipeline is has matured or not.
-    /// A pipeline is considered mature when it has an associated frame.
-    pub is_mature: bool,
 }
 
 /// Initial setup data needed to construct a pipeline.
@@ -283,7 +280,6 @@ impl Pipeline {
             running_animations: false,
             visible: visible,
             is_private: is_private,
-            is_mature: false,
         }
     }
 

components/net/http_loader.rs

@@ -784,7 +784,7 @@ pub fn obtain_response<A>(request_factory: &HttpRequestFactory<R=A>,
         }
 
         if log_enabled!(log::LogLevel::Info) {
-            info!("{}", method);
+            info!("{} {}", method, connection_url);
             for header in headers.iter() {
                 info!(" - {}", header);
             }

components/script/document_loader.rs

@@ -94,6 +94,7 @@ impl DocumentLoader {
 
     pub fn new_with_threads(resource_threads: ResourceThreads,
                             initial_load: Option<Url>) -> DocumentLoader {
+        debug!("Initial blocking load {:?}.", initial_load);
         let initial_loads = initial_load.into_iter().map(LoadType::PageSource).collect();
 
         DocumentLoader {
@@ -105,6 +106,7 @@ impl DocumentLoader {
 
     /// Add a load to the list of blocking loads.
     fn add_blocking_load(&mut self, load: LoadType) {
+        debug!("Adding blocking load {:?} ({}).", load, self.blocking_loads.len());
         self.blocking_loads.push(load);
     }
 
@@ -119,6 +121,7 @@ impl DocumentLoader {
 
     /// Mark an in-progress network request complete.
     pub fn finish_load(&mut self, load: &LoadType) {
+        debug!("Removing blocking load {:?} ({}).", load, self.blocking_loads.len());
         let idx = self.blocking_loads.iter().position(|unfinished| *unfinished == *load);
         self.blocking_loads.remove(idx.expect(&format!("unknown completed load {:?}", load)));
     }

components/script/dom/document.rs

@@ -1399,7 +1399,7 @@ impl Document {
             if let Some((parent_pipeline_id, _)) = self.window.parent_info() {
                 let global_scope = self.window.upcast::<GlobalScope>();
                 let event = ConstellationMsg::MozBrowserEvent(parent_pipeline_id,
-                                                              Some(global_scope.pipeline_id()),
+                                                              global_scope.pipeline_id(),
                                                               event);
                 global_scope.constellation_chan().send(event).unwrap();
             }
@@ -1512,8 +1512,10 @@ impl Document {
             }
         }
 
-        let loader = self.loader.borrow();
+        if !self.loader.borrow().is_blocked() && !self.loader.borrow().events_inhibited() {
-        if !loader.is_blocked() && !loader.events_inhibited() {
+            // Schedule a task to fire a "load" event (if no blocking loads have arrived in the mean time)
+            // NOTE: we can end up executing this code more than once, in case more blocking loads arrive.
+            debug!("Document loads are complete.");
             let win = self.window();
             let msg = MainThreadScriptMsg::DocumentLoadsComplete(
                 win.upcast::<GlobalScope>().pipeline_id());

components/script/dom/htmliframeelement.rs

@@ -231,7 +231,11 @@ impl HTMLIFrameElement {
     }
 
     /// https://html.spec.whatwg.org/multipage/#iframe-load-event-steps steps 1-4
-    pub fn iframe_load_event_steps(&self) {
+    pub fn iframe_load_event_steps(&self, loaded_pipeline: PipelineId) {
+        // TODO(#9592): assert that the load blocker is present at all times when we
+        //              can guarantee that it's created for the case of iframe.reload().
+        assert_eq!(loaded_pipeline, self.pipeline_id().unwrap());
+
         // TODO A cross-origin child document would not be easily accessible
         //      from this script thread. It's unclear how to implement
         //      steps 2, 3, and 5 efficiently in this case.

components/script/script_thread.rs

@@ -932,8 +932,8 @@ impl ScriptThread {
             ConstellationControlMsg::WebFontLoaded(pipeline_id) =>
                 self.handle_web_font_loaded(pipeline_id),
             ConstellationControlMsg::DispatchFrameLoadEvent {
-                target: frame_id, parent: parent_pipeline_id } =>
+                target: frame_id, parent: parent_id, child: child_id } =>
-                self.handle_frame_load_event(parent_pipeline_id, frame_id),
+                self.handle_frame_load_event(parent_id, frame_id, child_id),
             ConstellationControlMsg::FramedContentChanged(parent_pipeline_id, frame_id) =>
                 self.handle_framed_content_changed(parent_pipeline_id, frame_id),
             ConstellationControlMsg::ReportCSSError(pipeline_id, filename, line, column, msg) =>
@@ -1193,12 +1193,15 @@ impl ScriptThread {
             None => return warn!("Message sent to closed pipeline {}.", pipeline),
         };
         if doc.loader().is_blocked() {
+            debug!("Script thread got loads complete while loader is blocked.");
             return;
         }
 
         doc.mut_loader().inhibit_events();
 
         // https://html.spec.whatwg.org/multipage/#the-end step 7
+        // Schedule a task to fire a "load" event (if no blocking loads have arrived in the mean time)
+        // NOTE: we can end up executing this code more than once, in case more blocking loads arrive.
         let handler = box DocumentProgressHandler::new(Trusted::new(&doc));
         self.dom_manipulation_task_source.queue(handler, doc.window().upcast()).unwrap();
 
@@ -1573,13 +1576,15 @@ impl ScriptThread {
     }
 
     /// Notify the containing document of a child frame that has completed loading.
-    fn handle_frame_load_event(&self, parent_pipeline_id: PipelineId, id: FrameId) {
+    fn handle_frame_load_event(&self, parent_id: PipelineId, frame_id: FrameId, child_id: PipelineId) {
-        let document = match self.root_browsing_context().find(parent_pipeline_id) {
+        let document = match self.root_browsing_context().find(parent_id) {
             Some(browsing_context) => browsing_context.active_document(),
-            None => return warn!("Message sent to closed pipeline {}.", parent_pipeline_id),
+            None => return warn!("Message sent to closed pipeline {}.", parent_id),
         };
-        if let Some(iframe) = document.find_iframe(id) {
+        if let Some(iframe) = document.find_iframe(frame_id) {
-            iframe.iframe_load_event_steps();
+            if iframe.pipeline_id() == Some(child_id) {
+                iframe.iframe_load_event_steps(child_id);
+            }
         }
     }
 

components/script_traits/lib.rs

@@ -237,6 +237,8 @@ pub enum ConstellationControlMsg {
         target: FrameId,
         /// The pipeline that contains a frame loading the target pipeline.
         parent: PipelineId,
+        /// The pipeline that has completed loading.
+        child: PipelineId,
     },
     /// Notifies a parent pipeline that one of its child frames is now active.
     /// PipelineId is for the parent, FrameId is the child frame.

components/script_traits/script_msg.rs

@@ -83,10 +83,9 @@ pub enum ScriptMsg {
     /// 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,
+    /// Dispatch a mozbrowser event to the parent of this pipeline.
-    /// or to the window if no subpage id is provided.
+    /// The first PipelineId is for the parent, the second is for the originating pipeline.
-    /// First PipelineId is for the parent, second PipelineId is in the frame.
+    MozBrowserEvent(PipelineId, PipelineId, MozBrowserEvent),
-    MozBrowserEvent(PipelineId, Option<PipelineId>, MozBrowserEvent),
     /// HTMLIFrameElement Forward or Back traversal.
     TraverseHistory(Option<PipelineId>, TraversalDirection),
     /// Gets the length of the joint session history from the constellation.