Bug 170166

Summary:

Stop RTCDataChannel when closing page

Product:

WebKit

Reporter:

youenn fablet <youennf>

Component:

WebCore Misc.

Assignee:

youenn fablet <youennf>

Status:

RESOLVED FIXED

Severity:

Normal

CC:

achristensen, commit-queue, eric.carlson, ryanhaddad

Priority:

Version:

WebKit Nightly Build

Hardware:

Unspecified

OS:

Unspecified

Attachments:

Description	Flags
Patch	none
Patch	none
Patch for landing	none
Patch for landing	none

youenn fablet

Reported 2017-03-27 21:44:03 PDT

One possibility is to make RTCDataChannel an ActiveDOMObject.

Attachments
Patch (8.15 KB, patch) 2017-03-27 22:25 PDT, youenn fablet	no flags	Details Formatted Diff Diff
Patch (8.22 KB, patch) 2017-03-28 09:40 PDT, youenn fablet	no flags	Details Formatted Diff Diff
Patch for landing (8.22 KB, patch) 2017-03-28 11:19 PDT, youenn fablet	no flags	Details Formatted Diff Diff
Patch for landing (8.27 KB, patch) 2017-03-28 16:23 PDT, youenn fablet	no flags	Details Formatted Diff Diff
Show Obsolete (3) View All Add attachment proposed patch, testcase, etc.

youenn fablet

Comment 1 2017-03-27 22:25:47 PDT

Created attachment 305560 [details] Patch

youenn fablet

Comment 2 2017-03-28 09:40:09 PDT

Created attachment 305597 [details] Patch

Eric Carlson

Comment 3 2017-03-28 09:57:54 PDT

Comment on attachment 305597 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=305597&action=review > Source/WebCore/Modules/mediastream/RTCDataChannel.h:93 > + bool canSuspendForDocumentSuspension() const final { return false; } It seems this should at least return true when readyState is ReadyStateClosed.

youenn fablet

Comment 4 2017-03-28 11:19:38 PDT

Created attachment 305613 [details] Patch for landing

youenn fablet

Comment 5 2017-03-28 11:19:59 PDT

Thanks for the review. (In reply to Eric Carlson from comment #3) > Comment on attachment 305597 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=305597&action=review > > > Source/WebCore/Modules/mediastream/RTCDataChannel.h:93 > > + bool canSuspendForDocumentSuspension() const final { return false; } > > It seems this should at least return true when readyState is > ReadyStateClosed. Indeed!

WebKit Commit Bot

Comment 6 2017-03-28 12:03:25 PDT

Comment on attachment 305613 [details] Patch for landing Clearing flags on attachment: 305613 Committed r214485: <http://trac.webkit.org/changeset/214485>

WebKit Commit Bot

Comment 7 2017-03-28 12:03:30 PDT

All reviewed patches have been landed. Closing bug.

Ryan Haddad

Comment 8 2017-03-28 15:12:54 PDT

(In reply to WebKit Commit Bot from comment #6) > Comment on attachment 305613 [details] > Patch for landing > > Clearing flags on attachment: 305613 > > Committed r214485: <http://trac.webkit.org/changeset/214485> This change appears to have caused LayoutTest crashes: https://build.webkit.org/results/Apple%20Sierra%20Debug%20WK2%20(Tests)/r214485%20(177)/results.html

Ryan Haddad

Comment 9 2017-03-28 15:32:49 PDT

Reverted r214485 for reason: This change caused LayoutTest crashes. Committed r214500: <http://trac.webkit.org/changeset/214500>

youenn fablet

Comment 10 2017-03-28 16:23:40 PDT

Created attachment 305663 [details] Patch for landing

youenn fablet

Comment 11 2017-03-28 16:24:05 PDT

(In reply to youenn fablet from comment #10) > Created attachment 305663 [details] > Patch for landing Added the missing call to suspendIfNeeded.

WebKit Commit Bot

Comment 12 2017-03-28 17:06:14 PDT

Comment on attachment 305663 [details] Patch for landing Clearing flags on attachment: 305663 Committed r214508: <http://trac.webkit.org/changeset/214508>

WebKit Commit Bot

Comment 13 2017-03-28 17:06:17 PDT

All reviewed patches have been landed. Closing bug.

Note You need to log in before you can comment on or make changes to this bug.