Auto merge of #13498 - asajeffrey:script-iframe-stores-frameid, r=glennw

IFrame elements now manage FrameIds rather than the constellation.

<!-- Please describe your changes on the following line: -->

This PR stores the FrameId as well as the PipelineId in an html iframe. The iframes are now responsible for creating frame ids, not the constellation.

This is the first step in fixing #633, because it means we know the frame id of each script thread when it is created. It also means we can share the frame id, for example using it in the debugger.

cc @jdm, @ConnorGBrewster and @ejpbruel.

---
<!-- 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. -->

<!-- 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/13498)
<!-- Reviewable:end -->

7 files changed, 170 insertions, 178 deletions

diff --git a/components/constellation/constellation.rs b/components/constellation/constellation.rs
index fa10a76bbdc..364401af8a2 100644
--- a/components/constellation/constellation.rs
+++ b/components/constellation/constellation.rs
@@ -139,14 +139,11 @@ pub struct Constellation<Message, LTF, STF> {
     font_cache_thread: FontCacheThread,
 
     /// ID of the root frame.
-    root_frame_id: Option<FrameId>,
+    root_frame_id: FrameId,
 
     /// The next free ID to assign to a pipeline ID namespace.
     next_pipeline_namespace_id: PipelineNamespaceId,
 
-    /// The next free ID to assign to a frame.
-    next_frame_id: FrameId,
-
     /// Pipeline ID that has currently focused element for key events.
     focus_pipeline_id: Option<PipelineId>,
 
@@ -268,6 +265,7 @@ impl Frame {
 /// Represents a pending change in the frame tree, that will be applied
 /// once the new pipeline has loaded and completed initial layout / paint.
 struct FrameChange {
+    frame_id: FrameId,
     old_pipeline_id: Option<PipelineId>,
     new_pipeline_id: PipelineId,
     document_ready: bool,
@@ -476,6 +474,8 @@ impl<Message, LTF, STF> Constellation<Message, LTF, STF>
 
             let swmanager_receiver = ROUTER.route_ipc_receiver_to_new_mpsc_receiver(swmanager_receiver);
 
+            PipelineNamespace::install(PipelineNamespaceId(0));
+
             let mut constellation: Constellation<Message, LTF, STF> = Constellation {
                 script_sender: ipc_script_sender,
                 layout_sender: ipc_layout_sender,
@@ -495,9 +495,9 @@ impl<Message, LTF, STF> Constellation<Message, LTF, STF>
                 pipelines: HashMap::new(),
                 frames: HashMap::new(),
                 pending_frames: vec!(),
-                next_pipeline_namespace_id: PipelineNamespaceId(0),
-                root_frame_id: None,
-                next_frame_id: FrameId(0),
+                // We initialize the namespace at 1, since we reserved namespace 0 for the constellation
+                next_pipeline_namespace_id: PipelineNamespaceId(1),
+                root_frame_id: FrameId::new(),
                 focus_pipeline_id: None,
                 time_profiler_chan: state.time_profiler_chan,
                 mem_profiler_chan: state.mem_profiler_chan,
@@ -525,8 +525,7 @@ impl<Message, LTF, STF> Constellation<Message, LTF, STF>
                     (rng, prob)
                 }),
             };
-            let namespace_id = constellation.next_pipeline_namespace_id();
-            PipelineNamespace::install(namespace_id);
+
             constellation.run();
         });
         (compositor_sender, swmanager_sender)
@@ -614,9 +613,9 @@ impl<Message, LTF, STF> Constellation<Message, LTF, STF>
 
     // Get an iterator for the current frame tree. Specify self.root_frame_id to
     // iterate the entire tree, or a specific frame id to iterate only that sub-tree.
