Make the I Tried star appear when a top-level page load fails for network-related reasons.

Under the hood, this requires treating the I Tried pipeline as a new load instead of a replacement, since the failure-handling code interacts poorly with the rest of the replacement code when we get a series of staggered failures over time from the various pipeline components.

5 files changed, 60 insertions, 47 deletions

diff --git a/src/components/main/constellation.rs b/src/components/main/constellation.rs
index 2777c259ff7..7c27e1d97be 100644
--- a/src/components/main/constellation.rs
+++ b/src/components/main/constellation.rs
@@ -387,17 +387,38 @@ impl Constellation {
         }
 
         let old_pipeline = match self.pipelines.find(&pipeline_id) {
-            None => return, // already failed?
-            Some(id) => id.clone()
+            None => {
+                debug!("no existing pipeline found; bailing out of failure recovery.");
+                return; // already failed?
+            }
+            Some(pipeline) => pipeline.clone()
         };
 
-        let ScriptChan(ref old_script) = old_pipeline.script_chan;
-        old_script.try_send(ExitPipelineMsg(pipeline_id));
-        old_pipeline.render_chan.chan.try_send(render_task::ExitMsg(None));
-        let LayoutChan(ref old_layout) = old_pipeline.layout_chan;
-        old_layout.try_send(layout_interface::ExitNowMsg);
+        fn force_pipeline_exit(old_pipeline: &Rc<Pipeline>) {
+            let ScriptChan(ref old_script) = old_pipeline.script_chan;
+            old_script.try_send(ExitPipelineMsg(old_pipeline.id));
+            old_pipeline.render_chan.chan.try_send(render_task::ExitMsg(None));
+            let LayoutChan(ref old_layout) = old_pipeline.layout_chan;
+            old_layout.try_send(layout_interface::ExitNowMsg);
+        }
+        force_pipeline_exit(&old_pipeline);
         self.pipelines.remove(&pipeline_id);
 
+        loop {
+            let idx = self.pending_frames.iter().position(|pending| {
+                pending.after.pipeline.id == pipeline_id
+            });
+            idx.map(|idx| {
+                debug!("removing pending frame change for failed pipeline");
+                force_pipeline_exit(&self.pending_frames[idx].after.pipeline);
+                self.pending_frames.remove(idx)
+            });
+            if idx.is_none() {
+                break;
+            }
+        }
+        debug!("creating replacement pipeline for about:failure");
+
         let new_id = self.get_next_pipeline_id();
         let pipeline = Pipeline::create(new_id,
                                         subpage_id,
@@ -419,10 +440,10 @@ impl Constellation {
                 parent: RefCell::new(None),
                 children: RefCell::new(~[]),
             }),
-            navigation_type: constellation_msg::Navigate,
+            navigation_type: constellation_msg::Load,
         });
 
