generic-library/webrtc
1
0
Fork 0
Code Issues Pull Requests Releases Wiki Activity

PeerConnection(java): rationalize pointer-to-jlong conversion.

Browse Source 
In r4665 I went a bit crazy with the manual reinterpretation of a pointer to a
jlong (to avoid undefined behavior) but that's what reinterpret_cast<> is for.
So use it directly now.
Added a do-nothing DataChannel to AppRTCDemo to regression test this, since the
only repro I've found of the original bug requires ARM ABI (PeerConnectionTest
on ia32 fails to repro).

BUG=2302
R=henrike@webrtc.org

Review URL: https://webrtc-codereview.appspot.com/5489004

git-svn-id: http://webrtc.googlecode.com/svn/trunk@5269 4adac7df-926f-26a2-2b94-8c16560cd09d
This commit is contained in:
fischman@webrtc.org 2013-12-11 21:07:18 +00:00
parent 9caf2765b2
commit f41f06b916
3 changed files with 32 additions and 19 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

26
talk/app/webrtc/java/jni/peerconnection_jni.cc
View File

@ -195,22 +195,18 @@ static JNIEnv* AttachCurrentThreadIfNeeded() {
return jni; return jni;
} }
// Return a |jlong| that will automatically convert back to |ptr| when assigned // Return a |jlong| that will correctly convert back to |ptr|. This is needed
// to a |uint64| // because the alternative (of silently passing a 32-bit pointer to a vararg
// function expecting a 64-bit param) picks up garbage in the high 32 bits.
static jlong jlongFromPointer(void* ptr) { static jlong jlongFromPointer(void* ptr) {
COMPILE_ASSERT(sizeof(intptr_t) <= sizeof(uint64), COMPILE_ASSERT(sizeof(intptr_t) <= sizeof(jlong),
Time_to_rethink_the_use_of_jlongs); Time_to_rethink_the_use_of_jlongs);
// Guaranteed to fit by the COMPILE_ASSERT above. // Going through intptr_t to be obvious about the definedness of the
uint64 u64 = reinterpret_cast<intptr_t>(ptr); // conversion from pointer to integral type. intptr_t to jlong is a standard
// If the unsigned value fits in the signed type, return it directly. // widening by the COMPILE_ASSERT above.
if (u64 <= std::numeric_limits<int64>::max()) jlong ret = reinterpret_cast<intptr_t>(ptr);
return u64; assert(reinterpret_cast<void*>(ret) == ptr);
// Otherwise, we need to get move u64 into the range of [int64min, -1] subject return ret;
// to the constraints of remaining equal to |u64| modulo |2^64|.
u64 = std::numeric_limits<uint64>::max() - u64; // In [0,int64max].
int64 i64 = -u64; // In [-int64max, 0].
i64 -= 1; // In [int64min, -1], and i64+2^64==u64.
return i64;
} }
// Android's FindClass() is trickier than usual because the app-specific // Android's FindClass() is trickier than usual because the app-specific
@ -1104,7 +1100,7 @@ JOW(jlong, DataChannel_registerObserverNative)(
talk_base::scoped_ptr<DataChannelObserverWrapper> observer( talk_base::scoped_ptr<DataChannelObserverWrapper> observer(
new DataChannelObserverWrapper(jni, j_observer)); new DataChannelObserverWrapper(jni, j_observer));
ExtractNativeDC(jni, j_dc)->RegisterObserver(observer.get()); ExtractNativeDC(jni, j_dc)->RegisterObserver(observer.get());
return reinterpret_cast<jlong>(observer.release()); return jlongFromPointer(observer.release());
} }
JOW(void, DataChannel_unregisterObserverNative)( JOW(void, DataChannel_unregisterObserverNative)(

6
talk/app/webrtc/javatests/src/org/webrtc/PeerConnectionTest.java
View File

@ -533,9 +533,11 @@ public class PeerConnectionTest extends TestCase {
factory, offeringPC, videoSource, "oLMS", "oLMSv0", "oLMSa0", factory, offeringPC, videoSource, "oLMS", "oLMSv0", "oLMSa0",
offeringExpectations); offeringExpectations);
offeringExpectations.setDataChannel(offeringPC.createDataChannel( DataChannel offeringDC = offeringPC.createDataChannel(
"offeringDC", new DataChannel.Init())); "offeringDC", new DataChannel.Init());
assertEquals("offeringDC", offeringDC.label());
offeringExpectations.setDataChannel(offeringDC);
SdpObserverLatch sdpLatch = new SdpObserverLatch(); SdpObserverLatch sdpLatch = new SdpObserverLatch();
offeringPC.createOffer(sdpLatch, new MediaConstraints()); offeringPC.createOffer(sdpLatch, new MediaConstraints());
assertTrue(sdpLatch.await()); assertTrue(sdpLatch.await());

19
talk/examples/android/src/org/appspot/apprtc/AppRTCDemoActivity.java
View File

@ -171,11 +171,26 @@ public class AppRTCDemoActivity extends Activity
} }
} }
// Just for fun (and to regression-test bug 2302) make sure that DataChannels
// can be created, queried, and disposed.
private static void createDataChannelToRegressionTestBug2302(
PeerConnection pc) {
DataChannel dc = pc.createDataChannel("dcLabel", new DataChannel.Init());
abortUnless("dcLabel".equals(dc.label()), "WTF?");
dc.dispose();
}
@Override @Override
public void onIceServers(List<PeerConnection.IceServer> iceServers) { public void onIceServers(List<PeerConnection.IceServer> iceServers) {
factory = new PeerConnectionFactory(); factory = new PeerConnectionFactory();
pc = factory.createPeerConnection(
iceServers, appRtcClient.pcConstraints(), pcObserver); MediaConstraints pcConstraints = appRtcClient.pcConstraints();
pcConstraints.optional.add(
new MediaConstraints.KeyValuePair("RtpDataChannels", "true"));
pc = factory.createPeerConnection(iceServers, pcConstraints, pcObserver);
createDataChannelToRegressionTestBug2302(pc); // See method comment.
// Uncomment to get ALL WebRTC tracing and SENSITIVE libjingle logging. // Uncomment to get ALL WebRTC tracing and SENSITIVE libjingle logging.
// NOTE: this _must_ happen while |factory| is alive! // NOTE: this _must_ happen while |factory| is alive!