Skip to content

Replace use of callbacks in webxr by channels#23848

Merged
bors-servo merged 1 commit intoservo:masterfrom
asajeffrey:webxr-moar-channels
Jul 27, 2019
Merged

Replace use of callbacks in webxr by channels#23848
bors-servo merged 1 commit intoservo:masterfrom
asajeffrey:webxr-moar-channels

Conversation

@asajeffrey
Copy link
Copy Markdown
Contributor

@asajeffrey asajeffrey commented Jul 25, 2019 •
edited by SimonSapin
Loading

Use senders rather than callbacks in webxr.

  • ./mach build -d does not report any errors
  • ./mach test-tidy does not report any errors
  • These changes do not require tests because it's an internal change

This change is Reviewable

@asajeffrey asajeffrey added S-blocked-on-external Something, somewhere else, needs to happen before this PR can be merged. A-xr-zomg AR/VR/MR labels Jul 25, 2019
@highfive
Copy link
Copy Markdown

highfive commented Jul 25, 2019

Heads up! This PR modifies the following files:

  • @KiChjang: components/net/tests/main.rs, components/script/dom/xrtest.rs, components/script/dom/webglrenderingcontext.rs, components/script/dom/xrsession.rs, components/script/dom/xr.rs

@highfive highfive added the S-awaiting-review There is new code that needs to be reviewed. label Jul 25, 2019
@asajeffrey
Copy link
Copy Markdown
Contributor Author

asajeffrey commented Jul 25, 2019

Depends on #23847 and servo/webxr#29

@asajeffrey
Copy link
Copy Markdown
Contributor Author

asajeffrey commented Jul 25, 2019

r? @Manishearth

@highfive highfive assigned Manishearth and unassigned avadacatavra Jul 25, 2019
@asajeffrey asajeffrey changed the title Webxr moar channels Replace use of callbacks in webxr by channels Jul 25, 2019
@bors-servo
Copy link
Copy Markdown
Contributor

bors-servo commented Jul 25, 2019

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

@highfive highfive added the S-needs-rebase There are merge conflict errors. label Jul 25, 2019
asajeffrey pushed a commit to asajeffrey/servo that referenced this pull request Jul 25, 2019
PR servo#23848
dec9428
@asajeffrey asajeffrey force-pushed the webxr-moar-channels branch 2 times, most recently from 9efcc74 to 1cbf512 Compare July 25, 2019 23:27
@asajeffrey asajeffrey marked this pull request as ready for review July 25, 2019 23:28
@asajeffrey asajeffrey removed S-awaiting-review There is new code that needs to be reviewed. S-blocked-on-external Something, somewhere else, needs to happen before this PR can be merged. S-needs-rebase There are merge conflict errors. labels Jul 25, 2019
@asajeffrey
Copy link
Copy Markdown
Contributor Author

asajeffrey commented Jul 25, 2019

@bors-servo r=Manishearth

@bors-servo
Copy link
Copy Markdown
Contributor

bors-servo commented Jul 25, 2019

📌 Commit 1cbf512 has been approved by Manishearth

@highfive highfive added the S-awaiting-merge The PR is in the process of compiling and running tests on the automated CI. label Jul 25, 2019
@bors-servo
Copy link
Copy Markdown
Contributor

bors-servo commented Jul 25, 2019

⌛ Testing commit 1cbf512 with merge 913205a...

bors-servo pushed a commit that referenced this pull request Jul 25, 2019
Auto merge of #23848 - asajeffrey:webxr-moar-channels, r=Manishearth
913205a 
Replace use of callbacks in webxr by channels

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

Use senders rather than callbacks in webxr.

---
<!-- 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 an internal change

<!-- Also, please make sure that "Allow edits from maintainers" checkbox is checked, so that we can help you if you get stuck somewhere along the way.-->

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

bors-servo commented Jul 26, 2019

💔 Test failed - linux-rel-css

@highfive highfive added S-tests-failed The changes caused existing tests to fail. and removed S-awaiting-merge The PR is in the process of compiling and running tests on the automated CI. labels Jul 26, 2019
@jdm
Copy link
Copy Markdown
Member

jdm commented Jul 26, 2019 

./components/canvas/Cargo.toml:39: no newline at EOF

@asajeffrey asajeffrey force-pushed the webxr-moar-channels branch from 1cbf512 to 65a26ad Compare July 26, 2019 15:10
bors-servo pushed a commit that referenced this pull request Jul 27, 2019
Auto merge of #23848 - asajeffrey:webxr-moar-channels, r=Manishearth
9e7bb4c 
Replace use of callbacks in webxr by channels

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

Use senders rather than callbacks in webxr.

---
<!-- 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 an internal change

<!-- Also, please make sure that "Allow edits from maintainers" checkbox is checked, so that we can help you if you get stuck somewhere along the way.-->

<!-- 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/23848)
<!-- Reviewable:end -->
@highfive highfive added S-awaiting-merge The PR is in the process of compiling and running tests on the automated CI. and removed S-tests-failed The changes caused existing tests to fail. labels Jul 27, 2019
@CYBAI
Copy link
Copy Markdown
Member

CYBAI commented Jul 27, 2019

bors-servo retry

Note that it's #23290 intermittent failure.

@bors-servo
Copy link
Copy Markdown
Contributor

bors-servo commented Jul 27, 2019

💔 Test failed - linux-rel-css

@highfive highfive added S-tests-failed The changes caused existing tests to fail. and removed S-awaiting-merge The PR is in the process of compiling and running tests on the automated CI. labels Jul 27, 2019
@CYBAI
Copy link
Copy Markdown
Member

CYBAI commented Jul 27, 2019

@bors-servo retry

@bors-servo
Copy link
Copy Markdown
Contributor

