diff options
author | Chiachang Wang <chiachangwang@google.com> | 2021-06-28 01:57:08 +0000 |
---|---|---|
committer | Automerger Merge Worker <android-build-automerger-merge-worker@system.gserviceaccount.com> | 2021-06-28 01:57:08 +0000 |
commit | 6a53d96ea24a4acac07fe590af09d54e25930434 (patch) | |
tree | 75da09456a5ccea9c45ae1190e76bda88c4a4b8e | |
parent | 71f1bda0dd28affe70dac91f74df92979c9756a7 (diff) | |
parent | 74be4cd836a02c60dd6c0a011bf5af3932d64ddc (diff) | |
download | Mms-6a53d96ea24a4acac07fe590af09d54e25930434.tar.gz |
Merge "Fix the logic for handling network status change" am: f9a72a813d am: 74be4cd836
Original change: https://android-review.googlesource.com/c/platform/packages/services/Mms/+/1739413
Change-Id: I8b683f69c9e66a1791dbdaca13fc3bbf4435eced
-rw-r--r-- | src/com/android/mms/service/MmsNetworkManager.java | 91 | ||||
-rw-r--r-- | tests/robotests/src/com/android/mms/service/MmsNetworkManagerTest.java | 235 |
2 files changed, 300 insertions, 26 deletions
diff --git a/src/com/android/mms/service/MmsNetworkManager.java b/src/com/android/mms/service/MmsNetworkManager.java index af4d8a4..670ed81 100644 --- a/src/com/android/mms/service/MmsNetworkManager.java +++ b/src/com/android/mms/service/MmsNetworkManager.java @@ -32,8 +32,8 @@ import android.provider.DeviceConfig; import android.telephony.SubscriptionManager; import android.telephony.TelephonyManager; +import com.android.internal.annotations.VisibleForTesting; import com.android.internal.telephony.PhoneConstants; - import com.android.mms.service.exception.MmsNetworkException; /** @@ -90,6 +90,8 @@ public class MmsNetworkManager { // If ACTION_SIM_CARD_STATE_CHANGED intent receiver is registered private boolean mReceiverRegistered; + private final Dependencies mDeps; + /** * This receiver listens to ACTION_SIM_CARD_STATE_CHANGED after starting a new NetworkRequest. * If ACTION_SIM_CARD_STATE_CHANGED with SIM_STATE_ABSENT for a SIM card corresponding to the @@ -142,22 +144,12 @@ public class MmsNetworkManager { */ private class NetworkRequestCallback extends ConnectivityManager.NetworkCallback { @Override - public void onAvailable(Network network) { - super.onAvailable(network); - LogUtil.i("NetworkCallbackListener.onAvailable: network=" + network); - synchronized (MmsNetworkManager.this) { - mNetwork = network; - MmsNetworkManager.this.notifyAll(); - } - } - - @Override public void onLost(Network network) { super.onLost(network); LogUtil.w("NetworkCallbackListener.onLost: network=" + network); synchronized (MmsNetworkManager.this) { - releaseRequestLocked(this); - MmsNetworkManager.this.notifyAll(); + // Wait for other available network. Not notify. + if (network.equals(mNetwork)) mNetwork = null; } } @@ -170,10 +162,61 @@ public class MmsNetworkManager { MmsNetworkManager.this.notifyAll(); } } + + @Override + public void onCapabilitiesChanged(Network network, NetworkCapabilities nc) { + // onAvailable will always immediately be followed by a onCapabilitiesChanged. Check + // network status here is enough. + super.onCapabilitiesChanged(network, nc); + LogUtil.w("NetworkCallbackListener.onCapabilitiesChanged: network=" + + network + ", nc=" + nc); + synchronized (MmsNetworkManager.this) { + final boolean isAvailable = + nc.hasCapability(NetworkCapabilities.NET_CAPABILITY_NOT_SUSPENDED); + if (network.equals(mNetwork) && !isAvailable) { + // Current network becomes suspended. + mNetwork = null; + // Not notify. Either wait for other available network or current network to + // become available again. + return; + } + + // New available network + if (mNetwork == null && isAvailable) { + mNetwork = network; + MmsNetworkManager.this.notifyAll(); + } + } + } } - public MmsNetworkManager(Context context, int subId) { + /** + * Dependencies of MmsNetworkManager, for injection in tests. + */ + @VisibleForTesting + public static class Dependencies { + /** Get phone Id from the given subId */ + public int getPhoneId(int subId) { + return SubscriptionManager.getPhoneId(subId); + } + + // Timeout used to call ConnectivityManager.requestNetwork. Given that the telephony layer + // will retry on failures, this timeout should be high enough. + public int getNetworkRequestTimeoutMillis() { + return DeviceConfig.getInt( + DeviceConfig.NAMESPACE_TELEPHONY, MMS_SERVICE_NETWORK_REQUEST_TIMEOUT_MILLIS, + DEFAULT_MMS_SERVICE_NETWORK_REQUEST_TIMEOUT_MILLIS); + } + + public int getAdditionalNetworkAcquireTimeoutMillis() { + return ADDITIONAL_NETWORK_ACQUIRE_TIMEOUT_MILLIS; + } + } + + @VisibleForTesting + protected MmsNetworkManager(Context context, int subId, Dependencies dependencies) { mContext = context; + mDeps = dependencies; mNetworkCallback = null; mNetwork = null; mMmsRequestCount = 0; @@ -200,6 +243,10 @@ public class MmsNetworkManager { }; } + public MmsNetworkManager(Context context, int subId) { + this(context, subId, new Dependencies()); + } + /** * Acquire the MMS network * @@ -207,7 +254,7 @@ public class MmsNetworkManager { * @throws com.android.mms.service.exception.MmsNetworkException if we fail to acquire it */ public void acquireNetwork(final String requestId) throws MmsNetworkException { - int networkRequestTimeoutMillis = getNetworkRequestTimeoutMillis(); + int networkRequestTimeoutMillis = mDeps.getNetworkRequestTimeoutMillis(); synchronized (this) { // Since we are acquiring the network, remove the network release task if exists. @@ -220,7 +267,7 @@ public class MmsNetworkManager { } // Not available, so start a new request if not done yet if (mNetworkCallback == null) { - mPhoneId = SubscriptionManager.getPhoneId(mSubId); + mPhoneId = mDeps.getPhoneId(mSubId); if (mPhoneId == SubscriptionManager.INVALID_PHONE_INDEX || mPhoneId == SubscriptionManager.DEFAULT_PHONE_INDEX) { throw new MmsNetworkException("Invalid Phone Id: " + mPhoneId); @@ -236,7 +283,8 @@ public class MmsNetworkManager { mReceiverRegistered = true; } try { - this.wait(networkRequestTimeoutMillis + ADDITIONAL_NETWORK_ACQUIRE_TIMEOUT_MILLIS); + this.wait(networkRequestTimeoutMillis + + mDeps.getAdditionalNetworkAcquireTimeoutMillis()); } catch (InterruptedException e) { LogUtil.w(requestId, "MmsNetworkManager: acquire network wait interrupted"); } @@ -269,15 +317,6 @@ public class MmsNetworkManager { } } - // Timeout used to call ConnectivityManager.requestNetwork - // Given that the telephony layer will retry on failures, this timeout should be high enough. - private int getNetworkRequestTimeoutMillis() { - return DeviceConfig.getInt( - DeviceConfig.NAMESPACE_TELEPHONY, MMS_SERVICE_NETWORK_REQUEST_TIMEOUT_MILLIS, - DEFAULT_MMS_SERVICE_NETWORK_REQUEST_TIMEOUT_MILLIS); - } - - /** * Release the MMS network when nobody is holding on to it. * diff --git a/tests/robotests/src/com/android/mms/service/MmsNetworkManagerTest.java b/tests/robotests/src/com/android/mms/service/MmsNetworkManagerTest.java new file mode 100644 index 0000000..c2d3e05 --- /dev/null +++ b/tests/robotests/src/com/android/mms/service/MmsNetworkManagerTest.java @@ -0,0 +1,235 @@ +/* + * Copyright (C) 2021 The Android Open Source Project + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package com.android.mms.service; + +import static android.net.NetworkCapabilities.NET_CAPABILITY_NOT_SUSPENDED; + +import static junit.framework.Assert.assertEquals; +import static junit.framework.Assert.assertFalse; +import static junit.framework.Assert.fail; + +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.ArgumentMatchers.anyInt; +import static org.mockito.ArgumentMatchers.eq; +import static org.mockito.Mockito.doReturn; +import static org.mockito.Mockito.never; +import static org.mockito.Mockito.timeout; +import static org.mockito.Mockito.verify; + +import android.content.Context; +import android.net.ConnectivityManager; +import android.net.ConnectivityManager.NetworkCallback; +import android.net.Network; +import android.net.NetworkCapabilities; +import android.net.NetworkInfo; + +import org.junit.Before; +import org.junit.Test; +import org.junit.runner.RunWith; +import org.mockito.ArgumentCaptor; +import org.mockito.Mock; +import org.mockito.MockitoAnnotations; +import org.robolectric.RobolectricTestRunner; + +import java.util.concurrent.CompletableFuture; +import java.util.concurrent.ExecutorService; +import java.util.concurrent.Executors; +import java.util.concurrent.TimeUnit; +import java.util.concurrent.atomic.AtomicInteger; + +@RunWith(RobolectricTestRunner.class) +public final class MmsNetworkManagerTest { + private static final int TEST_SUBID = 1234; + private static final int CALLBACK_TIMEOUT_MS = 3000; + private static final int NETWORK_ACQUIRE_TIMEOUT_MS = 5000; + + private static final String MMS_APN = "mmsapn"; + private static final String MMS_APN2 = "mmsapn2"; + private static final NetworkCapabilities SUSPEND_NC = new NetworkCapabilities.Builder().build(); + private static final NetworkCapabilities USABLE_NC = + new NetworkCapabilities.Builder().addCapability(NET_CAPABILITY_NOT_SUSPENDED).build(); + + @Mock Network mTestNetwork; + @Mock Network mTestNetwork2; + @Mock NetworkInfo mNetworkInfo; + @Mock NetworkInfo mNetworkInfo2; + @Mock Context mCtx; + @Mock ConnectivityManager mCm; + @Mock MmsNetworkManager.Dependencies mDeps; + + + private MmsNetworkManager mMnm; + private final ExecutorService mExecutor = Executors.newSingleThreadExecutor(); + private final AtomicInteger mRequestId = new AtomicInteger(1); + + @Before + public void setup() { + MockitoAnnotations.initMocks(this); + + doReturn(mCm).when(mCtx).getSystemService(Context.CONNECTIVITY_SERVICE); + doReturn(mCm).when(mCtx).getSystemService(ConnectivityManager.class); + doReturn(1).when(mDeps).getPhoneId(anyInt()); + doReturn(mNetworkInfo).when(mCm).getNetworkInfo(eq(mTestNetwork)); + doReturn(MMS_APN).when(mNetworkInfo).getExtraInfo(); + doReturn(mNetworkInfo2).when(mCm).getNetworkInfo(eq(mTestNetwork2)); + doReturn(MMS_APN2).when(mNetworkInfo2).getExtraInfo(); + doReturn(NETWORK_ACQUIRE_TIMEOUT_MS).when(mDeps).getNetworkRequestTimeoutMillis(); + doReturn(NETWORK_ACQUIRE_TIMEOUT_MS).when(mDeps).getAdditionalNetworkAcquireTimeoutMillis(); + + mMnm = new MmsNetworkManager(mCtx, TEST_SUBID, mDeps); + } + + @Test + public void testAvailableNetwork_newNetworkAvailable() throws Exception { + acquireAvailableNetworkAndGetCallback( + mTestNetwork /* expectNetwork */, MMS_APN /* expectApn */); + } + + @Test + public void testAvailableNetwork_newNetworkIsSuspend() throws Exception { + final ArgumentCaptor<NetworkCallback> callbackCaptor = + ArgumentCaptor.forClass(NetworkCallback.class); + final CompletableFuture<String> future = + acquireNetwork(Integer.toString(mRequestId.getAndIncrement())); + verify(mCm, timeout(CALLBACK_TIMEOUT_MS).times(1)) + .requestNetwork(any(), callbackCaptor.capture(), anyInt()); + final NetworkCallback callback = callbackCaptor.getValue(); + + callback.onCapabilitiesChanged(mTestNetwork, SUSPEND_NC); + assertFalse(future.isDone()); + // mNetwork should be null. + assertEquals(null, mMnm.getApnName()); + verify(mCm, never()).unregisterNetworkCallback(eq(callback)); + } + + @Test + public void testAvailableNetwork_networkSuspendThenResume() throws Exception { + final NetworkCallback callback = acquireAvailableNetworkAndGetCallback( + mTestNetwork /* expectNetwork */, MMS_APN /* expectApn */); + + // Network becomes suspended. mNetwork should be null. + callback.onCapabilitiesChanged(mTestNetwork, SUSPEND_NC); + assertEquals(null, mMnm.getApnName()); + verify(mCm, never()).unregisterNetworkCallback(eq(callback)); + + // Network resume + callback.onCapabilitiesChanged(mTestNetwork, USABLE_NC); + assertEquals(MMS_APN, mMnm.getApnName()); + } + + @Test + public void testAvailableNetwork_networkReplaced() throws Exception { + final NetworkCallback callback = acquireAvailableNetworkAndGetCallback( + mTestNetwork /* expectNetwork */, MMS_APN /* expectApn */); + + // Previous network lost. Callback will not release to wait for possible network. + callback.onLost(mTestNetwork); + verify(mCm, never()).unregisterNetworkCallback(eq(callback)); + // New mTestNetwork2 available + callback.onCapabilitiesChanged(mTestNetwork2, USABLE_NC); + assertEquals(MMS_APN2, mMnm.getApnName()); + } + + @Test + public void testAvailableNetwork_networkBecomeSuspend() throws Exception { + final NetworkCallback callback = acquireAvailableNetworkAndGetCallback( + mTestNetwork /* expectNetwork */, MMS_APN /* expectApn */); + + callback.onCapabilitiesChanged(mTestNetwork, SUSPEND_NC); + // mNetwork should be null. + assertEquals(null, mMnm.getApnName()); + // Callback will not release to wait for possible network. + verify(mCm, never()).unregisterNetworkCallback(eq(callback)); + } + + @Test + public void testAvailableNetwork_networkUnavailable() throws Exception { + doReturn(100).when(mDeps).getNetworkRequestTimeoutMillis(); + doReturn(100).when(mDeps).getAdditionalNetworkAcquireTimeoutMillis(); + final ArgumentCaptor<NetworkCallback> callbackCaptor = + ArgumentCaptor.forClass(NetworkCallback.class); + final CompletableFuture<String> future = + acquireNetwork(Integer.toString(mRequestId.getAndIncrement())); + verify(mCm, timeout(CALLBACK_TIMEOUT_MS).times(1)) + .requestNetwork(any(), callbackCaptor.capture(), anyInt()); + final NetworkCallback callback = callbackCaptor.getValue(); + + assertFalse(future.isDone()); + // No network available after 100+100 ms. Callback will be released. + verify(mCm, timeout(CALLBACK_TIMEOUT_MS).times(1)) + .unregisterNetworkCallback(eq(callback)); + + // mNetwork should be null. + assertEquals(null, mMnm.getApnName()); + } + + @Test + public void testAvailableNetwork_newNetworkSuspendedEventuallyReleased() throws Exception { + doReturn(100).when(mDeps).getNetworkRequestTimeoutMillis(); + doReturn(100).when(mDeps).getAdditionalNetworkAcquireTimeoutMillis(); + final ArgumentCaptor<NetworkCallback> callbackCaptor = + ArgumentCaptor.forClass(NetworkCallback.class); + final CompletableFuture<String> future = + acquireNetwork(Integer.toString(mRequestId.getAndIncrement())); + verify(mCm, timeout(CALLBACK_TIMEOUT_MS).times(1)) + .requestNetwork(any(), callbackCaptor.capture(), anyInt()); + final NetworkCallback callback = callbackCaptor.getValue(); + + // New network but not a usable network. + callback.onCapabilitiesChanged(mTestNetwork, SUSPEND_NC); + + assertFalse(future.isDone()); + // No network available after 100+100 ms. Callback will be released. + verify(mCm, timeout(CALLBACK_TIMEOUT_MS).times(1)) + .unregisterNetworkCallback(eq(callback)); + + // mNetwork should be null. + assertEquals(null, mMnm.getApnName()); + } + + private NetworkCallback acquireAvailableNetworkAndGetCallback( + Network expectNetwork, String expectApn) throws Exception { + final ArgumentCaptor<NetworkCallback> callbackCaptor = + ArgumentCaptor.forClass(NetworkCallback.class); + final CompletableFuture<String> future = + acquireNetwork(Integer.toString(mRequestId.getAndIncrement())); + verify(mCm, timeout(CALLBACK_TIMEOUT_MS).times(1)) + .requestNetwork(any(), callbackCaptor.capture(), anyInt()); + final NetworkCallback callback = callbackCaptor.getValue(); + + // Network available + callback.onCapabilitiesChanged(expectNetwork, USABLE_NC); + assertEquals(expectApn, future.get(CALLBACK_TIMEOUT_MS, TimeUnit.MILLISECONDS)); + return callbackCaptor.getValue(); + } + + private CompletableFuture<String> acquireNetwork(String requestId) { + final CompletableFuture<String> future = new CompletableFuture(); + + mExecutor.execute(() -> { + try { + mMnm.acquireNetwork(requestId); + future.complete(mMnm.getApnName()); + android.util.Log.d("MmsNetworkManagerTest", "acquireNetwork done"); + } catch (Exception e) { + fail("Acquire network fail"); + } + }); + + return future; + } +} |