Patch crrev/c/5450018 and crrev/c/5458753

This should eliminate all the remaining divergence that was made for the native test runner.

Test: atest cronet_unittests_tester
Bug: 334809455
Change-Id: I8b68df1ecb96a5b900ff631fa0eae61ff7b70605

10 files changed, 64 insertions, 261 deletions

diff --git a/Android.bp b/Android.bp
index 7e743b956..e6e7475bd 100644
--- a/Android.bp
+++ b/Android.bp
@@ -34501,7 +34501,6 @@ java_library {
         "testing/android/native_test/java/src/org/chromium/native_test/NativeUnitTest.java",
         "testing/android/native_test/java/src/org/chromium/native_test/NativeUnitTestActivity.java",
         "testing/android/native_test/java/src/org/chromium/native_test/NativeUnitTestNativeActivity.java",
-        "testing/android/native_test/java/src/org/chromium/native_test/SignalMaskInfo.java",
     ],
     static_libs: [
         "androidx.fragment_fragment",
diff --git a/android/tools/gn2bp/Android.bp.swp b/android/tools/gn2bp/Android.bp.swp
index 7e743b956..e6e7475bd 100644
--- a/android/tools/gn2bp/Android.bp.swp
+++ b/android/tools/gn2bp/Android.bp.swp
@@ -34501,7 +34501,6 @@ java_library {
         "testing/android/native_test/java/src/org/chromium/native_test/NativeUnitTest.java",
         "testing/android/native_test/java/src/org/chromium/native_test/NativeUnitTestActivity.java",
         "testing/android/native_test/java/src/org/chromium/native_test/NativeUnitTestNativeActivity.java",
-        "testing/android/native_test/java/src/org/chromium/native_test/SignalMaskInfo.java",
     ],
     static_libs: [
         "androidx.fragment_fragment",
diff --git a/android/tools/gn2bp/gen_android_bp b/android/tools/gn2bp/gen_android_bp
index 4b282df6f..2372e35fb 100755
--- a/android/tools/gn2bp/gen_android_bp
+++ b/android/tools/gn2bp/gen_android_bp
@@ -192,11 +192,6 @@ additional_args = {
         # This is necessary because net-tests-utils compiles against private SDK.
         ('sdk_version', ""),
     ],
-    'cronet_aml_testing_android_native_test_native_test_java__testing': [
-        ('srcs', {
-            'testing/android/native_test/java/src/org/chromium/native_test/SignalMaskInfo.java',
-        }),
-    ],
     'cronet_aml_components_cronet_android_cronet__testing': [
         ('target',  ('android_riscv64', {'stem': "libmainlinecronet_riscv64"})),
         ('comment', """TODO: remove stem for riscv64
diff --git a/test_runner/AndroidNetTest.xml b/test_runner/AndroidNetTest.xml
index cd285773c..ea2ca782e 100644
--- a/test_runner/AndroidNetTest.xml
+++ b/test_runner/AndroidNetTest.xml
@@ -33,16 +33,15 @@
     </target_preparer>
     <!-- Push the testing data to the specified directory before running tests -->
     <target_preparer class="com.android.tradefed.targetprep.PushFilePreparer">
-        <option name="push-file" key="testdir" value="/data/local/tmp/net/third_party/quiche/src/quiche/common/platform/api/testdir/" />
-        <option name="push-file" key="nist-pkits" value="/data/local/tmp/net/third_party/nist-pkits/" />
-        <option name="push-file" key="data" value="/data/local/tmp/net/data/" />
-        <option name="push-file" key="anonymous_tokens" value="/data/local/tmp/third_party/anonymous_tokens/" />
+        <option name="push-file" key="testdir" value="/storage/emulated/0/chromium_tests_root/net/third_party/quiche/src/quiche/common/platform/api/testdir/" />
+        <option name="push-file" key="nist-pkits" value="/storage/emulated/0/chromium_tests_root/net/third_party/nist-pkits/" />
+        <option name="push-file" key="data" value="/storage/emulated/0/chromium_tests_root/net/data/" />
+        <option name="push-file" key="anonymous_tokens" value="/storage/emulated/0/chromium_tests_root/third_party/anonymous_tokens/" />
     </target_preparer>
     <!-- Runs the test -->
     <test class="com.android.tradefed.testtype.HostTest">
         <option name="jar" value="net_unittests_tester.jar"/>
         <option name="set-option" value="library-to-load:net_unittests__library"/>
-        <option name="set-option" value="dump-native-coverage:true"/>
         <option name="enable-pretty-logs" value="false" />
     </test>
 </configuration>
\ No newline at end of file
diff --git a/test_runner/AndroidTest.xml b/test_runner/AndroidTest.xml
index 8ba6d882b..84d26669a 100644
--- a/test_runner/AndroidTest.xml
+++ b/test_runner/AndroidTest.xml
@@ -33,7 +33,6 @@
     <test class="com.android.tradefed.testtype.HostTest">
         <option name="jar" value="cronet_unittests_tester.jar" />
         <option name="set-option" value="library-to-load:cronet_unittests_android__library" />
-        <option name="set-option" value="dump-native-coverage:true" />
         <option name="enable-pretty-logs" value="false" />
     </test>
 </configuration>
\ No newline at end of file
diff --git a/test_runner/src/com.android.tests.chromium.host/InstrumentationCommandBuilder.java b/test_runner/src/com.android.tests.chromium.host/InstrumentationCommandBuilder.java
index a3ba5ccef..3b3e76bc7 100644
--- a/test_runner/src/com.android.tests.chromium.host/InstrumentationCommandBuilder.java
+++ b/test_runner/src/com.android.tests.chromium.host/InstrumentationCommandBuilder.java
@@ -25,7 +25,7 @@ public class InstrumentationCommandBuilder {
     private final List<Pair<String, String>> arguments;
     private final String activityName;
     // Instrument and wait until execution has finished before returning
-    private static final String BASE_CMD = "am instrument -w ";
+    private static final String BASE_CMD = "am instrument -w --no-isolated-storage ";
 
     public InstrumentationCommandBuilder(String activity) {
         this.activityName = activity;
diff --git a/testing/android/native_test/java/src/org/chromium/native_test/NativeTest.java b/testing/android/native_test/java/src/org/chromium/native_test/NativeTest.java
index 60f4b5ff7..26e6436cf 100644
--- a/testing/android/native_test/java/src/org/chromium/native_test/NativeTest.java
+++ b/testing/android/native_test/java/src/org/chromium/native_test/NativeTest.java
@@ -10,7 +10,6 @@ import android.content.Intent;
 import android.os.Bundle;
 import android.os.Environment;
 import android.os.Handler;
-import android.os.Looper;
 import android.os.Process;
 import android.system.ErrnoException;
 import android.system.Os;
@@ -35,12 +34,6 @@ public class NativeTest {
     private TestStatusReporter mReporter;
     private boolean mRunInSubThread;
     private String mStdoutFilePath;
-    private static final String LOG_TAG = "NativeTestRunner";
-    // Signal used to trigger a dump of Clang coverage information.
-    private final int COVERAGE_SIGNAL = 37;
-    private boolean mDumpCoverage = false;
-    private static final String DUMP_COVERAGE =
-            "org.chromium.native_test.NativeTestInstrumentationTestRunner.DumpCoverage";
 
     private static class ReportingUncaughtExceptionHandler
             implements Thread.UncaughtExceptionHandler {
@@ -120,7 +113,6 @@ public class NativeTest {
         }
 
         mStdoutFilePath = intent.getStringExtra(NativeTestIntent.EXTRA_STDOUT_FILE);
-        mDumpCoverage = intent.hasExtra(DUMP_COVERAGE);
     }
 
     public void appendCommandLineFlags(String flags) {
@@ -169,97 +161,22 @@ public class NativeTest {
     }
 
     private void runTests(Activity activity) {
-        // Chromium pushes data to "chrome/test/data/chromium_test_root". But in AOSP, it's more
-        // common to push testing data to data/local/tmp.
-        // This has to be consistent with the AndroidTest configuration file.
-        NativeTestJni.get().runTests(mCommandLineFlags.toString(), mCommandLineFilePath,
-                mStdoutFilePath, activity.getApplicationContext(), "/data/local/tmp");
-        if (mDumpCoverage) {
-            new Handler(Looper.getMainLooper()).post(() -> {
-                maybeDumpNativeCoverage();
-                activity.finish();
-                mReporter.testRunFinished(Process.myPid());
-            });
-        } else {
-            activity.finish();
-            mReporter.testRunFinished(Process.myPid());
-        }
+        NativeTestJni.get()
+                .runTests(
+                        mCommandLineFlags.toString(),
+                        mCommandLineFilePath,
+                        mStdoutFilePath,
+                        activity.getApplicationContext(),
+                        UrlUtils.getIsolatedTestRoot());
+        activity.finish();
+        mReporter.testRunFinished(Process.myPid());
     }
 
-    /**
-     * If this test process is instrumented for native coverage, then trigger a dump
-     * of the coverage data and wait until either we detect the dumping has finished or 60 seconds,
-     * whichever is shorter.
-     *
-     * Background: Coverage builds install a signal handler for signal 37 which flushes coverage
-     * data to disk, which may take a few seconds.  Tests running as an app process will get
-     * killed with SIGKILL once the app code exits, even if the coverage handler is still running.
-     *
-     * Method: If a handler is installed for signal 37, then assume this is a coverage run and
-     * send signal 37.  The handler is non-reentrant and so signal 37 will then be blocked until
-     * the handler completes. So after we send the signal, we loop checking the blocked status
-     * for signal 37 until we hit the 60 second deadline.  If the signal is blocked then sleep for
-     * 2 seconds, and if it becomes unblocked then the handler exitted so we can return early.
-     * If the signal is not blocked at the start of the loop then most likely the handler has
-     * not yet been invoked.  This should almost never happen as it should get blocked on delivery
-     * when we call {@code Os.kill()}, so sleep for a shorter duration (100ms) and try again.  There
-     * is a race condition here where the handler is delayed but then runs for less than 100ms and
-     * gets missed, in which case this method will loop with 100ms sleeps until the deadline.
-     *
-     * In the case where the handler runs for more than 60 seconds, the test process will be allowed
-     * to exit so coverage information may be incomplete.
-     *
-     * There is no API for determining signal dispositions, so this method uses the
-     * {@link SignalMaskInfo} class to read the data from /proc.  If there is an error parsing
-     * the /proc data then this method will also loop until the 60s deadline passes.
-     */
-    private void maybeDumpNativeCoverage() {
-        SignalMaskInfo siginfo = new SignalMaskInfo();
-        if (!siginfo.isValid()) {
-            Log.e(LOG_TAG, "Invalid signal info");
-            return;
-        }
-
-        if (!siginfo.isCaught(COVERAGE_SIGNAL)) {
-            // Process is not instrumented for coverage
-            Log.i(LOG_TAG, "Not dumping coverage, no handler installed");
-            return;
-        }
-
-        Log.i(LOG_TAG,
-                String.format("Sending coverage dump signal %d to pid %d uid %d", COVERAGE_SIGNAL,
-                        Os.getpid(), Os.getuid()));
-        try {
-            Os.kill(Os.getpid(), COVERAGE_SIGNAL);
-        } catch (ErrnoException e) {
-            Log.e(LOG_TAG, "Unable to send coverage signal", e);
-            return;
-        }
-
-        long start = System.currentTimeMillis();
-        long deadline = start + 60 * 1000L;
-        while (System.currentTimeMillis() < deadline) {
-            siginfo.refresh();
-            try {
-                if (siginfo.isValid() && siginfo.isBlocked(COVERAGE_SIGNAL)) {
-                    // Signal is currently blocked so assume a handler is running
-                    Thread.sleep(2000L);
-                    siginfo.refresh();
-                    if (siginfo.isValid() && !siginfo.isBlocked(COVERAGE_SIGNAL)) {
-                        // Coverage handler exited while we were asleep
-                        Log.i(LOG_TAG,
-                                String.format("Coverage dump detected finished after %dms",
-                                        System.currentTimeMillis() - start));
-                        break;
-                    }
-                } else {
-                    // Coverage signal handler not yet started or invalid siginfo
-                    Thread.sleep(100L);
-                }
-            } catch (InterruptedException e) {
-                // ignored
-            }
-        }
+    // Signal a failure of the native test loader to python scripts
+    // which run tests.  For example, we look for
+    // RUNNER_FAILED build/android/test_package.py.
+    private void nativeTestFailed() {
+        Log.e(TAG, "[ RUNNER_FAILED ] could not load native library");
     }
 
     @NativeMethods
diff --git a/testing/android/native_test/java/src/org/chromium/native_test/NativeUnitTest.java b/testing/android/native_test/java/src/org/chromium/native_test/NativeUnitTest.java
index 0f007902e..bb7154f23 100644
--- a/testing/android/native_test/java/src/org/chromium/native_test/NativeUnitTest.java
+++ b/testing/android/native_test/java/src/org/chromium/native_test/NativeUnitTest.java
@@ -12,13 +12,27 @@ import org.chromium.base.Log;
 import org.chromium.base.PathUtils;
 import org.chromium.base.PowerMonitor;
 import org.chromium.base.library_loader.LibraryLoader;
+import org.chromium.build.NativeLibraries;
 
 /** A helper for running native unit tests (i.e., not browser tests) */
 public class NativeUnitTest extends NativeTest {
     private static final String TAG = "NativeTest";
-
+    // This key is only used by Cronet in AOSP to run tests instead of
+    // NativeLibraries.LIBRARIES constant.
+    //
+    // The reason for that is that Cronet translates GN build rules to
+    // AOSP Soong modules, the translation layer is incapable currently
+    // of replicating the logic embedded into GN to replace the temporary
+    // NativeLibraries with the real NativeLibraries at the root of the build
+    // graph, hence we depend on the value of this key to carry the name
+    // of the native library to load.
+    //
+    // Assumption: The code below assumes that the value is exactly a single
+    // library. There is no support for loading multiple libraries through this
+    // key at the moment.
     private static final String LIBRARY_UNDER_TEST_NAME =
             "org.chromium.native_test.NativeTestInstrumentationTestRunner.LibraryUnderTest";
+
     private static class NativeUnitTestLibraryLoader extends LibraryLoader {
         static void setLibrariesLoaded() {
             LibraryLoader.setLibrariesLoadedForNativeTests();
@@ -42,24 +56,26 @@ public class NativeUnitTest extends NativeTest {
         // Needed by system_monitor_unittest.cc
         PowerMonitor.createForTests();
 
-        // For NativeActivity based tests,
-        // dependency libraries must be loaded before NativeActivity::OnCreate,
-        // otherwise loading android.app.lib_name will fail
+        // For NativeActivity based tests, dependency libraries must be loaded before
+        // NativeActivity::OnCreate, otherwise loading android.app.lib_name will fail
         String libraryToLoad = activity.getIntent().getStringExtra(LIBRARY_UNDER_TEST_NAME);
-        if (libraryToLoad != null) {
-            loadLibrary(libraryToLoad);
-        } else {
-            Log.e(TAG, "No Library provided for native tests! Exiting");
-            activity.finish();
-        }
+        loadLibraries(
+                libraryToLoad != null ? new String[] {libraryToLoad} : NativeLibraries.LIBRARIES);
     }
 
-    private void loadLibrary(String library) {
-
+    private void loadLibraries(String[] librariesToLoad) {
         LibraryLoader.setEnvForNative();
-        Log.i(TAG, "loading: %s", library);
-        System.loadLibrary(library);
-        Log.i(TAG, "loaded: %s", library);
+        for (String library : librariesToLoad) {
+            // Do not load this library early so that
+            // |LibunwindstackUnwinderAndroidTest.ReparsesMapsOnNewDynamicLibraryLoad| test can
+            // observe the change in /proc/self/maps before and after loading the library.
+            if (library.equals("base_profiler_reparsing_test_support_library")) {
+                continue;
+            }
+            Log.i(TAG, "loading: %s", library);
+            System.loadLibrary(library);
+            Log.i(TAG, "loaded: %s", library);
+        }
         NativeUnitTestLibraryLoader.setLibrariesLoaded();
     }
-}
+}
\ No newline at end of file
diff --git a/testing/android/native_test/java/src/org/chromium/native_test/SignalMaskInfo.java b/testing/android/native_test/java/src/org/chromium/native_test/SignalMaskInfo.java
deleted file mode 100644
index 123b45644..000000000
--- a/testing/android/native_test/java/src/org/chromium/native_test/SignalMaskInfo.java
+++ /dev/null
@@ -1,134 +0,0 @@
-/*
- * Copyright (C) 2023 The Android Open Source Project
- *
- * Licensed under the Apache License, Version 2.0 (the "License");
- * you may not use this file except in compliance with the License.
- * You may obtain a copy of the License at
- *
- *      http://www.apache.org/licenses/LICENSE-2.0
- *
- * Unless required by applicable law or agreed to in writing, software
- * distributed under the License is distributed on an "AS IS" BASIS,
- * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
- * See the License for the specific language governing permissions and
- * limitations under the License.
- */
-
-package org.chromium.native_test;
-
-import java.io.BufferedReader;
-import java.io.FileReader;
-import java.io.IOException;
-import java.util.HashMap;
-import java.util.Map;
-
-/**
- * Copied from <a
- * href="https://cs.android.com/android/platform/superproject/+/master:external/conscrypt/test
- * -support/src/java/org/conscrypt/SignalMaskInfo.java;
- * l=35?q=SignalMaskInfo&sq=&ss=android%2Fplatform%2Fsuperproject">here</a>
- * Class for reading a process' signal masks from the /proc filesystem.  Looks for the
- * BLOCKED, CAUGHT, IGNORED and PENDING masks from /proc/self/status, each of which is a
- * 64 bit bitmask with one bit per signal.
- *
- * Maintains a map from SignalMaskInfo.Type to the bitmask.  The {@code isValid} method
- * will only return true if all 4 masks were successfully parsed. Provides lookup
- * methods per signal, e.g. {@code isPending(signum)} which will throw
- * {@code IllegalStateException} if the current data is not valid.
- */
-public class SignalMaskInfo {
-    private enum Type {
-        BLOCKED("SigBlk"),
-        CAUGHT("SigCgt"),
-        IGNORED("SigIgn"),
-        PENDING("SigPnd");
-
-        // The tag for this mask in /proc/self/status
-        private final String tag;
-
-        Type(String tag) {
-            this.tag = tag + ":\t";
-        }
-
-        public String getTag() {
-            return tag;
-        }
-
-        public static Map<Type, Long> parseProcinfo(String path) {
-            Map<Type, Long> map = new HashMap<>();
-            try (BufferedReader reader = new BufferedReader(new FileReader(path))) {
-                String line;
-                while ((line = reader.readLine()) != null) {
-                    for (Type mask : values()) {
-                        long value = mask.tryToParse(line);
-                        if (value >= 0) {
-                            map.put(mask, value);
-                        }
-                    }
-                }
-            } catch (NumberFormatException | IOException e) {
-                // Ignored - the map will end up being invalid instead.
-            }
-            return map;
-        }
-
-        private long tryToParse(String line) {
-            if (line.startsWith(tag)) {
-                return Long.valueOf(line.substring(tag.length()), 16);
-            } else {
-                return -1;
-            }
-        }
-    }
-
-    private static final String PROCFS_PATH = "/proc/self/status";
-    private Map<Type, Long> maskMap = null;
-
-    public SignalMaskInfo() {
-        refresh();
-    }
-
-    public void refresh() {
-        maskMap = Type.parseProcinfo(PROCFS_PATH);
-    }
-
-    public boolean isValid() {
-        return (maskMap != null && maskMap.size() == Type.values().length);
-    }
-
-    public boolean isCaught(int signal) {
-        return isSignalInMask(signal, Type.CAUGHT);
-    }
-
-    public boolean isBlocked(int signal) {
-        return isSignalInMask(signal, Type.BLOCKED);
-    }
-
-    public boolean isPending(int signal) {
-        return isSignalInMask(signal, Type.PENDING);
-    }
-
-    public boolean isIgnored(int signal) {
-        return isSignalInMask(signal, Type.IGNORED);
-    }
-
-    private void checkValid() {
-        if (!isValid()) {
-            throw new IllegalStateException();
-        }
-    }
-
-    private boolean isSignalInMask(int signal, Type mask) {
-        long bit = 1L << (signal - 1);
-        return (getSignalMask(mask) & bit) != 0;
-    }
-
-    private long getSignalMask(Type mask) {
-        checkValid();
-        Long value = maskMap.get(mask);
-        if (value == null) {
-            throw new IllegalStateException();
-        }
-        return value;
-    }
-}
diff --git a/testing/android/native_test/native_test_launcher.cc b/testing/android/native_test/native_test_launcher.cc
index 7c8a0fe77..0c0a3a08f 100644
--- a/testing/android/native_test/native_test_launcher.cc
+++ b/testing/android/native_test/native_test_launcher.cc
@@ -38,6 +38,11 @@
 #include "base/test/clang_profiling.h"
 #endif
 
+#if defined(__ANDROID_CLANG_COVERAGE__)
+// This is only used by Cronet in AOSP.
+extern "C" int __llvm_profile_dump(void);
+#endif
+
 using base::android::JavaParamRef;
 
 // The main function of the program to be wrapped as a test apk.
@@ -146,6 +151,14 @@ static void JNI_NativeTest_RunTests(
 // Explicitly write profiling data to LLVM profile file.
 #if BUILDFLAG(CLANG_PROFILING)
   base::WriteClangProfilingProfile();
+#elif defined(__ANDROID_CLANG_COVERAGE__)
+  // Cronet runs tests in AOSP, where due to build system constraints, compiler
+  // flags can be changed (to enable coverage), but source files cannot be
+  // conditionally linked (as is the case with `clang_profiling.cc`).
+  //
+  //  This will always get called from a single thread unlike
+  //  base::WriteClangProfilingProfile hence the lack of locks.
+  __llvm_profile_dump();
 #endif
 }