Reland chromium#2: Move side-loaded test data /sdcard -> /sdcard/gtes…

…tdata

Reason for reland:
- content_browsertests and component_browsertests updated to
clear the correct private data directory between test runs.

Uses delete_device_stale=True when pushing. This will prevent tests
mistakenly passing when they depend on stale data files from a previous test.

TBR=jbudorick@chromium.org,thakis@chromium.org,mef@chromium.org,torne@chromium.org
BUG=607169,617213,616155

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

android_webview/tools/system_webview_shell/layout_tests/src/org/chromium/webview_shell/test/WebViewLayoutTest.java

@@ -4,14 +4,14 @@
 
 package org.chromium.webview_shell.test;
 
-import android.os.Environment;
 import android.test.ActivityInstrumentationTestCase2;
 import android.test.suitebuilder.annotation.MediumTest;
 
 import junit.framework.ComparisonFailure;
 
 import org.chromium.base.Log;
 import org.chromium.base.test.util.DisabledTest;
+import org.chromium.base.test.util.UrlUtils;
 import org.chromium.webview_shell.WebViewLayoutTestActivity;
 
 import java.io.BufferedReader;
@@ -34,8 +34,7 @@ public class WebViewLayoutTest
 
     private static final String TAG = "WebViewLayoutTest";
 
-    private static final String EXTERNAL_PREFIX =
-            Environment.getExternalStorageDirectory().getAbsolutePath() + "/";
+    private static final String EXTERNAL_PREFIX = UrlUtils.getIsolatedTestRoot() + "/";
     private static final String BASE_WEBVIEW_TEST_PATH =
             "android_webview/tools/system_webview_shell/test/data/";
     private static final String BASE_BLINK_TEST_PATH = "third_party/WebKit/LayoutTests/";

android_webview/tools/system_webview_shell/test/data/run_tests.sh

@@ -5,8 +5,8 @@
 # found in the LICENSE file.
 
 PACKAGE_NAME="org.chromium.webview_shell.test"
-DEVICE_WEBVIEW_TEST_PATH=\
-    "/sdcard/android_webview/tools/system_webview_shell/test/data/"
+DEVICE_WEBVIEW_TEST_PATH="/sdcard/chromium_tests_root/android_webview/tools/"
+DEVICE_WEBVIEW_TEST_PATH+="system_webview_shell/test/data/"
 TESTRUNNER="../../../../build/android/test_runner.py"
 
 $TESTRUNNER instrumentation \

base/base_paths_android.cc

