Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

android: make Channel always enterIdle upon network recover #7611

Merged
merged 1 commit into from Nov 17, 2020
Merged

android: make Channel always enterIdle upon network recover #7611

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
23 changes: 3 additions & 20 deletions android/src/main/java/io/grpc/android/AndroidChannelBuilder.java
View file
Open in desktop
Expand Up @@ -151,7 +151,7 @@ public ManagedChannel build() {

/**
* Wraps an OkHttp channel and handles invoking the appropriate methods (e.g., {@link
* ManagedChannel#resetConnectBackoff}) when the device network state changes.
* ManagedChannel#enterIdle) when the device network state changes.
*/
@VisibleForTesting
static final class AndroidChannel extends ManagedChannel {
Expand Down Expand Up @@ -290,26 +290,9 @@ public void enterIdle() {
/** Respond to changes in the default network. Only used on API levels 24+. */
@TargetApi(Build.VERSION_CODES.N)
private class DefaultNetworkCallback extends ConnectivityManager.NetworkCallback {
// Registering a listener may immediate invoke onAvailable/onLost: the API docs do not specify
// if the methods are always invoked once, then again on any change, or only on change. When
// onAvailable() is invoked immediately without an actual network change, it's preferable to
// (spuriously) resetConnectBackoff() rather than enterIdle(), as the former is a no-op if the
// channel has already moved to CONNECTING.
private boolean isConnected = false;

@Override
public void onAvailable(Network network) {
if (isConnected) {
delegate.enterIdle();
} else {
delegate.resetConnectBackoff();
}
isConnected = true;
}

@Override
public void onLost(Network network) {
isConnected = false;
delegate.enterIdle();
}
}

Expand All @@ -325,7 +308,7 @@ public void onReceive(Context context, Intent intent) {
boolean wasConnected = isConnected;
isConnected = networkInfo != null && networkInfo.isConnected();
if (isConnected && !wasConnected) {
delegate.resetConnectBackoff();
delegate.enterIdle();
}
}
}
Expand Down
73 changes: 18 additions & 55 deletions android/src/test/java/io/grpc/android/AndroidChannelBuilderTest.java
View file
Open in desktop
Expand Up @@ -119,7 +119,7 @@ public void nullContextDoesNotThrow_api23() {
.sendBroadcast(new Intent(ConnectivityManager.CONNECTIVITY_ACTION));
androidChannel.shutdown();

assertThat(delegateChannel.resetCount).isEqualTo(0);
assertThat(delegateChannel.enterIdleCount).isEqualTo(0);
}

@Test
Expand All @@ -133,53 +133,52 @@ public void nullContextDoesNotThrow_api24() {
shadowOf(connectivityManager).setActiveNetworkInfo(MOBILE_CONNECTED);
androidChannel.shutdown();

assertThat(delegateChannel.resetCount).isEqualTo(0);
assertThat(delegateChannel.enterIdleCount).isEqualTo(0);
}

@Test
@Config(sdk = 23)
public void resetConnectBackoff_api23() {
public void networkChanges_api23() {
TestChannel delegateChannel = new TestChannel();
ManagedChannel androidChannel =
new AndroidChannelBuilder.AndroidChannel(
delegateChannel, ApplicationProvider.getApplicationContext());
assertThat(delegateChannel.resetCount).isEqualTo(0);
assertThat(delegateChannel.enterIdleCount).isEqualTo(0);

// On API levels < 24, the broadcast receiver will invoke resetConnectBackoff() on the first
// On API levels < 24, the broadcast receiver will invoke enterIdle() on the first
// connectivity action broadcast regardless of previous connection status
shadowOf(connectivityManager).setActiveNetworkInfo(WIFI_CONNECTED);
ApplicationProvider
.getApplicationContext()
.sendBroadcast(new Intent(ConnectivityManager.CONNECTIVITY_ACTION));
assertThat(delegateChannel.resetCount).isEqualTo(1);
assertThat(delegateChannel.enterIdleCount).isEqualTo(1);

// The broadcast receiver may fire when the active network status has not actually changed
ApplicationProvider
.getApplicationContext()
.sendBroadcast(new Intent(ConnectivityManager.CONNECTIVITY_ACTION));
assertThat(delegateChannel.resetCount).isEqualTo(1);
assertThat(delegateChannel.enterIdleCount).isEqualTo(1);

// Drop the connection
shadowOf(connectivityManager).setActiveNetworkInfo(null);
ApplicationProvider
.getApplicationContext()
.sendBroadcast(new Intent(ConnectivityManager.CONNECTIVITY_ACTION));
assertThat(delegateChannel.resetCount).isEqualTo(1);
assertThat(delegateChannel.enterIdleCount).isEqualTo(1);

// Notify that a new but not connected network is available
shadowOf(connectivityManager).setActiveNetworkInfo(MOBILE_DISCONNECTED);
ApplicationProvider
.getApplicationContext()
.sendBroadcast(new Intent(ConnectivityManager.CONNECTIVITY_ACTION));
assertThat(delegateChannel.resetCount).isEqualTo(1);
assertThat(delegateChannel.enterIdleCount).isEqualTo(1);

// Establish a connection
shadowOf(connectivityManager).setActiveNetworkInfo(MOBILE_CONNECTED);
ApplicationProvider
.getApplicationContext()
.sendBroadcast(new Intent(ConnectivityManager.CONNECTIVITY_ACTION));
assertThat(delegateChannel.resetCount).isEqualTo(2);
assertThat(delegateChannel.enterIdleCount).isEqualTo(2);

// Disconnect, then shutdown the channel and verify that the broadcast receiver has been
// unregistered
Expand All @@ -193,72 +192,42 @@ public void resetConnectBackoff_api23() {
.getApplicationContext()
.sendBroadcast(new Intent(ConnectivityManager.CONNECTIVITY_ACTION));

assertThat(delegateChannel.resetCount).isEqualTo(2);
// enterIdle is not called on API levels < 24
assertThat(delegateChannel.enterIdleCount).isEqualTo(0);
assertThat(delegateChannel.enterIdleCount).isEqualTo(2);
}

@Test
@Config(sdk = 24)
public void resetConnectBackoffAndEnterIdle_api24() {
public void networkChanges_api24() {
shadowOf(connectivityManager).setActiveNetworkInfo(MOBILE_DISCONNECTED);
TestChannel delegateChannel = new TestChannel();
ManagedChannel androidChannel =
new AndroidChannelBuilder.AndroidChannel(
delegateChannel, ApplicationProvider.getApplicationContext());
assertThat(delegateChannel.resetCount).isEqualTo(0);
assertThat(delegateChannel.enterIdleCount).isEqualTo(0);

// Establish an initial network connection
shadowOf(connectivityManager).setActiveNetworkInfo(MOBILE_CONNECTED);
assertThat(delegateChannel.resetCount).isEqualTo(1);
assertThat(delegateChannel.enterIdleCount).isEqualTo(0);
assertThat(delegateChannel.enterIdleCount).isEqualTo(1);

// Switch to another network to trigger enterIdle()
shadowOf(connectivityManager).setActiveNetworkInfo(WIFI_CONNECTED);
assertThat(delegateChannel.resetCount).isEqualTo(1);
assertThat(delegateChannel.enterIdleCount).isEqualTo(1);
assertThat(delegateChannel.enterIdleCount).isEqualTo(2);

// Switch to an offline network and then to null
shadowOf(connectivityManager).setActiveNetworkInfo(WIFI_DISCONNECTED);
shadowOf(connectivityManager).setActiveNetworkInfo(null);
assertThat(delegateChannel.resetCount).isEqualTo(1);
assertThat(delegateChannel.enterIdleCount).isEqualTo(1);
assertThat(delegateChannel.enterIdleCount).isEqualTo(2);

// Establish a connection
shadowOf(connectivityManager).setActiveNetworkInfo(MOBILE_CONNECTED);
assertThat(delegateChannel.resetCount).isEqualTo(2);
assertThat(delegateChannel.enterIdleCount).isEqualTo(1);
assertThat(delegateChannel.enterIdleCount).isEqualTo(3);

// Disconnect, then shutdown the channel and verify that the callback has been unregistered
shadowOf(connectivityManager).setActiveNetworkInfo(null);
androidChannel.shutdown();
shadowOf(connectivityManager).setActiveNetworkInfo(MOBILE_CONNECTED);

assertThat(delegateChannel.resetCount).isEqualTo(2);
assertThat(delegateChannel.enterIdleCount).isEqualTo(1);
}

@Test
@Config(sdk = 24)
public void newChannelWithConnection_entersIdleOnSecondConnectionChange_api24() {
shadowOf(connectivityManager).setActiveNetworkInfo(MOBILE_CONNECTED);
TestChannel delegateChannel = new TestChannel();
ManagedChannel androidChannel =
new AndroidChannelBuilder.AndroidChannel(
delegateChannel, ApplicationProvider.getApplicationContext());

// The first onAvailable() may just signal that the device was connected when the callback is
// registered, rather than indicating a changed network, so we do not enter idle.
shadowOf(connectivityManager).setActiveNetworkInfo(WIFI_CONNECTED);
assertThat(delegateChannel.resetCount).isEqualTo(1);
assertThat(delegateChannel.enterIdleCount).isEqualTo(0);

shadowOf(connectivityManager).setActiveNetworkInfo(MOBILE_CONNECTED);
assertThat(delegateChannel.resetCount).isEqualTo(1);
assertThat(delegateChannel.enterIdleCount).isEqualTo(1);

androidChannel.shutdown();
assertThat(delegateChannel.enterIdleCount).isEqualTo(3);
}

@Test
Expand All @@ -279,7 +248,7 @@ public void shutdownNowUnregistersBroadcastReceiver_api23() {
.getApplicationContext()
.sendBroadcast(new Intent(ConnectivityManager.CONNECTIVITY_ACTION));

assertThat(delegateChannel.resetCount).isEqualTo(0);
assertThat(delegateChannel.enterIdleCount).isEqualTo(0);
}

@Test
Expand All @@ -294,7 +263,7 @@ public void shutdownNowUnregistersNetworkCallback_api24() {
androidChannel.shutdownNow();
shadowOf(connectivityManager).setActiveNetworkInfo(WIFI_CONNECTED);

assertThat(delegateChannel.resetCount).isEqualTo(0);
assertThat(delegateChannel.enterIdleCount).isEqualTo(0);
}

/**
Expand Down Expand Up @@ -358,7 +327,6 @@ public void unregisterNetworkCallback(ConnectivityManager.NetworkCallback networ
}

private static class TestChannel extends ManagedChannel {
int resetCount;
int enterIdleCount;

@Override
Expand Down Expand Up @@ -397,11 +365,6 @@ public String authority() {
return null;
}

@Override
public void resetConnectBackoff() {
resetCount++;
}

@Override
public void enterIdle() {
enterIdleCount++;
Expand Down