Add network tester client [android]

Add network tester client [android] to be able to test mobile networks in terms of packet size and sending rates.

BUG=webrtc:7426
Review-Url: https://codereview.webrtc.org/2787863002
Cr-Commit-Position: refs/heads/master@{#17789}
Committed: https://chromium.googlesource.com/external/webrtc/+/2fe9ac376360f22dbdd18ca23a950c149e36a2d7

[michaelt, 2017-03-30 15:50:09]

Description was changed from

==========
add android app

BUG=webrtc:7426
==========

to

==========
add android app

BUG=webrtc:7426
==========

[michaelt, 2017-03-30 15:50:09]

michaelt@webrtc.org changed reviewers:
+ magjed@webrtc.org, minyue@webrtc.org, nisse@webrtc.org, stefan@webrtc.org

[minyue-webrtc, 2017-03-31 07:49:56]

On 2017/03/30 15:50:44, michaelt wrote:

Please update the title

[michaelt, 2017-04-03 07:01:05]

On 2017/03/31 07:49:56, minyue-webrtc wrote:
> On 2017/03/30 15:50:44, michaelt wrote:
> 
> Please update the title

Done

[magjed_webrtc, 2017-04-04 09:05:59]

Please make the CL description more descriptive.

https://codereview.webrtc.org/2787863002/diff/1/webrtc/tools/network_tester/a...
File
webrtc/tools/network_tester/androidapp/src/com/google/media/networktester/Tester.java
(right):

https://codereview.webrtc.org/2787863002/diff/1/webrtc/tools/network_tester/a...
webrtc/tools/network_tester/androidapp/src/com/google/media/networktester/Tester.java:13:
public class Tester extends Thread {
Make the class name more descriptive, or consider moving this to a nested class
in MainActivity, or even just an anonymous 'new Thread(new Runnable() {...'.

https://codereview.webrtc.org/2787863002/diff/1/webrtc/tools/network_tester/a...
webrtc/tools/network_tester/androidapp/src/com/google/media/networktester/Tester.java:22:
private static final String TAG = "NetworkTester";
This is not used.

https://codereview.webrtc.org/2787863002/diff/1/webrtc/tools/network_tester/a...
webrtc/tools/network_tester/androidapp/src/com/google/media/networktester/Tester.java:27:
public synchronized void startTest() {
Remove this and launch a new thread for every test run instead.

https://codereview.webrtc.org/2787863002/diff/1/webrtc/tools/network_tester/a...
webrtc/tools/network_tester/androidapp/src/com/google/media/networktester/Tester.java:37:
while (true) {
Do this instead:

public void run() {
  final long testController = CreateTestController();
  TestControllerConnect(testController);
  while (!Thread.interrupted() && !TestControllerIsDone(testController)) {
    TestControllerRun(testController);
  }
  DestroyTestController(testController);
}

and interrupt the thread from MainActivity to stop it.

https://codereview.webrtc.org/2787863002/diff/1/webrtc/tools/network_tester/j...
File webrtc/tools/network_tester/jni.cpp (right):

https://codereview.webrtc.org/2787863002/diff/1/webrtc/tools/network_tester/j...
webrtc/tools/network_tester/jni.cpp:22: return reinterpret_cast<jlong>(new
webrtc::TestController(
Use jlongFromPointer instead.

[michaelt, 2017-04-05 12:01:51]

Description was changed from

==========
add android app

BUG=webrtc:7426
==========

to

==========
Add network tester client [android] to be able to test mobile networks in terms
of packet size and sending rates.

BUG=webrtc:7426
==========

[michaelt, 2017-04-05 15:34:53]

Patchset #2 (id:20001) has been deleted

[michaelt, 2017-04-05 15:36:16]

https://codereview.webrtc.org/2787863002/diff/1/webrtc/tools/network_tester/a...
File
webrtc/tools/network_tester/androidapp/src/com/google/media/networktester/Tester.java
(right):

https://codereview.webrtc.org/2787863002/diff/1/webrtc/tools/network_tester/a...
webrtc/tools/network_tester/androidapp/src/com/google/media/networktester/Tester.java:13:
public class Tester extends Thread {
I'd like to capsulate the c functions in this class so i would prefer to keep
it.
Renamed it.

https://codereview.webrtc.org/2787863002/diff/1/webrtc/tools/network_tester/a...
webrtc/tools/network_tester/androidapp/src/com/google/media/networktester/Tester.java:22:
private static final String TAG = "NetworkTester";
On 2017/04/04 09:05:59, magjed_webrtc wrote:
> This is not used.

Acknowledged.

https://codereview.webrtc.org/2787863002/diff/1/webrtc/tools/network_tester/a...
webrtc/tools/network_tester/androidapp/src/com/google/media/networktester/Tester.java:27:
public synchronized void startTest() {
On 2017/04/04 09:05:59, magjed_webrtc wrote:
> Remove this and launch a new thread for every test run instead.

Done.

https://codereview.webrtc.org/2787863002/diff/1/webrtc/tools/network_tester/a...
webrtc/tools/network_tester/androidapp/src/com/google/media/networktester/Tester.java:37:
while (true) {
On 2017/04/04 09:05:59, magjed_webrtc wrote:
> Do this instead:
> 
> public void run() {
>   final long testController = CreateTestController();
>   TestControllerConnect(testController);
>   while (!Thread.interrupted() && !TestControllerIsDone(testController)) {
>     TestControllerRun(testController);
>   }
>   DestroyTestController(testController);
> }
> 
> and interrupt the thread from MainActivity to stop it.

Done.

https://codereview.webrtc.org/2787863002/diff/1/webrtc/tools/network_tester/j...
File webrtc/tools/network_tester/jni.cpp (right):

https://codereview.webrtc.org/2787863002/diff/1/webrtc/tools/network_tester/j...
webrtc/tools/network_tester/jni.cpp:22: return reinterpret_cast<jlong>(new
webrtc::TestController(
I would have to set the libjingle_peerconnection_jni as dep if i like to use
this function.
Seams a bit of overkill to me.

Or should we move this helper function in to a separate target so that i don't
have to link the libjingle_peerconnection_jni ?

[magjed_webrtc, 2017-04-05 19:08:41]

lgtm

https://codereview.webrtc.org/2787863002/diff/1/webrtc/tools/network_tester/j...
File webrtc/tools/network_tester/jni.cpp (right):

https://codereview.webrtc.org/2787863002/diff/1/webrtc/tools/network_tester/j...
webrtc/tools/network_tester/jni.cpp:22: return reinterpret_cast<jlong>(new
webrtc::TestController(
On 2017/04/05 15:36:16, michaelt wrote:
> I would have to set the libjingle_peerconnection_jni as dep if i like to use
> this function.
> Seams a bit of overkill to me.
> 
> Or should we move this helper function in to a separate target so that i don't
> have to link the libjingle_peerconnection_jni ?
> 

Ok, then it's an overkill. Let's go with reinterpret_cast.

[stefan-webrtc, 2017-04-06 12:40:24]

lgtm

https://codereview.webrtc.org/2787863002/diff/60001/webrtc/tools/network_test...
File webrtc/tools/network_tester/jni.cpp (right):

https://codereview.webrtc.org/2787863002/diff/60001/webrtc/tools/network_test...
webrtc/tools/network_tester/jni.cpp:33: ->SendConnectTo("85.195.237.107", 9090);
I guess this would be good to put inside a text box instead?

I'm fine with doing that separately though

[stefan-webrtc, 2017-04-07 13:01:56]

nisse, minyue, any comments or should michael land this?

[minyue-webrtc, 2017-04-07 13:11:13]

On 2017/04/07 13:01:56, stefan-webrtc wrote:
> nisse, minyue, any comments or should michael land this?

Sure. in addition to the hard coded IP address, I don't quite get how the config
proto get to pass down.

[michaelt, 2017-04-10 07:01:05]

Along the hard coded IP i pass down a hard coded path of the config file to the
tester.
I could add a pref. for both settings. But since this CL is already big, I would
prefer to do this in a follow up.

[minyue-webrtc, 2017-04-10 07:03:55]

lgtm

[michaelt, 2017-04-19 14:24:55]

The CQ bit was checked by michaelt@webrtc.org to run a CQ dry run

[commit-bot: I haz the power, 2017-04-19 14:25:04]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.webrtc.org/...

[commit-bot: I haz the power, 2017-04-19 14:26:33]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-04-19 14:26:34]

Dry run: Try jobs failed on following builders:
  ios32_sim_ios9_dbg on master.tryserver.webrtc (JOB_FAILED,
http://build.chromium.org/p/tryserver.webrtc/builders/ios32_sim_ios9_dbg/buil...)
  ios_arm64_dbg on master.tryserver.webrtc (JOB_FAILED,
http://build.chromium.org/p/tryserver.webrtc/builders/ios_arm64_dbg/builds/19620)
  ios_arm64_rel on master.tryserver.webrtc (JOB_FAILED,
http://build.chromium.org/p/tryserver.webrtc/builders/ios_arm64_rel/builds/19416)
  ios_dbg on master.tryserver.webrtc (JOB_FAILED,
http://build.chromium.org/p/tryserver.webrtc/builders/ios_dbg/builds/24923)
  ios_rel on master.tryserver.webrtc (JOB_FAILED,
http://build.chromium.org/p/tryserver.webrtc/builders/ios_rel/builds/23767)

[michaelt, 2017-04-20 12:49:37]

The CQ bit was checked by michaelt@webrtc.org to run a CQ dry run

[commit-bot: I haz the power, 2017-04-20 12:49:47]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.webrtc.org/...

[commit-bot: I haz the power, 2017-04-20 13:16:28]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-04-20 13:16:29]

Dry run: This issue passed the CQ dry run.

[michaelt, 2017-04-20 13:53:45]

The CQ bit was checked by michaelt@webrtc.org

[michaelt, 2017-04-20 13:53:46]

The patchset sent to the CQ was uploaded after l-g-t-m from magjed@webrtc.org,
stefan@webrtc.org, minyue@webrtc.org
Link to the patchset: https://codereview.webrtc.org/2787863002/#ps100001 (title:
"Rebased")

[commit-bot: I haz the power, 2017-04-20 13:53:53]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.webrtc.org/...

[commit-bot: I haz the power, 2017-04-20 13:56:27]

CQ is committing da patch.

Bot data: {"patchset_id": 100001, "attempt_start_ts": 1492696425923850,
"parent_rev": "fcea39d7ce3f1601d07fb34f04635be7f98eae25", "commit_rev":
"2fe9ac376360f22dbdd18ca23a950c149e36a2d7"}

[commit-bot: I haz the power, 2017-04-20 13:56:32]

Description was changed from

==========
Add network tester client [android] to be able to test mobile networks in terms
of packet size and sending rates.

BUG=webrtc:7426
==========

to

==========
Add network tester client [android] to be able to test mobile networks in terms
of packet size and sending rates.

BUG=webrtc:7426

Review-Url: https://codereview.webrtc.org/2787863002
Cr-Commit-Position: refs/heads/master@{#17789}
Committed:
https://chromium.googlesource.com/external/webrtc/+/2fe9ac376360f22dbdd18ca23...
==========

[commit-bot: I haz the power, 2017-04-20 13:56:33]

Committed patchset #5 (id:100001) as
https://chromium.googlesource.com/external/webrtc/+/2fe9ac376360f22dbdd18ca23...