Enabling usage of native PeerConnection v2 in libjingle.

content/renderer/media/media_stream_dependency_factory.cc

Index: content/renderer/media/media_stream_dependency_factory.cc
diff --git a/content/renderer/media/media_stream_dependency_factory.cc b/content/renderer/media/media_stream_dependency_factory.cc
index a9e9d469e45882519c97a065c68ef8d902884d06..f1834906c53da655631c0e1f4c593787a5abaf18 100644
--- a/content/renderer/media/media_stream_dependency_factory.cc
+++ b/content/renderer/media/media_stream_dependency_factory.cc
@@ -4,69 +4,137 @@
 
 #include "content/renderer/media/media_stream_dependency_factory.h"
 
+#include <vector>
+
 #include "content/renderer/media/video_capture_module_impl.h"
 #include "content/renderer/media/webrtc_audio_device_impl.h"
 #include "content/renderer/p2p/ipc_network_manager.h"
 #include "content/renderer/p2p/ipc_socket_factory.h"
 #include "content/renderer/p2p/port_allocator.h"
 #include "jingle/glue/thread_wrapper.h"
-#include "third_party/libjingle/source/talk/app/webrtcv1/peerconnection.h"
-#include "third_party/libjingle/source/talk/session/phone/dummydevicemanager.h"
-#include "third_party/libjingle/source/talk/session/phone/webrtcmediaengine.h"
+#include "third_party/libjingle/source/talk/app/webrtc/peerconnection.h"
 #include "third_party/WebKit/Source/WebKit/chromium/public/WebFrame.h"
 
+static std::string GetAddress(const std::string& hostname) {
+  std::string address;
+  size_t pos = hostname.find(':');
+  if (pos == std::string::npos) {
+    DVLOG(1) << "Invalid hostname.";

[tommi (sloooow) - chröme, 2012/01/24 15:18:56]

is this correct?  Feels like the hostname might be fine although there's no port
number.  Should this perhaps be "No port specified?"

[Henrik Grunell, 2012/01/26 13:03:16]

Right. Fixed.

+  } else {
+    address = hostname.substr(0, pos);
+  }
+  return address;

[tommi (sloooow) - chröme, 2012/01/24 15:18:56]

nit: Change the name of the method to be GetHostName, ParseHostName or
RemovePortFromHostName.  GetAddress implies (to me at least) name resolution.

[Henrik Grunell, 2012/01/26 13:03:16]

Done.

+}
+
+class P2PPortAllocatorFactory : public webrtc::PortAllocatorFactoryInterface {
+ public:
+  P2PPortAllocatorFactory(content::P2PSocketDispatcher* socket_dispatcher,
+      talk_base::NetworkManager* network_manager,
+      talk_base::PacketSocketFactory* socket_factory)
+      : socket_dispatcher_(socket_dispatcher),
+        network_manager_(network_manager),
+        socket_factory_(socket_factory) {
+  }
+  virtual cricket::PortAllocator* CreatePortAllocator(
+      const std::vector<StunConfiguration>& stun_servers,
+      const std::vector<TurnConfiguration>& turn_configurations) {

[tommi (sloooow) - chröme, 2012/01/24 15:18:56]

don't forget to use OVERRIDE for all overridden virtual methods.

[Henrik Grunell, 2012/01/26 13:03:16]

Done.

+    WebKit::WebFrame* web_frame = WebKit::WebFrame::frameForCurrentContext();
+    if (!web_frame) {
+      DVLOG(1) << "WebFrame is NULL.";

[tommi (sloooow) - chröme, 2012/01/24 15:18:56]

warning/error?

[Henrik Grunell, 2012/01/26 13:03:16]

Yes, I think it should be an error. Fixed.

+      return NULL;
+    }
+    webkit_glue::P2PTransport::Config config;
+    // P2PPortAllocator only supports one set of config.
+    if (stun_servers.size() > 0) {
+      config.stun_server = GetAddress(
+          stun_servers[0].server.hostname());

[tommi (sloooow) - chröme, 2012/01/24 15:18:56]

instead of having to strip the port away from the host name, should hostname()
perhaps only return the host name?
It feels like it's a bit of a misnomer if it can also return the port part. 
Besides we already have a port() accessor as well, so we shouldn't have that
information in hostname as well imho.
Then you can remove the GetAddress method.

[Henrik Grunell, 2012/01/26 13:03:16]

Yes I agree, but it's libjingle code that sets this up and calls this function.
I'll bring it up with that team.

+      config.stun_server_port = stun_servers[0].server.port();
+    }
+    if (turn_configurations.size() > 0) {
+      config.relay_server = GetAddress(
+          turn_configurations[0].server.hostname());
+      config.relay_server_port = turn_configurations[0].server.port();
+      config.relay_username = turn_configurations[0].username;
+      config.relay_password = turn_configurations[0].password;
+    }
+
+    return new content::P2PPortAllocator(web_frame,
+                                         socket_dispatcher_,
+                                         network_manager_,
+                                         socket_factory_,
+                                         config);
+  }
+
+ protected:
+  virtual ~P2PPortAllocatorFactory() {}
+
+ private:
+  // socket_dispatcher_ is a weak reference, owned by RenderView. It's valid
+  // for the lifetime of RenderView.
+  content::P2PSocketDispatcher* socket_dispatcher_;
+  // network_manager_ and socket_factory_ are a weak references, owned by
+  // MediaStreamImpl.
+  talk_base::NetworkManager* network_manager_;
+  talk_base::PacketSocketFactory* socket_factory_;
+};
+
+
 MediaStreamDependencyFactory::MediaStreamDependencyFactory() {}
 
 MediaStreamDependencyFactory::~MediaStreamDependencyFactory() {}
 
