Fixing NPE in cacheEvent of BTSUma
Looks like on some devices it is possible that shared preference of
type string set contains null. This patch adds code to sanitize
such set (both when reading and writing) to make sure users of the
read set cannot hit a null value.
Bug: 1064881
Change-Id: I7cc833c3f9e1eb104783341892b23808593a557a
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2122465
Reviewed-by: Tommy Nyquist <nyquist@chromium.org>
Commit-Queue: Filip Gorski <fgorski@chromium.org>
Cr-Commit-Position: refs/heads/master@{#753764}
diff --git a/components/background_task_scheduler/internal/android/java/src/org/chromium/components/background_task_scheduler/internal/BackgroundTaskSchedulerUma.java b/components/background_task_scheduler/internal/android/java/src/org/chromium/components/background_task_scheduler/internal/BackgroundTaskSchedulerUma.java
index 0cfdafa3..90d3677d 100644
--- a/components/background_task_scheduler/internal/android/java/src/org/chromium/components/background_task_scheduler/internal/BackgroundTaskSchedulerUma.java
+++ b/components/background_task_scheduler/internal/android/java/src/org/chromium/components/background_task_scheduler/internal/BackgroundTaskSchedulerUma.java
@@ -276,18 +276,26 @@
@VisibleForTesting
static Set<String> getCachedUmaEntries(SharedPreferences prefs) {
- return prefs.getStringSet(KEY_CACHED_UMA, new HashSet<String>(1));
+ Set<String> cachedUmaEntries = prefs.getStringSet(KEY_CACHED_UMA, new HashSet<>());
+ return sanitizeEntrySet(cachedUmaEntries);
}
@VisibleForTesting
static void updateCachedUma(SharedPreferences prefs, Set<String> cachedUma) {
ThreadUtils.assertOnUiThread();
SharedPreferences.Editor editor = prefs.edit();
- editor.putStringSet(KEY_CACHED_UMA, cachedUma);
+ editor.putStringSet(KEY_CACHED_UMA, sanitizeEntrySet(cachedUma));
editor.apply();
}
void assertNativeIsLoaded() {
assert LibraryLoader.getInstance().isInitialized();
}
+
+ private static Set<String> sanitizeEntrySet(Set<String> set) {
+ if (set != null && set.contains(null)) {
+ set.remove(null);
+ }
+ return set;
+ }
}
diff --git a/components/background_task_scheduler/internal/android/junit/src/org/chromium/components/background_task_scheduler/internal/BackgroundTaskSchedulerUmaTest.java b/components/background_task_scheduler/internal/android/junit/src/org/chromium/components/background_task_scheduler/internal/BackgroundTaskSchedulerUmaTest.java
index a9f60ae5..b0fd24c 100644
--- a/components/background_task_scheduler/internal/android/junit/src/org/chromium/components/background_task_scheduler/internal/BackgroundTaskSchedulerUmaTest.java
+++ b/components/background_task_scheduler/internal/android/junit/src/org/chromium/components/background_task_scheduler/internal/BackgroundTaskSchedulerUmaTest.java
@@ -5,6 +5,7 @@
package org.chromium.components.background_task_scheduler.internal;
import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertFalse;
import static org.junit.Assert.assertTrue;
import static org.mockito.Mockito.anyInt;
import static org.mockito.Mockito.anyString;
@@ -14,6 +15,8 @@
import static org.mockito.Mockito.times;
import static org.mockito.Mockito.verify;
+import android.content.SharedPreferences;
+
import org.junit.Before;
import org.junit.Test;
import org.junit.runner.RunWith;
@@ -28,6 +31,7 @@
import org.chromium.components.background_task_scheduler.BackgroundTaskSchedulerExternalUma;
import org.chromium.components.background_task_scheduler.TaskIds;
+import java.util.HashSet;
import java.util.Set;
/** Unit tests for {@link BackgroundTaskSchedulerUma}. */
@@ -150,6 +154,28 @@
@Test
@Feature({"BackgroundTaskScheduler"})
+ public void testCacheEvent_StringStartWithNpe() {
+ // Set up preferences with a null entry.
+ SharedPreferences.Editor editor = ContextUtils.getAppSharedPreferences().edit();
+ HashSet<String> setWithNullValue = new HashSet<>();
+ setWithNullValue.add(null);
+ editor.putStringSet(BackgroundTaskSchedulerUma.KEY_CACHED_UMA, setWithNullValue);
+ editor.apply();
+
+ Set<String> cachedUmaEntries = BackgroundTaskSchedulerUma.getCachedUmaEntries(
+ ContextUtils.getAppSharedPreferences());
+ assertTrue(cachedUmaEntries.isEmpty());
+
+ mUmaSpy.cacheEvent("NpeTestEvent", 77);
+
+ cachedUmaEntries = BackgroundTaskSchedulerUma.getCachedUmaEntries(
+ ContextUtils.getAppSharedPreferences());
+ assertTrue(cachedUmaEntries.contains("NpeTestEvent:77:1"));
+ assertFalse(cachedUmaEntries.contains(null));
+ }
+
+ @Test
+ @Feature({"BackgroundTaskScheduler"})
public void testFlushStats() {
doNothing().when(mUmaSpy).recordEnumeratedHistogram(anyString(), anyInt(), anyInt());
