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

Add input mocking, input sources change event #25463

Merged
merged 15 commits into from Jan 9, 2020

Conversation

@Manishearth
Copy link
Member

Manishearth commented Jan 8, 2020

Depends on servo/webxr#118

Also fixes some bugs I found.

Wanted to finish and merge this before I started on hit testing since the transient hit test stuff might have overlap.

There are a bunch of missing mock pieces that I'll probably do in a separate PR.

Still need to run tests.

Some things I skipped:

  • Doing handedness/target ray setting: See immersive-web/webxr-test-api#46 , this would require making our impl support these changing
  • Handling button initial state: Would require some mock changes, but I ran out of time
  • Handling profiles/etc: We don't yet have impl support for these

r? @jdm

@highfive
Copy link

highfive commented Jan 8, 2020

Heads up! This PR modifies the following files:

  • @asajeffrey: components/script/dom/webidls/FakeXRInputController.webidl, components/script/dom/webidls/XRSession.webidl, components/script/dom/mod.rs, components/script/dom/webidls/XRInputSourcesChangeEvent.webidl, components/script/dom/xrinputsourceschangeevent.rs and 8 more
  • @KiChjang: components/script/dom/webidls/FakeXRInputController.webidl, components/script/dom/webidls/XRSession.webidl, components/script/dom/mod.rs, components/script/dom/webidls/XRInputSourcesChangeEvent.webidl, components/script/dom/xrinputsourceschangeevent.rs and 8 more
@highfive
Copy link

highfive commented Jan 8, 2020

warning Warning warning

  • These commits modify unsafe code. Please review it carefully!
  • These commits modify script code, but no tests are modified. Please consider adding a test!
@Manishearth Manishearth force-pushed the Manishearth:input-mocking branch from 1f6b191 to 0732658 Jan 8, 2020
// let cx = global.get_cx();
// unsafe {
// rooted!(in(*cx) let mut added_val = UndefinedValue());
// added.to_jsval(*cx, added_val.handle_mut());

This comment has been minimized.

@Manishearth

Manishearth Jan 8, 2020

Author Member

Annoyingly, to_jsval() here throws a segfault at this line:

https://github.com/servo/mozjs/blob/a2957207f4b296f9ecd207d453224aafa3bd1e86/mozjs/js/src/builtin/Array.cpp#L4032

I've verified that cx is non null. Am I missing something?

@servo-wpt-sync
Copy link
Collaborator

servo-wpt-sync commented Jan 8, 2020

Opened new PR for upstreamable changes.

Completed upstream sync of web-platform-test changes at web-platform-tests/wpt#21085.

@Manishearth Manishearth force-pushed the Manishearth:input-mocking branch from 0732658 to 5ab4a27 Jan 8, 2020
@Manishearth Manishearth marked this pull request as ready for review Jan 8, 2020
@Manishearth
Copy link
Member Author

Manishearth commented Jan 8, 2020

Ready to land once servo/webxr#118 does

@servo-wpt-sync
Copy link
Collaborator

servo-wpt-sync commented Jan 8, 2020

Transplanted upstreamable changes to existing PR.

Completed upstream sync of web-platform-test changes at web-platform-tests/wpt#21085.

@Manishearth Manishearth force-pushed the Manishearth:input-mocking branch from 5ab4a27 to 794eac5 Jan 8, 2020
@servo-wpt-sync
Copy link
Collaborator

servo-wpt-sync commented Jan 8, 2020

Transplanted upstreamable changes to existing PR.

Completed upstream sync of web-platform-test changes at web-platform-tests/wpt#21085.

Copy link
Member

asajeffrey left a comment

LGTM, just some minor nits. r=me assuming the segvs have been fixed!

