Variations: add an option to avoid SharedPreferences

No change to logic for Chrome, this only affects WebView. This adds an
option to VariationsSeedStore to configure it to avoid accessing the
"first run" Java SharedPreferences. Any access to SharedPreferences may
perform disk IO; since WebView initializes variations on the UI thread,
this causes a strictmode violation.

WebView doesn't use the first run prefs, so this adds a configuration
parameter to completely avoid SharedPreferences. This also adds unit
tests to verify SharedPreferences are not modified when this parameter
is passed.

Fixed: 1131941
Test: out/Default/bin/run_components_unittests -f VariationsSeedStore*
Test: android_webview/tools/run_cts.py -m CtsWebViewStartupApp.apk
Test: Locally add 'assert false' next to SharedPreferences access, verify WebView doesn't crash
Change-Id: I137fcbc49b4619dcde84fa1d0375edf6d421c8eb
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2444473
Auto-Submit: Nate Fischer <ntfschr@chromium.org>
Commit-Queue: Alexei Svitkine <asvitkine@chromium.org>
Reviewed-by: Alexei Svitkine <asvitkine@chromium.org>
Reviewed-by: Robert Kaplow <rkaplow@chromium.org>
Reviewed-by: Richard Coles <torne@chromium.org>
Cr-Commit-Position: refs/heads/master@{#813944}

android_webview/browser/aw_feature_list_creator.cc

@@ -190,7 +190,8 @@ void AwFeatureListCreator::SetUpFieldTrials() {
   client_ = std::make_unique<AwVariationsServiceClient>();
   auto seed_store = std::make_unique<variations::VariationsSeedStore>(
       local_state_.get(), /*initial_seed=*/std::move(seed),
-      /*signature_verification_enabled=*/g_signature_verification_enabled);
+      /*signature_verification_enabled=*/g_signature_verification_enabled,
+      /*use_first_run_prefs=*/false);
 
   if (!seed_date.is_null())
     seed_store->RecordLastFetchTime(seed_date);

components/variations/android/java/src/org/chromium/components/variations/firstrun/VariationsSeedBridge.java

@@ -94,6 +94,7 @@ public class VariationsSeedBridge {
     /**
      * Returns the status of the variations seed storing on the C++ side: was it successful or not.
      */
+    @CalledByNative
     public static boolean hasNativePref() {
         return ContextUtils.getAppSharedPreferences().getBoolean(
                 VARIATIONS_FIRST_RUN_SEED_NATIVE_STORED, false);

components/variations/android/variations_seed_bridge.cc

@@ -70,5 +70,10 @@ void SetJavaFirstRunPrefsForTesting(const std::string& seed_data,
       static_cast<jboolean>(is_gzip_compressed));
 }
 
+bool HasMarkedPrefsForTesting() {
+  JNIEnv* env = AttachCurrentThread();
+  return Java_VariationsSeedBridge_hasNativePref(env);
+}
+
 }  // namespace android
 }  // namespace variations

components/variations/android/variations_seed_bridge.h

@@ -33,6 +33,8 @@ void SetJavaFirstRunPrefsForTesting(const std::string& seed_data,
                                     long response_date,
                                     bool is_gzip_compressed);
 
+bool HasMarkedPrefsForTesting();
+
 }  // namespace android
 }  // namespace variations
 

components/variations/variations_seed_store.cc

@@ -118,9 +118,11 @@ VariationsSeedStore::VariationsSeedStore(PrefService* local_state)
 VariationsSeedStore::VariationsSeedStore(
     PrefService* local_state,
     std::unique_ptr<SeedResponse> initial_seed,
-    bool signature_verification_enabled)
+    bool signature_verification_enabled,
+    bool use_first_run_prefs)
     : local_state_(local_state),
