From d7568a08c3039971cb7692147b2985a39db1cac7 Mon Sep 17 00:00:00 2001 From: "fischman@webrtc.org" Date: Mon, 13 Jan 2014 22:04:12 +0000 Subject: [PATCH] PeerConnection(java): Add OnRenegotiationNeeded support Also: - Make PeerConnectionObserver::OnRenegotiationNeeded() pure virtual to avoid this sort of mistake in the future. - Sprinkle @Override annotations on some callback definitions that were missing them. - Fix a JNI method-signature-lookup typo (s/(V)V/()V/) in PCOJava::OnError() - Add an explicit ScopedLocalFrameRef to PCOJava::OnError() to match all other C++-fired callbacks, for consistency. BUG=2771 R=wu@webrtc.org Review URL: https://webrtc-codereview.appspot.com/6829004 git-svn-id: http://webrtc.googlecode.com/svn/trunk@5376 4adac7df-926f-26a2-2b94-8c16560cd09d --- .../app/webrtc/java/jni/peerconnection_jni.cc | 11 ++++++++++- .../java/src/org/webrtc/PeerConnection.java | 3 +++ .../src/org/webrtc/PeerConnectionTest.java | 19 +++++++++++++++++++ talk/app/webrtc/peerconnectioninterface.h | 2 +- .../appspot/apprtc/AppRTCDemoActivity.java | 5 +++++ 5 files changed, 38 insertions(+), 2 deletions(-) diff --git a/talk/app/webrtc/java/jni/peerconnection_jni.cc b/talk/app/webrtc/java/jni/peerconnection_jni.cc index 9e005767b..b68c6aec2 100644 --- a/talk/app/webrtc/java/jni/peerconnection_jni.cc +++ b/talk/app/webrtc/java/jni/peerconnection_jni.cc @@ -515,7 +515,8 @@ class PCOJava : public PeerConnectionObserver { } virtual void OnError() OVERRIDE { - jmethodID m = GetMethodID(jni(), *j_observer_class_, "onError", "(V)V"); + ScopedLocalRefFrame local_ref_frame(jni()); + jmethodID m = GetMethodID(jni(), *j_observer_class_, "onError", "()V"); jni()->CallVoidMethod(*j_observer_global_, m); CHECK_EXCEPTION(jni(), "error during CallVoidMethod"); } @@ -648,6 +649,14 @@ class PCOJava : public PeerConnectionObserver { CHECK_EXCEPTION(jni(), "error during CallVoidMethod"); } + virtual void OnRenegotiationNeeded() OVERRIDE { + ScopedLocalRefFrame local_ref_frame(jni()); + jmethodID m = + GetMethodID(jni(), *j_observer_class_, "onRenegotiationNeeded", "()V"); + jni()->CallVoidMethod(*j_observer_global_, m); + CHECK_EXCEPTION(jni(), "error during CallVoidMethod"); + } + void SetConstraints(ConstraintsWrapper* constraints) { CHECK(!constraints_.get(), "constraints already set!"); constraints_.reset(constraints); diff --git a/talk/app/webrtc/java/src/org/webrtc/PeerConnection.java b/talk/app/webrtc/java/src/org/webrtc/PeerConnection.java index 1cd4dc5ad..c2617def1 100644 --- a/talk/app/webrtc/java/src/org/webrtc/PeerConnection.java +++ b/talk/app/webrtc/java/src/org/webrtc/PeerConnection.java @@ -82,6 +82,9 @@ public class PeerConnection { /** Triggered when a remote peer opens a DataChannel. */ public void onDataChannel(DataChannel dataChannel); + + /** Triggered when renegotiation is necessary. */ + public void onRenegotiationNeeded(); } /** Java version of PeerConnectionInterface.IceServer. */ diff --git a/talk/app/webrtc/javatests/src/org/webrtc/PeerConnectionTest.java b/talk/app/webrtc/javatests/src/org/webrtc/PeerConnectionTest.java index dfed4bf02..03be05c28 100644 --- a/talk/app/webrtc/javatests/src/org/webrtc/PeerConnectionTest.java +++ b/talk/app/webrtc/javatests/src/org/webrtc/PeerConnectionTest.java @@ -59,6 +59,7 @@ public class PeerConnectionTest extends TestCase { private final String name; private int expectedIceCandidates = 0; private int expectedErrors = 0; + private int expectedRenegotiations = 0; private int expectedSetSize = 0; private int previouslySeenWidth = 0; private int previouslySeenHeight = 0; @@ -103,6 +104,7 @@ public class PeerConnectionTest extends TestCase { expectedIceCandidates += count; } + @Override public synchronized void onIceCandidate(IceCandidate candidate) { --expectedIceCandidates; // We don't assert expectedIceCandidates >= 0 because it's hard to know @@ -115,6 +117,7 @@ public class PeerConnectionTest extends TestCase { ++expectedErrors; } + @Override public synchronized void onError() { assertTrue(--expectedErrors >= 0); } @@ -236,6 +239,15 @@ public class PeerConnectionTest extends TestCase { assertEquals(DataChannel.State.CONNECTING, dataChannel.state()); } + public synchronized void expectRenegotiationNeeded() { + ++expectedRenegotiations; + } + + @Override + public synchronized void onRenegotiationNeeded() { + assertTrue(--expectedRenegotiations >= 0); + } + public synchronized void expectMessage(ByteBuffer expectedBuffer, boolean expectedBinary) { expectedBuffers.add( @@ -375,20 +387,24 @@ public class PeerConnectionTest extends TestCase { public SdpObserverLatch() {} + @Override public void onCreateSuccess(SessionDescription sdp) { this.sdp = sdp; onSetSuccess(); } + @Override public void onSetSuccess() { success = true; latch.countDown(); } + @Override public void onCreateFailure(String error) { onSetFailure(error); } + @Override public void onSetFailure(String error) { this.error = error; latch.countDown(); @@ -529,10 +545,12 @@ public class PeerConnectionTest extends TestCase { // Drop |label| params from {Audio,Video}Track-related APIs once // https://code.google.com/p/webrtc/issues/detail?id=1253 is fixed. offeringExpectations.expectSetSize(); + offeringExpectations.expectRenegotiationNeeded(); WeakReference oLMS = addTracksToPC( factory, offeringPC, videoSource, "oLMS", "oLMSv0", "oLMSa0", offeringExpectations); + offeringExpectations.expectRenegotiationNeeded(); DataChannel offeringDC = offeringPC.createDataChannel( "offeringDC", new DataChannel.Init()); assertEquals("offeringDC", offeringDC.label()); @@ -557,6 +575,7 @@ public class PeerConnectionTest extends TestCase { assertNull(sdpLatch.getSdp()); answeringExpectations.expectSetSize(); + answeringExpectations.expectRenegotiationNeeded(); WeakReference aLMS = addTracksToPC( factory, answeringPC, videoSource, "aLMS", "aLMSv0", "aLMSa0", answeringExpectations); diff --git a/talk/app/webrtc/peerconnectioninterface.h b/talk/app/webrtc/peerconnectioninterface.h index 01f1e1ccc..4a54d3ce3 100644 --- a/talk/app/webrtc/peerconnectioninterface.h +++ b/talk/app/webrtc/peerconnectioninterface.h @@ -277,7 +277,7 @@ class PeerConnectionObserver { virtual void OnDataChannel(DataChannelInterface* data_channel) {} // Triggered when renegotation is needed, for example the ICE has restarted. - virtual void OnRenegotiationNeeded() {} + virtual void OnRenegotiationNeeded() = 0; // Called any time the IceConnectionState changes virtual void OnIceConnectionChange( diff --git a/talk/examples/android/src/org/appspot/apprtc/AppRTCDemoActivity.java b/talk/examples/android/src/org/appspot/apprtc/AppRTCDemoActivity.java index f5b403a89..e8723559c 100644 --- a/talk/examples/android/src/org/appspot/apprtc/AppRTCDemoActivity.java +++ b/talk/examples/android/src/org/appspot/apprtc/AppRTCDemoActivity.java @@ -419,6 +419,11 @@ public class AppRTCDemoActivity extends Activity } }); } + + @Override public void onRenegotiationNeeded() { + // No need to do anything; AppRTC follows a pre-agreed-upon + // signaling/negotiation protocol. + } } // Implementation detail: handle offer creation/signaling and answer setting,