-        self.pipelines.insert(pipeline_id, pipeline_wrapped);
+        self.pipelines.insert(new_id, pipeline_wrapped);
     }
 
     fn handle_init_load(&mut self, url: Url) {
@@ -729,12 +750,12 @@ impl Constellation {
 
             // If there are frames to revoke permission from, do so now.
             match frame_change.before {
-                Some(revoke_id) => {
+                Some(revoke_id) if self.current_frame().is_some() => {
                     debug!("Constellation: revoking permission from {:?}", revoke_id);
                     let current_frame = self.current_frame().get_ref();
 
                     let to_revoke = current_frame.find(revoke_id).expect(
-                        "Constellation: pending frame change refers to an old
+                        "Constellation: pending frame change refers to an old \
                         frame not contained in the current frame. This is a bug");
 
                     for frame in to_revoke.iter() {
@@ -758,7 +779,7 @@ impl Constellation {
                     }
                 }
 
-                None => {
+                _ => {
                     // Add to_add to parent's children, if it is not the root
                     let parent = &to_add.parent;
                     for parent in parent.borrow().iter() {
@@ -807,9 +828,10 @@ impl Constellation {
         for change in self.pending_frames.iter() {
             let frame_tree = &change.after;
             if frame_tree.parent.borrow().is_none() {
-                debug!("constellation sending resize message to pending outer frame");
+                debug!("constellation sending resize message to pending outer frame ({:?})",
+                       frame_tree.pipeline.id);
                 let ScriptChan(ref chan) = frame_tree.pipeline.script_chan;
-                chan.send(ResizeMsg(frame_tree.pipeline.id, new_size))
+                chan.try_send(ResizeMsg(frame_tree.pipeline.id, new_size));
             }
         }
 
@@ -847,10 +869,13 @@ impl Constellation {
         // parsed iframes that finish loading)
         match navigation_type {
             constellation_msg::Load => {
+                debug!("evicting old frames due to load");
                 let evicted = self.navigation_context.load(frame_tree);
                 self.handle_evicted_frames(evicted);
             }
-            _ => {}
+            _ => {
+                debug!("ignoring non-load navigation type");
+            }
         }
     }
 
diff --git a/src/components/main/pipeline.rs b/src/components/main/pipeline.rs
index 2ed978360ac..2439a73f262 100644
--- a/src/components/main/pipeline.rs
+++ b/src/components/main/pipeline.rs
@@ -208,6 +208,8 @@ impl Pipeline {
     }
 
     pub fn exit(&self) {
+        debug!("pipeline {:?} exiting", self.id);
+
         // Script task handles shutting down layout, and layout handles shutting down the renderer.
         // For now, if the script task has failed, we give up on clean shutdown.
         let ScriptChan(ref chan) = self.script_chan;
diff --git a/src/components/script/dom/window.rs b/src/components/script/dom/window.rs
index 54575882ed3..e29a2ceb886 100644
--- a/src/components/script/dom/window.rs
+++ b/src/components/script/dom/window.rs
@@ -26,7 +26,7 @@ use js::jsval::{NullValue, JSVal};
 
 use collections::hashmap::HashMap;
 use std::cmp;
-use std::comm::{channel, Sender, Receiver};
+use std::comm::{channel, Sender};
 use std::comm::Select;
 use std::hash::{Hash, sip};
 use std::io::timer::Timer;
@@ -35,12 +35,6 @@ use std::rc::Rc;
 use serialize::{Encoder, Encodable};
 use url::Url;
 
-pub enum TimerControlMsg {
-    TimerMessageFire(~TimerData),
-    TimerMessageClose,
-    TimerMessageTriggerExit //XXXjdm this is just a quick hack to talk to the script task
-}
-
 pub struct TimerHandle {
     handle: i32,
     cancel_chan: Option<Sender<()>>,
@@ -86,7 +80,6 @@ pub struct Window {
     active_timers: ~HashMap<i32, TimerHandle>,
     next_timer_handle: i32,
     compositor: Untraceable<~ScriptListener>,
-    timer_chan: Untraceable<Sender<TimerControlMsg>>,
     browser_context: Option<BrowserContext>,
     page: Rc<Page>,
 }
@@ -108,7 +101,6 @@ impl Window {
 #[unsafe_destructor]
 impl Drop for Window {
     fn drop(&mut self) {
-        self.timer_chan.send(TimerMessageClose);
         for timer_handle in self.active_timers.values() {
             timer_handle.cancel();
         }
@@ -132,7 +124,8 @@ impl Window {
     }
 
     pub fn Close(&self) {
-        self.timer_chan.deref().send(TimerMessageTriggerExit);
+        let ScriptChan(ref chan) = self.script_chan;
+        chan.send(ExitWindowMsg(self.page.id.clone()));
     }
 
     pub fn Document(&self) -> JS<Document> {
@@ -228,7 +221,8 @@ impl Window {
         // to the relevant script handler that will deal with it.
         let tm = Timer::new().unwrap();
         let (cancel_chan, cancel_port) = channel();
-        let chan = self.timer_chan.clone();
+        let chan = self.script_chan.clone();
+        let page_id = self.page.id.clone();
         let spawn_name = if is_interval {
             "Window:SetInterval"
         } else {
@@ -253,12 +247,14 @@ impl Window {
                 let id = select.wait();
                 if id == timeout_handle.id() {
                     timeout_port.recv();
-                    chan.send(TimerMessageFire(~TimerData {
+                    let data = ~TimerData {
                         handle: handle,
                         is_interval: is_interval,
                         funval: callback,
                         args: ~[],
-                    }));
+                    };
+                    let ScriptChan(ref chan) = chan;
+                    chan.send(FireTimerMsg(page_id, data));
                     if !is_interval {
                         break;
                     }
@@ -323,26 +319,11 @@ impl Window {
                compositor: ~ScriptListener,
                image_cache_task: ImageCacheTask)
                -> JS<Window> {
-        let script_chan_clone = script_chan.clone();
-        let (timer_chan, timer_port): (Sender<TimerControlMsg>, Receiver<TimerControlMsg>) = channel();
-        let id = page.id.clone();
-        spawn_named("timer controller", proc() {
-            let ScriptChan(script_chan) = script_chan;
-            loop {
-                match timer_port.recv() {
-                    TimerMessageClose => break,
-                    TimerMessageFire(td) => script_chan.send(FireTimerMsg(id, td)),
-                    TimerMessageTriggerExit => script_chan.send(ExitWindowMsg(id)),
-                }
-            }
-        });
-
         let win = ~Window {
             eventtarget: EventTarget::new_inherited(WindowTypeId),
-            script_chan: script_chan_clone,
+            script_chan: script_chan,
             console: None,
             compositor: Untraceable::new(compositor),
-            timer_chan: Untraceable::new(timer_chan),
             page: page.clone(),
             location: None,
             navigator: None,
diff --git a/src/components/script/script_task.rs b/src/components/script/script_task.rs
index a78108c057a..13d6c11c0f6 100644
--- a/src/components/script/script_task.rs
+++ b/src/components/script/script_task.rs
@@ -172,7 +172,7 @@ impl PageTree {
                 next_subpage_id: Untraceable::new(RefCell::new(SubpageId(0))),
                 resize_event: Untraceable::new(RefCell::new(None)),
                 fragment_node: Traceable::new(RefCell::new(None)),
-                last_reflow_id: Traceable::new(RefCell::new(0))
+                last_reflow_id: Traceable::new(RefCell::new(0)),
             }),
             inner: ~[],
         }
diff --git a/src/components/util/task.rs b/src/components/util/task.rs
index 8500649370d..ec12b1eaf58 100644
--- a/src/components/util/task.rs
+++ b/src/components/util/task.rs
@@ -16,10 +16,15 @@ pub fn spawn_named<S: IntoMaybeOwned<'static>>(name: S, f: proc()) {
 /// this `TaskBuilder` fails.
 pub fn send_on_failure<T: Send>(builder: &mut TaskBuilder, msg: T, dest: Sender<T>) {
     let port = builder.future_result();
-    spawn(proc() {
+    let watched_name = builder.opts.name.as_ref().unwrap().as_slice().to_owned();
+    let name = format!("{:s}Watcher", watched_name);
+    spawn_named(name, proc() {
         match port.recv() {
             Ok(()) => (),
-            Err(..) => dest.send(msg),
+            Err(..) => {
+                debug!("{:s} failed, notifying constellation", watched_name);
+                dest.send(msg);
+            }
         }
     })
 }