@@ -38,8 +38,10 @@ bool PathProviderAndroid(int key, FilePath* result) {
     case base::DIR_MODULE:
       return base::android::GetNativeLibraryDirectory(result);
     case base::DIR_SOURCE_ROOT:
-      // This const is only used for tests.
-      return base::android::GetExternalStorageDirectory(result);
+      // Used only by tests.
+      // In that context, hooked up via base/test/test_support_android.cc.
+      NOTIMPLEMENTED();
+      return false;
     case base::DIR_USER_DESKTOP:
       // Android doesn't support GetUserDesktop.
       NOTIMPLEMENTED();

base/test/android/javatests/src/org/chromium/base/test/util/UrlUtils.java

@@ -20,7 +20,7 @@ public class UrlUtils {
      */
     public static String getTestFilePath(String path) {
         // TODO(jbudorick): Remove DATA_DIR once everything has been isolated. crbug/400499
-        return PathUtils.getExternalStorageDirectory() + DATA_DIR + path;
+        return getIsolatedTestFilePath(DATA_DIR + path);
     }
 
     // TODO(jbudorick): Remove this function once everything has been isolated and switched back
@@ -30,7 +30,14 @@ public static String getTestFilePath(String path) {
      * @param path Pathname relative to external/
      */
     public static String getIsolatedTestFilePath(String path) {
-        return PathUtils.getExternalStorageDirectory() + "/" + path;
+        return getIsolatedTestRoot() + "/" + path;
+    }
+
+    /**
+     * Returns the root of the test data directory.
+     */
+    public static String getIsolatedTestRoot() {
+        return PathUtils.getExternalStorageDirectory() + "/chromium_tests_root";
     }
 
     /**

base/test/test_support_android.cc

@@ -18,6 +18,8 @@
 
 namespace {
 
+base::FilePath* g_test_data_dir = nullptr;
+
 struct RunState {
   RunState(base::MessagePump::Delegate* delegate, int run_depth)
       : delegate(delegate),
@@ -132,13 +134,16 @@ std::unique_ptr<base::MessagePump> CreateMessagePumpForUIStub() {
   return std::unique_ptr<base::MessagePump>(new MessagePumpForUIStub());
 };
 
-// Provides the test path for DIR_MODULE and DIR_ANDROID_APP_DATA.
+// Provides the test path for DIR_SOURCE_ROOT and DIR_ANDROID_APP_DATA.
 bool GetTestProviderPath(int key, base::FilePath* result) {
   switch (key) {
-    case base::DIR_ANDROID_APP_DATA: {
-      // For tests, app data is put in external storage.
-      return base::android::GetExternalStorageDirectory(result);
-    }
+    // TODO(agrieve): Stop overriding DIR_ANDROID_APP_DATA.
+    // https://crbug.com/617734
+    case base::DIR_ANDROID_APP_DATA:
+    case base::DIR_SOURCE_ROOT:
+      CHECK(g_test_data_dir != nullptr);
+      *result = *g_test_data_dir;
+      return true;
     default:
       return false;
   }
@@ -166,8 +171,13 @@ void InitAndroidTestLogging() {
                        false);   // Tick count
 }
 
-void InitAndroidTestPaths() {
-  InitPathProvider(DIR_MODULE);
+void InitAndroidTestPaths(const FilePath& test_data_dir) {
+  if (g_test_data_dir) {
+    CHECK(test_data_dir == *g_test_data_dir);
+    return;
+  }
+  g_test_data_dir = new FilePath(test_data_dir);
+  InitPathProvider(DIR_SOURCE_ROOT);
   InitPathProvider(DIR_ANDROID_APP_DATA);
 }
 
@@ -179,7 +189,6 @@ void InitAndroidTestMessageLoop() {
 void InitAndroidTest() {
   if (!base::AndroidIsChildProcess()) {
     InitAndroidTestLogging();
-    InitAndroidTestPaths();
   }
   InitAndroidTestMessageLoop();
 }

base/test/test_support_android.h

@@ -9,11 +9,13 @@
 
 namespace base {
 
+class FilePath;
+
 // Init logging for tests on Android. Logs will be output into Android's logcat.
 BASE_EXPORT void InitAndroidTestLogging();
 
 // Init path providers for tests on Android.
-BASE_EXPORT void InitAndroidTestPaths();
+BASE_EXPORT void InitAndroidTestPaths(const FilePath& test_data_dir);
 
 // Init the message loop for tests on Android.
 BASE_EXPORT void InitAndroidTestMessageLoop();

build/android/pylib/local/device/local_device_gtest_run.py

@@ -241,12 +241,16 @@ def install_apk():
 
       def push_test_data():
         # Push data dependencies.
-        external_storage = dev.GetExternalStoragePath()
+        device_root = posixpath.join(dev.GetExternalStoragePath(),
+                                     'chromium_tests_root')
         data_deps = self._test_instance.GetDataDependencies()
         host_device_tuples = [
-            (h, d if d is not None else external_storage)
+            (h, d if d is not None else device_root)
             for h, d in data_deps]
-        dev.PushChangedFiles(host_device_tuples)
+        dev.PushChangedFiles(host_device_tuples, delete_device_stale=True)
+        if not host_device_tuples:
+          dev.RunShellCommand(['rm', '-rf', device_root], check_return=True)
+          dev.RunShellCommand(['mkdir', '-p', device_root], check_return=True)
 
       def init_tool_and_start_servers():
         tool = self.GetTool(dev)

build/android/pylib/local/device/local_device_instrumentation_test_run.py

@@ -4,6 +4,7 @@
 
 import logging
 import os
+import posixpath
 import re
 import time
 
@@ -58,11 +59,11 @@ def TestPackage(self):
     return self._test_instance.suite
 
   def SetUp(self):
-    def substitute_external_storage(d, external_storage):
+    def substitute_device_root(d, device_root):
       if not d:
-        return external_storage
+        return device_root
       elif isinstance(d, list):
-        return '/'.join(p if p else external_storage for p in d)
+        return posixpath.join(p if p else device_root for p in d)
       else:
         return d
 
@@ -106,14 +107,19 @@ def install_apk():
                                 check_return=True)
 
       def push_test_data():
-        external_storage = dev.GetExternalStoragePath()
+        device_root = posixpath.join(dev.GetExternalStoragePath(),
+                                     'chromium_tests_root')
         host_device_tuples_substituted = [
-            (h, substitute_external_storage(d, external_storage))
+            (h, substitute_device_root(d, device_root))
             for h, d in host_device_tuples]
         logging.info('instrumentation data deps:')
         for h, d in host_device_tuples_substituted:
           logging.info('%r -> %r', h, d)
-        dev.PushChangedFiles(host_device_tuples_substituted)
+        dev.PushChangedFiles(host_device_tuples_substituted,
+                             delete_device_stale=True)
+        if not host_device_tuples_substituted:
+          dev.RunShellCommand(['rm', '-rf', device_root], check_return=True)
+          dev.RunShellCommand(['mkdir', '-p', device_root], check_return=True)
 
       def create_flag_changer():
         if self._test_instance.flags:

build/android/pylib/remote/device/remote_device_test_run.py

@@ -230,6 +230,8 @@ def _AmInstrumentTestSetup(self, app_path, test_path, runner_package,
 
     self._app_id = self._UploadAppToDevice(app_path)
 
+    # TODO(agrieve): If AMP is ever ressurected, this needs to be changed to put
+    #     test files under /sdcard/gtestdata.  http://crbug.com/607169
     data_deps = self._test_instance.GetDataDependencies()
     if data_deps:
       with tempfile.NamedTemporaryFile(suffix='.zip') as test_with_deps:

build/apk_test.gypi

@@ -24,6 +24,7 @@
     '<(DEPTH)/build/android/pylib/remote/device/dummy/dummy.gyp:require_remote_device_dummy_apk',
     '<(DEPTH)/testing/android/appurify_support.gyp:appurify_support_java',
     '<(DEPTH)/testing/android/on_device_instrumentation.gyp:reporter_java',
+    '<(DEPTH)/testing/android/native_test.gyp:native_test_java',
     '<(DEPTH)/tools/android/android_tools.gyp:android_tools',
   ],
   'conditions': [
@@ -35,7 +36,7 @@
          'intermediate_dir': '<(PRODUCT_DIR)/<(test_suite_name)_apk',
          'generated_src_dirs': [ '<(SHARED_INTERMEDIATE_DIR)/<(test_suite_name)_jinja', ],
          'final_apk_path': '<(intermediate_dir)/<(test_suite_name)-debug.apk',
-         'java_in_dir': '<(DEPTH)/testing/android/native_test/java',
+         'java_in_dir': '<(DEPTH)/build/android/empty',
          'native_lib_target': 'lib<(test_suite_name)',
          # TODO(yfriedman, cjhopman): Support managed installs for gtests.
          'gyp_managed_install': 0,

chrome/common/chrome_paths.cc

@@ -444,7 +444,7 @@ bool PathProvider(int key, base::FilePath* result) {
 #if defined(OS_ANDROID)
       // On Android, our tests don't have permission to write to DIR_MODULE.
       // gtest/test_runner.py pushes data to external storage.
-      if (!PathService::Get(base::DIR_ANDROID_EXTERNAL_STORAGE, &cur))
+      if (!PathService::Get(base::DIR_SOURCE_ROOT, &cur))
         return false;
 #else
       if (!PathService::Get(base::DIR_MODULE, &cur))

components/BUILD.gn

@@ -342,6 +342,7 @@ if (is_android) {
     deps = [
       ":components_browsertests_resources",
       "//base:base_java",
+      "//base:base_java_test_support",
       "//content/public/test/android:content_java_test_support",
       "//content/shell/android:content_shell_browsertests_java",
       "//testing/android/native_test:native_test_java",

components/cronet/android/BUILD.gn

@@ -429,6 +429,7 @@ shared_library("cronet_tests") {
     ":cronet_version_header",
     "//base",
     "//base:i18n",
+    "//base/test:test_support",
     "//net",
     "//net:simple_quic_tools",
     "//net:test_support",
@@ -479,6 +480,7 @@ android_library("cronet_test_apk_java") {
     ":cronet_api",
     ":cronet_java",
     "//base:base_java",
+    "//base:base_java_test_support",
     "//net/android:net_java_test_support",
     "//third_party/netty-tcnative:netty-tcnative",
     "//third_party/netty4:netty_all",

components/cronet/android/test/mock_cert_verifier.cc

@@ -9,6 +9,8 @@
 
 #include "base/android/jni_android.h"
 #include "base/android/jni_array.h"
+#include "base/android/jni_string.h"
+#include "base/test/test_support_android.h"
 #include "crypto/sha2.h"
 #include "jni/MockCertVerifier_jni.h"
 #include "net/base/net_errors.h"
@@ -49,9 +51,15 @@ static bool CalculatePublicKeySha256(const net::X509Certificate& cert,
 
 }  // namespace
 
-static jlong CreateMockCertVerifier(JNIEnv* env,
-                                    const JavaParamRef<jclass>& jcaller,
-                                    const JavaParamRef<jobjectArray>& jcerts) {
+static jlong CreateMockCertVerifier(
+    JNIEnv* env,
+    const JavaParamRef<jclass>& jcaller,
+    const JavaParamRef<jobjectArray>& jcerts,
+    const JavaParamRef<jstring>& jtest_data_dir) {
+  base::FilePath test_data_dir(
+      base::android::ConvertJavaStringToUTF8(env, jtest_data_dir));
+  base::InitAndroidTestPaths(test_data_dir);
+
   std::vector<std::string> certs;
   base::android::AppendJavaStringArrayToStringVector(env, jcerts, &certs);
   net::MockCertVerifier* mock_cert_verifier = new net::MockCertVerifier();

components/cronet/android/test/native_test_server.cc

@@ -18,6 +18,7 @@
 #include "base/path_service.h"
 #include "base/strings/string_util.h"
 #include "base/strings/stringprintf.h"
+#include "base/test/test_support_android.h"
 #include "components/cronet/android/test/cronet_test_util.h"
 #include "jni/NativeTestServer_jni.h"
 #include "net/base/host_port_pair.h"
@@ -164,10 +165,16 @@ std::unique_ptr<net::test_server::HttpResponse> SdchRequestHandler(
 
 jboolean StartNativeTestServer(JNIEnv* env,
                                const JavaParamRef<jclass>& jcaller,
-                               const JavaParamRef<jstring>& jtest_files_root) {
+                               const JavaParamRef<jstring>& jtest_files_root,
+                               const JavaParamRef<jstring>& jtest_data_dir) {
   // Shouldn't happen.
   if (g_test_server)
     return false;
+
+  base::FilePath test_data_dir(
+      base::android::ConvertJavaStringToUTF8(env, jtest_data_dir));
+  base::InitAndroidTestPaths(test_data_dir);
+
   g_test_server = new net::EmbeddedTestServer();
   g_test_server->RegisterRequestHandler(
       base::Bind(&NativeTestServerRequestHandler));

components/cronet/android/test/quic_test_server.cc

@@ -10,6 +10,7 @@
 #include "base/bind.h"
 #include "base/files/file_path.h"
 #include "base/files/file_util.h"
+#include "base/test/test_support_android.h"
 #include "base/threading/thread.h"
 #include "components/cronet/android/test/cronet_test_util.h"
 #include "jni/QuicTestServer_jni.h"
@@ -29,7 +30,8 @@ static const int kServerPort = 6121;
 base::Thread* g_quic_server_thread = nullptr;
 net::QuicSimpleServer* g_quic_server = nullptr;
 
-void StartOnServerThread(const base::FilePath& test_files_root) {
+void StartOnServerThread(const base::FilePath& test_files_root,
+                         const base::FilePath& test_data_dir) {
   DCHECK(g_quic_server_thread->task_runner()->BelongsToCurrentThread());
   DCHECK(!g_quic_server);
 
@@ -41,9 +43,7 @@ void StartOnServerThread(const base::FilePath& test_files_root) {
   net::QuicConfig config;
 
   // Set up server certs.
-  base::FilePath directory;
-  CHECK(base::android::GetExternalStorageDirectory(&directory));
-  directory = directory.Append("net/data/ssl/certificates");
+  base::FilePath directory = test_data_dir.Append("net/data/ssl/certificates");
   // TODO(xunjieli): Use scoped_ptr when crbug.com/545474 is fixed.
   net::ProofSourceChromium* proof_source = new net::ProofSourceChromium();
   CHECK(proof_source->Initialize(
@@ -73,8 +73,13 @@ void ShutdownOnServerThread() {
 // the device.
 void StartQuicTestServer(JNIEnv* env,
                          const JavaParamRef<jclass>& /*jcaller*/,
-                         const JavaParamRef<jstring>& jtest_files_root) {
+                         const JavaParamRef<jstring>& jtest_files_root,
+                         const JavaParamRef<jstring>& jtest_data_dir) {
   DCHECK(!g_quic_server_thread);
+  base::FilePath test_data_dir(
+      base::android::ConvertJavaStringToUTF8(env, jtest_data_dir));
+  base::InitAndroidTestPaths(test_data_dir);
+
   g_quic_server_thread = new base::Thread("quic server thread");
   base::Thread::Options thread_options;
   thread_options.message_loop_type = base::MessageLoop::TYPE_IO;
@@ -83,7 +88,8 @@ void StartQuicTestServer(JNIEnv* env,
   base::FilePath test_files_root(
       base::android::ConvertJavaStringToUTF8(env, jtest_files_root));
   g_quic_server_thread->task_runner()->PostTask(
-      FROM_HERE, base::Bind(&StartOnServerThread, test_files_root));
+      FROM_HERE,
+      base::Bind(&StartOnServerThread, test_files_root, test_data_dir));
 }
 
 void ShutdownQuicTestServer(JNIEnv* env,

components/cronet/android/test/src/org/chromium/net/MockCertVerifier.java

@@ -5,6 +5,7 @@
 package org.chromium.net;
 
 import org.chromium.base.annotations.JNINamespace;
+import org.chromium.base.test.util.UrlUtils;
 
 /**
  * A Java wrapper to supply a net::MockCertVerifier which can be then passed
@@ -22,8 +23,8 @@ private MockCertVerifier() {}
      * @return a pointer to the newly created net::MockCertVerifier.
      */
     public static long createMockCertVerifier(String[] certs) {
-        return nativeCreateMockCertVerifier(certs);
+        return nativeCreateMockCertVerifier(certs, UrlUtils.getIsolatedTestRoot());
     }
 
-    private static native long nativeCreateMockCertVerifier(String[] certs);
+    private static native long nativeCreateMockCertVerifier(String[] certs, String testDataDir);
 }