-cricket::WebRtcMediaEngine*
-MediaStreamDependencyFactory::CreateWebRtcMediaEngine() {
-  webrtc::AudioDeviceModule* adm = new WebRtcAudioDeviceImpl();
-  webrtc::AudioDeviceModule* adm_sc = new WebRtcAudioDeviceImpl();
-  return new cricket::WebRtcMediaEngine(adm, adm_sc, NULL);
-}
-
 bool MediaStreamDependencyFactory::CreatePeerConnectionFactory(
-    cricket::MediaEngineInterface* media_engine,
-    talk_base::Thread* worker_thread) {
+    talk_base::Thread* worker_thread,
+    talk_base::Thread* signaling_thread,
+    content::P2PSocketDispatcher* socket_dispatcher,
+    talk_base::NetworkManager* network_manager,
+    talk_base::PacketSocketFactory* socket_factory) {
   if (!pc_factory_.get()) {
-    scoped_ptr<webrtc::PeerConnectionFactory> factory(
-        new webrtc::PeerConnectionFactory(media_engine,
-                                          new cricket::DummyDeviceManager(),
-                                          worker_thread));
-    if (factory->Initialize())
-      pc_factory_.reset(factory.release());
+    talk_base::scoped_refptr<P2PPortAllocatorFactory> pa_factory =
+      new talk_base::RefCountedObject<P2PPortAllocatorFactory>(

[tommi (sloooow) - chröme, 2012/01/24 15:18:56]

indent

[Henrik Grunell, 2012/01/26 13:03:16]

Done.

+          socket_dispatcher, network_manager, socket_factory);
+
+    talk_base::scoped_refptr<webrtc::PeerConnectionFactoryInterface> factory(
+        webrtc::CreatePeerConnectionFactory(worker_thread,
+                                            signaling_thread,
+                                            pa_factory.release(),
+                                            new WebRtcAudioDeviceImpl()));
+    if (factory.get())
+      pc_factory_ = factory.release();
   }
   return pc_factory_.get() != NULL;
 }
 
