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

Get rid of dom::bindings::global #13596

Merged
merged 66 commits into from Oct 7, 2016
Merged

Get rid of dom::bindings::global #13596

merged 66 commits into from Oct 7, 2016

Conversation

@nox
Copy link
Member

nox commented Oct 5, 2016

Globals in that PR are now represented by the fake IDL interface GlobalScope.


This change is Reviewable

@highfive
Copy link

highfive commented Oct 5, 2016

Heads up! This PR modifies the following files:

  • @KiChjang: components/script/dom/request.rs, components/script/dom/workerlocation.rs, components/script/dom/textencoder.rs, components/script/dom/dompoint.rs, components/script/dom/webglframebuffer.rs, components/script/dom/crypto.rs, components/script/dom/client.rs, components/script/dom/focusevent.rs, components/script/dom/bindings/codegen/parser/inline.patch, components/script/dom/htmlcanvaselement.rs, components/script/dom/servohtmlparser.rs, components/script/dom/blob.rs, components/script/dom/location.rs, components/script/dom/eventtarget.rs, components/script/timers.rs, components/script/dom/popstateevent.rs, components/script/dom/mimetypearray.rs, components/script/dom/htmlformcontrolscollection.rs, components/script/dom/workerglobalscope.rs, components/script/dom/bindings/error.rs, components/script/dom/pluginarray.rs, components/script/dom/imagedata.rs, components/script/dom/domstringmap.rs, components/script/fetch.rs, components/script/dom/bindings/iterable.rs, components/script/dom/touchlist.rs, components/script/dom/file.rs, components/script/dom/webglbuffer.rs, components/script/dom/htmltextareaelement.rs, components/script/dom/validitystate.rs, components/script/dom/htmldetailselement.rs, components/script/dom/urlsearchparams.rs, components/script/dom/htmlimageelement.rs, components/script/script_thread.rs, components/script/dom/css.rs, components/script/dom/promise.rs, components/script/dom/worker.rs, components/script/dom/attr.rs, components/script/dom/console.rs, components/script/dom/globalscope.rs, components/script/dom/domrectreadonly.rs, components/script/dom/event.rs, components/script/dom/radionodelist.rs, components/script/dom/webglshader.rs, components/script/dom/domrect.rs, components/script/dom/node.rs, components/script/dom/workernavigator.rs, components/script/dom/canvasgradient.rs, components/script/dom/documentfragment.rs, components/script/dom/serviceworkercontainer.rs, components/script/dom/progressevent.rs, components/script/dom/textdecoder.rs, components/script/dom/canvaspattern.rs, components/script/dom/dommatrix.rs, components/script/dom/bluetoothcharacteristicproperties.rs, components/script/dom/mouseevent.rs, components/script/dom/range.rs, components/script/dom/xmldocument.rs, components/script/dom/mod.rs, components/script/dom/pagetransitionevent.rs, components/script/dom/bluetoothuuid.rs, components/script/dom/webidls/WorkerGlobalScope.webidl, components/script/dom/bluetoothdevice.rs, components/script/devtools.rs, components/script/dom/htmllinkelement.rs, components/script/dom/performance.rs, components/script/dom/testbinding.rs, components/script/dom/cssstyledeclaration.rs, components/script/dom/webglrenderingcontext.rs, components/script/dom/browsingcontext.rs, components/script/dom/messageevent.rs, components/script/dom/webglcontextevent.rs, components/script/dom/xmlhttprequest.rs, components/script/dom/htmlmediaelement.rs, components/script/dom/customevent.rs, components/script/dom/uievent.rs, components/script/dom/extendableevent.rs, components/script/dom/history.rs, components/script/webdriver_handlers.rs, components/script/dom/testbindingpairiterable.rs, components/script/dom/namednodemap.rs, components/script/dom/domrectlist.rs, components/script/dom/touch.rs, components/script/dom/webidls/GlobalScope.webidl, components/script/dom/htmlinputelement.rs, components/script/dom/response.rs, components/script/dom/bluetoothadvertisingdata.rs, components/script/dom/domimplementation.rs, components/script/dom/serviceworkerregistration.rs, components/script/dom/domtokenlist.rs, components/script/dom/servoxmlparser.rs, components/script/dom/bindings/reflector.rs, components/script/dom/xmlhttprequestupload.rs, components/script/dom/eventsource.rs, components/script/dom/closeevent.rs, components/script/dom/webglrenderbuffer.rs, components/script/dom/domexception.rs, components/script/dom/htmlcollection.rs, components/script/dom/treewalker.rs, components/script/dom/nodeiterator.rs, components/script/dom/filereader.rs, components/script/dom/beforeunloadevent.rs, components/script/dom/window.rs, components/script/dom/webidls/Window.webidl, components/script/dom/webgluniformlocation.rs, components/script/dom/hashchangeevent.rs, components/script/task_source/mod.rs, components/script/dom/bluetoothremotegattserver.rs, components/script/dom/document.rs, components/script/dom/domquad.rs, components/script/dom/element.rs, components/script/dom/extendablemessageevent.rs, components/script/dom/webglobject.rs, components/script/dom/bindings/codegen/CodegenRust.py, components/script/dom/text.rs, components/script/dom/bindings/codegen/parser/update.sh, components/script/dom/stylesheet.rs, components/script/dom/bluetoothremotegattservice.rs, components/script/dom/url.rs, components/script/dom/filereadersync.rs, components/script/dom/dompointreadonly.rs, components/script/dom/stylesheetlist.rs, components/script/dom/performancetiming.rs, components/script/dom/webglshaderprecisionformat.rs, components/script/dom/storage.rs, components/script/dom/canvasrenderingcontext2d.rs, components/script/dom/keyboardevent.rs, components/script/dom/webglactiveinfo.rs, components/script/task_source/dom_manipulation.rs, components/script/dom/bindings/codegen/parser/WebIDL.py, components/script/dom/storageevent.rs, components/script/dom/bindings/global.rs, components/script/dom/htmloptionscollection.rs, components/script/dom/navigator.rs, components/script/dom/screen.rs, components/script/dom/htmlscriptelement.rs, components/script/dom/bindings/codegen/Configuration.py, components/script/dom/webgltexture.rs, components/script/dom/touchevent.rs, components/script/dom/bindings/callback.rs, components/script/dom/webglprogram.rs, components/script/dom/filelist.rs, components/script/dom/forcetouchevent.rs, components/script/dom/comment.rs, components/script/dom/mediaerror.rs, components/script/dom/bindings/mod.rs, components/script/dom/eventdispatcher.rs, components/script/dom/serviceworker.rs, components/script/dom/errorevent.rs, components/script/dom/dedicatedworkerglobalscope.rs, components/script/dom/nodelist.rs, components/script/dom/promisenativehandler.rs, components/script/dom/headers.rs, components/script/dom/domparser.rs, components/script/dom/bluetooth.rs, components/script/dom/bluetoothremotegattdescriptor.rs, components/script/dom/bluetoothremotegattcharacteristic.rs, components/script/script_runtime.rs, components/script/dom/htmlbodyelement.rs, components/script/dom/htmliframeelement.rs, components/script/dom/dommatrixreadonly.rs, components/script/dom/testbindingiterable.rs, components/script/body.rs, components/net_traits/request.rs, components/net_traits/request.rs, components/script/dom/htmlformelement.rs, components/script/task_source/user_interaction.rs, components/script/dom/serviceworkerglobalscope.rs, components/script/dom/websocket.rs, components/script/dom/bindings/structuredclone.rs, components/script/dom/formdata.rs
  • @emilio: components/script/dom/webglframebuffer.rs, components/script/dom/webglbuffer.rs, components/script/dom/webglshader.rs, components/script/dom/webglrenderingcontext.rs, components/script/dom/webglcontextevent.rs, components/script/dom/webglrenderbuffer.rs, components/script/dom/webgluniformlocation.rs, components/script/dom/webglobject.rs, components/script/dom/webglshaderprecisionformat.rs, components/script/dom/webglactiveinfo.rs, components/script/dom/webgltexture.rs, components/script/dom/webglprogram.rs
