Bug 189016 - WebKitMediaSession should be GC collectable when its document is being stopped
Summary: WebKitMediaSession should be GC collectable when its document is being stopped
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebRTC (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: youenn fablet
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2018-08-27 15:26 PDT by youenn fablet
Modified: 2018-08-30 09:00 PDT (History)
8 users (show)

See Also:


Attachments
Patch (8.04 KB, patch)
2018-08-27 15:39 PDT, youenn fablet
no flags Details | Formatted Diff | Diff
Patch (7.50 KB, patch)
2018-08-28 08:34 PDT, youenn fablet
no flags Details | Formatted Diff | Diff
Unflake test (2.33 KB, patch)
2018-08-28 18:38 PDT, youenn fablet
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description youenn fablet 2018-08-27 15:26:54 PDT
MediaDevices and WebKitMediaSession should be GC collectable when their document is being stopped
Comment 1 youenn fablet 2018-08-27 15:39:25 PDT
Created attachment 348220 [details]
Patch
Comment 2 Simon Fraser (smfr) 2018-08-27 16:12:54 PDT
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 3 youenn fablet 2018-08-27 19:40:13 PDT
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.
Comment 4 youenn fablet 2018-08-28 08:34:51 PDT
Created attachment 348295 [details]
Patch
Comment 5 WebKit Commit Bot 2018-08-28 11:00:07 PDT
Comment on attachment 348295 [details]
Patch

Clearing flags on attachment: 348295

Committed r235429: <https://trac.webkit.org/changeset/235429>
Comment 6 WebKit Commit Bot 2018-08-28 11:00:08 PDT
All reviewed patches have been landed.  Closing bug.
Comment 7 Radar WebKit Bug Importer 2018-08-28 11:02:56 PDT
<rdar://problem/43804904>
Comment 8 Truitt Savell 2018-08-28 15:12:51 PDT
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"
Comment 9 youenn fablet 2018-08-28 18:38:18 PDT
Reopening to attach new patch.
Comment 10 youenn fablet 2018-08-28 18:38:19 PDT
Created attachment 348373 [details]
Unflake test
Comment 11 WebKit Commit Bot 2018-08-28 19:37:16 PDT
Comment on attachment 348373 [details]
Unflake test

Clearing flags on attachment: 348373

Committed r235453: <https://trac.webkit.org/changeset/235453>
Comment 12 WebKit Commit Bot 2018-08-28 19:37:18 PDT
All reviewed patches have been landed.  Closing bug.
Comment 13 Ryan Haddad 2018-08-29 15:41:49 PDT
(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.
Comment 14 youenn fablet 2018-08-30 09:00:28 PDT
(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.