components/script/dom/fakexrdevice.rs Outdated Show resolved Hide resolved
@@ -19,7 +19,7 @@ enum XRTargetRayMode {
[SecureContext, Exposed=Window, Pref="dom.webxr.enabled"]
interface XRInputSource {
readonly attribute XRHandedness handedness;
// [SameObject] readonly attribute XRTargetRayMode targetRayMode;
readonly attribute XRTargetRayMode targetRayMode;

This comment has been minimized.

@asajeffrey

asajeffrey Jan 8, 2020

Member

Why no [SameObject]?

This comment has been minimized.

@Manishearth

Manishearth Jan 8, 2020

Author Member

Can't SameObject enums. I believe this was fixed upstream in the spec, we just never updated the commented webidl.

@Manishearth
Copy link
Member Author

Manishearth commented Jan 8, 2020

@bors-servo r=asajeffrey

@bors-servo
Copy link
Contributor

bors-servo commented Jan 8, 2020

📌 Commit a5ffd2e has been approved by asajeffrey

@highfive highfive assigned asajeffrey and unassigned jdm Jan 8, 2020
@servo-wpt-sync
Copy link
Collaborator

servo-wpt-sync commented Jan 8, 2020

Transplanted upstreamable changes to existing PR.

Completed upstream sync of web-platform-test changes at web-platform-tests/wpt#21085.

@bors-servo
Copy link
Contributor

bors-servo commented Jan 9, 2020

Testing commit a5ffd2e with merge 1ad9c4d...

bors-servo added a commit that referenced this pull request Jan 9, 2020
Add input mocking, input sources change event

Depends on  servo/webxr#118

Also fixes some bugs I found.

Wanted to finish and merge this before I started on hit testing since the transient hit test stuff might have overlap.

There are a bunch of missing mock pieces that I'll probably do in a separate PR.

Still need to run tests.

Some things I skipped:
 - Doing handedness/target ray setting: See immersive-web/webxr-test-api#46 , this would require making our impl support these changing
 - Handling button initial state: Would require some mock changes, but I ran out of time
 - Handling profiles/etc: We don't yet have impl support for these

r? @jdm
@bors-servo
Copy link
Contributor

bors-servo commented Jan 9, 2020

💔 Test failed - status-taskcluster

@jdm
Copy link
Member

jdm commented Jan 9, 2020

./components/script/dom/webidls/FakeXRInputController.webidl:53: no newline at EOF
@jdm
Copy link
Member

jdm commented Jan 9, 2020

@bors-servo retry

@bors-servo
Copy link
Contributor

bors-servo commented Jan 9, 2020

Testing commit 94e8ce5 with merge 52337c7...

bors-servo added a commit that referenced this pull request Jan 9, 2020
Add input mocking, input sources change event

Depends on  servo/webxr#118

Also fixes some bugs I found.

Wanted to finish and merge this before I started on hit testing since the transient hit test stuff might have overlap.

There are a bunch of missing mock pieces that I'll probably do in a separate PR.

Still need to run tests.

Some things I skipped:
 - Doing handedness/target ray setting: See immersive-web/webxr-test-api#46 , this would require making our impl support these changing
 - Handling button initial state: Would require some mock changes, but I ran out of time
 - Handling profiles/etc: We don't yet have impl support for these

r? @jdm
@bors-servo
Copy link
Contributor

bors-servo commented Jan 9, 2020

💔 Test failed - status-taskcluster

@Manishearth
Copy link
Member Author

Manishearth commented Jan 9, 2020

@bors-servo retry

1 unexpected results that are NOT known-intermittents:
  ▶ FAIL [expected PASS] /css/CSS2/fonts/font-weight-applies-to-008.xht
  │   → /css/CSS2/fonts/font-weight-applies-to-008.xht 54a9df64f1476dd12020019d7cf22ac34d727bc0
  │   → /css/reference/pass_if_filler_text_match_bold.xht 68ca2ea769567706593ec6c195cb80a02c91b5a2
  └   → Screenshot is solid color 0xFFFFFF for /css/CSS2/fonts/font-weight-applies-to-008.xht
@bors-servo
Copy link
Contributor

bors-servo commented Jan 9, 2020

Testing commit 94e8ce5 with merge 2a742e7...

bors-servo added a commit that referenced this pull request Jan 9, 2020
Add input mocking, input sources change event

Depends on  servo/webxr#118

Also fixes some bugs I found.

Wanted to finish and merge this before I started on hit testing since the transient hit test stuff might have overlap.

There are a bunch of missing mock pieces that I'll probably do in a separate PR.

Still need to run tests.

Some things I skipped:
 - Doing handedness/target ray setting: See immersive-web/webxr-test-api#46 , this would require making our impl support these changing
 - Handling button initial state: Would require some mock changes, but I ran out of time
 - Handling profiles/etc: We don't yet have impl support for these

r? @jdm
@bors-servo
Copy link
Contributor

bors-servo commented Jan 9, 2020

💔 Test failed - status-taskcluster

@Manishearth
Copy link
Member Author

Manishearth commented Jan 9, 2020

@bors-servo retry

  ▶ FAIL [expected PASS] /css/CSS2/css1/c16-descendant-002.xht
  │   → /css/CSS2/css1/c16-descendant-002.xht 54a9df64f1476dd12020019d7cf22ac34d727bc0
  │   → /css/CSS2/reference/ref-this-text-should-be-green.xht cc1047e1c7ff45f6ce371b86003daf56e31e4f67
  └   → Screenshot is solid color 0xFFFFFF for /css/CSS2/css1/c16-descendant-002.xht
  ▶ FAIL [expected PASS] /_mozilla/css/transform_3d.html
  │   → /_mozilla/css/transform_3d.html 54a9df64f1476dd12020019d7cf22ac34d727bc0
  │   → /_mozilla/css/transform_3d_ref.html fc1036f05dcdf401b928df32bbf403df76bb1b9c
  └   → Screenshot is solid color 0xFFFFFF for /_mozilla/css/transform_3d.html
@bors-servo
Copy link
Contributor

bors-servo commented Jan 9, 2020

Testing commit 94e8ce5 with merge a35879e...

bors-servo added a commit that referenced this pull request Jan 9, 2020
Add input mocking, input sources change event

Depends on  servo/webxr#118

Also fixes some bugs I found.

Wanted to finish and merge this before I started on hit testing since the transient hit test stuff might have overlap.

There are a bunch of missing mock pieces that I'll probably do in a separate PR.

Still need to run tests.

Some things I skipped:
 - Doing handedness/target ray setting: See immersive-web/webxr-test-api#46 , this would require making our impl support these changing
 - Handling button initial state: Would require some mock changes, but I ran out of time
 - Handling profiles/etc: We don't yet have impl support for these

r? @jdm
@bors-servo
Copy link
Contributor

bors-servo commented Jan 9, 2020

💔 Test failed - status-taskcluster

@Manishearth
Copy link
Member Author

Manishearth commented Jan 9, 2020

@bors-servo retry

#24726 again

@bors-servo
Copy link
Contributor

bors-servo commented Jan 9, 2020

Testing commit 94e8ce5 with merge dbee7f7...

bors-servo added a commit that referenced this pull request Jan 9, 2020
Add input mocking, input sources change event

Depends on  servo/webxr#118

Also fixes some bugs I found.

Wanted to finish and merge this before I started on hit testing since the transient hit test stuff might have overlap.

There are a bunch of missing mock pieces that I'll probably do in a separate PR.

Still need to run tests.

Some things I skipped:
 - Doing handedness/target ray setting: See immersive-web/webxr-test-api#46 , this would require making our impl support these changing
 - Handling button initial state: Would require some mock changes, but I ran out of time
 - Handling profiles/etc: We don't yet have impl support for these

r? @jdm
@bors-servo
Copy link
Contributor

bors-servo commented Jan 9, 2020

☀️ Test successful - status-taskcluster
Approved by: asajeffrey
Pushing dbee7f7 to master...

@bors-servo bors-servo merged commit 94e8ce5 into servo:master Jan 9, 2020
2 checks passed
2 checks passed
Community-TC (pull_request) TaskGroup: success
Details
homu Test successful
Details
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.