bors-servo commented Jul 27, 2019

⌛ Testing commit 133a17e with merge 6db5fc6...

bors-servo pushed a commit that referenced this pull request Jul 27, 2019
Auto merge of #23848 - asajeffrey:webxr-moar-channels, r=Manishearth
6db5fc6 
Replace use of callbacks in webxr by channels

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

Use senders rather than callbacks in webxr.

---
<!-- 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 an internal change

<!-- Also, please make sure that "Allow edits from maintainers" checkbox is checked, so that we can help you if you get stuck somewhere along the way.-->

<!-- 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/23848)
<!-- Reviewable:end -->
@highfive highfive added S-awaiting-merge The PR is in the process of compiling and running tests on the automated CI. and removed S-tests-failed The changes caused existing tests to fail. labels Jul 27, 2019
@bors-servo
Copy link
Copy Markdown
Contributor

bors-servo commented Jul 27, 2019

💔 Test failed - linux-rel-css

@highfive highfive added S-tests-failed The changes caused existing tests to fail. and removed S-awaiting-merge The PR is in the process of compiling and running tests on the automated CI. labels Jul 27, 2019
@jdm
Copy link
Copy Markdown
Member

jdm commented Jul 27, 2019

@bors-servo retry

@bors-servo
Copy link
Copy Markdown
Contributor

bors-servo commented Jul 27, 2019

💣 Failed to start rebuilding: Unknown error

@bors-servo
Copy link
Copy Markdown
Contributor

bors-servo commented Jul 27, 2019

⌛ Testing commit 133a17e with merge dcc5ae9...

bors-servo pushed a commit that referenced this pull request Jul 27, 2019
Auto merge of #23848 - asajeffrey:webxr-moar-channels, r=Manishearth
dcc5ae9 
Replace use of callbacks in webxr by channels

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

Use senders rather than callbacks in webxr.

---
<!-- 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 an internal change

<!-- Also, please make sure that "Allow edits from maintainers" checkbox is checked, so that we can help you if you get stuck somewhere along the way.-->

<!-- 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/23848)
<!-- Reviewable:end -->
@highfive highfive added S-awaiting-merge The PR is in the process of compiling and running tests on the automated CI. and removed S-tests-failed The changes caused existing tests to fail. labels Jul 27, 2019
@bors-servo
Copy link
Copy Markdown
Contributor

bors-servo commented Jul 27, 2019

☀️ Test successful - linux-rel-css, linux-rel-wpt, status-taskcluster
Approved by: Manishearth
Pushing dcc5ae9 to master...

@bors-servo bors-servo merged commit 133a17e into servo:master Jul 27, 2019
@highfive highfive removed the S-awaiting-merge The PR is in the process of compiling and running tests on the automated CI. label Jul 27, 2019
This was referenced Jul 27, 2019
Merged
Merged
SimonSapin added a commit that referenced this pull request Jul 28, 2019
@SimonSapin
Revert "Replace use of callbacks in webxr by channels"
c0971b1 
This backs out commit 133a17e / PR #23848
because it relies on a git branch that was apparently removed:

```
$ cargo fetch
    Updating git repository `https://github.com/asajeffrey/webxr`
error: failed to resolve patches for `https://github.com/servo/webxr`

Caused by:
  failed to load source for a dependency on `webxr`

Caused by:
  Unable to update https://github.com/asajeffrey/webxr?branch=optional-glsync#da820a3a

Caused by:
  revspec 'da820a3ab266fce07c9a8abee3e6e9231cb93ec5' not found; class=Reference (4); code=NotFound (-3)
```
@SimonSapin SimonSapin mentioned this pull request Jul 28, 2019
Closed
bors-servo pushed a commit that referenced this pull request Jul 28, 2019
Auto merge of #23874 - servo:revert, r=SimonSapin
a323f81 
Revert "Replace use of callbacks in webxr by channels"

This backs out commit 133a17e / PR #23848 because it relies on a git branch that was apparently removed:

```
$ cargo fetch
    Updating git repository `https://github.com/asajeffrey/webxr`
error: failed to resolve patches for `https://github.com/servo/webxr`

Caused by:
  failed to load source for a dependency on `webxr`

Caused by:
  Unable to update https://github.com/asajeffrey/webxr?branch=optional-glsync#da820a3a

Caused by:
  revspec 'da820a3ab266fce07c9a8abee3e6e9231cb93ec5' not found; class=Reference (4); code=NotFound (-3)
```

<!-- 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/23874)
<!-- Reviewable:end -->
bors-servo pushed a commit that referenced this pull request Jul 28, 2019
Auto merge of #23874 - servo:revert, r=SimonSapin
139e7e8 
Revert "Replace use of callbacks in webxr by channels"

This backs out commit 133a17e / PR #23848 because it relies on a git branch that was apparently removed:

```
$ cargo fetch
    Updating git repository `https://github.com/asajeffrey/webxr`
error: failed to resolve patches for `https://github.com/servo/webxr`

Caused by:
  failed to load source for a dependency on `webxr`

Caused by:
  Unable to update https://github.com/asajeffrey/webxr?branch=optional-glsync#da820a3a

Caused by:
  revspec 'da820a3ab266fce07c9a8abee3e6e9231cb93ec5' not found; class=Reference (4); code=NotFound (-3)
```

<!-- 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/23874)
<!-- Reviewable:end -->
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@Manishearth Manishearth Manishearth approved these changes

Assignees

@Manishearth Manishearth

Labels

A-xr-zomg AR/VR/MR S-blocked-on-external Something, somewhere else, needs to happen before this PR can be merged.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

7 participants

@asajeffrey @highfive @bors-servo @jdm @CYBAI @Manishearth @avadacatavra