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 srcObject logic for Blob media providers #23103

Merged
merged 1 commit into from Apr 3, 2019

Conversation

Projects
None yet
8 participants
@ferjm
Copy link
Member

commented Mar 26, 2019

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

This change is Reviewable

@highfive

This comment has been minimized.

Copy link

commented Mar 26, 2019

Heads up! This PR modifies the following files:

  • @asajeffrey: components/script/dom/webidls/HTMLMediaElement.webidl, components/script/dom/htmlmediaelement.rs
  • @KiChjang: components/script/dom/webidls/HTMLMediaElement.webidl, components/script/dom/htmlmediaelement.rs
@highfive

This comment has been minimized.

Copy link

commented Mar 26, 2019

warning Warning warning

  • These commits include an empty title element (<title></title>). Consider adding appropriate metadata.
@servo-wpt-sync

This comment has been minimized.

Copy link
Collaborator

commented Mar 26, 2019

Opened new PR for upstreamable changes.

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

@CYBAI

This comment has been minimized.

Copy link
Collaborator

commented Mar 27, 2019

AppVeyor failed with timeout and Travis complained about WPT manifest 👀

 0:09.30 INFO Diffing old and new manifests /home/travis/build/servo/servo/tests/wpt/mozilla/meta/MANIFEST.json
 0:09.41 /home/travis/build/servo/servo/tests/wpt/mozilla/meta/MANIFEST.json  0  error  mozilla/movie_5.mp4 in manifest but removed from source.  (wpt-manifest)
 0:09.41 ERROR Manifest /home/travis/build/servo/servo/tests/wpt/mozilla/meta/MANIFEST.json is outdated, use |./mach update-manifest| to fix.
 0:09.41 INFO Diffing old and new manifests /home/travis/build/servo/servo/tests/wpt/metadata/MANIFEST.json
 0:15.47 WARNING Manifest /home/travis/build/servo/servo/tests/wpt/metadata/MANIFEST.json contains correct tests but file hashes changed.
 0:15.60 ERROR Manifest /home/travis/build/servo/servo/tests/wpt/metadata/MANIFEST.json is outdated, use |./mach update-manifest| to fix.
 0:15.60 INFO Diffing old and new manifests /home/travis/build/servo/servo/tests/wpt/webgl/meta/MANIFEST.json

@CYBAI CYBAI removed the S-needs-rebase label Mar 27, 2019

@CYBAI

This comment has been minimized.

Copy link
Collaborator

commented Mar 27, 2019

I saw there's a forced push after @jdm labeled the S-needs-rebase so I removed it 🙇

@ferjm ferjm force-pushed the ferjm:srcObject branch from bc69210 to d156400 Mar 27, 2019

@servo-wpt-sync

This comment has been minimized.

Copy link
Collaborator

commented Mar 27, 2019

Transplanted upstreamable changes to existing PR.

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

@ferjm

This comment has been minimized.

Copy link
Member Author

commented Mar 27, 2019

@bors-servo try=wpt

@bors-servo

This comment has been minimized.

Copy link
Contributor

commented Mar 27, 2019

⌛️ Trying commit d156400 with merge 32893f8...

bors-servo added a commit that referenced this pull request Mar 27, 2019

Auto merge of #23103 - ferjm:srcObject, r=<try>
Implement srcObject logic for Blob media providers

- [X] `./mach build -d` does not report any errors
- [X] `./mach test-tidy` does not report any errors
- [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/23103)
<!-- Reviewable:end -->
@ferjm

This comment has been minimized.

Copy link
Member Author

commented Mar 27, 2019

r? @Manishearth

I realize that this might not be very useful for the MediaStream case, as we are using the fetch_request path for blobs as well. I guess we would need to generalize and replicate some of the logic of the FetchResponseListener implementation for HTMLMediaElementFetchListener for the MediaStream and MediaSource cases.

@highfive highfive assigned Manishearth and unassigned SimonSapin Mar 27, 2019

@bors-servo

This comment has been minimized.

Copy link
Contributor

commented Mar 27, 2019

☀️ Test successful - linux-rel-css, linux-rel-wpt, status-taskcluster
State: approved= try=True

@Manishearth

This comment has been minimized.

Copy link
Member

commented Apr 3, 2019

@bors-servo r+

looks good!

@bors-servo

This comment has been minimized.

Copy link
Contributor

commented Apr 3, 2019

📌 Commit d156400 has been approved by Manishearth

@bors-servo

This comment has been minimized.

Copy link
Contributor

commented Apr 3, 2019

⌛️ Testing commit d156400 with merge 9454086...

bors-servo added a commit that referenced this pull request Apr 3, 2019

Auto merge of #23103 - ferjm:srcObject, r=Manishearth
Implement srcObject logic for Blob media providers

- [X] `./mach build -d` does not report any errors
- [X] `./mach test-tidy` does not report any errors
- [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/23103)
<!-- Reviewable:end -->
@bors-servo

This comment has been minimized.

Copy link
Contributor

commented Apr 3, 2019

💔 Test failed - linux-rel-wpt

@CYBAI

This comment has been minimized.

Copy link
Collaborator

commented Apr 3, 2019

@bors-servo

This comment has been minimized.

Copy link
Contributor

commented Apr 3, 2019

⌛️ Testing commit d156400 with merge b9b729c...

bors-servo added a commit that referenced this pull request Apr 3, 2019

Auto merge of #23103 - ferjm:srcObject, r=Manishearth
Implement srcObject logic for Blob media providers

- [X] `./mach build -d` does not report any errors
- [X] `./mach test-tidy` does not report any errors
- [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/23103)
<!-- Reviewable:end -->
@bors-servo

This comment has been minimized.

Copy link
Contributor

commented Apr 3, 2019

@bors-servo bors-servo merged commit d156400 into servo:master Apr 3, 2019

3 of 4 checks passed

continuous-integration/appveyor/pr AppVeyor build failed
Details
Taskcluster (pull_request) TaskGroup: success
Details
Travis CI - Pull Request Build Passed
Details
homu Test successful
Details

@bors-servo bors-servo referenced this pull request Apr 3, 2019

Merged

Refactoring RequestInit to use a Builder Pattern #22521

4 of 4 tasks complete
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.