Enable variations restrict param for Java-side fetches.

Prior to this change, when we fetched the variations seed from Java could, it was
not using the correct restrict param in the URL. This change plumbs it through.
See bug for more details.

BUG=708713

Review-Url: https://codereview.chromium.org/2804043002
Cr-Commit-Position: refs/heads/master@{#463033}

chrome/android/java/src/org/chromium/chrome/browser/ChromeActivitySessionTracker.java

@@ -5,6 +5,7 @@
 package org.chromium.chrome.browser;
 
 import android.app.Activity;
+import android.app.Application;
 import android.content.SharedPreferences;
 import android.provider.Settings;
 import android.text.TextUtils;
@@ -12,6 +13,7 @@ import android.text.TextUtils;
 import org.chromium.base.ApplicationState;
 import org.chromium.base.ApplicationStatus;
 import org.chromium.base.ApplicationStatus.ApplicationStateListener;
+import org.chromium.base.Callback;
 import org.chromium.base.ContextUtils;
 import org.chromium.base.LocaleUtils;
 import org.chromium.base.ThreadUtils;
@@ -47,7 +49,7 @@ public class ChromeActivitySessionTracker {
     // Used to trigger variation changes (such as seed fetches) upon application foregrounding.
     private VariationsSession mVariationsSession;
 
-    private ChromeApplication mApplication;
+    private Application mApplication;
     private boolean mIsInitialized;
     private boolean mIsStarted;
     private boolean mIsFinishedCachingNativeFlags;
@@ -67,7 +69,17 @@ public class ChromeActivitySessionTracker {
      * @see #getInstance()
      */
     protected ChromeActivitySessionTracker() {
-        mApplication = (ChromeApplication) ContextUtils.getApplicationContext();
+        mApplication = (Application) ContextUtils.getApplicationContext();
+        mVariationsSession = AppHooks.get().createVariationsSession();
+    }
+
+    /**
+     * Asynchronously returns the value of the "restrict" URL param that the variations service
+     * should use for variation seed requests.
+     * @param callback Callback that will be called with the param value when available.
+     */
+    public void getVariationsRestrictModeValue(Callback<String> callback) {
+        mVariationsSession.getRestrictModeValue(mApplication, callback);
     }
 
     /**
@@ -81,7 +93,6 @@ public class ChromeActivitySessionTracker {
         assert !mIsStarted;
 
         ApplicationStatus.registerApplicationStateListener(createApplicationStateListener());
-        mVariationsSession = AppHooks.get().createVariationsSession();
     }
 
     /**

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

@@ -6,12 +6,14 @@ package org.chromium.chrome.browser.init;
 
 import android.os.AsyncTask;
 
+import org.chromium.base.Callback;
 import org.chromium.base.ContextUtils;
 import org.chromium.base.ThreadUtils;
 import org.chromium.base.VisibleForTesting;
 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.ChromeActivitySessionTracker;
 import org.chromium.chrome.browser.ChromeVersionInfo;
 import org.chromium.components.variations.firstrun.VariationsSeedFetcher;
 import org.chromium.content.browser.ChildProcessLauncher;
@@ -67,9 +69,15 @@ public abstract class AsyncInitTaskRunner {
     }
 
     private class FetchSeedTask extends AsyncTask<Void, Void, Void> {
+        private final String mRestrictMode;
+
+        public FetchSeedTask(String restrictMode) {
+            mRestrictMode = restrictMode;
+        }
+
         @Override
         protected Void doInBackground(Void... params) {
-            VariationsSeedFetcher.get().fetchSeed();
+            VariationsSeedFetcher.get().fetchSeed(mRestrictMode);
             return null;
         }
 
@@ -92,8 +100,15 @@ public abstract class AsyncInitTaskRunner {
         assert mLoadTask == null;
         if (fetchVariationSeed && shouldFetchVariationsSeedDuringFirstRun()) {
             mFetchingVariations = true;
-            mFetchSeedTask = new FetchSeedTask();
-            mFetchSeedTask.executeOnExecutor(getExecutor());
+            ChromeActivitySessionTracker sessionTracker =
+                    ChromeActivitySessionTracker.getInstance();
+            sessionTracker.getVariationsRestrictModeValue(new Callback<String>() {
+                @Override
+                public void onResult(String restrictMode) {
+                    mFetchSeedTask = new FetchSeedTask(restrictMode);
+                    mFetchSeedTask.executeOnExecutor(getExecutor());
+                }
+            });
         }
 
         if (allocateChildConnection) {

chrome/android/java/src/org/chromium/chrome/browser/metrics/VariationsSession.java

@@ -13,39 +13,61 @@ import org.chromium.base.Callback;
  * triggering seed fetches on application startup.
  */
 public class VariationsSession {
-    private boolean mInitialized;
+    private boolean mRestrictModeFetchStarted;
     private String mRestrictMode;
 
     /**
      * Triggers to the native VariationsService that the application has entered the foreground.
      */
     public void start(Context context) {
-        if (!mInitialized) {
-            mInitialized = true;
-            // Check the restrict mode only once initially to avoid doing extra work each time the
-            // app enters foreground.
-            getRestrictMode(context, new Callback<String>() {
-                @Override
-                public void onResult(String restrictMode) {
-                    assert restrictMode != null;
-                    mRestrictMode = restrictMode;
-                    nativeStartVariationsSession(mRestrictMode);
-                }
-            });
-        // If |mRestrictMode| is null, async initialization is in progress and
-        // nativeStartVariationsSession will be called when it completes.
-        } else if (mRestrictMode != null) {
-            nativeStartVariationsSession(mRestrictMode);
+        // If |mRestrictModeFetchStarted| is true and |mRestrictMode| is null, then async
+        // initializationn is in progress and nativeStartVariationsSession() will be called
+        // when it completes.
+        if (mRestrictModeFetchStarted && mRestrictMode == null) {
+            return;
         }
+
+        mRestrictModeFetchStarted = true;
+        getRestrictModeValue(context, new Callback<String>() {
+            @Override
+            public void onResult(String restrictMode) {
+                nativeStartVariationsSession(mRestrictMode);
+            }
+        });
+    }
+
+    /**
+     * Asynchronously returns the value of the "restrict" URL param that the variations service
+     * should use for variation seed requests. Public version that can be called externally.
+     * Uses the protected version (that could be overridden by subclasses) to actually get the
+     * value and also sets that value internally when retrieved.
+     * @param callback Callback that will be called with the param value when available.
+     */
+    public final void getRestrictModeValue(Context context, final Callback<String> callback) {
+        // If |mRestrictMode| is not null, the value has already been fetched and so it can
+        // simply be provided to the callback.
+        if (mRestrictMode != null) {
+            callback.onResult(mRestrictMode);
+            return;
+        }
+        getRestrictMode(context, new Callback<String>() {
+            @Override
+            public void onResult(String restrictMode) {
+                assert restrictMode != null;
+                mRestrictMode = restrictMode;
+                callback.onResult(restrictMode);
+            }
+        });
     }
 
     /**
      * Asynchronously returns the value of the "restrict" URL param that the variations service
-     * should use for variation seed requests.
+     * should use for variation seed requests. This can be overriden by subclass to provide actual
+     * restrict values, which must not be null.
      */
     protected void getRestrictMode(Context context, Callback<String> callback) {
         callback.onResult("");
     }
 
-    private native void nativeStartVariationsSession(String restrictMode);
+    protected native void nativeStartVariationsSession(String restrictMode);
 }

chrome/android/java_sources.gni

@@ -1625,6 +1625,7 @@ chrome_junit_test_java_sources = [
   "junit/src/org/chromium/chrome/browser/media/router/cast/TestUtils.java",
   "junit/src/org/chromium/chrome/browser/media/ui/MediaImageManagerTest.java",
   "junit/src/org/chromium/chrome/browser/media/ui/MediaNotificationButtonComputationTest.java",
+  "junit/src/org/chromium/chrome/browser/metrics/VariationsSessionTest.java",
   "junit/src/org/chromium/chrome/browser/notifications/NotificationPlatformBridgeUnitTest.java",
   "junit/src/org/chromium/chrome/browser/ntp/NativePageFactoryTest.java",
   "junit/src/org/chromium/chrome/browser/ntp/TitleUtilTest.java",

chrome/android/junit/src/org/chromium/chrome/browser/init/AsyncInitTaskRunnerTest.java

@@ -86,7 +86,7 @@ public class AsyncInitTaskRunnerTest {
         verify(mLoader).ensureInitialized();
         verify(mLoader).asyncPrefetchLibrariesToMemory();
         verify(mRunner).onSuccess();
-        verify(mVariationsSeedFetcher, never()).fetchSeed();
+        verify(mVariationsSeedFetcher, never()).fetchSeed("");
     }
 
     @Test
@@ -100,7 +100,7 @@ public class AsyncInitTaskRunnerTest {
         Robolectric.flushForegroundThreadScheduler();
         assertTrue(mLatch.await(0, TimeUnit.SECONDS));
         verify(mRunner).onFailure();
-        verify(mVariationsSeedFetcher, never()).fetchSeed();
+        verify(mVariationsSeedFetcher, never()).fetchSeed("");
     }
 
     @Test
@@ -113,7 +113,7 @@ public class AsyncInitTaskRunnerTest {
         verify(mLoader).ensureInitialized();
         verify(mLoader).asyncPrefetchLibrariesToMemory();
         verify(mRunner).onSuccess();
-        verify(mVariationsSeedFetcher).fetchSeed();
+        verify(mVariationsSeedFetcher).fetchSeed("");
     }
 
     // TODO(aberent) Test for allocateChildConnection. Needs refactoring of ChildProcessLauncher to

chrome/android/junit/src/org/chromium/chrome/browser/metrics/VariationsSessionTest.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.metrics;
+
+import android.content.Context;
+
+import static org.mockito.Mockito.any;
+import static org.mockito.Mockito.never;
+import static org.mockito.Mockito.spy;
+import static org.mockito.Mockito.times;
+import static org.mockito.Mockito.verify;
+
+import org.junit.Before;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.robolectric.annotation.Config;
+
+import org.chromium.base.Callback;
+import org.chromium.base.ContextUtils;
+import org.chromium.testing.local.LocalRobolectricTestRunner;
+
+/**
+ * Tests for VariationsSession
+ */
+@RunWith(LocalRobolectricTestRunner.class)
+@Config(manifest = Config.NONE)
+public class VariationsSessionTest {
+    private TestVariationsSession mSession;
+
+    private static class TestVariationsSession extends VariationsSession {
+        private Callback<String> mCallback;
+
+        @Override
+        protected void getRestrictMode(Context context, Callback<String> callback) {
+            mCallback = callback;
+        }
+
+        public void runCallback(String value) {
+            mCallback.onResult(value);
+        }
+
+        @Override
+        protected void nativeStartVariationsSession(String restrictMode) {
+            // No-op for tests.
+        }
+    }
+
+    @Before
+    public void setUp() {
+        mSession = spy(new TestVariationsSession());
+    }
+
+    @Test
+    public void testStart() {
+        mSession.start(ContextUtils.getApplicationContext());
+        verify(mSession, never()).nativeStartVariationsSession(any(String.class));
+
+        String restrictValue = "test";
+        mSession.runCallback(restrictValue);
+        verify(mSession, times(1)).nativeStartVariationsSession(restrictValue);
+    }
+
+    @Test
+    public void testGetRestrictModeValue() {
+        mSession.getRestrictModeValue(ContextUtils.getApplicationContext(), new Callback<String>() {
+            @Override
+            public void onResult(String restrictMode) {}
+        });
+        String restrictValue = "test";
+        mSession.runCallback(restrictValue);
+        verify(mSession, never()).nativeStartVariationsSession(any(String.class));
+
+        mSession.start(ContextUtils.getApplicationContext());
+        verify(mSession, times(1)).nativeStartVariationsSession(restrictValue);
+    }
+}

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

@@ -77,15 +77,21 @@ public class VariationsSeedFetcher {
     }
 
     @VisibleForTesting
-    protected HttpURLConnection getServerConnection() throws MalformedURLException, IOException {
-        URL url = new URL(VARIATIONS_SERVER_URL);
+    protected HttpURLConnection getServerConnection(String restrictMode)
+            throws MalformedURLException, IOException {
+        String urlString = VARIATIONS_SERVER_URL;
+        if (restrictMode != null && !restrictMode.isEmpty()) {
+            urlString += "&restrict=" + restrictMode;
+        }
+        URL url = new URL(urlString);
         return (HttpURLConnection) url.openConnection();
     }
 
     /**
      * Fetch the first run variations seed.
+     * @param restrictMode The restrict mode parameter to pass to the server via a URL param.
      */
-    public void fetchSeed() {
+    public void fetchSeed(String restrictMode) {
         assert !ThreadUtils.runningOnUiThread();
         // Prevent multiple simultaneous fetches
         synchronized (sLock) {
@@ -100,7 +106,7 @@ public class VariationsSeedFetcher {
                     || VariationsSeedBridge.hasNativePref(context)) {
                 return;
             }
-            downloadContent(context);
+            downloadContent(context, restrictMode);
             prefs.edit().putBoolean(VARIATIONS_INITIALIZED_PREF, true).apply();
         }
     }
@@ -124,11 +130,11 @@ public class VariationsSeedFetcher {
         histogram.record(timeDeltaMillis);
     }
 
-    private void downloadContent(Context context) {
+    private void downloadContent(Context context, String restrictMode) {
         HttpURLConnection connection = null;
         try {
             long startTimeMillis = SystemClock.elapsedRealtime();
-            connection = getServerConnection();
+            connection = getServerConnection(restrictMode);
             connection.setReadTimeout(READ_TIMEOUT);
             connection.setConnectTimeout(REQUEST_TIMEOUT);
             connection.setDoInput(true);

components/variations/android/junit/src/org/chromium/components/variations/firstrun/VariationsSeedFetcherTest.java

@@ -53,7 +53,7 @@ public class VariationsSeedFetcherTest {
         ThreadUtils.setUiThread(mock(Looper.class));
         mFetcher = spy(VariationsSeedFetcher.get());
         mConnection = mock(HttpURLConnection.class);
-        doReturn(mConnection).when(mFetcher).getServerConnection();
+        doReturn(mConnection).when(mFetcher).getServerConnection("");
         mPrefs = ContextUtils.getAppSharedPreferences();
         mPrefs.edit().clear().apply();
     }
@@ -80,7 +80,7 @@ public class VariationsSeedFetcherTest {
         when(mConnection.getHeaderField("IM")).thenReturn("gzip");
         when(mConnection.getInputStream()).thenReturn(new ByteArrayInputStream("1234".getBytes()));
 
-        mFetcher.fetchSeed();
+        mFetcher.fetchSeed("");
 
         assertThat(mPrefs.getString(VariationsSeedBridge.VARIATIONS_FIRST_RUN_SEED_SIGNATURE, ""),
                 equalTo("signature"));
@@ -101,7 +101,7 @@ public class VariationsSeedFetcherTest {
     public void testFetchSeed_noFetchNeeded() throws IOException {
         mPrefs.edit().putBoolean(VariationsSeedFetcher.VARIATIONS_INITIALIZED_PREF, true).apply();
 
-        mFetcher.fetchSeed();
+        mFetcher.fetchSeed("");
 
         verify(mConnection, never()).connect();
     }
@@ -113,7 +113,7 @@ public class VariationsSeedFetcherTest {
     public void testFetchSeed_badResponse() throws IOException {
         when(mConnection.getResponseCode()).thenReturn(HttpURLConnection.HTTP_NOT_FOUND);
 
-        mFetcher.fetchSeed();
+        mFetcher.fetchSeed("");
 
         assertTrue(mPrefs.getBoolean(VariationsSeedFetcher.VARIATIONS_INITIALIZED_PREF, false));
         assertFalse(VariationsSeedBridge.hasJavaPref(ContextUtils.getApplicationContext()));
@@ -126,7 +126,7 @@ public class VariationsSeedFetcherTest {
     public void testFetchSeed_IOException() throws IOException {
         doThrow(new IOException()).when(mConnection).connect();
 
-        mFetcher.fetchSeed();
+        mFetcher.fetchSeed("");
 
         assertTrue(mPrefs.getBoolean(VariationsSeedFetcher.VARIATIONS_INITIALIZED_PREF, false));
         assertFalse(VariationsSeedBridge.hasJavaPref(ContextUtils.getApplicationContext()));