Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(103)

Issues Search
    My Issues | Starred     Open | Closed | All

Issue 10008077: Adding JSEP PeerConnection glue - attempt 2 (Closed)

Created:
8 years, 8 months ago by perkj_chrome
Modified:
8 years, 8 months ago
CC:
chromium-reviews, joi+watch-content_chromium.org, darin-cc_chromium.org, jam, feature-media-reviews_chromium.org
Visibility:
Public.

Description

Adding JSEP PeerConnection glue - attempt 2 (cloned http://codereview.chromium.org/9699069/#ps35007 since it was reverted) This adds glue for JSEP PeerConnection. PeerConnectionHandler is split in two classes and a base class. The class name is kept for the old ROAP PeerConnection to be aligned with WebKit naming. ROAP is planned to be removed pretty soon, then the classes can be refactored. See also main WebKit bug https://bugs.webkit.org/show_bug.cgi?id=80589 (In particular https://bugs.webkit.org/show_bug.cgi?id=82450) TEST=content_unittests and manual webrtc test. BUG=123130 TBR=darin Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=132210

Patch Set 1 #

Patch Set 2 : Fix memory leak in unit test. Bug http://code.google.com/p/chromium/issues/detail?id=123130 #

Patch Set 3 : Fix memory leak in media_stream_center.cc #

Patch Set 4 : Add missing file. Sorry... #

Total comments: 1

Patch Set 5 : rebased #

Unified diffs Side-by-side diffs Delta from patch set Stats (+1396 lines, -228 lines) Patch
M content/content_renderer.gypi View 1 chunk +4 lines, -0 lines 0 comments Download
M content/content_tests.gypi View 1 2 3 4 2 chunks +3 lines, -0 lines 0 comments Download
M content/renderer/media/media_stream_center.h View 1 chunk +6 lines, -0 lines 0 comments Download
M content/renderer/media/media_stream_center.cc View 1 2 2 chunks +40 lines, -0 lines 0 comments Download
M content/renderer/media/media_stream_dependency_factory.h View 1 chunk +6 lines, -0 lines 0 comments Download
M content/renderer/media/media_stream_dependency_factory.cc View 1 chunk +11 lines, -0 lines 0 comments Download
M content/renderer/media/media_stream_impl.h View 6 chunks +20 lines, -8 lines 0 comments Download
M content/renderer/media/media_stream_impl.cc View 5 chunks +55 lines, -33 lines 0 comments Download
M content/renderer/media/media_stream_impl_unittest.cc View 2 chunks +65 lines, -3 lines 0 comments Download
M content/renderer/media/mock_media_stream_dependency_factory.h View 1 chunk +5 lines, -0 lines 0 comments Download
M content/renderer/media/mock_media_stream_dependency_factory.cc View 4 chunks +74 lines, -2 lines 0 comments Download
M content/renderer/media/mock_media_stream_impl.h View 1 chunk +2 lines, -1 line 0 comments Download
M content/renderer/media/mock_media_stream_impl.cc View 1 chunk +2 lines, -1 line 0 comments Download
M content/renderer/media/mock_peer_connection_impl.h View 1 2 chunks +26 lines, -1 line 0 comments Download
M content/renderer/media/mock_peer_connection_impl.cc View 1 4 chunks +33 lines, -21 lines 0 comments Download
A content/renderer/media/mock_web_peer_connection_00_handler_client.h View 1 2 3 1 chunk +51 lines, -0 lines 0 comments Download
A content/renderer/media/mock_web_peer_connection_00_handler_client.cc View 1 chunk +56 lines, -0 lines 0 comments Download
M content/renderer/media/peer_connection_handler.h View 3 chunks +6 lines, -43 lines 0 comments Download
M content/renderer/media/peer_connection_handler.cc View 6 chunks +13 lines, -107 lines 0 comments Download
A content/renderer/media/peer_connection_handler_base.h View 1 chunk +71 lines, -0 lines 0 comments Download
A content/renderer/media/peer_connection_handler_base.cc View 1 chunk +123 lines, -0 lines 0 comments Download
A content/renderer/media/peer_connection_handler_jsep.h View 1 chunk +85 lines, -0 lines 0 comments Download
A content/renderer/media/peer_connection_handler_jsep.cc View 1 chunk +368 lines, -0 lines 0 comments Download
A content/renderer/media/peer_connection_handler_jsep_unittest.cc View 1 1 chunk +231 lines, -0 lines 0 comments Download
M content/renderer/render_view_impl.h View 2 chunks +4 lines, -0 lines 0 comments Download
M content/renderer/render_view_impl.cc View 1 2 3 4 5 chunks +20 lines, -8 lines 0 comments Download
M content/renderer/renderer_webkitplatformsupport_impl.h View 1 chunk +2 lines, -0 lines 0 comments Download
M content/renderer/renderer_webkitplatformsupport_impl.cc View 1 2 3 4 2 chunks +14 lines, -0 lines 0 comments Download

Messages

Total messages: 6 (0 generated)
perkj_chrome
Can you please review. It have passed the linux try bots. linux_valgrind pass all memory ...
8 years, 8 months ago (2012-04-12 14:43:53 UTC) #1
tommi (sloooow) - chröme
lgtm. Thanks for fixing the leaks. https://chromiumcodereview.appspot.com/10008077/diff/1060/content/renderer/media/media_stream_center.cc File content/renderer/media/media_stream_center.cc (right): https://chromiumcodereview.appspot.com/10008077/diff/1060/content/renderer/media/media_stream_center.cc#newcode54 content/renderer/media/media_stream_center.cc:54: scoped_ptr<webrtc::IceCandidateInterface> native_candidate( yikes!
8 years, 8 months ago (2012-04-13 08:11:34 UTC) #2
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/perkj@chromium.org/10008077/9001
8 years, 8 months ago (2012-04-13 14:00:34 UTC) #3
commit-bot: I haz the power
Try job failure for 10008077-9001 on linux_chromeos for step "update". http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=linux_chromeos&number=10303 Step "update" is always ...
8 years, 8 months ago (2012-04-13 14:02:23 UTC) #4
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/perkj@chromium.org/10008077/9001
8 years, 8 months ago (2012-04-13 15:40:41 UTC) #5
commit-bot: I haz the power
8 years, 8 months ago (2012-04-13 17:47:49 UTC) #6 
Change committed as 132210

Powered by Google App Engine
This is Rietveld 408576698