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

Shutdown player when HTMLMediaElement is dropped #22944

Merged
merged 4 commits into from Mar 25, 2019

Conversation

@ferjm
Copy link
Member

ferjm commented Feb 27, 2019

  • ./mach build -d does not report any errors
  • ./mach test-tidy does not report any errors
  • These changes fix #22931
  • There are tests for these changes

This change is Reviewable

@highfive
Copy link

highfive commented Feb 27, 2019

Heads up! This PR modifies the following files:

  • @asajeffrey: components/script/dom/htmlmediaelement.rs
  • @KiChjang: components/script/dom/htmlmediaelement.rs
@ferjm ferjm force-pushed the ferjm:player.shutdown branch from 8303995 to a63d742 Feb 27, 2019
@ceyusa
Copy link
Collaborator

ceyusa commented Feb 27, 2019

@bors-servo
Copy link
Contributor

bors-servo commented Feb 27, 2019

@ceyusa: 🔑 Insufficient privileges: Not in reviewers

@highfive highfive assigned ceyusa and unassigned asajeffrey Feb 27, 2019
@ferjm
Copy link
Member Author

ferjm commented Feb 27, 2019

@bors-servo r=ceyusa

@bors-servo
Copy link
Contributor

bors-servo commented Feb 27, 2019

📌 Commit a63d742 has been approved by ceyusa

@bors-servo
Copy link
Contributor

bors-servo commented Feb 27, 2019

Testing commit a63d742 with merge def295e...

bors-servo added a commit that referenced this pull request Feb 27, 2019
Shutdown player when HTMLMediaElement is dropped

- [X] `./mach build -d` does not report any errors
- [X] `./mach test-tidy` does not report any errors
- [X] These changes fix #22931
- [X] There are tests for these changes

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

bors-servo commented Feb 27, 2019

💔 Test failed - linux-rel-wpt

@ferjm
Copy link
Member Author

ferjm commented Feb 27, 2019

@bors-servo
Copy link
Contributor

bors-servo commented Feb 27, 2019

Testing commit a63d742 with merge e69aad0...

bors-servo added a commit that referenced this pull request Feb 27, 2019
Shutdown player when HTMLMediaElement is dropped

- [X] `./mach build -d` does not report any errors
- [X] `./mach test-tidy` does not report any errors
- [X] These changes fix #22931
- [X] There are tests for these changes

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

bors-servo commented Feb 27, 2019

💔 Test failed - linux-rel-css

@ferjm
Copy link
Member Author

ferjm commented Feb 27, 2019

bors-servo added a commit that referenced this pull request Feb 27, 2019
Shutdown player when HTMLMediaElement is dropped

- [X] `./mach build -d` does not report any errors
- [X] `./mach test-tidy` does not report any errors
- [X] These changes fix #22931
- [X] There are tests for these changes

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

bors-servo commented Feb 27, 2019

Testing commit a63d742 with merge da4ff1b...

@ferjm ferjm force-pushed the ferjm:player.shutdown branch 2 times, most recently from 8328825 to 31c4b30 Mar 25, 2019
bors-servo added a commit to servo/media that referenced this pull request Mar 25, 2019
Temporarily disable GL functionality

This should unblock updating servo-media in Servo while we work on a different workaround for the [existing build issues](servo/servo#22944 (comment)).
@ferjm ferjm force-pushed the ferjm:player.shutdown branch from 31c4b30 to dc41d2f Mar 25, 2019
@ferjm ferjm removed the S-needs-rebase label Mar 25, 2019
@ferjm
Copy link
Member Author

ferjm commented Mar 25, 2019

@bors-servo r=ceyusa

@bors-servo
Copy link
Contributor

bors-servo commented Mar 25, 2019

📌 Commit dc41d2f has been approved by ceyusa

@highfive highfive assigned ceyusa and unassigned jdm Mar 25, 2019
@bors-servo
Copy link
Contributor

bors-servo commented Mar 25, 2019

Testing commit dc41d2f with merge 5ae562b...

bors-servo added a commit that referenced this pull request Mar 25, 2019
Shutdown player when HTMLMediaElement is dropped

- [X] `./mach build -d` does not report any errors
- [X] `./mach test-tidy` does not report any errors
- [X] These changes fix #22931
- [X] There are tests for these changes

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

bors-servo commented Mar 25, 2019

💔 Test failed - linux-rel-wpt

@ferjm
Copy link
Member Author

ferjm commented Mar 25, 2019

@bors-servo
Copy link
Contributor

bors-servo commented Mar 25, 2019

@bors-servo
Copy link
Contributor

bors-servo commented Mar 25, 2019

@bors-servo bors-servo merged commit dc41d2f into servo:master Mar 25, 2019
4 checks passed
4 checks passed
Taskcluster (pull_request) TaskGroup: success
Details
Travis CI - Pull Request Build Passed
Details
continuous-integration/appveyor/pr AppVeyor build succeeded
Details
homu Test successful
Details
ceyusa added a commit to ceyusa/media that referenced this pull request Apr 6, 2019
This object is in charge of two tasks:

- Create the video sink
- Create the frames to push to the Renders

The purpose of this design is to handle GL by another create (in order
to workaround
servo/servo#22944 (comment)), and
that other object will proxy these tasks if available in the target
OS.

This patch removes temporarly the GL support in player.
ceyusa added a commit to ceyusa/media that referenced this pull request Apr 10, 2019
This object is in charge of two tasks:

- Create the video sink
- Create the frames to push to the Renders

The purpose of this design is to handle GL by another create (in order
to workaround
servo/servo#22944 (comment)), and
that other object will proxy these tasks if available in the target
OS.

This patch removes temporarly the GL support in player.
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.

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