From a9ef5a9252761c73959cfb16a838d3c61fee77f3 Mon Sep 17 00:00:00 2001 From: lucaslin Date: Thu, 29 Jul 2021 13:25:46 +0800 Subject: [PATCH 1/8] Add test for [stop|start]VpnProfile() Ignore-AOSP-First: It's a part of security patches. Bug: 191382886 Test: atest FrameworksNetTests:VpnTest Change-Id: Ie9c0c626f404efe0dd6dc79ca891639bc224090a --- .../com/android/server/ConnectivityServiceTest.java | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/tests/unit/java/com/android/server/ConnectivityServiceTest.java b/tests/unit/java/com/android/server/ConnectivityServiceTest.java index 103ed8bbe6..6cde62a5ea 100644 --- a/tests/unit/java/com/android/server/ConnectivityServiceTest.java +++ b/tests/unit/java/com/android/server/ConnectivityServiceTest.java @@ -9469,6 +9469,19 @@ public class ConnectivityServiceTest { assertContainsExactly(uidCaptor.getValue(), APP1_UID, APP2_UID); } + @Test + public void testStartVpnProfileFromDiffPackage() throws Exception { + final String notMyVpnPkg = "com.not.my.vpn"; + assertThrows( + SecurityException.class, () -> mVpnManagerService.startVpnProfile(notMyVpnPkg)); + } + + @Test + public void testStopVpnProfileFromDiffPackage() throws Exception { + final String notMyVpnPkg = "com.not.my.vpn"; + assertThrows(SecurityException.class, () -> mVpnManagerService.stopVpnProfile(notMyVpnPkg)); + } + @Test public void testUidUpdateChangesInterfaceFilteringRule() throws Exception { LinkProperties lp = new LinkProperties(); From 893494289b45b98718c2bacb8792fc9caf2e281c Mon Sep 17 00:00:00 2001 From: Remi NGUYEN VAN Date: Wed, 4 Aug 2021 07:59:48 +0000 Subject: [PATCH 2/8] Add overlay options for no internet notifications Add an option to display the no internet dialog directly instead of showing a notification when the notification would have been high priority (typically when the network was explicitly selected). This is disabled by default, but allows device manufacturers to use a slightly more disruptive UX to ensure that the user is aware that the network has no connectivity, and can take action. Also add an option to show the same notification as "no internet" instead of the "partial connectivity" notification. This is also disabled by default, but allows device manufacturers to use the "no internet" text if they feel that "partial connectivity" text is hard to understand for the user. Bug: 193847396 Test: atest NetworkNotificationManagerTest Original-Change: https://android-review.googlesource.com/1782433 Merged-In: Ib5bd74d8cf973bf70d373dd63648c178fae0ebae Change-Id: Ib5bd74d8cf973bf70d373dd63648c178fae0ebae --- .../res/values/config.xml | 11 ++ .../res/values/overlayable.xml | 2 + .../NetworkNotificationManager.java | 31 ++++- tests/unit/Android.bp | 1 + tests/unit/AndroidManifest.xml | 2 + .../NetworkNotificationManagerTest.java | 112 ++++++++++++++++++ 6 files changed, 158 insertions(+), 1 deletion(-) diff --git a/service/ServiceConnectivityResources/res/values/config.xml b/service/ServiceConnectivityResources/res/values/config.xml index bf32ad5ac5..b22457a9bb 100644 --- a/service/ServiceConnectivityResources/res/values/config.xml +++ b/service/ServiceConnectivityResources/res/values/config.xml @@ -114,4 +114,15 @@ true + + false + + + false + diff --git a/service/ServiceConnectivityResources/res/values/overlayable.xml b/service/ServiceConnectivityResources/res/values/overlayable.xml index 6ac6a0e6a6..5af13d764b 100644 --- a/service/ServiceConnectivityResources/res/values/overlayable.xml +++ b/service/ServiceConnectivityResources/res/values/overlayable.xml @@ -32,6 +32,8 @@ + + diff --git a/service/src/com/android/server/connectivity/NetworkNotificationManager.java b/service/src/com/android/server/connectivity/NetworkNotificationManager.java index ae98d92073..155f6c4395 100644 --- a/service/src/com/android/server/connectivity/NetworkNotificationManager.java +++ b/service/src/com/android/server/connectivity/NetworkNotificationManager.java @@ -198,11 +198,22 @@ public class NetworkNotificationManager { } final Resources r = mResources.get(); + if (highPriority && maybeNotifyViaDialog(r, notifyType, intent)) { + Log.d(TAG, "Notified via dialog for event " + nameOf(eventId)); + return; + } + final CharSequence title; final CharSequence details; Icon icon = Icon.createWithResource( mResources.getResourcesContext(), getIcon(transportType)); - if (notifyType == NotificationType.NO_INTERNET && transportType == TRANSPORT_WIFI) { + final boolean showAsNoInternet = notifyType == NotificationType.PARTIAL_CONNECTIVITY + && r.getBoolean(R.bool.config_partialConnectivityNotifiedAsNoInternet); + if (showAsNoInternet) { + Log.d(TAG, "Showing partial connectivity as NO_INTERNET"); + } + if ((notifyType == NotificationType.NO_INTERNET || showAsNoInternet) + && transportType == TRANSPORT_WIFI) { title = r.getString(R.string.wifi_no_internet, name); details = r.getString(R.string.wifi_no_internet_detailed); } else if (notifyType == NotificationType.PRIVATE_DNS_BROKEN) { @@ -306,6 +317,24 @@ public class NetworkNotificationManager { } } + private boolean maybeNotifyViaDialog(Resources res, NotificationType notifyType, + PendingIntent intent) { + if (notifyType != NotificationType.NO_INTERNET + && notifyType != NotificationType.PARTIAL_CONNECTIVITY) { + return false; + } + if (!res.getBoolean(R.bool.config_notifyNoInternetAsDialogWhenHighPriority)) { + return false; + } + + try { + intent.send(); + } catch (PendingIntent.CanceledException e) { + Log.e(TAG, "Error sending dialog PendingIntent", e); + } + return true; + } + /** * Clear the notification with the given id, only if it matches the given type. */ diff --git a/tests/unit/Android.bp b/tests/unit/Android.bp index c8476cb726..71bd6089cc 100644 --- a/tests/unit/Android.bp +++ b/tests/unit/Android.bp @@ -62,6 +62,7 @@ android_library { jarjar_rules: "jarjar-rules.txt", static_libs: [ "androidx.test.rules", + "androidx.test.uiautomator", "bouncycastle-repackaged-unbundled", "core-tests-support", "FrameworksNetCommonTests", diff --git a/tests/unit/AndroidManifest.xml b/tests/unit/AndroidManifest.xml index 4c60ccf606..887f17158b 100644 --- a/tests/unit/AndroidManifest.xml +++ b/tests/unit/AndroidManifest.xml @@ -53,6 +53,8 @@ + finish()); + setContentView(txt); + } + } + @Mock Context mCtx; @Mock Resources mResources; @Mock DisplayMetrics mDisplayMetrics; @@ -345,4 +379,82 @@ public class NetworkNotificationManagerTest { mManager.clearNotification(id, PARTIAL_CONNECTIVITY); verify(mNotificationManager, never()).cancel(eq(tag), eq(PARTIAL_CONNECTIVITY.eventId)); } + + @Test + public void testNotifyNoInternetAsDialogWhenHighPriority() throws Exception { + doReturn(true).when(mResources).getBoolean( + R.bool.config_notifyNoInternetAsDialogWhenHighPriority); + + mManager.showNotification(TEST_NOTIF_ID, NETWORK_SWITCH, mWifiNai, mCellNai, null, false); + // Non-"no internet" notifications are not affected + verify(mNotificationManager).notify(eq(TEST_NOTIF_TAG), eq(NETWORK_SWITCH.eventId), any()); + + final Instrumentation instr = InstrumentationRegistry.getInstrumentation(); + final Context ctx = instr.getContext(); + final String testAction = "com.android.connectivity.coverage.TEST_DIALOG"; + final Intent intent = new Intent(testAction) + .addFlags(Intent.FLAG_ACTIVITY_NEW_TASK) + .setClassName(ctx.getPackageName(), TestDialogActivity.class.getName()); + final PendingIntent pendingIntent = PendingIntent.getActivity(ctx, 0 /* requestCode */, + intent, PendingIntent.FLAG_CANCEL_CURRENT | PendingIntent.FLAG_IMMUTABLE); + + mManager.showNotification(TEST_NOTIF_ID, NO_INTERNET, mWifiNai, null /* switchToNai */, + pendingIntent, true /* highPriority */); + + // Previous notifications are still dismissed + verify(mNotificationManager).cancel(TEST_NOTIF_TAG, NETWORK_SWITCH.eventId); + + // Verify that the activity is shown (the activity shows the action on screen) + final UiObject actionText = UiDevice.getInstance(instr).findObject( + new UiSelector().text(testAction)); + assertTrue("Activity not shown", actionText.waitForExists(TEST_TIMEOUT_MS)); + + // Tapping the text should dismiss the dialog + actionText.click(); + assertTrue("Activity not dismissed", actionText.waitUntilGone(TEST_TIMEOUT_MS)); + + // Verify no NO_INTERNET notification was posted + verify(mNotificationManager, never()).notify(any(), eq(NO_INTERNET.eventId), any()); + } + + private void doNotificationTextTest(NotificationType type, @StringRes int expectedTitleRes, + String expectedTitleArg, @StringRes int expectedContentRes) { + final String expectedTitle = "title " + expectedTitleArg; + final String expectedContent = "expected content"; + doReturn(expectedTitle).when(mResources).getString(expectedTitleRes, expectedTitleArg); + doReturn(expectedContent).when(mResources).getString(expectedContentRes); + + mManager.showNotification(TEST_NOTIF_ID, type, mWifiNai, mCellNai, null, false); + final ArgumentCaptor notifCap = ArgumentCaptor.forClass(Notification.class); + + verify(mNotificationManager).notify(eq(TEST_NOTIF_TAG), eq(type.eventId), + notifCap.capture()); + final Notification notif = notifCap.getValue(); + + assertEquals(expectedTitle, notif.extras.getString(Notification.EXTRA_TITLE)); + assertEquals(expectedContent, notif.extras.getString(Notification.EXTRA_TEXT)); + } + + @Test + public void testNotificationText_NoInternet() { + doNotificationTextTest(NO_INTERNET, + R.string.wifi_no_internet, TEST_EXTRA_INFO, + R.string.wifi_no_internet_detailed); + } + + @Test + public void testNotificationText_Partial() { + doNotificationTextTest(PARTIAL_CONNECTIVITY, + R.string.network_partial_connectivity, TEST_EXTRA_INFO, + R.string.network_partial_connectivity_detailed); + } + + @Test + public void testNotificationText_PartialAsNoInternet() { + doReturn(true).when(mResources).getBoolean( + R.bool.config_partialConnectivityNotifiedAsNoInternet); + doNotificationTextTest(PARTIAL_CONNECTIVITY, + R.string.wifi_no_internet, TEST_EXTRA_INFO, + R.string.wifi_no_internet_detailed); + } } From 5234f3acc64056ec290e0c61ae954e0bf5455322 Mon Sep 17 00:00:00 2001 From: Chalard Jean Date: Thu, 29 Jul 2021 20:03:04 +0900 Subject: [PATCH 3/8] Fix a crash when changing preferences The crash occurs when some app has more than half its limit in requests that will need to be moved to some other default network upon changing the preferences. This will send the requests for this app over the limit temporarily when creating new requests for the reevaluated ones. While ConnectivityService has a provision for making a transaction-like addition/removal of requests that is meant to avoid exactly this kind of crash with the transact() method on PerUidCounter, the code only transacts on mSystemNetworkRequestCounter. But these requests are counted in the mNetworkRequestCounters, which is not part of the transaction, causing the crash anyway. To avoid the problem, this patch allows the request counters to go over the max if and only if the system server is updating the request counts for a UID other than its own. This should allow only the case where ConnectivityService is moving the requests over to the new per-uid default, while keeping the exception when registering from an app (then the calling UID is not the system server), or when the system server registers its own requests (then the UID inside the request is that of the system server). A much better solution than this patch would be to completely eliminate the transact() method by somehow unregistering the old ones before creating the new ones. However this would be a much bigger and difficult patch than this, and much more dangerous, because callers depend on the list of requests to find out the old requests to remove, so they have to be created first. Another possible clean solution would be to count the requests not in the NRI constructor, but later. This would be more error-prone though because it would be very easy to create an NRI without counting it. Bug: 192470012 Test: ConnectivityServiceTest. Improve tests so they catch this case. Original change: https://android-review.googlesource.com/c/platform/packages/modules/Connectivity/+/1781202 Merged-In: Ia482e6fbf2bf300ce6cbaca72810d394ed201b98 Change-Id: I6744d2f60d6bd664f048b532a58461c110a5b7fe (cherry picked from commit 916aeb7b0dab0198858fe265c5675140276700bd) --- .../android/server/ConnectivityService.java | 25 ++- .../server/ConnectivityServiceTest.java | 178 +++++++++++++----- 2 files changed, 145 insertions(+), 58 deletions(-) diff --git a/service/src/com/android/server/ConnectivityService.java b/service/src/com/android/server/ConnectivityService.java index c8a0b7f0d9..e34c0640c2 100644 --- a/service/src/com/android/server/ConnectivityService.java +++ b/service/src/com/android/server/ConnectivityService.java @@ -324,7 +324,8 @@ public class ConnectivityService extends IConnectivityManager.Stub private static final int DEFAULT_NASCENT_DELAY_MS = 5_000; // The maximum number of network request allowed per uid before an exception is thrown. - private static final int MAX_NETWORK_REQUESTS_PER_UID = 100; + @VisibleForTesting + static final int MAX_NETWORK_REQUESTS_PER_UID = 100; // The maximum number of network request allowed for system UIDs before an exception is thrown. @VisibleForTesting @@ -344,7 +345,8 @@ public class ConnectivityService extends IConnectivityManager.Stub @VisibleForTesting protected final PermissionMonitor mPermissionMonitor; - private final PerUidCounter mNetworkRequestCounter; + @VisibleForTesting + final PerUidCounter mNetworkRequestCounter; @VisibleForTesting final PerUidCounter mSystemNetworkRequestCounter; @@ -1154,9 +1156,20 @@ public class ConnectivityService extends IConnectivityManager.Stub private void incrementCountOrThrow(final int uid, final int numToIncrement) { final int newRequestCount = mUidToNetworkRequestCount.get(uid, 0) + numToIncrement; - if (newRequestCount >= mMaxCountPerUid) { + if (newRequestCount >= mMaxCountPerUid + // HACK : the system server is allowed to go over the request count limit + // when it is creating requests on behalf of another app (but not itself, + // so it can still detect its own request leaks). This only happens in the + // per-app API flows in which case the old requests for that particular + // UID will be removed soon. + // TODO : instead of this hack, addPerAppDefaultNetworkRequests and other + // users of transact() should unregister the requests to decrease the count + // before they increase it again by creating a new NRI. Then remove the + // transact() method. + && (Process.myUid() == uid || Process.myUid() != Binder.getCallingUid())) { throw new ServiceSpecificException( - ConnectivityManager.Errors.TOO_MANY_REQUESTS); + ConnectivityManager.Errors.TOO_MANY_REQUESTS, + "Uid " + uid + " exceeded its allotted requests limit"); } mUidToNetworkRequestCount.put(uid, newRequestCount); } @@ -5817,7 +5830,7 @@ public class ConnectivityService extends IConnectivityManager.Stub mUid = nri.mUid; mAsUid = nri.mAsUid; mPendingIntent = nri.mPendingIntent; - mPerUidCounter = getRequestCounter(this); + mPerUidCounter = nri.mPerUidCounter; mPerUidCounter.incrementCountOrThrow(mUid); mCallbackFlags = nri.mCallbackFlags; mCallingAttributionTag = nri.mCallingAttributionTag; @@ -10147,7 +10160,7 @@ public class ConnectivityService extends IConnectivityManager.Stub final NetworkRequestInfo trackingNri = getDefaultRequestTrackingUid(callbackRequest.mAsUid); - // If this nri is not being tracked, the change it back to an untracked nri. + // If this nri is not being tracked, then change it back to an untracked nri. if (trackingNri == mDefaultRequest) { callbackRequestsToRegister.add(new NetworkRequestInfo( callbackRequest, diff --git a/tests/unit/java/com/android/server/ConnectivityServiceTest.java b/tests/unit/java/com/android/server/ConnectivityServiceTest.java index 0f0ac1223c..9dde31ab91 100644 --- a/tests/unit/java/com/android/server/ConnectivityServiceTest.java +++ b/tests/unit/java/com/android/server/ConnectivityServiceTest.java @@ -125,6 +125,7 @@ import static android.net.resolv.aidl.IDnsResolverUnsolicitedEventListener.VALID import static android.os.Process.INVALID_UID; import static android.system.OsConstants.IPPROTO_TCP; +import static com.android.server.ConnectivityService.MAX_NETWORK_REQUESTS_PER_SYSTEM_UID; import static com.android.server.ConnectivityService.PREFERENCE_PRIORITY_MOBILE_DATA_PREFERERRED; import static com.android.server.ConnectivityService.PREFERENCE_PRIORITY_OEM; import static com.android.server.ConnectivityService.PREFERENCE_PRIORITY_PROFILE; @@ -539,6 +540,9 @@ public class ConnectivityServiceTest { private final LinkedBlockingQueue mStartedActivities = new LinkedBlockingQueue<>(); // Map of permission name -> PermissionManager.Permission_{GRANTED|DENIED} constant + // For permissions granted across the board, the key is only the permission name. + // For permissions only granted to a combination of uid/pid, the key + // is ",,". PID+UID permissons have priority over generic ones. private final HashMap mMockedPermissions = new HashMap<>(); MockContext(Context base, ContentProvider settingsProvider) { @@ -640,30 +644,40 @@ public class ConnectivityServiceTest { return mPackageManager; } - private int checkMockedPermission(String permission, Supplier ifAbsent) { - final Integer granted = mMockedPermissions.get(permission); - return granted != null ? granted : ifAbsent.get(); + private int checkMockedPermission(String permission, int pid, int uid, + Supplier ifAbsent) { + final Integer granted = mMockedPermissions.get(permission + "," + pid + "," + uid); + if (null != granted) { + return granted; + } + final Integer allGranted = mMockedPermissions.get(permission); + if (null != allGranted) { + return allGranted; + } + return ifAbsent.get(); } @Override public int checkPermission(String permission, int pid, int uid) { - return checkMockedPermission( - permission, () -> super.checkPermission(permission, pid, uid)); + return checkMockedPermission(permission, pid, uid, + () -> super.checkPermission(permission, pid, uid)); } @Override public int checkCallingOrSelfPermission(String permission) { - return checkMockedPermission( - permission, () -> super.checkCallingOrSelfPermission(permission)); + return checkMockedPermission(permission, Process.myPid(), Process.myUid(), + () -> super.checkCallingOrSelfPermission(permission)); } @Override public void enforceCallingOrSelfPermission(String permission, String message) { - final Integer granted = mMockedPermissions.get(permission); - if (granted == null) { - super.enforceCallingOrSelfPermission(permission, message); - return; - } + final Integer granted = checkMockedPermission(permission, + Process.myPid(), Process.myUid(), + () -> { + super.enforceCallingOrSelfPermission(permission, message); + // enforce will crash if the permission is not granted + return PERMISSION_GRANTED; + }); if (!granted.equals(PERMISSION_GRANTED)) { throw new SecurityException("[Test] permission denied: " + permission); @@ -673,6 +687,8 @@ public class ConnectivityServiceTest { /** * Mock checks for the specified permission, and have them behave as per {@code granted}. * + * This will apply across the board no matter what the checked UID and PID are. + * *

