[Android] Add Java observers to the NetworkChangeNotifier

[Android] Add Java observers to the NetworkChangeNotifier

Adds the ability for Java classes to be alerted by the NetworkChangeNotifier
instead of having to loop through the native side first.  Also adds a test.

BUG=136984
Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=158432

[Philippe, 2012-09-21 23:47:03]

LGTM with a really minor nit.

https://chromiumcodereview.appspot.com/10968016/diff/6003/net/android/javates...
File net/android/javatests/src/org/chromium/net/NetworkChangeNotifierTest.java
(right):

https://chromiumcodereview.appspot.com/10968016/diff/6003/net/android/javates...
net/android/javatests/src/org/chromium/net/NetworkChangeNotifierTest.java:21:
private class NetworkChangeNotifierTestObserver
Can this be made static?

[nilesh, 2012-09-22 00:40:24]

https://chromiumcodereview.appspot.com/10968016/diff/6003/net/android/java/sr...
File net/android/java/src/org/chromium/net/NetworkChangeNotifier.java (right):

https://chromiumcodereview.appspot.com/10968016/diff/6003/net/android/java/sr...
net/android/java/src/org/chromium/net/NetworkChangeNotifier.java:21: * function.
WOrth adding that this class is not thread safe.

https://chromiumcodereview.appspot.com/10968016/diff/6003/net/android/java/sr...
net/android/java/src/org/chromium/net/NetworkChangeNotifier.java:25: /** Alerted
when the connection type of the network changes. */
Please add a comment.
onConnectionTypeChanged will be called on the UI thread.

https://chromiumcodereview.appspot.com/10968016/diff/6003/net/android/java/sr...
net/android/java/src/org/chromium/net/NetworkChangeNotifier.java:169: public
static void addConnectionTypeObserver(ConnectionTypeObserver observer) {
No removeConnectionTypeObserver ?
I think this is restrictive, and puts unnecessary limits on the lifecycle of the
observer.

https://chromiumcodereview.appspot.com/10968016/diff/6003/net/android/javates...
File net/android/javatests/src/org/chromium/net/NetworkChangeNotifierTest.java
(right):

https://chromiumcodereview.appspot.com/10968016/diff/6003/net/android/javates...
net/android/javatests/src/org/chromium/net/NetworkChangeNotifierTest.java:92:
new MockConnectivityManagerDelegate();
Looks like no line wrap is needed here.

[gone, 2012-09-24 18:28:24]

https://chromiumcodereview.appspot.com/10968016/diff/6003/net/android/java/sr...
File net/android/java/src/org/chromium/net/NetworkChangeNotifier.java (right):

https://chromiumcodereview.appspot.com/10968016/diff/6003/net/android/java/sr...
net/android/java/src/org/chromium/net/NetworkChangeNotifier.java:21: * function.
On 2012/09/22 00:40:24, nileshagrawal1 wrote:
> WOrth adding that this class is not thread safe.

Done.

https://chromiumcodereview.appspot.com/10968016/diff/6003/net/android/java/sr...
net/android/java/src/org/chromium/net/NetworkChangeNotifier.java:25: /** Alerted
when the connection type of the network changes. */
On 2012/09/22 00:40:24, nileshagrawal1 wrote:
> Please add a comment.
> onConnectionTypeChanged will be called on the UI thread.

Done.

https://chromiumcodereview.appspot.com/10968016/diff/6003/net/android/java/sr...
net/android/java/src/org/chromium/net/NetworkChangeNotifier.java:169: public
static void addConnectionTypeObserver(ConnectionTypeObserver observer) {
On 2012/09/22 00:40:24, nileshagrawal1 wrote:
> No removeConnectionTypeObserver ?
> I think this is restrictive, and puts unnecessary limits on the lifecycle of
the
> observer.

Done.

https://chromiumcodereview.appspot.com/10968016/diff/6003/net/android/javates...
File net/android/javatests/src/org/chromium/net/NetworkChangeNotifierTest.java
(right):

https://chromiumcodereview.appspot.com/10968016/diff/6003/net/android/javates...
net/android/javatests/src/org/chromium/net/NetworkChangeNotifierTest.java:21:
private class NetworkChangeNotifierTestObserver
On 2012/09/21 23:47:03, Philippe wrote:
> Can this be made static?

Done.

https://chromiumcodereview.appspot.com/10968016/diff/6003/net/android/javates...
net/android/javatests/src/org/chromium/net/NetworkChangeNotifierTest.java:92:
new MockConnectivityManagerDelegate();
On 2012/09/22 00:40:24, nileshagrawal1 wrote:
> Looks like no line wrap is needed here.
It's over by exactly 1 character :/

[gone, 2012-09-24 18:39:13]

+yfriedman for OWNERS

[nilesh, 2012-09-24 18:43:31]

https://chromiumcodereview.appspot.com/10968016/diff/14001/net/android/java/s...
File net/android/java/src/org/chromium/net/NetworkChangeNotifier.java (right):

https://chromiumcodereview.appspot.com/10968016/diff/14001/net/android/java/s...
net/android/java/src/org/chromium/net/NetworkChangeNotifier.java:45: private
ArrayList<ConnectionTypeObserver> mConnectionTypeObservers;
Please use CopyOnWriteArrayList to avoid ConcurrentModificationException when an
observer removes itself while processing the onConnectionTypeChanged
(ArrayList.remove will be called while we are traversing the list).

Also we can make this list final.

https://chromiumcodereview.appspot.com/10968016/diff/14001/net/android/java/s...
net/android/java/src/org/chromium/net/NetworkChangeNotifier.java:179:
mConnectionTypeObservers.add(observer);
if (!mConnectionTypeObservers.contains(observer)) ...

[gone, 2012-09-24 20:11:43]

https://chromiumcodereview.appspot.com/10968016/diff/14001/net/android/java/s...
File net/android/java/src/org/chromium/net/NetworkChangeNotifier.java (right):

https://chromiumcodereview.appspot.com/10968016/diff/14001/net/android/java/s...
net/android/java/src/org/chromium/net/NetworkChangeNotifier.java:45: private
ArrayList<ConnectionTypeObserver> mConnectionTypeObservers;
On 2012/09/24 18:43:31, nileshagrawal1 wrote:
> Please use CopyOnWriteArrayList to avoid ConcurrentModificationException when
an
> observer removes itself while processing the onConnectionTypeChanged
> (ArrayList.remove will be called while we are traversing the list).
> 
> Also we can make this list final.

Done.

https://chromiumcodereview.appspot.com/10968016/diff/14001/net/android/java/s...
net/android/java/src/org/chromium/net/NetworkChangeNotifier.java:179:
mConnectionTypeObservers.add(observer);
On 2012/09/24 18:43:31, nileshagrawal1 wrote:
> if (!mConnectionTypeObservers.contains(observer)) ...

Done.

[nilesh, 2012-09-24 20:17:05]

LGTM

[Yaron, 2012-09-24 20:38:19]

rubbestamp lgtm

[commit-bot: I haz the power, 2012-09-24 20:48:10]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/dfalcantara@chromium.org/10968016/17001

[commit-bot: I haz the power, 2012-09-24 22:41:56]

Change committed as 158432