Implement window.frameElement, change window.parent to make use of it.

components/compositing/constellation.rs

@@ -191,7 +191,7 @@ impl FrameTreeTraversal for Rc<FrameTree> {
 
     /// Returns the frame tree whose subpage is id
     fn find_with_subpage_id(&self, id: Option<SubpageId>) -> Option<Rc<FrameTree>> {
-        self.iter().find(|frame_tree| id == frame_tree.pipeline.borrow().subpage_id)
+        self.iter().find(|frame_tree| id == frame_tree.pipeline.borrow().subpage_id())
     }
 
     /// Replaces a node of the frame tree in place. Returns the node that was removed or the
@@ -392,14 +392,12 @@ impl<LTF: LayoutTaskFactory, STF: ScriptTaskFactory> Constellation<LTF, STF> {
     /// Helper function for creating a pipeline
     fn new_pipeline(&mut self,
                     id: PipelineId,
-                    subpage_id: Option<SubpageId>,
+                    parent: Option<(PipelineId, SubpageId)>,
-                    parent_id: Option<PipelineId>,
                     script_pipeline: Option<Rc<Pipeline>>,
                     load_data: LoadData)
                     -> Rc<Pipeline> {
         let pipe = Pipeline::create::<LTF, STF>(id,
-                                                subpage_id,
+                                                parent,
-                                                parent_id,
                                                 self.chan.clone(),
                                                 self.compositor_proxy.clone_compositor_proxy(),
                                                 self.devtools_chan.clone(),
@@ -454,8 +452,8 @@ impl<LTF: LayoutTaskFactory, STF: ScriptTaskFactory> Constellation<LTF, STF> {
                 self.handle_exit();
                 return false;
             }
-            ConstellationMsg::Failure(Failure { pipeline_id, subpage_id }) => {
+            ConstellationMsg::Failure(Failure { pipeline_id, parent }) => {
-                self.handle_failure_msg(pipeline_id, subpage_id);
+                self.handle_failure_msg(pipeline_id, parent);
             }
             // This should only be called once per constellation, and only by the browser
             ConstellationMsg::InitLoadUrl(url) => {
@@ -530,8 +528,8 @@ impl<LTF: LayoutTaskFactory, STF: ScriptTaskFactory> Constellation<LTF, STF> {
         self.compositor_proxy.send(CompositorMsg::ShutdownComplete);
     }
 
-    fn handle_failure_msg(&mut self, pipeline_id: PipelineId, subpage_id: Option<SubpageId>) {
+    fn handle_failure_msg(&mut self, pipeline_id: PipelineId, parent: Option<(PipelineId, SubpageId)>) {
-        debug!("handling failure message from pipeline {:?}, {:?}", pipeline_id, subpage_id);
+        debug!("handling failure message from pipeline {:?}, {:?}", pipeline_id, parent);
 
         if opts::get().hard_fail {
             // It's quite difficult to make Servo exit cleanly if some tasks have failed.
@@ -572,7 +570,7 @@ impl<LTF: LayoutTaskFactory, STF: ScriptTaskFactory> Constellation<LTF, STF> {
 
         let new_id = self.get_next_pipeline_id();
         let new_frame_id = self.get_next_frame_id();
-        let pipeline = self.new_pipeline(new_id, subpage_id, None, None,
+        let pipeline = self.new_pipeline(new_id, parent, None,
                                          LoadData::new(Url::parse("about:failure").unwrap()));
 
         self.browse(Some(pipeline_id),
@@ -599,7 +597,7 @@ impl<LTF: LayoutTaskFactory, STF: ScriptTaskFactory> Constellation<LTF, STF> {
     fn handle_init_load(&mut self, url: Url) {
         let next_pipeline_id = self.get_next_pipeline_id();
         let next_frame_id = self.get_next_frame_id();
-        let pipeline = self.new_pipeline(next_pipeline_id, None, None, None, LoadData::new(url));
+        let pipeline = self.new_pipeline(next_pipeline_id, None, None, LoadData::new(url));
         self.browse(None,
                     Rc::new(FrameTree::new(next_frame_id, pipeline.clone(), None)),
                     NavigationType::Load);
@@ -614,7 +612,7 @@ impl<LTF: LayoutTaskFactory, STF: ScriptTaskFactory> Constellation<LTF, STF> {
         // Returns true if a child frame tree's subpage id matches the given subpage id
         let subpage_eq = |&:child_frame_tree: & &mut ChildFrameTree| {
             child_frame_tree.frame_tree.pipeline.borrow().
-                subpage_id.expect("Constellation:
+                subpage_id().expect("Constellation:
                 child frame does not have a subpage id. This should not be possible.")
                 == subpage_id
         };
@@ -784,8 +782,7 @@ impl<LTF: LayoutTaskFactory, STF: ScriptTaskFactory> Constellation<LTF, STF> {
         let new_frame_pipeline_id = self.get_next_pipeline_id();
         let pipeline = self.new_pipeline(
             new_frame_pipeline_id,
-            Some(new_subpage_id),
+            Some((containing_page_pipeline_id, new_subpage_id)),
-            Some(containing_page_pipeline_id),
             script_pipeline,
             LoadData::new(url)
         );
@@ -829,11 +826,10 @@ impl<LTF: LayoutTaskFactory, STF: ScriptTaskFactory> Constellation<LTF, STF> {
         // changes would be overridden by changing the subframe associated with source_id.
 
         let parent = source_frame.parent.clone();
-        let subpage_id = source_frame.pipeline.borrow().subpage_id;
+        let parent_id = source_frame.pipeline.borrow().parent;
-        let parent_id = source_frame.pipeline.borrow().parent_id;
         let next_pipeline_id = self.get_next_pipeline_id();
         let next_frame_id = self.get_next_frame_id();
-        let pipeline = self.new_pipeline(next_pipeline_id, subpage_id, parent_id, None, load_data);
+        let pipeline = self.new_pipeline(next_pipeline_id, parent_id, None, load_data);
         self.browse(Some(source_id),
                     Rc::new(FrameTree::new(next_frame_id,
                                            pipeline.clone(),
@@ -979,7 +975,7 @@ impl<LTF: LayoutTaskFactory, STF: ScriptTaskFactory> Constellation<LTF, STF> {
                         // Add to_add to parent's children, if it is not the root
                         let parent = &to_add.parent;
                         for parent in parent.borrow().iter() {
-                            let subpage_id = to_add.pipeline.borrow().subpage_id
+                            let subpage_id = to_add.pipeline.borrow().subpage_id()
                                 .expect("Constellation:
                                 Child frame's subpage id is None. This should be impossible.");
                             let rect = self.pending_sizes.remove(&(parent.id, subpage_id));

components/compositing/pipeline.rs

@@ -23,8 +23,7 @@ use std::sync::mpsc::{Receiver, channel};
 /// A uniquely-identifiable pipeline of script task, layout task, and paint task.
 pub struct Pipeline {
     pub id: PipelineId,
-    pub subpage_id: Option<SubpageId>,
+    pub parent: Option<(PipelineId, SubpageId)>,
-    pub parent_id: Option<PipelineId>,
     pub script_chan: ScriptControlChan,
     pub layout_chan: LayoutControlChan,
     pub paint_chan: PaintChan,
@@ -49,8 +48,7 @@ impl Pipeline {
     /// Returns the channels wrapped in a struct.
     /// If script_pipeline is not None, then subpage_id must also be not None.
     pub fn create<LTF,STF>(id: PipelineId,
-                           subpage_id: Option<SubpageId>,
+                           parent: Option<(PipelineId, SubpageId)>,
-                           parent_id: Option<PipelineId>,
                            constellation_chan: ConstellationChan,
                            compositor_proxy: Box<CompositorProxy+'static+Send>,
                            devtools_chan: Option<DevtoolsControlChan>,
@@ -72,7 +70,7 @@ impl Pipeline {
 
         let failure = Failure {
             pipeline_id: id,
-            subpage_id: subpage_id,
+            parent: parent,
         };
 
         let script_chan = match script_pipeline {
@@ -97,7 +95,7 @@ impl Pipeline {
                 let new_layout_info = NewLayoutInfo {
                     old_pipeline_id: spipe.id.clone(),
                     new_pipeline_id: id,
-                    subpage_id: subpage_id.expect("script_pipeline != None but subpage_id == None"),
+                    subpage_id: parent.expect("script_pipeline != None but subpage_id == None").1,
                     layout_chan: ScriptTaskFactory::clone_layout_channel(None::<&mut STF>, &layout_pair),
                 };
 
@@ -131,8 +129,7 @@ impl Pipeline {
                                   layout_shutdown_chan);
 
         Pipeline::new(id,
-                      subpage_id,
+                      parent,
-                      parent_id,
                       script_chan,
                       LayoutControlChan(pipeline_chan),
                       paint_chan,
@@ -142,8 +139,7 @@ impl Pipeline {
     }
 
     pub fn new(id: PipelineId,
-               subpage_id: Option<SubpageId>,
+               parent: Option<(PipelineId, SubpageId)>,
-               parent_id: Option<PipelineId>,
                script_chan: ScriptControlChan,
                layout_chan: LayoutControlChan,
                paint_chan: PaintChan,
@@ -153,8 +149,7 @@ impl Pipeline {
                -> Pipeline {
         Pipeline {
             id: id,
-            subpage_id: subpage_id,
+            parent: parent,
-            parent_id: parent_id,
             script_chan: script_chan,
             layout_chan: layout_chan,
             paint_chan: paint_chan,
@@ -167,7 +162,9 @@ impl Pipeline {
 
     pub fn load(&self) {
         let ScriptControlChan(ref chan) = self.script_chan;
-        chan.send(ConstellationControlMsg::Load(self.id, self.parent_id, self.load_data.clone())).unwrap();
+        chan.send(ConstellationControlMsg::Load(self.id,
+                                                self.parent,
+                                                self.load_data.clone())).unwrap();
     }
 
     pub fn grant_paint_permission(&self) {
@@ -212,4 +209,8 @@ impl Pipeline {
             paint_chan: self.paint_chan.clone(),
         }
     }
+
+    pub fn subpage_id(&self) -> Option<SubpageId> {
+      self.parent.map(|parent| parent.1)
+    }
 }

components/msg/constellation_msg.rs

@@ -36,7 +36,7 @@ pub enum IFrameSandboxState {
 #[derive(Clone, Copy)]
 pub struct Failure {
     pub pipeline_id: PipelineId,
-    pub subpage_id: Option<SubpageId>,
+    pub parent: Option<(PipelineId, SubpageId)>,
 }
 
 #[derive(Copy)]

components/script/dom/browsercontext.rs

@@ -11,6 +11,7 @@ use dom::bindings::proxyhandler::{get_property_descriptor, fill_property_descrip
 use dom::bindings::utils::{Reflectable, WindowProxyHandler};
 use dom::bindings::utils::get_array_index_from_id;
 use dom::document::{Document, DocumentHelpers};
+use dom::element::Element;
 use dom::window::Window;
 use dom::window::WindowHelpers;
 
@@ -33,16 +34,16 @@ pub struct BrowserContext {
     history: Vec<SessionHistoryEntry>,
     active_index: uint,
     window_proxy: *mut JSObject,
-    parent: Option<JS<Window>>,
+    frame_element: Option<JS<Element>>,
 }
 
 impl BrowserContext {
-    pub fn new(document: JSRef<Document>, parent: Option<JSRef<Window>>) -> BrowserContext {
+    pub fn new(document: JSRef<Document>, frame_element: Option<JSRef<Element>>) -> BrowserContext {
         let mut context = BrowserContext {
             history: vec!(SessionHistoryEntry::new(document)),
             active_index: 0,
             window_proxy: ptr::null_mut(),
-            parent: parent.map(|p| JS::from_rooted(p)),
+            frame_element: frame_element.map(JS::from_rooted),
         };
         context.create_window_proxy();
         context
@@ -57,17 +58,15 @@ impl BrowserContext {
         doc.r().window()
     }
 
+    pub fn frame_element(&self) -> Option<Temporary<Element>> {
+        self.frame_element.map(Temporary::new)
+    }
+
     pub fn window_proxy(&self) -> *mut JSObject {
         assert!(!self.window_proxy.is_null());
         self.window_proxy
     }
 
-    pub fn parent(&self) -> Option<Temporary<Window>> {
-        self.parent.map(|p| {
-            p.root().r().browser_context().as_ref().unwrap().active_window()
-        })
-    }
-
     #[allow(unsafe_blocks)]
     fn create_window_proxy(&mut self) {
         let win = self.active_window().root();

components/script/dom/webidls/Window.webidl

@@ -36,7 +36,7 @@
   //         attribute any opener;
   //readonly attribute WindowProxy parent;
   readonly attribute Window parent;
-  //readonly attribute Element? frameElement;
+  readonly attribute Element? frameElement;
   //WindowProxy open(optional DOMString url = "about:blank", optional DOMString target = "_blank", optional DOMString features = "", optional boolean replace = false);
   //getter WindowProxy (unsigned long index);
   //getter object (DOMString name);

components/script/dom/window.rs

@@ -18,9 +18,11 @@ use dom::bindings::utils::Reflectable;
 use dom::browsercontext::BrowserContext;
 use dom::console::Console;
 use dom::document::Document;
+use dom::element::Element;
 use dom::eventtarget::{EventTarget, EventTargetHelpers, EventTargetTypeId};
 use dom::location::Location;
 use dom::navigator::Navigator;
+use dom::node::window_from_node;
 use dom::performance::Performance;
 use dom::screen::Screen;
 use dom::storage::Storage;
@@ -68,7 +70,7 @@ pub struct Window {
     navigation_start_precise: f64,
     screen: MutNullableJS<Screen>,
     session_storage: MutNullableJS<Storage>,
-    timers: TimerManager
+    timers: TimerManager,
 }
 
 impl Window {
@@ -215,6 +217,10 @@ impl<'a> WindowMethods for JSRef<'a, Window> {
         self.console.or_init(|| Console::new(GlobalRef::Window(self)))
     }
 
+    fn GetFrameElement(self) -> Option<Temporary<Element>> {
+        self.browser_context().as_ref().unwrap().frame_element()
+    }
+
     fn Navigator(self) -> Temporary<Navigator> {
         self.navigator.or_init(|| Navigator::new(self))
     }
@@ -278,7 +284,14 @@ impl<'a> WindowMethods for JSRef<'a, Window> {
 
     // https://html.spec.whatwg.org/multipage/browsers.html#dom-parent
     fn Parent(self) -> Temporary<Window> {
-        self.browser_context().as_ref().unwrap().parent().unwrap_or(self.Window())
+        let browser_context = self.browser_context();
+        let browser_context = browser_context.as_ref().unwrap();
+
+        browser_context.frame_element().map_or(self.Window(), |fe| {
+            let frame_element = fe.root();
+            let window = window_from_node(frame_element.r()).root();
+            window.r().browser_context().as_ref().unwrap().active_window()
+        })
     }
 
     fn Performance(self) -> Temporary<Performance> {
@@ -318,7 +331,7 @@ impl<'a> WindowMethods for JSRef<'a, Window> {
 
 pub trait WindowHelpers {
     fn flush_layout(self, goal: ReflowGoal, query: ReflowQueryType);
-    fn init_browser_context(self, doc: JSRef<Document>, parent: Option<JSRef<Window>>);
+    fn init_browser_context(self, doc: JSRef<Document>, frame_element: Option<JSRef<Element>>);
     fn load_url(self, href: DOMString);
     fn handle_fire_timer(self, timer_id: TimerId);
     fn IndexedGetter(self, _index: u32, _found: &mut bool) -> Option<Temporary<Window>>;
@@ -361,8 +374,8 @@ impl<'a> WindowHelpers for JSRef<'a, Window> {
         self.page().flush_layout(goal, query);
     }
 
-    fn init_browser_context(self, doc: JSRef<Document>, parent: Option<JSRef<Window>>) {
+    fn init_browser_context(self, doc: JSRef<Document>, frame_element: Option<JSRef<Element>>) {
-        *self.browser_context.borrow_mut() = Some(BrowserContext::new(doc, parent));
+        *self.browser_context.borrow_mut() = Some(BrowserContext::new(doc, frame_element));
     }
 
     /// Commence a new URL load which will either replace this window or scroll to a fragment.
@@ -413,7 +426,7 @@ impl Window {
             navigation_start_precise: time::precise_time_ns() as f64,
             screen: Default::default(),
             session_storage: Default::default(),
-            timers: TimerManager::new()
+            timers: TimerManager::new(),
         };
 
         WindowBinding::Wrap(cx, win)

components/script/script_task.rs

@@ -12,7 +12,7 @@ use dom::bindings::codegen::Bindings::DocumentBinding::{DocumentMethods, Documen
 use dom::bindings::codegen::Bindings::EventBinding::EventMethods;
 use dom::bindings::codegen::Bindings::EventTargetBinding::EventTargetMethods;
 use dom::bindings::codegen::Bindings::WindowBinding::WindowMethods;
-use dom::bindings::codegen::InheritTypes::{ElementCast, EventTargetCast, NodeCast, EventCast};
+use dom::bindings::codegen::InheritTypes::{ElementCast, EventTargetCast, HTMLIFrameElementCast, NodeCast, EventCast};
 use dom::bindings::conversions::FromJSValConvertible;
 use dom::bindings::conversions::StringificationBehavior;
 use dom::bindings::global::GlobalRef;
@@ -28,6 +28,7 @@ use dom::event::{Event, EventHelpers, EventBubbles, EventCancelable};
 use dom::uievent::UIEvent;
 use dom::eventtarget::{EventTarget, EventTargetHelpers};
 use dom::htmlelement::HTMLElementTypeId;
+use dom::htmliframeelement::HTMLIFrameElement;
 use dom::keyboardevent::KeyboardEvent;
 use dom::mouseevent::MouseEvent;
 use dom::node::{self, Node, NodeHelpers, NodeDamage, NodeTypeId};
@@ -53,7 +54,7 @@ use script_traits::ScriptTaskFactory;
 use servo_msg::compositor_msg::ReadyState::{FinishedLoading, Loading, PerformingLayout};
 use servo_msg::compositor_msg::{LayerId, ScriptListener};
 use servo_msg::constellation_msg::{ConstellationChan};
-use servo_msg::constellation_msg::{LoadData, NavigationDirection, PipelineId};
+use servo_msg::constellation_msg::{LoadData, NavigationDirection, PipelineId, SubpageId};
 use servo_msg::constellation_msg::{Failure, Msg, WindowSizeData, Key, KeyState};
 use servo_msg::constellation_msg::{KeyModifiers, SUPER, SHIFT, CONTROL, ALT};
 use servo_msg::constellation_msg::{PipelineExitType};
@@ -586,8 +587,8 @@ impl ScriptTask {
         match msg {
             ConstellationControlMsg::AttachLayout(_) =>
                 panic!("should have handled AttachLayout already"),
-            ConstellationControlMsg::Load(id, parent_id, load_data) =>
+            ConstellationControlMsg::Load(id, parent, load_data) =>
-                self.load(id, parent_id, load_data),
+                self.load(id, parent, load_data),
             ConstellationControlMsg::SendEvent(id, event) =>
                 self.handle_event(id, event),
             ConstellationControlMsg::ReflowComplete(id, reflow_id) =>
@@ -767,13 +768,14 @@ impl ScriptTask {
 
     /// The entry point to document loading. Defines bindings, sets up the window and document
     /// objects, parses HTML and CSS, and kicks off initial layout.
-    fn load(&self, pipeline_id: PipelineId, parent_id: Option<PipelineId>, load_data: LoadData) {
+    fn load(&self, pipeline_id: PipelineId,
+            parent: Option<(PipelineId, SubpageId)>, load_data: LoadData) {
         let url = load_data.url.clone();
         debug!("ScriptTask: loading {:?} on page {:?}", url, pipeline_id);
 
         let borrowed_page = self.page.borrow_mut();
 
-        let parent_window = parent_id.and_then(|pid| {
+        let frame_element = parent.and_then(|(parent_id, subpage_id)| {
           // In the case a parent id exists but the matching page
           // cannot be found, this means the page exists in a different
           // script task (due to origin) so it shouldn't be returned.
@@ -781,10 +783,20 @@ impl ScriptTask {
           // case, which is wrong. We should be returning an object that
           // denies access to most properties (per
           // https://github.com/servo/servo/issues/3939#issuecomment-62287025).
-            borrowed_page.find(pid).map(|page| {
+          borrowed_page.find(parent_id).and_then(|page| {
-                page.frame.borrow().as_ref().unwrap().window.root()
+            let match_iframe = |&:&node: &JSRef<HTMLIFrameElement>| {
+              node.subpage_id().map_or(false, |id| id == subpage_id)
+            };
+
+            let doc = page.frame().as_ref().unwrap().document.root();
+            let doc: JSRef<Node> = NodeCast::from_ref(doc.r());
+
+            doc.traverse_preorder()
+               .filter_map(|node| HTMLIFrameElementCast::to_ref(node))
+               .find(match_iframe)
+               .map(|node| Temporary::from_rooted(ElementCast::from_ref(node)))
           })
-        });
+        }).root();
 
         let page = borrowed_page.find(pipeline_id).expect("ScriptTask: received a load
             message for a layout channel that is not associated with this script task. This
@@ -865,7 +877,7 @@ impl ScriptTask {
         if let Some(tm) = last_modified {
             document.r().set_last_modified(dom_last_modified(&tm));
         }
-        window.r().init_browser_context(document.r(), parent_window.r());
+        window.r().init_browser_context(document.r(), frame_element.r());
 
 
         {

components/script_traits/lib.rs

@@ -56,7 +56,7 @@ pub struct NewLayoutInfo {
 /// Messages sent from the constellation to the script task
 pub enum ConstellationControlMsg {
     /// Loads a new URL on the specified pipeline.
-    Load(PipelineId, Option<PipelineId>, LoadData),
+    Load(PipelineId, Option<(PipelineId, SubpageId)>, LoadData),
     /// Gives a channel and ID to a layout task, as well as the ID of that layout's parent
     AttachLayout(NewLayoutInfo),
     /// Window resized.  Sends a DOM event eventually, but first we combine events.

tests/wpt/metadata/html/browsers/windows/nested-browsing-contexts/frameElement.sub.html.ini

@@ -6,7 +6,3 @@
 
   [The SecurityError must be thrown if the container\'s document does not have the same effective script origin]
     expected: FAIL
-
-  [The window\'s frameElement attribute must return null if it is not a nested browsing context]
-    expected: FAIL
-

tests/wpt/metadata/html/dom/interfaces.html.ini

@@ -8292,9 +8292,6 @@
   [Window interface: window must inherit property "parent" with the proper type (22)]
     expected: FAIL
 
-  [Window interface: window must inherit property "frameElement" with the proper type (23)]
-    expected: FAIL
-
   [Window interface: window must inherit property "open" with the proper type (24)]
     expected: FAIL