| Summary: | WebKitMediaSession should be GC collectable when its document is being stopped | ||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|
| Product: | WebKit | Reporter: | youenn fablet <youennf> | ||||||||
| Component: | WebRTC | Assignee: | youenn fablet <youennf> | ||||||||
| Status: | RESOLVED FIXED | ||||||||||
| Severity: | Normal | CC: | commit-queue, eric.carlson, jer.noble, ryanhaddad, simon.fraser, tsavell, webkit-bug-importer, youennf | ||||||||
| Priority: | P2 | Keywords: | InRadar | ||||||||
| Version: | WebKit Nightly Build | ||||||||||
| Hardware: | Unspecified | ||||||||||
| OS: | Unspecified | ||||||||||
| See Also: | https://bugs.webkit.org/show_bug.cgi?id=189018 | ||||||||||
| Attachments: |
|
||||||||||
|
Description
youenn fablet
2018-08-27 15:26:54 PDT
Created attachment 348220 [details]
Patch
Comment on attachment 348220 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=348220&action=review > Source/WebCore/Modules/encryptedmedia/legacy/WebKitMediaKeySession.h:44 > +class WebKitMediaKeySession final : public RefCounted<WebKitMediaKeySession>, public EventTargetWithInlineData, public ActiveDOMObject, private LegacyCDMSessionClient { Why was this change required? > LayoutTests/http/tests/media/clearkey/collect-webkit-media-session.html:42 > +}, "Ensuring frame document gets collected after being stopped while doing some webkit media session calls"); This reads oddly. Maybe "Ensure that the frame's document get collected after being stopped while doing some webkit media session calls" Comment on attachment 348220 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=348220&action=review >> Source/WebCore/Modules/encryptedmedia/legacy/WebKitMediaKeySession.h:44 >> +class WebKitMediaKeySession final : public RefCounted<WebKitMediaKeySession>, public EventTargetWithInlineData, public ActiveDOMObject, private LegacyCDMSessionClient { > > Why was this change required? Needed for bug 189018, but not needed for this one, I'll remove the change here. Created attachment 348295 [details]
Patch
Comment on attachment 348295 [details] Patch Clearing flags on attachment: 348295 Committed r235429: <https://trac.webkit.org/changeset/235429> All reviewed patches have been landed. Closing bug. The new test http/tests/media/clearkey/collect-webkit-media-session.html is flakey, Test History: http://webkit-test-results.webkit.org/dashboards/flakiness_dashboard.html#showAllRuns=true&tests=http%2Ftests%2Fmedia%2Fclearkey%2Fcollect-webkit-media-session.html Diff: --- /Volumes/Data/slave/highsierra-leaks/build/layout-test-results/http/tests/media/clearkey/collect-webkit-media-session-expected.txt +++ /Volumes/Data/slave/highsierra-leaks/build/layout-test-results/http/tests/media/clearkey/collect-webkit-media-session-actual.txt @@ -1,4 +1,4 @@ -PASS Ensure that the frame's document get collected after being stopped while doing some webkit media session calls +FAIL Ensure that the frame's document get collected after being stopped while doing some webkit media session calls promise_test: Unhandled rejection with value: "Test failed" Reopening to attach new patch. Created attachment 348373 [details]
Unflake test
Comment on attachment 348373 [details] Unflake test Clearing flags on attachment: 348373 Committed r235453: <https://trac.webkit.org/changeset/235453> All reviewed patches have been landed. Closing bug. (In reply to WebKit Commit Bot from comment #11) > Comment on attachment 348373 [details] > Unflake test > > Clearing flags on attachment: 348373 > > Committed r235453: <https://trac.webkit.org/changeset/235453> http/tests/media/clearkey/collect-webkit-media-session.html is still flaky after this change. It fails with the same diff as above. (In reply to Ryan Haddad from comment #13) > (In reply to WebKit Commit Bot from comment #11) > > Comment on attachment 348373 [details] > > Unflake test > > > > Clearing flags on attachment: 348373 > > > > Committed r235453: <https://trac.webkit.org/changeset/235453> > http/tests/media/clearkey/collect-webkit-media-session.html is still flaky > after this change. It fails with the same diff as above. I saw that and tried to reproduce it on my local machine but was not able to reproduce. I'll try again. |