Add AwVariationsSeedFetcher and refactory VariationsSeedFetcher

android_webview/java/src/org/chromium/android_webview/AwVariationsSeedFetcher.java

Index: android_webview/java/src/org/chromium/android_webview/AwVariationsSeedFetcher.java
diff --git a/android_webview/java/src/org/chromium/android_webview/AwVariationsSeedFetcher.java b/android_webview/java/src/org/chromium/android_webview/AwVariationsSeedFetcher.java
new file mode 100644
index 0000000000000000000000000000000000000000..5d2ead7f29787c518966df9c500945f4b10416a4
--- /dev/null
+++ b/android_webview/java/src/org/chromium/android_webview/AwVariationsSeedFetcher.java
@@ -0,0 +1,94 @@
+// Copyright 2017 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+package org.chromium.android_webview;
+
+import android.content.Context;
+
+import org.chromium.base.ContextUtils;
+import org.chromium.base.Log;
+import org.chromium.base.ThreadUtils;
+import org.chromium.components.variations.firstrun.VariationsSeedFetcher;
+
+import java.io.FileNotFoundException;
+import java.io.FileOutputStream;
+import java.io.IOException;
+import java.io.ObjectOutputStream;
+import java.util.HashMap;
+import java.util.Map;
+
+/**
+ * Fetches the variations seed before the actual first run of Android WebView.

[paulmiller, 2017/07/06 23:09:05]

I don't think this is true in WebView's case.

[sgurun-gerrit only, 2017/07/06 23:38:10]

yep not true. Please update class doc to explain in more detail.

+ */
+public class AwVariationsSeedFetcher {
+    private static final String TAG = "AwVariatnsSeedFetch";
+
+    public static final String VARIATIONS_PLATFORM = "android_webview";

[Alexei Svitkine (slow), 2017/07/06 22:01:10]

I think this is fine to put in the VariationsSeedFetcher as
VARIATIONS_PLATFORM_ANDROID or just PLATFORM_ANDROID.

+    private static final String SEED_DATA_FILENAME = "seed";
+    private static final String SEED_DATA_PREF_FILENAME = "seed_pref";
+
+    // Synchronization lock

[paulmiller, 2017/07/06 23:09:05]

Superfluous comment (not yours, but still).

+    private static final Object sLock = new Object();
+
+    private static AwVariationsSeedFetcher sInstance;
+
+    AwVariationsSeedFetcher() {}

[sgurun-gerrit only, 2017/07/06 23:38:11]

is this class supposed to be publicly instantiatable? Otherwise private.

Himm, actually something is weird in this class. It has no state, so no need to
instantiate it at all. the get method is not necessary.

+
+    public static AwVariationsSeedFetcher get() {
+        // TODO(aberent) Check not running on UI thread. Doing so however makes Robolectric testing

[sgurun-gerrit only, 2017/07/06 23:38:11]

why are we expecting aberent to fix that? is this a copy paste error?

+        // of dependent classes difficult.
+        synchronized (sLock) {
+            if (sInstance == null) {
+                Log.d(TAG, "using android webview fetcher");

[sgurun-gerrit only, 2017/07/06 23:38:11]

does not look necessary, remove.

+                sInstance = new AwVariationsSeedFetcher();
+            }
+            return sInstance;
+        }
+    }
+
+    public void fetchSeed(String restrictMode) {

[sgurun-gerrit only, 2017/07/06 23:38:11]

and who is going to call that? write tests maybe?

[sgurun-gerrit only, 2017/07/06 23:38:11]

static

+        assert !ThreadUtils.runningOnUiThread();
+        // Prevent multiple simultaneous fetches

[sgurun-gerrit only, 2017/07/06 23:38:11]

document how multiple simultanous fetches are possible. 

+        synchronized (sLock) {
+            Map<String, String> headerFields = new HashMap<String, String>();
+            byte[] rawSeed = VariationsSeedFetcher.get().downloadContent(
+                    VariationsSeedFetcher.VARIATIONS_SERVER_URL + VARIATIONS_PLATFORM, restrictMode,
+                    headerFields);
+            storeSeed(rawSeed, headerFields);
+        }
+    }
+
+    private void storeSeed(byte[] rawSeed, Map<String, String> headerFields) {

[sgurun-gerrit only, 2017/07/06 23:38:11]

static

+        FileOutputStream fosSeed = null;
+        ObjectOutputStream fosSeedPref = null;
+        try {
+            fosSeed = ContextUtils.getApplicationContext().openFileOutput(
+                    SEED_DATA_FILENAME, Context.MODE_PRIVATE);
+            fosSeed.write(rawSeed, 0, rawSeed.length);
+            fosSeedPref =
+                    new ObjectOutputStream(ContextUtils.getApplicationContext().openFileOutput(
+                            SEED_DATA_PREF_FILENAME, Context.MODE_PRIVATE));
+            fosSeedPref.writeObject(headerFields);
+        } catch (FileNotFoundException e) {
+            Log.e(TAG, "FileNotFoundException storing seed: ", e);

[paulmiller, 2017/07/06 23:09:05]

Mention Finch in these error messages, so people have some idea where they're
coming from. Remember, logcat is noisy, and these will be mixed in with logs
from all over the place. Also, I think printing the stack (which is what happens
when you pass in e) is unnecessarily verbose, since there's only one codepath
leading to this function. logcat already has too many non-actionable stack
traces in it.

+        } catch (IOException e) {
+            Log.e(TAG, "IOException storing seed: ", e);
+        } finally {
+            if (fosSeed != null) {
+                try {
+                    fosSeed.close();
+                } catch (IOException e) {
+                    Log.e(TAG, "IOException file close: ", e);
+                }
+            }
+            if (fosSeedPref != null) {
+                try {
+                    fosSeedPref.close();
+                } catch (IOException e) {
+                    Log.e(TAG, "IOException file close: ", e);
+                }
+            }
+        }
+    }
+}