Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Implement window indexed getter #20615

Merged
merged 5 commits into from May 18, 2018
Merged
Changes from all commits
Commits
File filter...
Filter file types
Jump to…
Jump to file
Failed to load files.

Always

Just for now

@@ -1231,6 +1231,15 @@ impl<Message, LTF, STF> Constellation<Message, LTF, STF>
warn!("Sending reply to get parent info failed ({:?}).", e);
}
}
FromScriptMsg::GetChildBrowsingContextId(browsing_context_id, index, sender) => {
// We increment here because the 0th element is the parent browsing context itself
let result = self.all_descendant_browsing_contexts_iter(browsing_context_id)
.nth(index + 1)

This comment has been minimized.

Copy link
@asajeffrey

asajeffrey May 2, 2018

Member

Doesn't this return the nth descendant rather than the nth child?

This comment has been minimized.

Copy link
@KiChjang

KiChjang May 3, 2018

Author Member

Hmm, I can't quite parse the following excerpt from the spec:

  1. Set value to the WindowProxy object of the indexth document-tree child browsing context of document's browsing context, sorted in the order that their browsing context container elements were most recently inserted into document, the WindowProxy object of the most recently inserted browsing context container's nested browsing context being last.

Initially I believed that this meant the child's child browsing context are ordered right after the child browsing context itself, which corresponds exactly to the ordering we have in AllDescendantBrowsingContextsIter, since the grandchild is inserted before the child's next sibling; however, the spec also mentions the document-tree child browsing context of document's browsing context, which may mean that we only care about child browsing context only, and (great-to-the-nth) grandchildren browsing contexts do not matter?

.map(|bc| bc.id);
if let Err(e) = sender.send(result) {
warn!("Sending reply to get child browsing context ID failed ({:?}).", e);
}
}
FromScriptMsg::RegisterServiceWorker(scope_things, scope) => {
debug!("constellation got store registration scope message");
self.handle_register_serviceworker(scope_things, scope);
@@ -77,22 +77,26 @@ impl DissimilarOriginWindow {
pub fn origin(&self) -> &MutableOrigin {
self.upcast::<GlobalScope>().origin()
}

pub fn window_proxy(&self) -> DomRoot<WindowProxy> {
DomRoot::from_ref(&*self.window_proxy)
}
}

impl DissimilarOriginWindowMethods for DissimilarOriginWindow {
// https://html.spec.whatwg.org/multipage/#dom-window
fn Window(&self) -> DomRoot<WindowProxy> {
DomRoot::from_ref(&*self.window_proxy)
self.window_proxy()
}

// https://html.spec.whatwg.org/multipage/#dom-self
fn Self_(&self) -> DomRoot<WindowProxy> {
DomRoot::from_ref(&*self.window_proxy)
self.window_proxy()
}

// https://html.spec.whatwg.org/multipage/#dom-frames
fn Frames(&self) -> DomRoot<WindowProxy> {
DomRoot::from_ref(&*self.window_proxy)
self.window_proxy()
}

// https://html.spec.whatwg.org/multipage/#dom-parent
@@ -3624,7 +3624,7 @@ impl DocumentMethods for Document {
let y = *y as f32;
let point = &Point2D::new(x, y);
let window = window_from_node(self);
let viewport = window.window_size().unwrap().initial_viewport;
let viewport = window.window_size()?.initial_viewport;

if self.browsing_context().is_none() {
return None;
@@ -3658,7 +3658,10 @@ impl DocumentMethods for Document {
let y = *y as f32;
let point = &Point2D::new(x, y);
let window = window_from_node(self);
let viewport = window.window_size().unwrap().initial_viewport;
let viewport = match window.window_size() {
Some(size) => size.initial_viewport,
None => return vec![]
};

if self.browsing_context().is_none() {
return vec!();
@@ -1634,11 +1634,6 @@ impl Window {
had_clip_rect
}

// https://html.spec.whatwg.org/multipage/#accessing-other-browsing-contexts
pub fn IndexedGetter(&self, _index: u32, _found: &mut bool) -> Option<DomRoot<Window>> {
None
}

pub fn suspend(&self) {
// Suspend timer events.
self.upcast::<GlobalScope>().suspend();
@@ -17,11 +17,12 @@ use dom::element::Element;
use dom::globalscope::GlobalScope;
use dom::window::Window;
use dom_struct::dom_struct;
use ipc_channel::ipc;
use js::JSCLASS_IS_GLOBAL;
use js::glue::{CreateWrapperProxyHandler, ProxyTraps};
use js::glue::{GetProxyPrivate, SetProxyExtra, GetProxyExtra};
use js::jsapi::{JSAutoCompartment, JSContext, JSErrNum, JSFreeOp, JSObject};
use js::jsapi::{JSPROP_READONLY, JSTracer, JS_DefinePropertyById};
use js::jsapi::{JSPROP_ENUMERATE, JSPROP_READONLY, JSTracer, JS_DefinePropertyById};
use js::jsapi::{JS_ForwardGetPropertyTo, JS_ForwardSetPropertyTo};
use js::jsapi::{JS_HasPropertyById, JS_HasOwnPropertyById};
use js::jsapi::{JS_IsExceptionPending, JS_GetOwnPropertyDescriptorById};
@@ -40,6 +41,8 @@ use js::rust::wrappers::{NewWindowProxy, SetWindowProxy, JS_TransplantObject};
use msg::constellation_msg::BrowsingContextId;
use msg::constellation_msg::PipelineId;
use msg::constellation_msg::TopLevelBrowsingContextId;
use script_thread::ScriptThread;
use script_traits::ScriptMsg;
use std::cell::Cell;
use std::ptr;

@@ -301,17 +304,45 @@ impl WindowProxy {

// This is only called from extern functions,
// there's no use using the lifetimed handles here.
// https://html.spec.whatwg.org/multipage/#accessing-other-browsing-contexts
#[allow(unsafe_code)]
unsafe fn GetSubframeWindow(cx: *mut JSContext,
proxy: RawHandleObject,
id: RawHandleId)
-> Option<DomRoot<Window>> {
unsafe fn GetSubframeWindowProxy(
cx: *mut JSContext,
proxy: RawHandleObject,
id: RawHandleId
) -> Option<(DomRoot<WindowProxy>, u32)> {

This comment has been minimized.

Copy link
@asajeffrey

asajeffrey May 2, 2018

Member

What is the intended semantics of the u32?

This comment has been minimized.

Copy link
@KiChjang

KiChjang May 3, 2018

Author Member

That's the property descriptor's attributes, i.e. is it enumerable? Is it read-only? Or is it configurable?

let index = get_array_index_from_id(cx, Handle::from_raw(id));
if let Some(index) = index {
rooted!(in(cx) let target = GetProxyPrivate(*proxy).to_object());
let win = root_from_handleobject::<Window>(target.handle()).unwrap();
let mut found = false;
return win.IndexedGetter(index, &mut found);
if let Ok(win) = root_from_handleobject::<Window>(target.handle()) {
let browsing_context_id = win.window_proxy().browsing_context_id();
let (result_sender, result_receiver) = ipc::channel().unwrap();

let _ = win.upcast::<GlobalScope>().script_to_constellation_chan().send(
ScriptMsg::GetChildBrowsingContextId(
browsing_context_id,
index as usize,
result_sender
)
);
return result_receiver.recv().ok()
.and_then(|maybe_bcid| maybe_bcid)
.and_then(ScriptThread::find_window_proxy)
.map(|proxy| (proxy, JSPROP_ENUMERATE | JSPROP_READONLY));

This comment has been minimized.

Copy link
@asajeffrey

asajeffrey May 2, 2018

Member

I don't see why being dissimilar-origin or not should make a difference to the props. Is there a spec reason why it does?

This comment has been minimized.

Copy link
@KiChjang
} else if let Ok(win) = root_from_handleobject::<DissimilarOriginWindow>(target.handle()) {
let browsing_context_id = win.window_proxy().browsing_context_id();
let (result_sender, result_receiver) = ipc::channel().unwrap();

let _ = win.global().script_to_constellation_chan().send(ScriptMsg::GetChildBrowsingContextId(
browsing_context_id,
index as usize,
result_sender
));
return result_receiver.recv().ok()
.and_then(|maybe_bcid| maybe_bcid)
.and_then(ScriptThread::find_window_proxy)
.map(|proxy| (proxy, JSPROP_READONLY));
}
}

None
@@ -323,12 +354,12 @@ unsafe extern "C" fn getOwnPropertyDescriptor(cx: *mut JSContext,
id: RawHandleId,
mut desc: RawMutableHandle<PropertyDescriptor>)
-> bool {
let window = GetSubframeWindow(cx, proxy, id);
if let Some(window) = window {
let window = GetSubframeWindowProxy(cx, proxy, id);
if let Some((window, attrs)) = window {
rooted!(in(cx) let mut val = UndefinedValue());
window.to_jsval(cx, val.handle_mut());
desc.value = val.get();
fill_property_descriptor(MutableHandle::from_raw(desc), proxy.get(), JSPROP_READONLY);
fill_property_descriptor(MutableHandle::from_raw(desc), proxy.get(), attrs);
return true;
}

@@ -372,7 +403,7 @@ unsafe extern "C" fn has(cx: *mut JSContext,
id: RawHandleId,
bp: *mut bool)
-> bool {
let window = GetSubframeWindow(cx, proxy, id);
let window = GetSubframeWindowProxy(cx, proxy, id);
if window.is_some() {
*bp = true;
return true;
@@ -395,8 +426,8 @@ unsafe extern "C" fn get(cx: *mut JSContext,
id: RawHandleId,
vp: RawMutableHandleValue)
-> bool {
let window = GetSubframeWindow(cx, proxy, id);
if let Some(window) = window {
let window = GetSubframeWindowProxy(cx, proxy, id);
if let Some((window, _attrs)) = window {
window.to_jsval(cx, MutableHandle::from_raw(vp));
return true;
}
@@ -90,6 +90,8 @@ pub enum ScriptMsg {
GetBrowsingContextId(PipelineId, IpcSender<Option<BrowsingContextId>>),
/// Get the parent info for a given pipeline.
GetParentInfo(PipelineId, IpcSender<Option<PipelineId>>),
/// Get the nth child browsing context ID for a given browsing context, sorted in tree order.
GetChildBrowsingContextId(BrowsingContextId, usize, IpcSender<Option<BrowsingContextId>>),
/// <head> tag finished parsing
HeadParsed,
/// All pending loads are complete, and the `load` event for this pipeline

This file was deleted.

@@ -1,62 +1,5 @@
[current-realm.html]
type: testharness
[querySelectorAll]
expected: FAIL

[createElement]
expected: FAIL

[createElementNS]
expected: FAIL

[createDocumentFragment]
expected: FAIL

[createTextNode]
expected: FAIL

[createComment]
expected: FAIL

[createProcessingInstruction]
expected: FAIL

[createAttribute]
expected: FAIL

[createAttributeNS]
expected: FAIL

[createEvent]
expected: FAIL

[createRange]
expected: FAIL

[createNodeIterator]
expected: FAIL

[createTreeWalker]
expected: FAIL

[getElementsByTagName]
expected: FAIL

[getElementsByTagNameNS]
expected: FAIL

[getElementsByClassName]
expected: FAIL

[createDocumentType]
expected: FAIL

[createDocument]
expected: FAIL

[createHTMLDocument]
expected: FAIL

[NamedNodeMap item]
expected: FAIL

@@ -66,24 +9,6 @@
[NamedNodeMap getNamedItemNS]
expected: FAIL

[splitText]
expected: FAIL

[extractContents]
expected: FAIL

[cloneContents]
expected: FAIL

[cloneRange]
expected: FAIL

[getContext 2d]
expected: FAIL

[getContext webgl]
expected: FAIL

[createImageData]
expected: FAIL

@@ -1,7 +1,4 @@
[elementFromPoint.html]
[co-ordinates larger than the viewport from in iframe]
expected: FAIL

[SVG element at x,y]
expected: FAIL

@@ -1,13 +1,7 @@
[elementsFromPoint.html]
[co-ordinates larger than the viewport from in iframe]
expected: FAIL

type: testharness
[SVG element at x,y]
expected: FAIL

[transformed element at x,y]
expected: FAIL

[no hit target at x,y]
expected: FAIL

@@ -1,5 +1,20 @@
[matchMedia.xht]
type: testharness
[CSS Test: CSSOM View matchMedia and MediaQueryList]
expected: TIMEOUT
[window.matchMedia exists]
expected: FAIL
[MediaQueryList.matches for "(max-width: 199px), all and (min-width: 200px)"]
expected: FAIL
[MediaQueryList.matches for "(min-aspect-ratio: 1/1)"]
expected: FAIL
[MediaQueryList.matches for "(width: 200px)"]
expected: FAIL
[MediaQueryList.matches for "(min-width: 150px)"]
expected: FAIL
[Resize iframe from 200x100 to 200x50, then to 100x50]

This comment has been minimized.

Copy link
@asajeffrey

asajeffrey May 2, 2018

Member

Tests that used to pass now fail?

This comment has been minimized.

Copy link
@KiChjang

KiChjang May 3, 2018

Author Member

This is the same reasoning as the one above -- the window_size for iframes are None, so these tests are failing.

This comment has been minimized.

Copy link
@emilio

emilio May 3, 2018

Member

That means that they haven't been synced properly, I don't think it's acceptable to regress elementsFromPoint et. all.

This comment has been minimized.

Copy link
@KiChjang

KiChjang May 5, 2018

Author Member

Oh, it looks like new iframes aren't set with any window size at all! Let me try and dig through the spec and see whether it specifies a point in time where they're supposed to be set...

expected: NOTRUN
[Listeners are called in the order which they have been added]
expected: NOTRUN
[Listener added twice is only called once.]
expected: NOTRUN

This file was deleted.

This file was deleted.

This file was deleted.

This file was deleted.

ProTip! Use n and p to navigate between commits in a pull request.
You can’t perform that action at this time.