-    fn current_frame_tree_iter(&self, frame_id_root: Option<FrameId>) -> FrameTreeIterator {
+    fn current_frame_tree_iter(&self, frame_id_root: FrameId) -> FrameTreeIterator {
         FrameTreeIterator {
-            stack: frame_id_root.into_iter().collect(),
+            stack: vec!(frame_id_root),
             pipelines: &self.pipelines,
             frames: &self.frames,
         }
@@ -656,20 +655,14 @@ impl<Message, LTF, STF> Constellation<Message, LTF, STF>
     }
 
     // Create a new frame and update the internal bookkeeping.
-    fn new_frame(&mut self, pipeline_id: PipelineId) -> FrameId {
-        let id = self.next_frame_id;
-        let FrameId(ref mut i) = self.next_frame_id;
-        *i += 1;
-
-        let frame = Frame::new(id, pipeline_id);
+    fn new_frame(&mut self, frame_id: FrameId, pipeline_id: PipelineId) {
+        let frame = Frame::new(frame_id, pipeline_id);
 
         assert!(self.pipelines.get(&pipeline_id).and_then(|pipeline| pipeline.frame).is_none());
-        assert!(!self.frames.contains_key(&id));
-
-        self.pipelines.get_mut(&pipeline_id).map(|pipeline| pipeline.frame = Some(id));
-        self.frames.insert(id, frame);
+        assert!(!self.frames.contains_key(&frame_id));
 
-        id
+        self.pipelines.get_mut(&pipeline_id).map(|pipeline| pipeline.frame = Some(frame_id));
+        self.frames.insert(frame_id, frame);
     }
 
     /// Handles loading pages, navigation, and granting access to the compositor
@@ -1032,10 +1025,9 @@ impl<Message, LTF, STF> Constellation<Message, LTF, STF>
         if self.shutting_down { return; }
         self.shutting_down = true;
 
-        // TODO: exit before the root frame is set?
-        if let Some(root_id) = self.root_frame_id {
-            self.close_frame(root_id, ExitPipelineMode::Normal);
-        }
+        // TODO: exit before the root frame is initialized?
+        let root_frame_id = self.root_frame_id;
+        self.close_frame(root_frame_id, ExitPipelineMode::Normal);
     }
 
     fn handle_shutdown(&mut self) {
@@ -1121,6 +1113,7 @@ impl<Message, LTF, STF> Constellation<Message, LTF, STF>
             let pipeline_url = self.pipelines.get(&pipeline_id).map(|pipeline| pipeline.url.clone());
             let parent_info = self.pipelines.get(&pipeline_id).and_then(|pipeline| pipeline.parent_info);
             let window_size = self.pipelines.get(&pipeline_id).and_then(|pipeline| pipeline.size);
+            let frame_id = self.pipelines.get(&pipeline_id).and_then(|pipeline| pipeline.frame);
 
             self.close_pipeline(pipeline_id, ExitPipelineMode::Force);
             self.pipelines.remove(&pipeline_id);
@@ -1142,16 +1135,19 @@ impl<Message, LTF, STF> Constellation<Message, LTF, STF>
 
             warn!("creating replacement pipeline for about:failure");
 
-            let new_pipeline_id = PipelineId::new();
-            let load_data = LoadData::new(failure_url, None, None);
-            self.new_pipeline(new_pipeline_id, parent_info, Some(pipeline_id), window_size, None, load_data, false);
+            if let Some(frame_id) = frame_id {
+                let new_pipeline_id = PipelineId::new();
+                let load_data = LoadData::new(failure_url, None, None);
+                self.new_pipeline(new_pipeline_id, parent_info, Some(pipeline_id), window_size, None, load_data, false);
 
-            self.pending_frames.push(FrameChange {
-                old_pipeline_id: Some(pipeline_id),
-                new_pipeline_id: new_pipeline_id,
-                document_ready: false,
-                replace: false,
-            });
+                self.pending_frames.push(FrameChange {
+                    frame_id: frame_id,
+                    old_pipeline_id: Some(pipeline_id),
+                    new_pipeline_id: new_pipeline_id,
+                    document_ready: false,
+                    replace: false,
+                });
+            }
         }
     }
 
