Fetch Finch seed during restore

chrome/android/java/src/org/chromium/chrome/browser/init/AsyncInitTaskRunner.java

+// 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.chrome.browser.init;
+
+import android.content.BroadcastReceiver;
+import android.content.Context;
+import android.content.Intent;
+import android.content.IntentFilter;
+import android.content.SharedPreferences;
+import android.content.SharedPreferences.Editor;
+import android.os.StrictMode;
+import android.support.v4.content.LocalBroadcastManager;
+
+import org.chromium.base.ContextUtils;
+import org.chromium.base.Log;
+import org.chromium.base.ThreadUtils;
+import org.chromium.base.library_loader.LibraryLoader;
+import org.chromium.base.library_loader.LibraryProcessType;
+import org.chromium.base.library_loader.ProcessInitException;
+import org.chromium.chrome.browser.ChromeVersionInfo;
+import org.chromium.components.variations.firstrun.VariationsSeedService;
+import org.chromium.content.browser.ChildProcessLauncher;
+
+/**
+ * Class for running the Asynchronous startup tasks that need to be run before the native side is
+ * started. Currently it runs two tasks:
+ * - Native library loading
+ * - Fetching the variations seed on first run.
+ */
+public abstract class AsyncInitTaskRunner {
+

[Alexei Svitkine (slow), 2017/01/12 16:06:28]

Nit: Remove empty line. You can add it after TAG if you'd like.

[aberent, 2017/01/12 17:31:50]

Done.

+    private static final String TAG = "AsyncTaskRunner";
+    private static final String VARIATIONS_INITIALIZED = "variations_initialized";
+    private boolean mWaitingForVariationsFetch;
+    private boolean mLibraryLoaded;
+
+    private static boolean shouldFetchVariationsSeedDuringFirstRun() {

[Alexei Svitkine (slow), 2017/01/12 16:06:28]

This logic changed on TOT in the meantime - this function was deleted and
replace with a call to ChromeVersionInfo.isOfficialBuild() directly.

[aberent, 2017/01/12 17:31:50]

Done.

+        // For now, only do the fetching on official canary and dev builds, as there is a concern
+        // about the extra latency this adds.
+        // TODO(asvitkine): Revise this logic based on histogram data.
+        return ChromeVersionInfo.isOfficialBuild()
+                && (ChromeVersionInfo.isCanaryBuild() || ChromeVersionInfo.isDevBuild());
+    }
+
+    /**
+     * Start the background tasks.

[Alexei Svitkine (slow), 2017/01/12 16:06:28]

Nit: Javadoc for params.

[aberent, 2017/01/12 17:31:50]

Done.

+     */
+    public void startBackgroundTasks(final boolean allocateChildConnection,
+            final boolean initVariationSeed) {

[Alexei Svitkine (slow), 2017/01/12 16:06:28]

Nit: No need for final on the second var.

[aberent, 2017/01/12 17:31:50]

Done.

+
+        if (initVariationSeed && shouldFetchVariationsSeedDuringFirstRun()) {
+            StrictMode.ThreadPolicy policy = StrictMode.allowThreadDiskReads();
+            SharedPreferences prefs = ContextUtils.getAppSharedPreferences();
+            boolean variationsInitialized = prefs.getBoolean(VARIATIONS_INITIALIZED, false);

[Alexei Svitkine (slow), 2017/01/12 16:06:28]

I don't understand how this will work.

If this Java pref will be restored by B&R, then it will have a value of true
while the native side would have been lost.

For this to work, we would need to make sure this pref is cleared in the case of
B&R. Is that done somewhere - or if not, how would this work?

[aberent, 2017/01/12 17:31:50]

Backup only saves and restores a very limited subset of the both the native and
the Java prefs (see the list in ChromeBackupAgent), not including this one, so
this will not be restored by B&R. 

[Alexei Svitkine (slow), 2017/01/12 18:55:43]

Ah, I did not realise this! It means that my fix for crbug.com/669543 was not
sufficient - sending another CL now.

+            // Only attempt to get the initial seed once, even if the attempt fails.
+            Editor prefsEditor = prefs.edit();
+            prefsEditor.putBoolean(VARIATIONS_INITIALIZED, true);
+            prefsEditor.apply();
+            StrictMode.setThreadPolicy(policy);
+
+            if (!variationsInitialized) {
+

[Alexei Svitkine (slow), 2017/01/12 16:06:28]

Nit: Remove empty line.

[aberent, 2017/01/12 17:31:50]

Done.

+                Context context = ContextUtils.getApplicationContext();
+                mWaitingForVariationsFetch = true;
+                IntentFilter filter = new IntentFilter(VariationsSeedService.COMPLETE_BROADCAST);
+                final LocalBroadcastManager manager = LocalBroadcastManager.getInstance(context);
+                manager.registerReceiver(
+                        new BroadcastReceiver() {
+                            @Override
+                            public void onReceive(Context context, Intent intent) {
+                                // This check is needed because onReceive() can be called multiple
+                                // times even after having unregistered below if two broadcasts
+                                // arrive in rapid succession.
+                                if (!mWaitingForVariationsFetch) return;
+                                mWaitingForVariationsFetch = false;
+                                manager.unregisterReceiver(this);
+                                Log.i(TAG, "Variations fetched");
+                                tasksPossiblyComplete();
+                            }
+                        },
+                        filter);
+                Log.i(TAG, "Fetching variations");
+                context.startService(new Intent(context, VariationsSeedService.class));
+            }
+        }
+
+        // TODO(yusufo) : Investigate using an AsyncTask for this.
+        new Thread() {
+            @Override
+            public void run() {
+                try {
+                    LibraryLoader libraryLoader =
+                            LibraryLoader.get(LibraryProcessType.PROCESS_BROWSER);
+                    libraryLoader.ensureInitialized();
+                    // The prefetch is done after the library load for two reasons:
+                    // - It is easier to know the library location after it has
+                    // been loaded.
+                    // - Testing has shown that this gives the best compromise,
+                    // by avoiding performance regression on any tested
+                    // device, and providing performance improvement on
+                    // some. Doing it earlier delays UI inflation and more
+                    // generally startup on some devices, most likely by
+                    // competing for IO.
+                    // For experimental results, see http://crbug.com/460438.
+                    libraryLoader.asyncPrefetchLibrariesToMemory();
+                } catch (ProcessInitException e) {
+                    Log.e(TAG, "Unable to load native library.", e);
+                    onFailure();
+                    return;
+                }
+                if (allocateChildConnection) {
+                    ChildProcessLauncher.warmUp(ContextUtils.getApplicationContext());
+                }
+                ThreadUtils.runOnUiThread(new Runnable() {
+                    @Override
+                    public void run() {
+                        mLibraryLoaded = true;
+                        tasksPossiblyComplete();
+                    }
+                });
+            }
+        }.start();
+
+    }
+
+    private void tasksPossiblyComplete() {
+        ThreadUtils.assertOnUiThread();
+
+        if (mLibraryLoaded && !mWaitingForVariationsFetch) {
+            onSuccess();
+        }
+    }
+
+    public abstract void onSuccess();
+    public abstract void onFailure();
+}