Skip to content

Commit 7ee8658

Browse files
jsharkeyjsharkey
committed
Conservatively trim data usage stats.
Instead of trusting NTP time alone, use the most-conservative of system clock and NTP. Bug: 5584564 Change-Id: I5dd87fc009959b1cf0a7d660e385a0b1a8be238b
1 parent cde433c commit 7ee8658

File tree

3 files changed

+43
-22
lines changed

3 files changed

+43
-22
lines changed

‎core/tests/coretests/src/android/net/NetworkStatsHistoryTest.java‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -256,6 +256,10 @@ public void testRemove() throws Exception {
256256
stats.recordData(TEST_START, TEST_START + DAY_IN_MILLIS, 24L, 24L);
257257
assertEquals(24, stats.size());
258258

259+
// try removing invalid data; should be no change
260+
stats.removeBucketsBefore(0 - DAY_IN_MILLIS);
261+
assertEquals(24, stats.size());
262+
259263
// try removing far before buckets; should be no change
260264
stats.removeBucketsBefore(TEST_START - YEAR_IN_MILLIS);
261265
assertEquals(24, stats.size());

‎services/java/com/android/server/net/NetworkStatsService.java‎

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1243,7 +1243,8 @@ private void writeNetworkStats(
12431243

12441244
// trim any history beyond max
12451245
if (mTime.hasCache()) {
1246-
final long currentTime = mTime.currentTimeMillis();
1246+
final long currentTime = Math.min(
1247+
System.currentTimeMillis(), mTime.currentTimeMillis());
12471248
final long maxHistory = mSettings.getNetworkMaxHistory();
12481249
for (NetworkStatsHistory history : input.values()) {
12491250
history.removeBucketsBefore(currentTime - maxHistory);
@@ -1287,7 +1288,8 @@ private void writeUidStatsLocked() {
12871288

12881289
// trim any history beyond max
12891290
if (mTime.hasCache()) {
1290-
final long currentTime = mTime.currentTimeMillis();
1291+
final long currentTime = Math.min(
1292+
System.currentTimeMillis(), mTime.currentTimeMillis());
12911293
final long maxUidHistory = mSettings.getUidMaxHistory();
12921294
final long maxTagHistory = mSettings.getTagMaxHistory();
12931295
for (UidStatsKey key : mUidStats.keySet()) {

‎services/tests/servicestests/src/com/android/server/NetworkPolicyManagerServiceTest.java‎

Lines changed: 35 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -80,7 +80,6 @@
8080
import org.easymock.Capture;
8181
import org.easymock.EasyMock;
8282
import org.easymock.IAnswer;
83-
import org.easymock.IExpectationSetters;
8483

8584
import java.io.File;
8685
import java.util.LinkedHashSet;
@@ -537,6 +536,7 @@ public void testNetworkPolicyAppliedCycleLastMonth() throws Exception {
537536
.addIfaceValues(TEST_IFACE, 256L, 2L, 256L, 2L);
538537
expect(mStatsService.getSummaryForNetwork(sTemplateWifi, TIME_FEB_15, TIME_MAR_10))
539538
.andReturn(stats).atLeastOnce();
539+
expectPolicyDataEnable(TYPE_WIFI, true);
540540

541541
// TODO: consider making strongly ordered mock
542542
expectRemoveInterfaceQuota(TEST_IFACE);
@@ -580,7 +580,7 @@ public void testOverWarningLimitNotification() throws Exception {
580580
NetworkState[] state = null;
581581
NetworkStats stats = null;
582582
Future<Void> future;
583-
Capture<String> tag;
583+
Future<String> tagFuture;
584584

585585
final long TIME_FEB_15 = 1171497600000L;
586586
final long TIME_MAR_10 = 1173484800000L;
@@ -598,6 +598,7 @@ public void testOverWarningLimitNotification() throws Exception {
598598
expect(mConnManager.getAllNetworkState()).andReturn(state).atLeastOnce();
599599
expect(mStatsService.getSummaryForNetwork(sTemplateWifi, TIME_FEB_15, currentTimeMillis()))
600600
.andReturn(stats).atLeastOnce();
601+
expectPolicyDataEnable(TYPE_WIFI, true);
601602

602603
expectClearNotifications();
603604
future = expectMeteredIfacesChanged();
@@ -620,6 +621,7 @@ public void testOverWarningLimitNotification() throws Exception {
620621
expect(mConnManager.getAllNetworkState()).andReturn(state).atLeastOnce();
621622
expect(mStatsService.getSummaryForNetwork(sTemplateWifi, TIME_FEB_15, currentTimeMillis()))
622623
.andReturn(stats).atLeastOnce();
624+
expectPolicyDataEnable(TYPE_WIFI, true);
623625

624626
expectRemoveInterfaceQuota(TEST_IFACE);
625627
expectSetInterfaceQuota(TEST_IFACE, 2 * MB_IN_BYTES);
@@ -642,14 +644,15 @@ public void testOverWarningLimitNotification() throws Exception {
642644
expectCurrentTime();
643645
expect(mStatsService.getSummaryForNetwork(sTemplateWifi, TIME_FEB_15, currentTimeMillis()))
644646
.andReturn(stats).atLeastOnce();
647+
expectPolicyDataEnable(TYPE_WIFI, true);
645648

646649
expectForceUpdate();
647650
expectClearNotifications();
648-
tag = expectEnqueueNotification();
651+
tagFuture = expectEnqueueNotification();
649652

650653
replay();
651654
mNetworkObserver.limitReached(null, TEST_IFACE);
652-
assertNotificationType(TYPE_WARNING, tag.getValue());
655+
assertNotificationType(TYPE_WARNING, tagFuture.get());
653656
verifyAndReset();
654657
}
655658

@@ -662,15 +665,15 @@ public void testOverWarningLimitNotification() throws Exception {
662665
expectCurrentTime();
663666
expect(mStatsService.getSummaryForNetwork(sTemplateWifi, TIME_FEB_15, currentTimeMillis()))
664667
.andReturn(stats).atLeastOnce();
665-
expectPolicyDataEnable(TYPE_WIFI, false).atLeastOnce();
668+
expectPolicyDataEnable(TYPE_WIFI, false);
666669

667670
expectForceUpdate();
668671
expectClearNotifications();
669-
tag = expectEnqueueNotification();
672+
tagFuture = expectEnqueueNotification();
670673

671674
replay();
672675
mNetworkObserver.limitReached(null, TEST_IFACE);
673-
assertNotificationType(TYPE_LIMIT, tag.getValue());
676+
assertNotificationType(TYPE_LIMIT, tagFuture.get());
674677
verifyAndReset();
675678
}
676679

@@ -682,21 +685,20 @@ public void testOverWarningLimitNotification() throws Exception {
682685
expect(mConnManager.getAllNetworkState()).andReturn(state).atLeastOnce();
683686
expect(mStatsService.getSummaryForNetwork(sTemplateWifi, TIME_FEB_15, currentTimeMillis()))
684687
.andReturn(stats).atLeastOnce();
685-
expectPolicyDataEnable(TYPE_WIFI, true).atLeastOnce();
688+
expectPolicyDataEnable(TYPE_WIFI, true);
686689

687690
// snoozed interface still has high quota so background data is
688691
// still restricted.
689692
expectRemoveInterfaceQuota(TEST_IFACE);
690693
expectSetInterfaceQuota(TEST_IFACE, Long.MAX_VALUE);
694+
expectMeteredIfacesChanged(TEST_IFACE);
691695

692696
expectClearNotifications();
693-
tag = expectEnqueueNotification();
694-
future = expectMeteredIfacesChanged(TEST_IFACE);
697+
tagFuture = expectEnqueueNotification();
695698

696699
replay();
697700
mService.snoozePolicy(sTemplateWifi);
698-
future.get();
699-
assertNotificationType(TYPE_LIMIT_SNOOZED, tag.getValue());
701+
assertNotificationType(TYPE_LIMIT_SNOOZED, tagFuture.get());
700702
verifyAndReset();
701703
}
702704
}
@@ -737,9 +739,9 @@ private void expectClearNotifications() throws Exception {
737739
expectLastCall().anyTimes();
738740
}
739741

740-
private Capture<String> expectEnqueueNotification() throws Exception {
741-
final Capture<String> tag = new Capture<String>();
742-
mNotifManager.enqueueNotificationWithTag(isA(String.class), capture(tag), anyInt(),
742+
private Future<String> expectEnqueueNotification() throws Exception {
743+
final FutureCapture<String> tag = new FutureCapture<String>();
744+
mNotifManager.enqueueNotificationWithTag(isA(String.class), capture(tag.capture), anyInt(),
743745
isA(Notification.class), isA(int[].class));
744746
return tag;
745747
}
@@ -789,29 +791,42 @@ private Future<Void> expectMeteredIfacesChanged(String... ifaces) throws Excepti
789791
return future;
790792
}
791793

792-
private <T> IExpectationSetters<T> expectPolicyDataEnable(int type, boolean enabled)
793-
throws Exception {
794+
private Future<Void> expectPolicyDataEnable(int type, boolean enabled) throws Exception {
795+
final FutureAnswer future = new FutureAnswer();
794796
mConnManager.setPolicyDataEnable(type, enabled);
795-
return expectLastCall();
797+
expectLastCall().andAnswer(future);
798+
return future;
796799
}
797800

798-
private static class FutureAnswer extends AbstractFuture<Void> implements IAnswer<Void> {
801+
private static class TestAbstractFuture<T> extends AbstractFuture<T> {
799802
@Override
800-
public Void get() throws InterruptedException, ExecutionException {
803+
public T get() throws InterruptedException, ExecutionException {
801804
try {
802805
return get(5, TimeUnit.SECONDS);
803806
} catch (TimeoutException e) {
804807
throw new RuntimeException(e);
805808
}
806809
}
810+
}
807811

812+
private static class FutureAnswer extends TestAbstractFuture<Void> implements IAnswer<Void> {
808813
@Override
809814
public Void answer() {
810815
set(null);
811816
return null;
812817
}
813818
}
814819

820+
private static class FutureCapture<T> extends TestAbstractFuture<T> {
821+
public Capture<T> capture = new Capture<T>() {
822+
@Override
823+
public void setValue(T value) {
824+
super.setValue(value);
825+
set(value);
826+
}
827+
};
828+
}
829+
815830
private static class IdleFuture extends AbstractFuture<Void> implements IdleHandler {
816831
@Override
817832
public Void get() throws InterruptedException, ExecutionException {

0 commit comments

Comments
 (0)