Move ownership of PeerConnectionObserver from Java to C++.
New OwnedPeerConnection takes ownership of the observer. This is done
to allow NativePeerConnectionFactory to return a capsulated object.
Bug: webrtc:8662
Change-Id: Ie876f7b9a1a17ebcfbe51537f712a32ab1a7cbfb
Reviewed-on: https://webrtc-review.googlesource.com/35300
Commit-Queue: Sami Kalliomäki <sakal@webrtc.org>
Reviewed-by: Magnus Jedvert <magjed@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#21610}diff --git a/sdk/android/api/org/webrtc/NativePeerConnectionFactory.java b/sdk/android/api/org/webrtc/NativePeerConnectionFactory.java
index 63ecf10..aeb91e1 100644
--- a/sdk/android/api/org/webrtc/NativePeerConnectionFactory.java
+++ b/sdk/android/api/org/webrtc/NativePeerConnectionFactory.java
@@ -10,10 +10,10 @@
package org.webrtc;
-/** Factory for creating webrtc::PeerConnectionInterface instances. */
+/** Factory for creating webrtc::jni::OwnedPeerConnection instances. */
public interface NativePeerConnectionFactory {
/**
- * Create a new webrtc::PeerConnectionInterface instance and returns a pointer to it.
+ * Create a new webrtc::jni::OwnedPeerConnection instance and returns a pointer to it.
* The caller takes ownership of the object.
*/
long createNativePeerConnection();
diff --git a/sdk/android/api/org/webrtc/PeerConnection.java b/sdk/android/api/org/webrtc/PeerConnection.java
index 238814d..20b06d5 100644
--- a/sdk/android/api/org/webrtc/PeerConnection.java
+++ b/sdk/android/api/org/webrtc/PeerConnection.java
@@ -548,7 +548,6 @@
private final List<MediaStream> localStreams = new ArrayList<>();
private final long nativePeerConnection;
- private final long nativeObserver;
private List<RtpSender> senders = new ArrayList<>();
private List<RtpReceiver> receivers = new ArrayList<>();
@@ -557,12 +556,11 @@
* their PeerConnection creation in JNI.
*/
public PeerConnection(NativePeerConnectionFactory factory) {
- this(factory.createNativePeerConnection(), 0 /* nativeObserver */);
+ this(factory.createNativePeerConnection());
}
- PeerConnection(long nativePeerConnection, long nativeObserver) {
+ PeerConnection(long nativePeerConnection) {
this.nativePeerConnection = nativePeerConnection;
- this.nativeObserver = nativeObserver;
}
// JsepInterface.
@@ -611,7 +609,7 @@
}
public boolean setConfiguration(RTCConfiguration config) {
- return nativeSetConfiguration(config, nativeObserver);
+ return nativeSetConfiguration(config);
}
public boolean addIceCandidate(IceCandidate candidate) {
@@ -781,14 +779,16 @@
receiver.dispose();
}
receivers.clear();
- JniCommon.nativeReleaseRef(nativePeerConnection);
- if (nativeObserver != 0) {
- nativeFreePeerConnectionObserver(nativeObserver);
- }
+ nativeFreeOwnedPeerConnection(nativePeerConnection);
+ }
+
+ /** Returns a pointer to the native webrtc::PeerConnectionInterface. */
+ public long getNativePeerConnection() {
+ return nativeGetNativePeerConnection();
}
@CalledByNative
- public long getNativePeerConnection() {
+ long getNativeOwnedPeerConnection() {
return nativePeerConnection;
}
@@ -796,10 +796,7 @@
return nativeCreatePeerConnectionObserver(observer);
}
- public static void freeNativePeerConnectionObserver(long observer) {
- nativeFreePeerConnectionObserver(observer);
- }
-
+ private native long nativeGetNativePeerConnection();
private native SessionDescription nativeGetLocalDescription();
private native SessionDescription nativeGetRemoteDescription();
private native DataChannel nativeCreateDataChannel(String label, DataChannel.Init init);
@@ -815,8 +812,8 @@
private native IceGatheringState nativeIceGatheringState();
private native void nativeClose();
private static native long nativeCreatePeerConnectionObserver(Observer observer);
- private static native void nativeFreePeerConnectionObserver(long observer);
- private native boolean nativeSetConfiguration(RTCConfiguration config, long nativeObserver);
+ private static native void nativeFreeOwnedPeerConnection(long ownedPeerConnection);
+ private native boolean nativeSetConfiguration(RTCConfiguration config);
private native boolean nativeAddIceCandidate(
String sdpMid, int sdpMLineIndex, String iceCandidateSdp);
private native boolean nativeRemoveIceCandidates(final IceCandidate[] candidates);
diff --git a/sdk/android/api/org/webrtc/PeerConnectionFactory.java b/sdk/android/api/org/webrtc/PeerConnectionFactory.java
index 5c9dc61..e17d655 100644
--- a/sdk/android/api/org/webrtc/PeerConnectionFactory.java
+++ b/sdk/android/api/org/webrtc/PeerConnectionFactory.java
@@ -232,7 +232,7 @@
if (nativePeerConnection == 0) {
return null;
}
- return new PeerConnection(nativePeerConnection, nativeObserver);
+ return new PeerConnection(nativePeerConnection);
}
/**
diff --git a/sdk/android/src/jni/pc/peerconnection.cc b/sdk/android/src/jni/pc/peerconnection.cc
index d904f07..a94d835 100644
--- a/sdk/android/src/jni/pc/peerconnection.cc
+++ b/sdk/android/src/jni/pc/peerconnection.cc
@@ -58,8 +58,9 @@
PeerConnectionInterface* ExtractNativePC(JNIEnv* jni,
const JavaRef<jobject>& j_pc) {
- return reinterpret_cast<PeerConnectionInterface*>(
- Java_PeerConnection_getNativePeerConnection(jni, j_pc));
+ return reinterpret_cast<OwnedPeerConnection*>(
+ Java_PeerConnection_getNativeOwnedPeerConnection(jni, j_pc))
+ ->pc();
}
PeerConnectionInterface::IceServers JavaToNativeIceServers(
@@ -284,12 +285,6 @@
NativeToJavaMediaStreamArray(env, streams));
}
-void PeerConnectionObserverJni::SetConstraints(
- std::unique_ptr<MediaConstraintsInterface> constraints) {
- RTC_CHECK(!constraints_.get()) << "constraints already set!";
- constraints_ = std::move(constraints);
-}
-
// If the NativeToJavaStreamsMap contains the stream, return it.
// Otherwise, create a new Java MediaStream.
JavaMediaStream& PeerConnectionObserverJni::GetOrCreateJavaStream(
@@ -318,6 +313,19 @@
});
}
+OwnedPeerConnection::OwnedPeerConnection(
+ rtc::scoped_refptr<PeerConnectionInterface> peer_connection,
+ std::unique_ptr<PeerConnectionObserver> observer,
+ std::unique_ptr<MediaConstraintsInterface> constraints)
+ : peer_connection_(peer_connection),
+ observer_(std::move(observer)),
+ constraints_(std::move(constraints)) {}
+
+OwnedPeerConnection::~OwnedPeerConnection() {
+ // Ensure that PeerConnection is destroyed before the observer.
+ peer_connection_ = nullptr;
+}
+
static jlong JNI_PeerConnection_CreatePeerConnectionObserver(
JNIEnv* jni,
const JavaParamRef<jclass>&,
@@ -325,12 +333,17 @@
return jlongFromPointer(new PeerConnectionObserverJni(jni, j_observer));
}
-static void JNI_PeerConnection_FreePeerConnectionObserver(
+static void JNI_PeerConnection_FreeOwnedPeerConnection(
JNIEnv*,
const JavaParamRef<jclass>&,
jlong j_p) {
- PeerConnectionObserver* p = reinterpret_cast<PeerConnectionObserver*>(j_p);
- delete p;
+ delete reinterpret_cast<OwnedPeerConnection*>(j_p);
+}
+
+static jlong JNI_PeerConnection_GetNativePeerConnection(
+ JNIEnv* jni,
+ const JavaParamRef<jobject>& j_pc) {
+ return jlongFromPointer(ExtractNativePC(jni, j_pc));
}
static ScopedJavaLocalRef<jobject> JNI_PeerConnection_GetLocalDescription(
@@ -426,19 +439,18 @@
static jboolean JNI_PeerConnection_SetConfiguration(
JNIEnv* jni,
const JavaParamRef<jobject>& j_pc,
- const JavaParamRef<jobject>& j_rtc_config,
- jlong native_observer) {
+ const JavaParamRef<jobject>& j_rtc_config) {
// Need to merge constraints into RTCConfiguration again, which are stored
- // in the observer object.
- PeerConnectionObserverJni* observer =
- reinterpret_cast<PeerConnectionObserverJni*>(native_observer);
+ // in the OwnedPeerConnection object.
+ OwnedPeerConnection* owned_pc = reinterpret_cast<OwnedPeerConnection*>(
+ Java_PeerConnection_getNativeOwnedPeerConnection(jni, j_pc));
PeerConnectionInterface::RTCConfiguration rtc_config(
PeerConnectionInterface::RTCConfigurationType::kAggressive);
JavaToNativeRTCConfiguration(jni, j_rtc_config, &rtc_config);
- if (observer && observer->constraints()) {
- CopyConstraintsIntoRtcConfiguration(observer->constraints(), &rtc_config);
+ if (owned_pc->constraints()) {
+ CopyConstraintsIntoRtcConfiguration(owned_pc->constraints(), &rtc_config);
}
- return ExtractNativePC(jni, j_pc)->SetConfiguration(rtc_config);
+ return owned_pc->pc()->SetConfiguration(rtc_config);
}
static jboolean JNI_PeerConnection_AddIceCandidate(
diff --git a/sdk/android/src/jni/pc/peerconnection.h b/sdk/android/src/jni/pc/peerconnection.h
index 7e14b51..6b021a0 100644
--- a/sdk/android/src/jni/pc/peerconnection.h
+++ b/sdk/android/src/jni/pc/peerconnection.h
@@ -61,9 +61,6 @@
const std::vector<rtc::scoped_refptr<MediaStreamInterface>>&
streams) override;
- void SetConstraints(std::unique_ptr<MediaConstraintsInterface> constraints);
- const MediaConstraintsInterface* constraints() { return constraints_.get(); }
-
private:
typedef std::map<MediaStreamInterface*, JavaMediaStream>
NativeToJavaStreamsMap;
@@ -86,6 +83,36 @@
// C++ -> Java remote streams.
NativeToJavaStreamsMap remote_streams_;
std::vector<JavaRtpReceiverGlobalOwner> rtp_receivers_;
+};
+
+// PeerConnection doesn't take ownership of the observer. In Java API, we don't
+// want the client to have to manually dispose the observer. To solve this, this
+// wrapper class is used for object ownership.
+//
+// Also stores reference to the deprecated PeerConnection constraints for now.
+class OwnedPeerConnection {
+ public:
+ OwnedPeerConnection(
+ rtc::scoped_refptr<PeerConnectionInterface> peer_connection,
+ std::unique_ptr<PeerConnectionObserver> observer)
+ : OwnedPeerConnection(peer_connection,
+ std::move(observer),
+ nullptr /* constraints */) {}
+ // Deprecated. PC constraints are deprecated.
+ OwnedPeerConnection(
+ rtc::scoped_refptr<PeerConnectionInterface> peer_connection,
+ std::unique_ptr<PeerConnectionObserver> observer,
+ std::unique_ptr<MediaConstraintsInterface> constraints);
+ ~OwnedPeerConnection();
+
+ PeerConnectionInterface* pc() const { return peer_connection_.get(); }
+ const MediaConstraintsInterface* constraints() const {
+ return constraints_.get();
+ }
+
+ private:
+ rtc::scoped_refptr<PeerConnectionInterface> peer_connection_;
+ std::unique_ptr<PeerConnectionObserver> observer_;
std::unique_ptr<MediaConstraintsInterface> constraints_;
};
diff --git a/sdk/android/src/jni/pc/peerconnectionfactory.cc b/sdk/android/src/jni/pc/peerconnectionfactory.cc
index b625df8..633a0fe 100644
--- a/sdk/android/src/jni/pc/peerconnectionfactory.cc
+++ b/sdk/android/src/jni/pc/peerconnectionfactory.cc
@@ -417,6 +417,8 @@
rtc::scoped_refptr<PeerConnectionFactoryInterface> f(
reinterpret_cast<PeerConnectionFactoryInterface*>(
factoryFromJava(factory)));
+ std::unique_ptr<PeerConnectionObserver> observer(
+ reinterpret_cast<PeerConnectionObserver*>(observer_p));
PeerConnectionInterface::RTCConfiguration rtc_config(
PeerConnectionInterface::RTCConfigurationType::kAggressive);
@@ -436,15 +438,15 @@
rtc_config.certificates.push_back(certificate);
}
- PeerConnectionObserverJni* observer =
- reinterpret_cast<PeerConnectionObserverJni*>(observer_p);
+ std::unique_ptr<MediaConstraintsInterface> constraints;
if (!j_constraints.is_null()) {
- observer->SetConstraints(JavaToNativeMediaConstraints(jni, j_constraints));
- CopyConstraintsIntoRtcConfiguration(observer->constraints(), &rtc_config);
+ constraints = JavaToNativeMediaConstraints(jni, j_constraints);
+ CopyConstraintsIntoRtcConfiguration(constraints.get(), &rtc_config);
}
rtc::scoped_refptr<PeerConnectionInterface> pc(
- f->CreatePeerConnection(rtc_config, nullptr, nullptr, observer));
- return jlongFromPointer(pc.release());
+ f->CreatePeerConnection(rtc_config, nullptr, nullptr, observer.get()));
+ return jlongFromPointer(
+ new OwnedPeerConnection(pc, std::move(observer), std::move(constraints)));
}
static jlong JNI_PeerConnectionFactory_CreateVideoSource(