@@ -1172,11 +1168,11 @@ impl<Message, LTF, STF> Constellation<Message, LTF, STF>
     fn handle_init_load(&mut self, url: Url) {
         let window_size = self.window_size.visible_viewport;
         let root_pipeline_id = PipelineId::new();
-        debug_assert!(PipelineId::fake_root_pipeline_id() == root_pipeline_id);
         self.new_pipeline(root_pipeline_id, None, None, Some(window_size), None,
                           LoadData::new(url.clone(), None, None), false);
         self.handle_load_start_msg(root_pipeline_id);
         self.pending_frames.push(FrameChange {
+            frame_id: self.root_frame_id,
             old_pipeline_id: None,
             new_pipeline_id: root_pipeline_id,
             document_ready: false,
@@ -1304,6 +1300,7 @@ impl<Message, LTF, STF> Constellation<Message, LTF, STF>
                           is_private);
 
         self.pending_frames.push(FrameChange {
+            frame_id: load_info.frame_id,
             old_pipeline_id: load_info.old_pipeline_id,
             new_pipeline_id: load_info.new_pipeline_id,
             document_ready: false,
@@ -1351,8 +1348,7 @@ impl<Message, LTF, STF> Constellation<Message, LTF, STF>
         let display_alert_dialog = if PREFS.is_mozbrowser_enabled() {
             let parent_pipeline_info = self.pipelines.get(&pipeline_id).and_then(|source| source.parent_info);
             if parent_pipeline_info.is_some() {
-                let root_pipeline_id = self.root_frame_id
-                    .and_then(|root_frame_id| self.frames.get(&root_frame_id))
+                let root_pipeline_id = self.frames.get(&self.root_frame_id)
                     .map(|root_frame| root_frame.current.pipeline_id);
 
                 let ancestor_info = self.get_mozbrowser_ancestor_info(pipeline_id);
@@ -1441,6 +1437,7 @@ impl<Message, LTF, STF> Constellation<Message, LTF, STF>
                 let new_pipeline_id = PipelineId::new();
                 self.new_pipeline(new_pipeline_id, None, None, window_size, None, load_data, false);
                 self.pending_frames.push(FrameChange {
+                    frame_id: self.root_frame_id,
                     old_pipeline_id: Some(source_id),
                     new_pipeline_id: new_pipeline_id,
                     document_ready: false,
@@ -1458,11 +1455,10 @@ impl<Message, LTF, STF> Constellation<Message, LTF, STF>
     }
 
     fn handle_load_start_msg(&mut self, pipeline_id: PipelineId) {
-        if let Some(frame_id) = self.get_top_level_frame_for_pipeline(Some(pipeline_id)) {
-            let forward = !self.joint_session_future(frame_id).is_empty();
-            let back = !self.joint_session_past(frame_id).is_empty();
-            self.compositor_proxy.send(ToCompositorMsg::LoadStart(back, forward));
-        }
+        let frame_id = self.get_top_level_frame_for_pipeline(Some(pipeline_id));
+        let forward = !self.joint_session_future(frame_id).is_empty();
+        let back = !self.joint_session_past(frame_id).is_empty();
+        self.compositor_proxy.send(ToCompositorMsg::LoadStart(back, forward));
     }
 
     fn handle_load_complete_msg(&mut self, pipeline_id: PipelineId) {
@@ -1477,22 +1473,18 @@ impl<Message, LTF, STF> Constellation<Message, LTF, STF>
         if webdriver_reset {
             self.webdriver.load_channel = None;
         }
-        if let Some(frame_id) = self.get_top_level_frame_for_pipeline(Some(pipeline_id)) {
-            let forward = !self.joint_session_future(frame_id).is_empty();
-            let back = !self.joint_session_past(frame_id).is_empty();
-            let root = self.root_frame_id.is_none() || self.root_frame_id == Some(frame_id);
-            self.compositor_proxy.send(ToCompositorMsg::LoadComplete(back, forward, root));
-        }
+        let frame_id = self.get_top_level_frame_for_pipeline(Some(pipeline_id));
+        let forward = !self.joint_session_future(frame_id).is_empty();
+        let back = !self.joint_session_past(frame_id).is_empty();
+        let root = self.root_frame_id == frame_id;
+        self.compositor_proxy.send(ToCompositorMsg::LoadComplete(back, forward, root));
         self.handle_subframe_loaded(pipeline_id);
     }
 
     fn handle_traverse_history_msg(&mut self,
                                    pipeline_id: Option<PipelineId>,
                                    direction: TraversalDirection) {
-        let frame_id = match self.get_top_level_frame_for_pipeline(pipeline_id) {
-            Some(frame_id) => frame_id,
-            None => return warn!("Traverse message received after root's closure."),
-        };
+        let frame_id = self.get_top_level_frame_for_pipeline(pipeline_id);
 
         let mut traversal_info = HashMap::new();
 
@@ -1526,14 +1518,7 @@ impl<Message, LTF, STF> Constellation<Message, LTF, STF>
     }
 
     fn handle_joint_session_history_length(&self, pipeline_id: PipelineId, sender: IpcSender<u32>) {
-        let frame_id = match self.get_top_level_frame_for_pipeline(Some(pipeline_id)) {
-            Some(frame_id) => frame_id,
-            None => {
-                warn!("Jsh length message received after root's closure.");
-                let _ = sender.send(0);
-                return;
-            },
-        };
+        let frame_id = self.get_top_level_frame_for_pipeline(Some(pipeline_id));
 
         // Initialize length at 1 to count for the current active entry
         let mut length = 1;
@@ -1548,8 +1533,7 @@ impl<Message, LTF, STF> Constellation<Message, LTF, STF>
         // Send to the explicitly focused pipeline (if it exists), or the root
         // frame's current pipeline. If neither exist, fall back to sending to
         // the compositor below.
-        let root_pipeline_id = self.root_frame_id
-            .and_then(|root_frame_id| self.frames.get(&root_frame_id))
+        let root_pipeline_id = self.frames.get(&self.root_frame_id)
             .map(|root_frame| root_frame.current.pipeline_id);
         let pipeline_id = self.focus_pipeline_id.or(root_pipeline_id);
 
@@ -1574,8 +1558,7 @@ impl<Message, LTF, STF> Constellation<Message, LTF, STF>
 
     fn handle_reload_msg(&mut self) {
         // Send Reload constellation msg to root script channel.
-        let root_pipeline_id = self.root_frame_id
-            .and_then(|root_frame_id| self.frames.get(&root_frame_id))
+        let root_pipeline_id = self.frames.get(&self.root_frame_id)
             .map(|root_frame| root_frame.current.pipeline_id);
 
         if let Some(pipeline_id) = root_pipeline_id {
@@ -1618,8 +1601,8 @@ impl<Message, LTF, STF> Constellation<Message, LTF, STF>
 
     fn handle_get_pipeline(&mut self, frame_id: Option<FrameId>,
                            resp_chan: IpcSender<Option<(PipelineId, bool)>>) {
-        let current_pipeline_id = frame_id.or(self.root_frame_id)
-            .and_then(|frame_id| self.frames.get(&frame_id))
+        let frame_id = frame_id.unwrap_or(self.root_frame_id);
+        let current_pipeline_id = self.frames.get(&frame_id)
             .map(|frame| frame.current.pipeline_id);
         let current_pipeline_id_loaded = current_pipeline_id
             .map(|id| (id, true));
@@ -1793,8 +1776,7 @@ impl<Message, LTF, STF> Constellation<Message, LTF, STF>
                 }
             },
             WebDriverCommandMsg::TakeScreenshot(pipeline_id, reply) => {
-                let current_pipeline_id = self.root_frame_id
-                    .and_then(|root_frame_id| self.frames.get(&root_frame_id))
+                let current_pipeline_id = self.frames.get(&self.root_frame_id)
                     .map(|root_frame| root_frame.current.pipeline_id);
                 if Some(pipeline_id) == current_pipeline_id {
                     self.compositor_proxy.send(ToCompositorMsg::CreatePng(reply));
@@ -1898,12 +1880,12 @@ impl<Message, LTF, STF> Constellation<Message, LTF, STF>
         }
     }
 
-    fn get_top_level_frame_for_pipeline(&self, pipeline_id: Option<PipelineId>) -> Option<FrameId> {
+    fn get_top_level_frame_for_pipeline(&self, pipeline_id: Option<PipelineId>) -> FrameId {
         if PREFS.is_mozbrowser_enabled() {
             pipeline_id.and_then(|id| self.get_mozbrowser_ancestor_info(id))
                        .and_then(|pipeline_info| self.pipelines.get(&pipeline_info.1))
                        .and_then(|pipeline| pipeline.frame)
-                       .or(self.root_frame_id)
+                       .unwrap_or(self.root_frame_id)
         } else {
             // If mozbrowser is not enabled, the root frame is the only top-level frame
             self.root_frame_id
@@ -1933,51 +1915,41 @@ impl<Message, LTF, STF> Constellation<Message, LTF, STF>
             }
         }
 
-        let frame_id = frame_change.old_pipeline_id.and_then(|old_pipeline_id| {
+        if let Some(old_pipeline_id) = frame_change.old_pipeline_id {
             // The new pipeline is replacing an old one.
             // Remove paint permissions for the pipeline being replaced.
             self.revoke_paint_permission(old_pipeline_id);
+        };
 
-            self.pipelines.get(&old_pipeline_id).and_then(|pipeline| pipeline.frame)
-        });
+        if self.frames.contains_key(&frame_change.frame_id) {
+            // Add new pipeline to navigation frame, and return frames evicted from history.
+            if let Some(ref mut pipeline) = self.pipelines.get_mut(&frame_change.new_pipeline_id) {
+                pipeline.frame = Some(frame_change.frame_id);
+            }
 
-        match frame_id {
-            Some(frame_id) => {
-                // Add new pipeline to navigation frame, and return frames evicted from history.
-                if let Some(ref mut pipeline) = self.pipelines.get_mut(&frame_change.new_pipeline_id) {
-                    pipeline.frame = Some(frame_id);
+            if frame_change.replace {
+                let evicted = self.frames.get_mut(&frame_change.frame_id).map(|frame| {
+                    frame.replace_current(frame_change.new_pipeline_id)
+                });
+                if let Some(evicted) = evicted {
+                    self.close_pipeline(evicted.pipeline_id, ExitPipelineMode::Normal);
                 }
-
-                if frame_change.replace {
-                    let evicted = self.frames.get_mut(&frame_id).map(|frame| {
-                        frame.replace_current(frame_change.new_pipeline_id)
-                    });
-                    if let Some(evicted) = evicted {
-                        self.close_pipeline(evicted.pipeline_id, ExitPipelineMode::Normal);
-                    }
-                } else {
-                    if let Some(ref mut frame) = self.frames.get_mut(&frame_id) {
-                        frame.load(frame_change.new_pipeline_id);
-                    }
+            } else {
+                if let Some(ref mut frame) = self.frames.get_mut(&frame_change.frame_id) {
+                    frame.load(frame_change.new_pipeline_id);
                 }
             }
-            None => {
-                // The new pipeline is in a new frame with no history
-                let frame_id = self.new_frame(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!
-                match self.pipelines.get(&frame_change.new_pipeline_id)
-                                    .and_then(|pipeline| pipeline.parent_info) {
-                    Some((parent_id, _)) => {
-                        if let Some(parent) = self.pipelines.get_mut(&parent_id) {
-                            parent.add_child(frame_id);
-                        }
-                    }
-                    None => {
-                        assert!(self.root_frame_id.is_none());
-                        self.root_frame_id = Some(frame_id);
-                    }
+        } 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);
                 }
             }
         }
@@ -1987,11 +1959,8 @@ impl<Message, LTF, STF> Constellation<Message, LTF, STF>
             // This is the result of a link being clicked and a navigation completing.
             self.trigger_mozbrowserlocationchange(frame_change.new_pipeline_id);
 
-            let frame_id = match self.get_top_level_frame_for_pipeline(Some(frame_change.new_pipeline_id)) {
-                Some(frame_id) => frame_id,
-                None => return warn!("Tried to remove forward history after root frame closure."),
-            };
-            self.clear_joint_session_future(frame_id);
+            let top_level_frame_id = self.get_top_level_frame_for_pipeline(Some(frame_change.new_pipeline_id));
+            self.clear_joint_session_future(top_level_frame_id);
         }
 
         // Build frame tree and send permission
@@ -2052,13 +2021,9 @@ impl<Message, LTF, STF> Constellation<Message, LTF, STF>
         debug!("handle_window_size_msg: {:?} {:?}", new_size.initial_viewport.to_untyped(),
                                                        new_size.visible_viewport.to_untyped());
 
-        if let Some(root_frame_id) = self.root_frame_id {
+        if let Some(frame) = self.frames.get(&self.root_frame_id) {
             // Send Resize (or ResizeInactive) messages to each
             // pipeline in the frame tree.
-            let frame = match self.frames.get(&root_frame_id) {
-                None => return warn!("Frame {:?} resized after closing.", root_frame_id),
-                Some(frame) => frame,
-            };
             let pipeline_id = frame.current.pipeline_id;
             let pipeline = match self.pipelines.get(&pipeline_id) {
                 None => return warn!("Pipeline {:?} resized after closing.", pipeline_id),
@@ -2127,7 +2092,7 @@ impl<Message, LTF, STF> Constellation<Message, LTF, STF>
         //
         // If there is no root frame yet, the initial page has
         // not loaded, so there is nothing to save yet.
-        if self.root_frame_id.is_none() {
+        if !self.frames.contains_key(&self.root_frame_id) {
             return ReadyToSave::NoRootFrame;
         }
 
@@ -2384,9 +2349,10 @@ impl<Message, LTF, STF> Constellation<Message, LTF, STF>
 
     // Revoke paint permission from a pipeline, and all children.
     fn revoke_paint_permission(&self, pipeline_id: PipelineId) {
-        let frame_id = self.pipelines.get(&pipeline_id).and_then(|pipeline| pipeline.frame);
-        for frame in self.current_frame_tree_iter(frame_id) {
-            self.pipelines.get(&frame.current.pipeline_id).map(|pipeline| pipeline.revoke_paint_permission());
+        if let Some(frame_id) = self.pipelines.get(&pipeline_id).and_then(|pipeline| pipeline.frame) {
+            for frame in self.current_frame_tree_iter(frame_id) {
+                self.pipelines.get(&frame.current.pipeline_id).map(|pipeline| pipeline.revoke_paint_permission());
+            }
         }
     }
 
@@ -2396,15 +2362,13 @@ 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.
-        if let Some(root_frame_id) = self.root_frame_id {
-            if let Some(frame_tree) = self.frame_to_sendable(root_frame_id) {
-                let (chan, port) = ipc::channel().expect("Failed to create IPC channel!");
-                self.compositor_proxy.send(ToCompositorMsg::SetFrameTree(frame_tree,
-                                                                         chan));
-                if port.recv().is_err() {
-                    warn!("Compositor has discarded SetFrameTree");
-                    return; // Our message has been discarded, probably shutting down.
-                }
+        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,
+                                                                     chan));
+            if port.recv().is_err() {
+                warn!("Compositor has discarded SetFrameTree");
+                return; // Our message has been discarded, probably shutting down.
             }
         }
 
@@ -2489,11 +2453,9 @@ impl<Message, LTF, STF> Constellation<Message, LTF, STF>
             }
         }
 
-        if let Some(root_frame_id) = self.root_frame_id {
-            if let Some(root_frame) = self.frames.get(&root_frame_id) {
-                if let Some(root_pipeline) = self.pipelines.get(&root_frame.current.pipeline_id) {
-                    return root_pipeline.trigger_mozbrowser_event(None, event);
-                }
+        if let Some(root_frame) = self.frames.get(&self.root_frame_id) {
+            if let Some(root_pipeline) = self.pipelines.get(&root_frame.current.pipeline_id) {
+                return root_pipeline.trigger_mozbrowser_event(None, event);
             }
         }
 
@@ -2502,7 +2464,7 @@ impl<Message, LTF, STF> Constellation<Message, LTF, STF>
 
     fn focused_pipeline_in_tree(&self, frame_id: FrameId) -> bool {
         self.focus_pipeline_id.map_or(false, |pipeline_id| {
-            self.pipeline_exists_in_tree(pipeline_id, Some(frame_id))
+            self.pipeline_exists_in_tree(pipeline_id, frame_id)
         })
     }
 
@@ -2512,7 +2474,7 @@ impl<Message, LTF, STF> Constellation<Message, LTF, STF>
 
     fn pipeline_exists_in_tree(&self,
                                pipeline_id: PipelineId,
-                               root_frame_id: Option<FrameId>) -> bool {
+                               root_frame_id: FrameId) -> bool {
         self.current_frame_tree_iter(root_frame_id)
             .any(|current_frame| current_frame.current.pipeline_id == pipeline_id)
     }
diff --git a/components/msg/constellation_msg.rs b/components/msg/constellation_msg.rs
index 042a3e14458..cc192b73f06 100644
--- a/components/msg/constellation_msg.rs
+++ b/components/msg/constellation_msg.rs
@@ -221,9 +221,6 @@ pub enum TraversalDirection {
     Back(usize),
 }
 
-#[derive(Clone, PartialEq, Eq, Copy, Hash, Debug, Deserialize, Serialize, PartialOrd, Ord)]
-pub struct FrameId(pub u32);
-
 /// Each pipeline ID needs to be unique. However, it also needs to be possible to
 /// generate the pipeline ID from an iframe element (this simplifies a lot of other
 /// code that makes use of pipeline IDs).
@@ -242,7 +239,7 @@ pub struct FrameId(pub u32);
 #[derive(Clone, Copy)]
 pub struct PipelineNamespace {
     id: PipelineNamespaceId,
-    next_index: PipelineIndex,
+    index: u32,
 }
 
 impl PipelineNamespace {
@@ -251,21 +248,29 @@ impl PipelineNamespace {
             assert!(tls.get().is_none());
             tls.set(Some(PipelineNamespace {
                 id: namespace_id,
-                next_index: PipelineIndex(0),
+                index: 0,
             }));
         });
     }
 
-    fn next(&mut self) -> PipelineId {
-        let pipeline_id = PipelineId {
-            namespace_id: self.id,
-            index: self.next_index,
-        };
+    fn next_index(&mut self) -> u32 {
+        let result = self.index;
+        self.index = result + 1;
+        result
+    }
 
-        let PipelineIndex(current_index) = self.next_index;
-        self.next_index = PipelineIndex(current_index + 1);
+    fn next_pipeline_id(&mut self) -> PipelineId {
+        PipelineId {
+            namespace_id: self.id,
+            index: PipelineIndex(self.next_index()),
+        }
+    }
 
-        pipeline_id
+    fn next_frame_id(&mut self) -> FrameId {
+        FrameId {
+            namespace_id: self.id,
+            index: FrameIndex(self.next_index()),
+        }
     }
 }
 
@@ -289,24 +294,12 @@ impl PipelineId {
     pub fn new() -> PipelineId {
         PIPELINE_NAMESPACE.with(|tls| {
             let mut namespace = tls.get().expect("No namespace set for this thread!");
-            let new_pipeline_id = namespace.next();
+            let new_pipeline_id = namespace.next_pipeline_id();
             tls.set(Some(namespace));
             new_pipeline_id
         })
     }
 
-    // TODO(gw): This should be removed. It's only required because of the code
-    // that uses it in the devtools lib.rs file (which itself is a TODO). Once
-    // that is fixed, this should be removed. It also relies on the first
-    // call to PipelineId::new() returning (0,0), which is checked with an
-    // assert in handle_init_load().
-    pub fn fake_root_pipeline_id() -> PipelineId {
-        PipelineId {
-            namespace_id: PipelineNamespaceId(0),
-            index: PipelineIndex(0),
-        }
-    }
-
     pub fn to_webrender(&self) -> webrender_traits::PipelineId {
         let PipelineNamespaceId(namespace_id) = self.namespace_id;
         let PipelineIndex(index) = self.index;
@@ -331,6 +324,41 @@ impl fmt::Display for PipelineId {
     }
 }
 
+#[derive(Clone, PartialEq, Eq, PartialOrd, Ord, Copy, Hash, Debug, Deserialize, Serialize, HeapSizeOf)]
+pub struct FrameIndex(pub u32);
+
+#[derive(Clone, PartialEq, Eq, PartialOrd, Ord, Copy, Hash, Debug, Deserialize, Serialize, HeapSizeOf)]
+pub struct FrameId {
+    pub namespace_id: PipelineNamespaceId,
+    pub index: FrameIndex
+}
+
+impl FrameId {
+    pub fn new() -> FrameId {
+        PIPELINE_NAMESPACE.with(|tls| {
+            let mut namespace = tls.get().expect("No namespace set for this thread!");
+            let new_frame_id = namespace.next_frame_id();
+            tls.set(Some(namespace));
+            new_frame_id
+        })
+    }
+}
+
+impl fmt::Display for FrameId {
+    fn fmt(&self, fmt: &mut fmt::Formatter) -> fmt::Result {
+        let PipelineNamespaceId(namespace_id) = self.namespace_id;
+        let FrameIndex(index) = self.index;
+        write!(fmt, "({},{})", namespace_id, index)
+    }
+}
+
+// We provide ids just for unit testing.
+pub const TEST_NAMESPACE: PipelineNamespaceId = PipelineNamespaceId(1234);
+pub const TEST_PIPELINE_INDEX: PipelineIndex = PipelineIndex(5678);
+pub const TEST_PIPELINE_ID: PipelineId = PipelineId { namespace_id: TEST_NAMESPACE, index: TEST_PIPELINE_INDEX };
+pub const TEST_FRAME_INDEX: FrameIndex = FrameIndex(8765);
+pub const TEST_FRAME_ID: FrameId = FrameId { namespace_id: TEST_NAMESPACE, index: TEST_FRAME_INDEX };
+
 #[derive(Clone, PartialEq, Eq, Copy, Hash, Debug, Deserialize, Serialize, HeapSizeOf)]
 pub enum FrameType {
     IFrame,
diff --git a/components/script/dom/bindings/trace.rs b/components/script/dom/bindings/trace.rs
index f71c2b83ef1..6331a1c3bf3 100644
--- a/components/script/dom/bindings/trace.rs
+++ b/components/script/dom/bindings/trace.rs
@@ -57,7 +57,7 @@ use js::jsapi::{GCTraceKindToAscii, Heap, JSObject, JSTracer, TraceKind};
 use js::jsval::JSVal;
 use js::rust::Runtime;
 use libc;
-use msg::constellation_msg::{FrameType, PipelineId, ReferrerPolicy, WindowSizeType};
+use msg::constellation_msg::{FrameId, FrameType, PipelineId, ReferrerPolicy, WindowSizeType};
 use net_traits::{Metadata, NetworkError, ResourceThreads};
 use net_traits::filemanager_thread::RelativePos;
 use net_traits::image::base::{Image, ImageMetadata};
@@ -308,7 +308,7 @@ no_jsmanaged_fields!(PropertyDeclarationBlock);
 no_jsmanaged_fields!(HashSet<T>);
 // These three are interdependent, if you plan to put jsmanaged data
 // in one of these make sure it is propagated properly to containing structs
-no_jsmanaged_fields!(FrameType, WindowSizeData, WindowSizeType, PipelineId);
+no_jsmanaged_fields!(FrameId, FrameType, WindowSizeData, WindowSizeType, PipelineId);
 no_jsmanaged_fields!(TimerEventId, TimerSource);
 no_jsmanaged_fields!(WorkerId);
 no_jsmanaged_fields!(QuirksMode);
diff --git a/components/script/dom/htmliframeelement.rs b/components/script/dom/htmliframeelement.rs
index 75d828e73c9..3a8e4c126f3 100644
--- a/components/script/dom/htmliframeelement.rs
+++ b/components/script/dom/htmliframeelement.rs
@@ -38,7 +38,7 @@ use dom::window::{ReflowReason, Window};
 use ipc_channel::ipc;
 use js::jsapi::{JSAutoCompartment, JSContext, MutableHandleValue};
 use js::jsval::{NullValue, UndefinedValue};
-use msg::constellation_msg::{FrameType, LoadData, PipelineId, TraversalDirection};
+use msg::constellation_msg::{FrameType, FrameId, LoadData, PipelineId, TraversalDirection};
 use net_traits::response::HttpsState;
 use script_layout_interface::message::ReflowQueryType;
 use script_traits::{IFrameLoadInfo, MozBrowserEvent, ScriptMsg as ConstellationMsg};
@@ -67,6 +67,7 @@ bitflags! {
 #[dom_struct]
 pub struct HTMLIFrameElement {
     htmlelement: HTMLElement,
+    frame_id: FrameId,
     pipeline_id: Cell<Option<PipelineId>>,
     sandbox: MutNullableHeap<JS<DOMTokenList>>,
     sandbox_allowance: Cell<Option<SandboxAllowance>>,
@@ -130,6 +131,7 @@ impl HTMLIFrameElement {
         let load_info = IFrameLoadInfo {
             load_data: load_data,
             parent_pipeline_id: global_scope.pipeline_id(),
+            frame_id: self.frame_id,
             old_pipeline_id: old_pipeline_id,
             new_pipeline_id: new_pipeline_id,
             sandbox: sandboxed,
@@ -181,6 +183,7 @@ impl HTMLIFrameElement {
                      document: &Document) -> HTMLIFrameElement {
         HTMLIFrameElement {
             htmlelement: HTMLElement::new_inherited(local_name, prefix, document),
+            frame_id: FrameId::new(),
             pipeline_id: Cell::new(None),
             sandbox: Default::default(),
             sandbox_allowance: Cell::new(None),
diff --git a/components/script_traits/lib.rs b/components/script_traits/lib.rs
index 65c52153a63..006b663268d 100644
--- a/components/script_traits/lib.rs
+++ b/components/script_traits/lib.rs
@@ -449,6 +449,8 @@ pub struct IFrameLoadInfo {
     pub load_data: Option<LoadData>,
     /// Pipeline ID of the parent of this iframe
     pub parent_pipeline_id: PipelineId,
+    /// The ID for this iframe.
+    pub frame_id: FrameId,
     /// The old pipeline ID for this iframe, if a page was previously loaded.
     pub old_pipeline_id: Option<PipelineId>,
     /// The new pipeline ID that the iframe has generated.
diff --git a/tests/unit/net/fetch.rs b/tests/unit/net/fetch.rs
index 217cc8ccee5..7f368516656 100644
--- a/tests/unit/net/fetch.rs
+++ b/tests/unit/net/fetch.rs
@@ -19,7 +19,7 @@ use hyper::server::{Handler, Listening, Server};
 use hyper::server::{Request as HyperRequest, Response as HyperResponse};
 use hyper::status::StatusCode;
 use hyper::uri::RequestUri;
-use msg::constellation_msg::{PipelineId, ReferrerPolicy};
+use msg::constellation_msg::{ReferrerPolicy, TEST_PIPELINE_ID};
 use net::fetch::cors_cache::CORSCache;
 use net::fetch::methods::{FetchContext, fetch, fetch_with_cors_cache};
 use net::http_loader::HttpState;
@@ -776,8 +776,7 @@ fn test_fetch_with_devtools() {
     let (mut server, url) = make_server(handler);
 
     let origin = Origin::Origin(url.origin());
-    let pipeline_id = PipelineId::fake_root_pipeline_id();
-    let request = Request::new(url.clone(), Some(origin), false, Some(pipeline_id));
+    let request = Request::new(url.clone(), Some(origin), false, Some(TEST_PIPELINE_ID));
     *request.referrer.borrow_mut() = Referrer::NoReferrer;
 
     let (devtools_chan, devtools_port) = channel::<DevtoolsControlMsg>();
@@ -815,7 +814,7 @@ fn test_fetch_with_devtools() {
         method: Method::Get,
         headers: headers,
         body: None,
-        pipeline_id: pipeline_id,
+        pipeline_id: TEST_PIPELINE_ID,
         startedDateTime: devhttprequest.startedDateTime,
         timeStamp: devhttprequest.timeStamp,
         connect_time: devhttprequest.connect_time,
@@ -832,7 +831,7 @@ fn test_fetch_with_devtools() {
         headers: Some(response_headers),
         status: Some((200, b"OK".to_vec())),
         body: None,
-        pipeline_id: pipeline_id,
+        pipeline_id: TEST_PIPELINE_ID,
     };
 
     assert_eq!(devhttprequest, httprequest);
diff --git a/tests/unit/net/http_loader.rs b/tests/unit/net/http_loader.rs
index e6545a9c818..aee43ac0e2b 100644
--- a/tests/unit/net/http_loader.rs
+++ b/tests/unit/net/http_loader.rs
@@ -18,7 +18,7 @@ use hyper::http::RawStatus;
 use hyper::method::Method;
 use hyper::mime::{Mime, SubLevel, TopLevel};
 use hyper::status::StatusCode;
-use msg::constellation_msg::{PipelineId, ReferrerPolicy};
+use msg::constellation_msg::{PipelineId, ReferrerPolicy, TEST_PIPELINE_ID};
 use net::cookie::Cookie;
 use net::cookie_storage::CookieStorage;
 use net::hsts::HstsEntry;
@@ -47,7 +47,7 @@ impl LoadOrigin for HttpTest {
         None
     }
     fn pipeline_id(&self) -> Option<PipelineId> {
-        Some(PipelineId::fake_root_pipeline_id())
+        Some(TEST_PIPELINE_ID)
     }
 }
 
@@ -472,8 +472,6 @@ fn test_request_and_response_data_with_network_messages() {
 
     let url = Url::parse("https://mozilla.com").unwrap();
     let (devtools_chan, devtools_port) = mpsc::channel::<DevtoolsControlMsg>();
-    // This will probably have to be changed as it uses fake_root_pipeline_id which is marked for removal.
-    let pipeline_id = PipelineId::fake_root_pipeline_id();
     let mut load_data = LoadData::new(LoadContext::Browsing, url.clone(), &HttpTest);
     let mut request_headers = Headers::new();
     request_headers.set(Host { hostname: "bar.foo".to_owned(), port: None });
@@ -521,7 +519,7 @@ fn test_request_and_response_data_with_network_messages() {
         method: Method::Get,
         headers: headers,
         body: None,
-        pipeline_id: pipeline_id,
+        pipeline_id: TEST_PIPELINE_ID,
         startedDateTime: devhttprequest.startedDateTime,
         timeStamp: devhttprequest.timeStamp,
         connect_time: devhttprequest.connect_time,
@@ -538,7 +536,7 @@ fn test_request_and_response_data_with_network_messages() {
         headers: Some(response_headers),
         status: Some((200, b"OK".to_vec())),
         body: None,
-        pipeline_id: pipeline_id,
+        pipeline_id: TEST_PIPELINE_ID,
     };
 
     assert_eq!(devhttprequest, httprequest);