Passing null reverts to default behavior, which does a real permission check on the * test package. * @param granted One of {@link PackageManager#PERMISSION_GRANTED} or @@ -682,6 +698,21 @@ public class ConnectivityServiceTest { mMockedPermissions.put(permission, granted); } + /** + * Mock checks for the specified permission, and have them behave as per {@code granted}. + * + * This will only apply to the passed UID and PID. + * + *

Passing null reverts to default behavior, which does a real permission check on the + * test package. + * @param granted One of {@link PackageManager#PERMISSION_GRANTED} or + * {@link PackageManager#PERMISSION_DENIED}. + */ + public void setPermission(String permission, int pid, int uid, Integer granted) { + final String key = permission + "," + pid + "," + uid; + mMockedPermissions.put(key, granted); + } + @Override public Intent registerReceiverForAllUsers(@Nullable BroadcastReceiver receiver, @NonNull IntentFilter filter, @Nullable String broadcastPermission, @@ -1569,15 +1600,21 @@ public class ConnectivityServiceTest { } private void withPermission(String permission, ExceptionalRunnable r) throws Exception { - if (mServiceContext.checkCallingOrSelfPermission(permission) == PERMISSION_GRANTED) { - r.run(); - return; - } try { mServiceContext.setPermission(permission, PERMISSION_GRANTED); r.run(); } finally { - mServiceContext.setPermission(permission, PERMISSION_DENIED); + mServiceContext.setPermission(permission, null); + } + } + + private void withPermission(String permission, int pid, int uid, ExceptionalRunnable r) + throws Exception { + try { + mServiceContext.setPermission(permission, pid, uid, PERMISSION_GRANTED); + r.run(); + } finally { + mServiceContext.setPermission(permission, pid, uid, null); } } @@ -13368,17 +13405,45 @@ public class ConnectivityServiceTest { @Test public void testProfileNetworkPrefCountsRequestsCorrectlyOnSet() throws Exception { final UserHandle testHandle = setupEnterpriseNetwork(); - testRequestCountLimits(() -> { - // Set initially to test the limit prior to having existing requests. - final TestOnCompleteListener listener = new TestOnCompleteListener(); - mCm.setProfileNetworkPreference(testHandle, PROFILE_NETWORK_PREFERENCE_ENTERPRISE, - Runnable::run, listener); + final TestOnCompleteListener listener = new TestOnCompleteListener(); + // Leave one request available so the profile preference can be set. + testRequestCountLimits(1 /* countToLeaveAvailable */, () -> { + withPermission(NetworkStack.PERMISSION_MAINLINE_NETWORK_STACK, + Process.myPid(), Process.myUid(), () -> { + // Set initially to test the limit prior to having existing requests. + mCm.setProfileNetworkPreference(testHandle, + PROFILE_NETWORK_PREFERENCE_ENTERPRISE, + Runnable::run, listener); + }); listener.expectOnComplete(); - // re-set so as to test the limit as part of replacing existing requests. - mCm.setProfileNetworkPreference(testHandle, PROFILE_NETWORK_PREFERENCE_ENTERPRISE, - Runnable::run, listener); + // Simulate filing requests as some app on the work profile + final int otherAppUid = UserHandle.getUid(TEST_WORK_PROFILE_USER_ID, + UserHandle.getAppId(Process.myUid() + 1)); + final int remainingCount = ConnectivityService.MAX_NETWORK_REQUESTS_PER_UID + - mService.mNetworkRequestCounter.mUidToNetworkRequestCount.get(otherAppUid) + - 1; + final NetworkCallback[] callbacks = new NetworkCallback[remainingCount]; + doAsUid(otherAppUid, () -> { + for (int i = 0; i < remainingCount; ++i) { + callbacks[i] = new TestableNetworkCallback(); + mCm.registerDefaultNetworkCallback(callbacks[i]); + } + }); + + withPermission(NetworkStack.PERMISSION_MAINLINE_NETWORK_STACK, + Process.myPid(), Process.myUid(), () -> { + // re-set so as to test the limit as part of replacing existing requests. + mCm.setProfileNetworkPreference(testHandle, + PROFILE_NETWORK_PREFERENCE_ENTERPRISE, Runnable::run, listener); + }); listener.expectOnComplete(); + + doAsUid(otherAppUid, () -> { + for (final NetworkCallback callback : callbacks) { + mCm.unregisterNetworkCallback(callback); + } + }); }); } @@ -13390,39 +13455,45 @@ public class ConnectivityServiceTest { mockHasSystemFeature(PackageManager.FEATURE_AUTOMOTIVE, true); @OemNetworkPreferences.OemNetworkPreference final int networkPref = OEM_NETWORK_PREFERENCE_OEM_PRIVATE_ONLY; - testRequestCountLimits(() -> { - // Set initially to test the limit prior to having existing requests. - final TestOemListenerCallback listener = new TestOemListenerCallback(); - mService.setOemNetworkPreference( - createDefaultOemNetworkPreferences(networkPref), listener); - listener.expectOnComplete(); + // Leave one request available so the OEM preference can be set. + testRequestCountLimits(1 /* countToLeaveAvailable */, () -> + withPermission(NetworkStack.PERMISSION_MAINLINE_NETWORK_STACK, () -> { + // Set initially to test the limit prior to having existing requests. + final TestOemListenerCallback listener = new TestOemListenerCallback(); + mService.setOemNetworkPreference( + createDefaultOemNetworkPreferences(networkPref), listener); + listener.expectOnComplete(); - // re-set so as to test the limit as part of replacing existing requests. - mService.setOemNetworkPreference( - createDefaultOemNetworkPreferences(networkPref), listener); - listener.expectOnComplete(); - }); + // re-set so as to test the limit as part of replacing existing requests. + mService.setOemNetworkPreference( + createDefaultOemNetworkPreferences(networkPref), listener); + listener.expectOnComplete(); + })); } - private void testRequestCountLimits(@NonNull final Runnable r) throws Exception { + private void testRequestCountLimits(final int countToLeaveAvailable, + @NonNull final ExceptionalRunnable r) throws Exception { final ArraySet callbacks = new ArraySet<>(); try { final int requestCount = mService.mSystemNetworkRequestCounter .mUidToNetworkRequestCount.get(Process.myUid()); - // The limit is hit when total requests <= limit. - final int maxCount = - ConnectivityService.MAX_NETWORK_REQUESTS_PER_SYSTEM_UID - requestCount; + // The limit is hit when total requests = limit - 1, and exceeded with a crash when + // total requests >= limit. + final int countToFile = + MAX_NETWORK_REQUESTS_PER_SYSTEM_UID - requestCount - countToLeaveAvailable; // Need permission so registerDefaultNetworkCallback uses mSystemNetworkRequestCounter withPermission(NetworkStack.PERMISSION_MAINLINE_NETWORK_STACK, () -> { - for (int i = 1; i < maxCount - 1; i++) { + for (int i = 1; i < countToFile; i++) { final TestNetworkCallback cb = new TestNetworkCallback(); mCm.registerDefaultNetworkCallback(cb); callbacks.add(cb); } - - // Code to run to check if it triggers a max request count limit error. - r.run(); + assertEquals(MAX_NETWORK_REQUESTS_PER_SYSTEM_UID - 1 - countToLeaveAvailable, + mService.mSystemNetworkRequestCounter + .mUidToNetworkRequestCount.get(Process.myUid())); }); + // Code to run to check if it triggers a max request count limit error. + r.run(); } finally { for (final TestNetworkCallback cb : callbacks) { mCm.unregisterNetworkCallback(cb); @@ -13667,15 +13738,18 @@ public class ConnectivityServiceTest { public void testMobileDataPreferredUidsChangedCountsRequestsCorrectlyOnSet() throws Exception { ConnectivitySettingsManager.setMobileDataPreferredUids(mServiceContext, Set.of(PRIMARY_USER_HANDLE.getUid(TEST_PACKAGE_UID))); - testRequestCountLimits(() -> { - // Set initially to test the limit prior to having existing requests. - mService.updateMobileDataPreferredUids(); - waitForIdle(); + // Leave one request available so MDO preference set up above can be set. + testRequestCountLimits(1 /* countToLeaveAvailable */, () -> + withPermission(NetworkStack.PERMISSION_MAINLINE_NETWORK_STACK, + Process.myPid(), Process.myUid(), () -> { + // Set initially to test the limit prior to having existing requests. + mService.updateMobileDataPreferredUids(); + waitForIdle(); - // re-set so as to test the limit as part of replacing existing requests. - mService.updateMobileDataPreferredUids(); - waitForIdle(); - }); + // re-set so as to test the limit as part of replacing existing requests + mService.updateMobileDataPreferredUids(); + waitForIdle(); + })); } @Test From 4dd0f9c1940373c884800af2e660bcef65d8d7fa Mon Sep 17 00:00:00 2001 From: Chiachang Wang Date: Tue, 3 Aug 2021 12:04:51 +0800 Subject: [PATCH 4/8] Revert "Allow network providers to set yield to bad wifi" This reverts commit c4660c98f6a63a73df1a79c34fbbf2266a6b381e. The reverted commit removed the POLICY_YIELD_TO_BAD_WIFI exclusive bit from the EXTERNAL_POLICIES_MASK. The new policy calculation is done by bitwise-or-ing with existing policy. If the POLICY_YIELD_TO_BAD_WIFI was ever set to policy in the FullScore , CS will not be able to remove the policy to reflect the avoid bad wifi setting change since the result is always be true unless the policy is updated from factories. Eventually, the original commit is the intended design but current design could not work well with it. Thus, revert it to keep the control on CS now. Bug: 195612849 Test: atest CtsNetTestCases FrameworksNetTests Change-Id: I002e206ffd41796cb0996b9c559afed3d619b67c Ignore-AOSP-First: Commit is only available in internal branch --- service/src/com/android/server/connectivity/FullScore.java | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/service/src/com/android/server/connectivity/FullScore.java b/service/src/com/android/server/connectivity/FullScore.java index fbfa7a18c9..14cec09560 100644 --- a/service/src/com/android/server/connectivity/FullScore.java +++ b/service/src/com/android/server/connectivity/FullScore.java @@ -108,10 +108,9 @@ public class FullScore { // and all bits managed by FullScore unset. As bits are handled from 0 up in NetworkScore and // from 63 down in FullScore, cut at the 32nd bit for simplicity, but change this if some day // there are more than 32 bits handled on either side. - // YIELD_TO_BAD_WIFI is temporarily handled by ConnectivityService, but the factory is still - // allowed to set it, so that it's possible to transition from handling it in CS to handling - // it in the factory. - private static final long EXTERNAL_POLICIES_MASK = 0x00000000FFFFFFFFL; + // YIELD_TO_BAD_WIFI is temporarily handled by ConnectivityService. + private static final long EXTERNAL_POLICIES_MASK = + 0x00000000FFFFFFFFL & ~(1L << POLICY_YIELD_TO_BAD_WIFI); @VisibleForTesting static @NonNull String policyNameOf(final int policy) { From 0a37e72f0c2c41e1c9ccda70ed2950b5339596be Mon Sep 17 00:00:00 2001 From: Remi NGUYEN VAN Date: Wed, 4 Aug 2021 19:19:49 +0900 Subject: [PATCH 5/8] Mock connectivity resources in integration tests The resources may have different values depending on devices, and resources IDs may mismatch if the test was not built at the same time as the installed module, so mock the resources to allow running the integration tests on more builds. Bug: 193847396 Test: atest FrameworksNetIntegrationTests Ignore-AOSP-First: cherry-pick created Change-Id: Ic33d897690a8ea84a78c01dc5f2b2e2c473d57df --- tests/integration/Android.bp | 1 + .../ConnectivityServiceIntegrationTest.kt | 43 ++++++++++++++++++- 2 files changed, 42 insertions(+), 2 deletions(-) diff --git a/tests/integration/Android.bp b/tests/integration/Android.bp index 26f7f4aaee..7b5b44f731 100644 --- a/tests/integration/Android.bp +++ b/tests/integration/Android.bp @@ -30,6 +30,7 @@ android_test { ], libs: [ "android.test.mock", + "ServiceConnectivityResources", ], static_libs: [ "NetworkStackApiStableLib", diff --git a/tests/integration/src/com/android/server/net/integrationtests/ConnectivityServiceIntegrationTest.kt b/tests/integration/src/com/android/server/net/integrationtests/ConnectivityServiceIntegrationTest.kt index e039ef0725..80338aa2c0 100644 --- a/tests/integration/src/com/android/server/net/integrationtests/ConnectivityServiceIntegrationTest.kt +++ b/tests/integration/src/com/android/server/net/integrationtests/ConnectivityServiceIntegrationTest.kt @@ -23,7 +23,9 @@ import android.content.Context.BIND_AUTO_CREATE import android.content.Context.BIND_IMPORTANT import android.content.Intent import android.content.ServiceConnection +import android.content.res.Resources import android.net.ConnectivityManager +import android.net.ConnectivityResources import android.net.IDnsResolver import android.net.INetd import android.net.LinkProperties @@ -35,6 +37,7 @@ import android.net.NetworkRequest import android.net.TestNetworkStackClient import android.net.Uri import android.net.metrics.IpConnectivityLog +import android.net.util.MultinetworkPolicyTracker import android.os.ConditionVariable import android.os.IBinder import android.os.SystemConfigManager @@ -43,6 +46,7 @@ import android.testing.TestableContext import android.util.Log import androidx.test.ext.junit.runners.AndroidJUnit4 import androidx.test.platform.app.InstrumentationRegistry +import com.android.connectivity.resources.R import com.android.server.ConnectivityService import com.android.server.NetworkAgentWrapper import com.android.server.TestNetIdManager @@ -59,6 +63,7 @@ import org.mockito.ArgumentMatchers.anyString import org.mockito.Mock import org.mockito.Mockito.any import org.mockito.Mockito.anyInt +import org.mockito.Mockito.doAnswer import org.mockito.Mockito.doNothing import org.mockito.Mockito.doReturn import org.mockito.Mockito.eq @@ -93,6 +98,10 @@ class ConnectivityServiceIntegrationTest { private lateinit var dnsResolver: IDnsResolver @Mock private lateinit var systemConfigManager: SystemConfigManager + @Mock + private lateinit var resources: Resources + @Mock + private lateinit var resourcesContext: Context @Spy private var context = TestableContext(realContext) @@ -110,9 +119,11 @@ class ConnectivityServiceIntegrationTest { private val realContext get() = InstrumentationRegistry.getInstrumentation().context private val httpProbeUrl get() = - realContext.getResources().getString(R.string.config_captive_portal_http_url) + realContext.getResources().getString(com.android.server.net.integrationtests.R.string + .config_captive_portal_http_url) private val httpsProbeUrl get() = - realContext.getResources().getString(R.string.config_captive_portal_https_url) + realContext.getResources().getString(com.android.server.net.integrationtests.R.string + .config_captive_portal_https_url) private class InstrumentationServiceConnection : ServiceConnection { override fun onServiceConnected(name: ComponentName?, service: IBinder?) { @@ -156,6 +167,27 @@ class ConnectivityServiceIntegrationTest { .getSystemService(Context.SYSTEM_CONFIG_SERVICE) doReturn(IntArray(0)).`when`(systemConfigManager).getSystemPermissionUids(anyString()) + doReturn(60000).`when`(resources).getInteger(R.integer.config_networkTransitionTimeout) + doReturn("").`when`(resources).getString(R.string.config_networkCaptivePortalServerUrl) + doReturn(arrayOf("test_wlan_wol")).`when`(resources) + .getStringArray(R.array.config_wakeonlan_supported_interfaces) + doReturn(arrayOf("0,1", "1,3")).`when`(resources) + .getStringArray(R.array.config_networkSupportedKeepaliveCount) + doReturn(emptyArray()).`when`(resources) + .getStringArray(R.array.config_networkNotifySwitches) + doReturn(intArrayOf(10, 11, 12, 14, 15)).`when`(resources) + .getIntArray(R.array.config_protectedNetworks) + // We don't test the actual notification value strings, so just return an empty array. + // It doesn't matter what the values are as long as it's not null. + doReturn(emptyArray()).`when`(resources).getStringArray( + R.array.network_switch_type_name) + doReturn(1).`when`(resources).getInteger(R.integer.config_networkAvoidBadWifi) + doReturn(R.array.config_networkSupportedKeepaliveCount).`when`(resources) + .getIdentifier(eq("config_networkSupportedKeepaliveCount"), eq("array"), any()) + + doReturn(resources).`when`(resourcesContext).getResources() + ConnectivityResources.setResourcesContextForTest(resourcesContext) + networkStackClient = TestNetworkStackClient(realContext) networkStackClient.start() @@ -176,12 +208,19 @@ class ConnectivityServiceIntegrationTest { doReturn(mock(ProxyTracker::class.java)).`when`(deps).makeProxyTracker(any(), any()) doReturn(mock(MockableSystemProperties::class.java)).`when`(deps).systemProperties doReturn(TestNetIdManager()).`when`(deps).makeNetIdManager() + doAnswer { inv -> + object : MultinetworkPolicyTracker(inv.getArgument(0), inv.getArgument(1), + inv.getArgument(2)) { + override fun getResourcesForActiveSubId() = resources + } + }.`when`(deps).makeMultinetworkPolicyTracker(any(), any(), any()) return deps } @After fun tearDown() { nsInstrumentation.clearAllState() + ConnectivityResources.setResourcesContextForTest(null) } @Test From ae00382c6787b4ddafe2f3a8f3169ea1eeb8a8e7 Mon Sep 17 00:00:00 2001 From: Benedict Wong Date: Mon, 3 Aug 2020 18:48:09 -0700 Subject: [PATCH 6/8] Update ConnDiags CTS test to expect validation result SKIPPED. This CL updates ConnectivityDiagnosticsManagerTest to expect NETWORK_VALIDATION_RESULT_SKIPPED for its TestNetworks. The tests also expect a ConnectivityReport to be sent for all calls to ConnectivityManager#reportNetworkConnectivity. This change is different from the one pushed into AOSP & the mainline modules; this allows both SKIPPED and VALID in the intermediate time while module prebuilts are generated. MTS will enforce the stricter SKIPPED check, while this allows both the S-release behavior and mainline update behavior. Bug: 162407730 Bug: 195727283 Test: atest ConnectivityDiagnosticsManagerTest Change-Id: Ia0bf1bb53289b079f26597f09b0759a89deb681f Merged-In: I78b78919d5b0f09348dfdd5fdb37418b8c7f861f --- .../ConnectivityDiagnosticsManagerTest.java | 39 ++++++++++++++++--- 1 file changed, 33 insertions(+), 6 deletions(-) diff --git a/tests/cts/net/src/android/net/cts/ConnectivityDiagnosticsManagerTest.java b/tests/cts/net/src/android/net/cts/ConnectivityDiagnosticsManagerTest.java index ccbdbd35b5..60a20f4edf 100644 --- a/tests/cts/net/src/android/net/cts/ConnectivityDiagnosticsManagerTest.java +++ b/tests/cts/net/src/android/net/cts/ConnectivityDiagnosticsManagerTest.java @@ -22,6 +22,7 @@ import static android.net.ConnectivityDiagnosticsManager.ConnectivityReport; import static android.net.ConnectivityDiagnosticsManager.ConnectivityReport.KEY_NETWORK_PROBES_ATTEMPTED_BITMASK; import static android.net.ConnectivityDiagnosticsManager.ConnectivityReport.KEY_NETWORK_PROBES_SUCCEEDED_BITMASK; import static android.net.ConnectivityDiagnosticsManager.ConnectivityReport.KEY_NETWORK_VALIDATION_RESULT; +import static android.net.ConnectivityDiagnosticsManager.ConnectivityReport.NETWORK_VALIDATION_RESULT_SKIPPED; import static android.net.ConnectivityDiagnosticsManager.ConnectivityReport.NETWORK_VALIDATION_RESULT_VALID; import static android.net.ConnectivityDiagnosticsManager.DataStallReport; import static android.net.ConnectivityDiagnosticsManager.DataStallReport.DETECTION_METHOD_DNS_EVENTS; @@ -78,6 +79,7 @@ import androidx.test.InstrumentationRegistry; import com.android.internal.telephony.uicc.IccUtils; import com.android.internal.util.ArrayUtils; +import com.android.modules.utils.build.SdkLevel; import com.android.net.module.util.ArrayTrackRecord; import com.android.testutils.DevSdkIgnoreRule.IgnoreUpTo; import com.android.testutils.DevSdkIgnoreRunner; @@ -427,6 +429,12 @@ public class ConnectivityDiagnosticsManagerTest { // revalidated which will trigger another onConnectivityReportAvailable callback. if (!hasConnectivity) { cb.expectOnConnectivityReportAvailable(mTestNetwork, interfaceName); + } else if (SdkLevel.isAtLeastS()) { + // All calls to #onNetworkConnectivityReported are expected to be accompanied by a call + // to #onConnectivityReportAvailable after a mainline update in the S timeframe. + // Optionally validate this, but do not fail if it does not exist. + cb.maybeVerifyOnConnectivityReportAvailable(mTestNetwork, interfaceName, TRANSPORT_TEST, + false /* requireCallbackFired */); } cb.assertNoCallback(); @@ -479,13 +487,25 @@ public class ConnectivityDiagnosticsManagerTest { public void expectOnConnectivityReportAvailable( @NonNull Network network, @NonNull String interfaceName) { - expectOnConnectivityReportAvailable(network, interfaceName, TRANSPORT_TEST); + expectOnConnectivityReportAvailable( + network, interfaceName, TRANSPORT_TEST); } - public void expectOnConnectivityReportAvailable( - @NonNull Network network, @NonNull String interfaceName, int transportType) { + public void expectOnConnectivityReportAvailable(@NonNull Network network, + @NonNull String interfaceName, int transportType) { + maybeVerifyOnConnectivityReportAvailable(network, interfaceName, transportType, + true /* requireCallbackFired */); + } + + public void maybeVerifyOnConnectivityReportAvailable(@NonNull Network network, + @NonNull String interfaceName, int transportType, boolean requireCallbackFired) { final ConnectivityReport result = (ConnectivityReport) mHistory.poll(CALLBACK_TIMEOUT_MILLIS, x -> true); + + // If callback is not required and there is no report, exit early. + if (!requireCallbackFired && result == null) { + return; + } assertEquals(network, result.getNetwork()); final NetworkCapabilities nc = result.getNetworkCapabilities(); @@ -496,9 +516,16 @@ public class ConnectivityDiagnosticsManagerTest { final PersistableBundle extras = result.getAdditionalInfo(); assertTrue(extras.containsKey(KEY_NETWORK_VALIDATION_RESULT)); - final int validationResult = extras.getInt(KEY_NETWORK_VALIDATION_RESULT); - assertEquals("Network validation result is not 'valid'", - NETWORK_VALIDATION_RESULT_VALID, validationResult); + final int actualValidationResult = extras.getInt(KEY_NETWORK_VALIDATION_RESULT); + + // Allow RESULT_VALID for networks that are expected to be skipped. Android S shipped + // with validation results being reported as VALID, but the behavior will be updated via + // mainline update. Allow both behaviors, and let MTS enforce stricter behavior + if (actualValidationResult != NETWORK_VALIDATION_RESULT_SKIPPED + && actualValidationResult != NETWORK_VALIDATION_RESULT_VALID) { + fail("Network validation result was incorrect; expected skipped or valid, but " + + "got " + actualValidationResult); + } assertTrue(extras.containsKey(KEY_NETWORK_PROBES_SUCCEEDED_BITMASK)); final int probesSucceeded = extras.getInt(KEY_NETWORK_VALIDATION_RESULT); From 0ccf3fb46122e1217d5ae0d263dade1147c19393 Mon Sep 17 00:00:00 2001 From: Remi NGUYEN VAN Date: Fri, 6 Aug 2021 14:15:31 +0900 Subject: [PATCH 7/8] Use dns resolver v9 in connectivity and set resolverOptions to null. The resolverOptions member of the ResolverParamsParcel has never been set by AOSP code but was only used by OEMs modifying DnsManager. Now that DnsManager is mainline code, this is no longer possible. So the DNS resolver introduces a new setResolverOptions IPC to allow OEMs to set the options and makes the resolverOptions nullable. Make DnsManager set resolverOptions to null, to ensure that when DnsManager calls setResolverConfiguration, it does not overwrite any options set by the OEM. Bug: 194048056 Test: Device boots and has connectivity Change-Id: I310a79521f5a365e50e2c65e9dd87d9b68f105d7 Merged-In: I310a79521f5a365e50e2c65e9dd87d9b68f105d7 --- service/Android.bp | 2 +- .../com/android/server/connectivity/DnsManager.java | 2 -- .../android/server/connectivity/DnsManagerTest.java | 13 ++++++++++--- 3 files changed, 11 insertions(+), 6 deletions(-) diff --git a/service/Android.bp b/service/Android.bp index 7fe0e2b382..39f970d209 100644 --- a/service/Android.bp +++ b/service/Android.bp @@ -65,7 +65,7 @@ java_library { "ServiceConnectivityResources", ], static_libs: [ - "dnsresolver_aidl_interface-V8-java", + "dnsresolver_aidl_interface-V9-java", "modules-utils-os", "net-utils-device-common", "net-utils-framework-common", diff --git a/service/src/com/android/server/connectivity/DnsManager.java b/service/src/com/android/server/connectivity/DnsManager.java index 05b12bad55..1493cae79a 100644 --- a/service/src/com/android/server/connectivity/DnsManager.java +++ b/service/src/com/android/server/connectivity/DnsManager.java @@ -38,7 +38,6 @@ import android.net.IDnsResolver; import android.net.InetAddresses; import android.net.LinkProperties; import android.net.Network; -import android.net.ResolverOptionsParcel; import android.net.ResolverParamsParcel; import android.net.Uri; import android.net.shared.PrivateDnsConfig; @@ -384,7 +383,6 @@ public class DnsManager { .collect(Collectors.toList())) : useTls ? paramsParcel.servers // Opportunistic : new String[0]; // Off - paramsParcel.resolverOptions = new ResolverOptionsParcel(); paramsParcel.transportTypes = transportTypes; // Prepare to track the validation status of the DNS servers in the // resolver config when private DNS is in opportunistic or strict mode. diff --git a/tests/unit/java/com/android/server/connectivity/DnsManagerTest.java b/tests/unit/java/com/android/server/connectivity/DnsManagerTest.java index 9ef558fcea..24aecdb90a 100644 --- a/tests/unit/java/com/android/server/connectivity/DnsManagerTest.java +++ b/tests/unit/java/com/android/server/connectivity/DnsManagerTest.java @@ -43,6 +43,7 @@ import static org.mockito.Mockito.verify; import static org.mockito.Mockito.when; import android.annotation.NonNull; +import android.annotation.Nullable; import android.content.Context; import android.net.ConnectivitySettingsManager; import android.net.IDnsResolver; @@ -106,8 +107,14 @@ public class DnsManagerTest { @Mock IDnsResolver mMockDnsResolver; private void assertResolverOptionsEquals( - @NonNull ResolverOptionsParcel actual, - @NonNull ResolverOptionsParcel expected) { + @Nullable ResolverOptionsParcel actual, + @Nullable ResolverOptionsParcel expected) { + if (actual == null) { + assertNull(expected); + return; + } else { + assertNotNull(expected); + } assertEquals(actual.hosts, expected.hosts); assertEquals(actual.tcMode, expected.tcMode); assertEquals(actual.enforceDnsUid, expected.enforceDnsUid); @@ -365,7 +372,7 @@ public class DnsManagerTest { expectedParams.tlsName = ""; expectedParams.tlsServers = new String[]{"3.3.3.3", "4.4.4.4"}; expectedParams.transportTypes = TEST_TRANSPORT_TYPES; - expectedParams.resolverOptions = new ResolverOptionsParcel(); + expectedParams.resolverOptions = null; assertResolverParamsEquals(actualParams, expectedParams); } From 58b493abf64230bb9095f42cc313bfcd7fd8e2ba Mon Sep 17 00:00:00 2001 From: Aaron Huang Date: Fri, 6 Aug 2021 17:53:14 +0800 Subject: [PATCH 8/8] Skip wifi stats test if link layer stats is not supported Wifi link layer is an optional feature so this test will be failed on wifi stats check if a device does not support it. Add a check to know if the device supports wifi link layer stats and skip it if it is not supported. Ignore-AOSP-First: Needs cherry-picks Bug: 195518957 Test: CtsNetTestCases:BatteryStatsManagerTest Change-Id: I592dd5f1d6e13b020beadb11b9d913857a82e524 --- .../net/src/android/net/cts/BatteryStatsManagerTest.java | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/tests/cts/net/src/android/net/cts/BatteryStatsManagerTest.java b/tests/cts/net/src/android/net/cts/BatteryStatsManagerTest.java index dfb03c753e..47a84202a3 100644 --- a/tests/cts/net/src/android/net/cts/BatteryStatsManagerTest.java +++ b/tests/cts/net/src/android/net/cts/BatteryStatsManagerTest.java @@ -31,6 +31,7 @@ import android.content.Context; import android.net.ConnectivityManager; import android.net.Network; import android.net.cts.util.CtsNetUtils; +import android.net.wifi.WifiManager; import android.os.BatteryStatsManager; import android.os.Build; import android.os.connectivity.CellularBatteryStats; @@ -70,6 +71,7 @@ public class BatteryStatsManagerTest{ private Context mContext; private BatteryStatsManager mBsm; private ConnectivityManager mCm; + private WifiManager mWm; private CtsNetUtils mCtsNetUtils; @Before @@ -77,6 +79,7 @@ public class BatteryStatsManagerTest{ mContext = getContext(); mBsm = mContext.getSystemService(BatteryStatsManager.class); mCm = mContext.getSystemService(ConnectivityManager.class); + mWm = mContext.getSystemService(WifiManager.class); mCtsNetUtils = new CtsNetUtils(mContext); } @@ -128,6 +131,11 @@ public class BatteryStatsManagerTest{ cellularStatsAfter -> cellularBatteryStatsIncreased( cellularStatsBefore, cellularStatsAfter))); + if (!mWm.isEnhancedPowerReportingSupported()) { + Log.d(TAG, "Skip wifi stats test because wifi does not support link layer stats."); + return; + } + WifiBatteryStats wifiStatsBefore = runAsShell(UPDATE_DEVICE_STATS, mBsm::getWifiBatteryStats);