-
Notifications
You must be signed in to change notification settings - Fork 425
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
test: fix tests in chrome 92 by disposing media elements/ blob url #1174
Conversation
Codecov Report
@@ Coverage Diff @@
## main #1174 +/- ##
=======================================
Coverage 86.47% 86.47%
=======================================
Files 39 39
Lines 9577 9577
Branches 2210 2210
=======================================
Hits 8282 8282
Misses 1295 1295 Continue to review full report at Codecov.
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just one question
@@ -831,7 +830,7 @@ QUnit.module('SegmentLoader', function(hooks) { | |||
}); | |||
|
|||
QUnit.test('only appends one segment at a time', function(assert) { | |||
return setupMediaSource(loader.mediaSource_, loader.sourceUpdater_).then(() => { | |||
return this.setupMediaSource(loader.mediaSource_, loader.sourceUpdater_).then(() => { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
where does this.setupMediaSource
is coming from here? I'm not seeing where it is defined, and setupMediaSource
is no longer being imported.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
its defined in the beforeEach/afterEach to handle the creation of the video element and then the removal.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
aaah, cool. Missed that this file used the default hooks.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Description
Chrome 92 requires media elements to be torn down completely before they are removed from memory. This includes unsetting the src property and removing the src attribute.
This is only an issue when a lot of players are created and disposed on a page. For us this mostly happens in tests.
The error in question:
Blocked attempt to create a WebMediaPlayer as there are too many WebMediaPlayers already in existence. See crbug.com/1144736#c27