Detect connectivity changes to bind the process to the active network

[alalamav]

Fixes a bug that occurs when, after the device looses connectivity, it is not possible to reconnect the VPN unless the app is restarted. We now detect connectivity changes and bind the process to a connected, non-VPN network when available.

---

This change is

[bemasc]

Review status: 0 of 2 files reviewed at latest revision, 4 unresolved discussions.

---

src/android/org/uproxy/tun2socks/NetworkManager.java, line 50 [r1] (raw file):

    // Destroys the network receiver.
    public void destroy() {
      if (m_broadcastReceiver != null) {

Nit: an early return can be nicer than indenting the whole function body.

---

_src/android/org/uproxy/tun2socks/NetworkManager.java, line 93 [r1] (raw file):_

    private void connectivityChanged(Intent intent) {
      NetworkInfo activeNetworkInfo = m_connectivityManager.getActiveNetworkInfo();
      Log.d(LOG_TAG, "ACTIVE NETWORK " + ((activeNetworkInfo != null) ? activeNetworkInfo.toString() : "null" ));

I think Java automatically calls toString so you don't have to?

---

src/android/org/uproxy/tun2socks/NetworkManager.java, line 140 [r1] (raw file):

        return networks.get(ConnectivityManager.TYPE_MOBILE);
      }
      return null;

I think this should probably return an arbitrary network , e.g.

for (Integer key : networks.keySet()) {
  return networks.get(key);
}

---

_src/android/org/uproxy/tun2socks/NetworkManager.java, line 145 [r1] (raw file):_

   // Returns true if the supplied network is connected, available, and not
   // a VPN.
   private boolean isConnectedNonVpnNetwork(NetworkInfo networkInfo) {

Could this be static?

---

Comments from Reviewable

[alalamav]

Review status: 0 of 2 files reviewed at latest revision, 4 unresolved discussions.

---

src/android/org/uproxy/tun2socks/NetworkManager.java, line 50 [r1] (raw file):

Previously, bemasc (Benjamin M. Schwartz) wrote…

> Nit: an early return can be nicer than indenting the whole function body. >

Done.

---

src/android/org/uproxy/tun2socks/NetworkManager.java, line 93 [r1] (raw file):

Previously, bemasc (Benjamin M. Schwartz) wrote…

> I think Java automatically calls `toString` so you don't have to? >

I believe this to be true for System.print, not for Log methods.

---

src/android/org/uproxy/tun2socks/NetworkManager.java, line 140 [r1] (raw file):

Previously, bemasc (Benjamin M. Schwartz) wrote…

> I think this should probably return an arbitrary network , e.g. > > ``` java > for (Integer key : networks.keySet()) { > return networks.get(key); > } > ``` > >

I'm not so sure. I added an additional network type (TYPE_MOBILE_DUN), so that the only possible network types left are TYPE_DUMMY and TYPE_BLUETOOTH. Would we want to bind to those?

---

_src/android/org/uproxy/tun2socks/NetworkManager.java, line 145 [r1] (raw file):_

Previously, bemasc (Benjamin M. Schwartz) wrote…

> Could this be `static`? >

Done.

---

Comments from Reviewable

[bemasc]

src/android/org/uproxy/tun2socks/NetworkManager.java, line 140 [r1] (raw file):

Previously, albertolalama (alberto lalama) wrote…

> I'm not so sure. I added an additional network type (TYPE_MOBILE_DUN), so that the only possible network types left are TYPE_DUMMY and TYPE_BLUETOOTH. Would we want to bind to those? >

OK. No need to risk binding to bad networks, since the likelihood of new good network types being introduced is probably low.

---

Comments from Reviewable

[bemasc]

:+1: with one nit

---

Review status: 0 of 2 files reviewed at latest revision, 1 unresolved discussion.

---

_src/android/org/uproxy/tun2socks/NetworkManager.java, line 93 [r1] (raw file):_

Previously, albertolalama (alberto lalama) wrote…

> I believe this to be true for System.print, not for Log methods. >

The docs say "The Java language provides special support for the string concatenation operator ( + ), and for conversion of other objects to strings. ... String conversions are implemented through the method toString, defined by Object and inherited by all classes in Java.".

So I think just constructing the string is sufficient. You don't even need the temporary variable.

---

Comments from Reviewable