Issue 472163002: Add method to notify blink when PeerConnection is closed in Chrome

Issue 472163002: Add method to notify blink when PeerConnection is closed in Chrome (Closed)

Created:
6 years, 4 months ago by vrk (LEFT CHROMIUM)

Modified:
6 years, 4 months ago

Reviewers:
jiayl, tommi (sloooow) - chröme, abarth-chromium

CC:
abarth-chromium, blink-reviews, dglazkov+blink, jamesr, tommyw+watchlist_chromium.org

Base URL:
https://chromium.googlesource.com/chromium/blink.git@master

Project:
blink

Visibility:
Public.

More Reviews

Description

Add method to notify blink when PeerConnection is closed in Chrome BUG=403618 Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=180508

Patch Set 4 : rename #

Created: 6 years, 4 months ago

Download [raw] [tar.bz2]

	Unified diffs	Side-by-side diffs	Delta from patch set	Stats (+22 lines, -6 lines)			Patch
M	Source/modules/mediastream/RTCPeerConnection.h	View	1 2 3	2 chunks	+3 lines, -0 lines	0 comments	Download
M	Source/modules/mediastream/RTCPeerConnection.cpp	View	1 2 3	3 chunks	+18 lines, -6 lines	0 comments	Download
M	public/platform/WebRTCPeerConnectionHandlerClient.h	View	1 2	1 chunk	+1 line, -0 lines	0 comments	Download

Messages

Total messages: 12 (0 generated)

Expand Messages | Collapse Messages

vrk (LEFT CHROMIUM)

Oops, sorry, sent this too soon. Will reply when ready.

6 years, 4 months ago (2014-08-14 23:33:44 UTC) #2

tommi (sloooow) - chröme

lgtm https://codereview.chromium.org/472163002/diff/20001/public/platform/WebRTCPeerConnectionHandlerClient.h File public/platform/WebRTCPeerConnectionHandlerClient.h (right): https://codereview.chromium.org/472163002/diff/20001/public/platform/WebRTCPeerConnectionHandlerClient.h#newcode80 public/platform/WebRTCPeerConnectionHandlerClient.h:80: virtual void closePeerConnection() { }; nit: remove semicolon

6 years, 4 months ago (2014-08-15 09:53:13 UTC) #4

jiayl

lgtm with one comment that should be address here or in the other Cl. https://codereview.chromium.org/472163002/diff/20001/Source/modules/mediastream/RTCPeerConnection.cpp ...

6 years, 4 months ago (2014-08-15 16:12:38 UTC) #5

vrk (LEFT CHROMIUM)

Thanks for reviewing! https://codereview.chromium.org/472163002/diff/20001/Source/modules/mediastream/RTCPeerConnection.cpp File Source/modules/mediastream/RTCPeerConnection.cpp (right): https://codereview.chromium.org/472163002/diff/20001/Source/modules/mediastream/RTCPeerConnection.cpp#newcode705 Source/modules/mediastream/RTCPeerConnection.cpp:705: void RTCPeerConnection::closePeerConnection() On 2014/08/15 16:12:38, jiayl ...

6 years, 4 months ago (2014-08-16 00:36:25 UTC) #6

abarth-chromium

One nit: I'd rename doClose to closeInternal. We generally avoid the "do" prefix because it ...

6 years, 4 months ago (2014-08-18 17:11:38 UTC) #8

vrk (LEFT CHROMIUM)

On 2014/08/18 17:11:38, abarth wrote: > One nit: I'd rename doClose to closeInternal. We generally ...

6 years, 4 months ago (2014-08-19 00:46:21 UTC) #9

commit-bot: I haz the power

CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/vrk@chromium.org/472163002/80001

6 years, 4 months ago (2014-08-19 00:47:51 UTC) #11

commit-bot: I haz the power

6 years, 4 months ago (2014-08-19 01:48:56 UTC) #12

Message was sent while issue was closed.

Committed patchset #4 (80001) as 180508

Expand Messages | Collapse Messages