@highfive
Copy link

highfive commented Oct 5, 2016

warning Warning warning

  • These commits modify unsafe code. Please review it carefully!
@nox
Copy link
Member Author

nox commented Oct 5, 2016

@jdm
Copy link
Member

jdm commented Oct 5, 2016

For the record, when I glanced through a few of the first and last commits the other day, I was very much in favour of these changes.

@nox
Copy link
Member Author

nox commented Oct 5, 2016

@jdm To be honest, I was expecting the opposite. Woooo! :D

@nox
Copy link
Member Author

nox commented Oct 5, 2016

bors-servo added a commit that referenced this pull request Oct 5, 2016
Get rid of dom::bindings::global

Globals in that PR are now represented by the fake IDL interface `GlobalScope`.

<!-- 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/13596)
<!-- Reviewable:end -->
@bors-servo
Copy link
Contributor

bors-servo commented Oct 5, 2016

Trying commit b644157 with merge 12f8180...

@bors-servo
Copy link
Contributor

bors-servo commented Oct 5, 2016

Copy link
Contributor

Ms2ger left a comment

LGTM, I guess.

@@ -76,7 +76,11 @@ impl File {
// NOTE: Following behaviour might be removed in future,
// see https://github.com/w3c/FileAPI/issues/41
let replaced_filename = DOMString::from_string(filename.replace("/", ":"));
Ok(File::new(global, BlobImpl::new_from_bytes(bytes), replaced_filename, modified, typeString))
Ok(File::new(global.as_global_scope(),
BlobImpl::new_from_bytes(bytes),

This comment has been minimized.

@Ms2ger

Ms2ger Oct 6, 2016

Contributor

Indentation

@@ -308,8 +306,8 @@ impl XMLHttpRequestMethods for XMLHttpRequest {
fn Open_(&self, method: ByteString, url: USVString, async: bool,
username: Option<USVString>, password: Option<USVString>) -> ErrorResult {
// Step 1
match self.global() {
GlobalRoot::Window(ref window) => {
match Root::downcast::<Window>(self.global_scope()) {

This comment has been minimized.

@Ms2ger

Ms2ger Oct 6, 2016

Contributor

if let

match *self.type_id() {
// https://html.spec.whatwg.org/multipage/#script-settings-for-browsing-contexts:api-base-url
GlobalScopeTypeId::Window => {
self.downcast::<Window>().unwrap().Document().base_url()

This comment has been minimized.

@Ms2ger

Ms2ger Oct 6, 2016

Contributor

I'd prefer two if lets on self.downcast::<T>().

@@ -200,6 +200,12 @@ impl GlobalScope {
},
}
}

/// Extract a `Window`, causing thread failure if the global object is not

This comment has been minimized.

@Ms2ger

Ms2ger Oct 6, 2016

Contributor

We call that a panic now.

@@ -144,8 +144,7 @@ impl Blob {
let (buffer, is_new_buffer) = match *f.cache.borrow() {
Some(ref bytes) => (bytes.clone(), false),
None => {
let global = self.global();
let bytes = read_file(global.r(), f.id.clone())?;
let bytes = read_file(&self.global_scope(), f.id.clone())?;

This comment has been minimized.

@Ms2ger

Ms2ger Oct 6, 2016

Contributor

Who's been putting ? in my code...

pub fn script_chan(&self) -> Box<ScriptChan + Send> {
match *self.type_id() {
GlobalScopeTypeId::Window => {
let window = self.downcast::<Window>().unwrap();

This comment has been minimized.

@Ms2ger

Ms2ger Oct 6, 2016

Contributor

As before.

/// this of this global scope.
pub fn networking_task_source(&self) -> Box<ScriptChan + Send> {
match *self.type_id() {
GlobalScopeTypeId::Window => {

This comment has been minimized.

@Ms2ger

Ms2ger Oct 6, 2016

Contributor

You guessed it...

@@ -43,7 +43,7 @@
//! [`Fallible<T>`](error/type.Fallible.html).
//! Methods that use certain WebIDL types like `any` or `object` will get a
//! `*mut JSContext` argument prepended to the argument list. Static methods
//! will be passed a [`GlobalRef`](global/enum.GlobalRef.html) for the relevant
//! will be passed a `&GlobalScope` for the relevant

This comment has been minimized.

@Ms2ger

Ms2ger Oct 6, 2016

Contributor

Keep linking, please.

@@ -861,7 +861,7 @@ impl WindowMethods for Window {
#[allow(unrooted_must_root)]
// https://fetch.spec.whatwg.org/#fetch-method
fn Fetch(&self, input: RequestOrUSVString, init: &RequestInit) -> Rc<Promise> {
fetch::Fetch(self.global().r(), input, init)
fetch::Fetch(&self.global_scope(), input, init)

This comment has been minimized.

@Ms2ger

Ms2ger Oct 6, 2016

Contributor

upcast?

@@ -319,7 +319,7 @@ impl WorkerGlobalScopeMethods for WorkerGlobalScope {
#[allow(unrooted_must_root)]
// https://fetch.spec.whatwg.org/#fetch-method
fn Fetch(&self, input: RequestOrUSVString, init: &RequestInit) -> Rc<Promise> {
fetch::Fetch(self.global().r(), input, init)
fetch::Fetch(&self.global_scope(), input, init)

This comment has been minimized.

@Ms2ger

Ms2ger Oct 6, 2016

Contributor

Ditto

@nox nox force-pushed the nox:inline branch from b644157 to 1dc6664 Oct 6, 2016
@bors-servo
Copy link
Contributor

bors-servo commented Oct 6, 2016

The latest upstream changes (presumably #13597) made this pull request unmergeable. Please resolve the merge conflicts.

@nox nox force-pushed the nox:inline branch from 1dc6664 to d8e92bb Oct 6, 2016
@nox nox removed the S-needs-rebase label Oct 6, 2016
@Ms2ger
Copy link
Contributor

Ms2ger commented Oct 7, 2016

@bors-servo delegate+

LGTM assuming only changes were rebasing and fixing my comments.

@bors-servo
Copy link
Contributor

bors-servo commented Oct 7, 2016

✌️ @nox can now approve this pull request

@nox
Copy link
Member Author

nox commented Oct 7, 2016

@bors-servo r=Ms2ger p=10

@bors-servo
Copy link
Contributor

bors-servo commented Oct 7, 2016

📌 Commit d8e92bb has been approved by Ms2ger

@highfive highfive assigned Ms2ger and unassigned emilio Oct 7, 2016
@bors-servo
Copy link
Contributor

bors-servo commented Oct 7, 2016

Testing commit d8e92bb with merge a6e4b5b...

bors-servo added a commit that referenced this pull request Oct 7, 2016
Get rid of dom::bindings::global

Globals in that PR are now represented by the fake IDL interface `GlobalScope`.

<!-- 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/13596)
<!-- Reviewable:end -->
@bors-servo
Copy link
Contributor

bors-servo commented Oct 7, 2016

💔 Test failed - linux-rel-wpt

@highfive
Copy link

highfive commented Oct 7, 2016

  ▶ FAIL [expected PASS] /_mozilla/css/inline_block_opacity_change.html
  └   → /_mozilla/css/inline_block_opacity_change.html aed6845267bba6c52d3aa69c4889449b454d8117
/_mozilla/css/inline_block_opacity_change_ref.html febec1b4730afab195f280694dad47200b09ea3c
Testing aed6845267bba6c52d3aa69c4889449b454d8117 == febec1b4730afab195f280694dad47200b09ea3c
@nox
Copy link
Member Author

nox commented Oct 7, 2016

@bors-servo
Copy link
Contributor

bors-servo commented Oct 7, 2016

Previous build results for arm32, arm64, linux-dev, linux-rel-css, mac-dev-unit, mac-rel-wpt1, mac-rel-wpt2, windows-dev are reusable. Rebuilding only linux-rel-wpt, mac-rel-css...

@bors-servo
Copy link
Contributor

bors-servo commented Oct 7, 2016

@bors-servo bors-servo merged commit d8e92bb into servo:master Oct 7, 2016
2 of 3 checks passed
2 of 3 checks passed
continuous-integration/appveyor/pr AppVeyor build failed
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
homu Test successful
Details
@bors-servo bors-servo mentioned this pull request Oct 7, 2016
2 of 5 tasks complete
@nox nox deleted the nox:inline branch Oct 7, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

6 participants
You can’t perform that action at this time.