-      signature_verification_enabled_(signature_verification_enabled) {
+      signature_verification_enabled_(signature_verification_enabled),
+      use_first_run_prefs_(use_first_run_prefs) {
 #if defined(OS_ANDROID)
   if (initial_seed)
     ImportInitialSeed(std::move(initial_seed));
@@ -411,7 +413,9 @@ void VariationsSeedStore::ImportInitialSeed(
   // needed there. This is done regardless if we fail or succeed
   // below - since if we succeed, we're good to go and if we fail,
   // we probably don't want to keep around the bad content anyway.
-  android::ClearJavaFirstRunPrefs();
+  if (use_first_run_prefs_) {
+    android::ClearJavaFirstRunPrefs();
+  }
 
   if (initial_seed->date == 0) {
     RecordFirstRunSeedImportResult(
@@ -518,8 +522,10 @@ bool VariationsSeedStore::StoreSeedDataNoDelta(
 #if defined(OS_ANDROID)
   // If currently we do not have any stored pref then we mark seed storing as
   // successful on the Java side to avoid repeated seed fetches.
-  if (local_state_->GetString(prefs::kVariationsCompressedSeed).empty())
+  if (local_state_->GetString(prefs::kVariationsCompressedSeed).empty() &&
+      use_first_run_prefs_) {
     android::MarkVariationsSeedAsStored();
+  }
 #endif
 
   // Update the saved country code only if one was returned from the server.

components/variations/variations_seed_store.h

@@ -35,10 +35,14 @@ class VariationsSeedStore {
   // seed store. This is used by Android Chrome to supply the first run seed,
   // and by Android WebView to supply the seed on every run.
   // |signature_verification_enabled| can be used in unit tests to disable
-  // signature checks on the seed.
+  // signature checks on the seed. If |use_first_run_prefs| is true (default),
+  // then this VariationsSeedStore may modify the Java SharedPreferences ("first
+  // run prefs") which are set during first run; otherwise this will not access
+  // SharedPreferences at all.
   VariationsSeedStore(PrefService* local_state,
                       std::unique_ptr<SeedResponse> initial_seed,
-                      bool signature_verification_enabled);
+                      bool signature_verification_enabled,
+                      bool use_first_run_prefs = true);
   virtual ~VariationsSeedStore();
 
   // Loads the variations seed data from local state into |seed|, as well as the
@@ -204,7 +208,10 @@ class VariationsSeedStore {
   std::string latest_serial_number_;
 
   // Whether to validate signatures on the seed. Always on except in tests.
-  bool signature_verification_enabled_ = true;
+  const bool signature_verification_enabled_;
+
+  // Whether this may read or write to Java "first run" SharedPreferences.
+  const bool use_first_run_prefs_;
 
   DISALLOW_COPY_AND_ASSIGN(VariationsSeedStore);
 };

components/variations/variations_seed_store_unittest.cc

@@ -51,10 +51,14 @@ constexpr char kIdenticalToSafeSeedSentinel[] = "safe_seed_content";
 
 class TestVariationsSeedStore : public VariationsSeedStore {
  public:
-  explicit TestVariationsSeedStore(PrefService* local_state)
+  explicit TestVariationsSeedStore(
+      PrefService* local_state,
+      std::unique_ptr<SeedResponse> initial_seed = nullptr,
+      bool use_first_run_prefs = true)
       : VariationsSeedStore(local_state,
-                            nullptr,
-                            /*signature_verification_enabled=*/false) {}
+                            std::move(initial_seed),
+                            /*signature_verification_enabled=*/false,
+                            use_first_run_prefs) {}
   ~TestVariationsSeedStore() override = default;
 
   bool StoreSeedForTesting(const std::string& seed_data) {
@@ -1208,6 +1212,70 @@ TEST(VariationsSeedStoreTest, ImportFirstRunJavaSeed) {
   EXPECT_EQ(0, seed->date);
   EXPECT_FALSE(seed->is_gzip_compressed);
 }
+
+class VariationsSeedStoreFirstRunPrefsTest
+    : public testing::TestWithParam<bool> {};
+
+INSTANTIATE_TEST_SUITE_P(VariationsSeedStoreTest,
+                         VariationsSeedStoreFirstRunPrefsTest,
+                         testing::Bool());
+
+TEST_P(VariationsSeedStoreFirstRunPrefsTest, FirstRunPrefsAllowed) {
+  bool use_first_run_prefs = GetParam();
+
+  const std::string test_seed_data = "raw_seed_data_test";
+  const std::string test_seed_signature = "seed_signature_test";
+  const std::string test_seed_country = "seed_country_code_test";
+  const long test_response_date = 1234567890;
+  const bool test_is_gzip_compressed = true;
+  android::SetJavaFirstRunPrefsForTesting(test_seed_data, test_seed_signature,
+                                          test_seed_country, test_response_date,
+                                          test_is_gzip_compressed);
+
+  const VariationsSeed test_seed = CreateTestSeed();
+  const std::string seed_data = SerializeSeed(test_seed);
+  const std::string base64_seed_data = SerializeSeedBase64(test_seed);
+  auto seed = std::make_unique<SeedResponse>();
+  seed->data = seed_data;
+  seed->signature = "java_seed_signature";
+  seed->country = "java_seed_country";
+  seed->date = 1234554321;
+  seed->is_gzip_compressed = false;
+
+  TestingPrefServiceSimple prefs;
+  VariationsSeedStore::RegisterPrefs(prefs.registry());
+  TestVariationsSeedStore seed_store(&prefs, /*initial_seed=*/std::move(seed),
+                                     use_first_run_prefs);
+
+  seed = android::GetVariationsFirstRunSeed();
+
+  if (use_first_run_prefs) {
+    // VariationsSeedStore should clear the first run seed prefs, so
+    // GetVariationsFirstRunSeed() should return default values.
+    EXPECT_EQ("", seed->data);
+    EXPECT_EQ("", seed->signature);
+    EXPECT_EQ("", seed->country);
+    EXPECT_EQ(0, seed->date);
+    EXPECT_FALSE(seed->is_gzip_compressed);
+
+    // VariationsSeedStore should signal to Java that it has saved the seed in
+    // native prefs.
+    EXPECT_TRUE(android::HasMarkedPrefsForTesting());
+  } else {
+    // VariationsSeedStore must not modify Java prefs at all.
+    EXPECT_EQ(test_seed_data, seed->data);
+    EXPECT_EQ(test_seed_signature, seed->signature);
+    EXPECT_EQ(test_seed_country, seed->country);
+    EXPECT_EQ(test_response_date, seed->date);
+    EXPECT_EQ(test_is_gzip_compressed, seed->is_gzip_compressed);
+    EXPECT_FALSE(android::HasMarkedPrefsForTesting());
+  }
+
+  // Seed should be stored in prefs.
+  EXPECT_FALSE(PrefHasDefaultValue(prefs, prefs::kVariationsCompressedSeed));
+  EXPECT_EQ(base64_seed_data,
+            prefs.GetString(prefs::kVariationsCompressedSeed));
+}
 #endif  // OS_ANDROID
 
 }  // namespace variations