-void MediaStreamDependencyFactory::DeletePeerConnectionFactory() {
-  pc_factory_.reset();
+void MediaStreamDependencyFactory::ReleasePeerConnectionFactory() {
+  if (pc_factory_.get())
+    pc_factory_.release();

[tommi (sloooow) - chröme, 2012/01/24 15:18:56]

hmm... are you sure that scoped_refptr.release() does what you think it does? :)
I have a feeling that this should be:

pc_factory_ = NULL;

[Henrik Grunell, 2012/01/26 13:03:16]

Indeed, complete brain loss. Fixed.

 }
 
 bool MediaStreamDependencyFactory::PeerConnectionFactoryCreated() {
   return pc_factory_.get() != NULL;
 }
 
-cricket::PortAllocator* MediaStreamDependencyFactory::CreatePortAllocator(
-    content::P2PSocketDispatcher* socket_dispatcher,
-    talk_base::NetworkManager* network_manager,
-    talk_base::PacketSocketFactory* socket_factory,
-    const webkit_glue::P2PTransport::Config& config) {
-  WebKit::WebFrame* web_frame = WebKit::WebFrame::frameForCurrentContext();
-  if (!web_frame) {
-    DVLOG(1) << "WebFrame is NULL.";
-    return NULL;
-  }
-  return new content::P2PPortAllocator(web_frame,
-                                       socket_dispatcher,
-                                       network_manager,
-                                       socket_factory,
-                                       config);
+talk_base::scoped_refptr<webrtc::PeerConnectionInterface>

[tommi (sloooow) - chröme, 2012/01/24 15:18:56]

nit: add this at the top of the file to save a bit of horizontal space:

using webrtc::PeerConnectionInterface;

also, if you primarily use talk_base::scoped_refptr in this file, you could also
have a 'using' line for that type at the top of the file.  Then you're down to
just 'scoped_refptr<PeerConnectionInterface>' which isn't so bad.

[Henrik Grunell, 2012/01/26 13:03:16]

Chrome's scoped_refptr is in the global namespace, so it will be problematic.
Given that, we don't gain anything having using for PCInterface.

+MediaStreamDependencyFactory::CreatePeerConnection(
+    const std::string& config, webrtc::PeerConnectionObserver* observer) {

[tommi (sloooow) - chröme, 2012/01/24 15:18:56]

does |config| fit in the line above?

[Henrik Grunell, 2012/01/26 13:03:16]

Yes, but observer doesn't if aligned under. Not allowed to have different
alignment. Moved both to separate lines though; according to the style guide.

+  // TODO(grunell): Can we move this out to pc handler?
+  return pc_factory_->CreatePeerConnection(config, observer);
+}
+
+talk_base::scoped_refptr<webrtc::LocalMediaStreamInterface>
+MediaStreamDependencyFactory::CreateLocalMediaStream(
+    const std::string& label) {
+  return pc_factory_->CreateLocalMediaStream(label);
+}
+
+talk_base::scoped_refptr<webrtc::LocalVideoTrackInterface>
+MediaStreamDependencyFactory::CreateLocalVideoTrack(
+    const std::string& label, cricket::VideoCapturer* video_device) {
+  return pc_factory_->CreateLocalVideoTrack(label, video_device);
 }
 
-webrtc::PeerConnection* MediaStreamDependencyFactory::CreatePeerConnection(
-    cricket::PortAllocator* port_allocator,
-    talk_base::Thread* signaling_thread) {
-  return pc_factory_->CreatePeerConnection(port_allocator, signaling_thread);
+talk_base::scoped_refptr<webrtc::LocalAudioTrackInterface>
+MediaStreamDependencyFactory::CreateLocalAudioTrack(
+    const std::string& label, webrtc::AudioDeviceModule* audio_device) {
+  return pc_factory_->CreateLocalAudioTrack(label, audio_device);
 }