Ensure low-memory renderers retry failed loads correctly.

LibraryLoader uses isUsingBrowserSharedRelros() to decide whether to retry
a fixed address load for the browser. On a low-memory target, this function
will currently return true for both browser and renderer processes. This
means that any renderer fixed load retry for low-memory will also occur in
LibraryLoader, rather than in ChildProcessService.

This works, but a side effect is that UMA statistics for failed renderer
fixed address load are not updated correctly by ChildProcessService (such
an event will however not register as a failed browser load either,
because the function to push UMA values on browser load is not called for
a child process).

Fixed by making isUsingBrowserSharedRelros() return false if not in the
browser process, regardless of any low-memory/normal-memory setting.

Additionally, make ...ForTesting functions final, and clean up a few
portions of testing code for tighter operation.

BUG=460499

Review URL: https://codereview.chromium.org/1349673003

Cr-Commit-Position: refs/heads/master@{#349420}

2 files changed, 22 insertions, 22 deletions

diff --git a/base/android/java/src/org/chromium/base/library_loader/LegacyLinker.java b/base/android/java/src/org/chromium/base/library_loader/LegacyLinker.java
index 86de11b..e40ef22 100644
--- a/base/android/java/src/org/chromium/base/library_loader/LegacyLinker.java
+++ b/base/android/java/src/org/chromium/base/library_loader/LegacyLinker.java
@@ -126,7 +126,7 @@ class LegacyLinker extends Linker {
     public boolean isUsingBrowserSharedRelros() {
         synchronized (mLock) {
             ensureInitializedLocked();
-            return mBrowserUsesSharedRelro;
+            return mInBrowserProcess && mBrowserUsesSharedRelro;
         }
     }
 
diff --git a/base/android/java/src/org/chromium/base/library_loader/Linker.java b/base/android/java/src/org/chromium/base/library_loader/Linker.java
index 11f9876..989158a 100644
--- a/base/android/java/src/org/chromium/base/library_loader/Linker.java
+++ b/base/android/java/src/org/chromium/base/library_loader/Linker.java
@@ -305,8 +305,6 @@ public abstract class Linker {
                 sSingleton = ModernLinker.create();
             } else if (type == LINKER_IMPLEMENTATION_LEGACY) {
                 sSingleton = LegacyLinker.create();
-            } else {
-                return;
             }
             Log.i(TAG, "Forced linker: " + sSingleton.getClass().getName());
         }
@@ -318,19 +316,21 @@ public abstract class Linker {
      *
      * @return LINKER_IMPLEMENTATION_LEGACY or LINKER_IMPLEMENTATION_MODERN
      */
-    public int getImplementationForTesting() {
+    public final int getImplementationForTesting() {
         // Sanity check. This method may only be called during tests.
         assertLinkerTestsAreEnabled();
 
         synchronized (sSingletonLock) {
-            assertForTesting(sSingleton != null);
+            assertForTesting(sSingleton == this);
 
             if (sSingleton instanceof ModernLinker) {
                 return LINKER_IMPLEMENTATION_MODERN;
             } else if (sSingleton instanceof LegacyLinker) {
                 return LINKER_IMPLEMENTATION_LEGACY;
+            } else {
+                Log.wtf(TAG, "Invalid linker: " + sSingleton.getClass().getName());
+                assertForTesting(false);
             }
-            Log.e(TAG, "Invalid linker: " + sSingleton.getClass().getName());
             return 0;
         }
     }
@@ -359,7 +359,7 @@ public abstract class Linker {
      * @param testRunnerClassName null or a String for the class name of the
      * TestRunner to use.
      */
-    public void setTestRunnerClassNameForTesting(String testRunnerClassName) {
+    public final void setTestRunnerClassNameForTesting(String testRunnerClassName) {
         if (DEBUG) {
             Log.i(TAG, "setTestRunnerClassNameForTesting(" + testRunnerClassName + ") called");
         }
@@ -380,7 +380,7 @@ public abstract class Linker {
      * @return null or a String holding the name of the class implementing
      * the TestRunner set by calling setTestRunnerClassNameForTesting() previously.
      */
-    public String getTestRunnerClassNameForTesting() {
+    public final String getTestRunnerClassNameForTesting() {
         // Sanity check. This method may only be called during tests.
         assertLinkerTestsAreEnabled();
 
@@ -399,7 +399,7 @@ public abstract class Linker {
      * runner class name. On subsequent calls, checks that the singleton produced by
      * the first call matches the requested type and test runner class name.
      */
-    public static void setupForTesting(int type, String testRunnerClassName) {
+    public static final void setupForTesting(int type, String testRunnerClassName) {
         if (DEBUG) {
             Log.i(TAG, "setupForTesting(" + type + ", " + testRunnerClassName + ") called");
         }
@@ -433,8 +433,8 @@ public abstract class Linker {
      * @param memoryDeviceConfig LegacyLinker memory config, or 0 if unused
      * @param inBrowserProcess true if in the browser process
      */
-    protected void runTestRunnerClassForTesting(int memoryDeviceConfig,
-                                                boolean inBrowserProcess) {
+    protected final void runTestRunnerClassForTesting(int memoryDeviceConfig,
+                                                      boolean inBrowserProcess) {
         if (DEBUG) {
             Log.i(TAG, "runTestRunnerClassForTesting called");
         }
@@ -443,7 +443,8 @@ public abstract class Linker {
 
         synchronized (mLock) {
             if (mTestRunnerClassName == null) {
-                return;
+                Log.wtf(TAG, "Linker runtime tests not set up for this process");
+                assertForTesting(false);
             }
             if (DEBUG) {
                 Log.i(TAG, "Instantiating " + mTestRunnerClassName);
@@ -452,17 +453,16 @@ public abstract class Linker {
             try {
                 testRunner = (TestRunner) Class.forName(mTestRunnerClassName).newInstance();
             } catch (Exception e) {
-                Log.e(TAG, "Could not instantiate test runner class by name", e);
-                testRunner = null;
+                Log.wtf(TAG, "Could not instantiate test runner class by name", e);
+                assertForTesting(false);
             }
-            if (testRunner != null) {
-                if (!testRunner.runChecks(memoryDeviceConfig, inBrowserProcess)) {
-                    Log.wtf(TAG, "Linker runtime tests failed in this process!!");
-                    assertForTesting(false);
-                } else {
-                    Log.i(TAG, "All linker tests passed!");
-                }
+
+            if (!testRunner.runChecks(memoryDeviceConfig, inBrowserProcess)) {
+                Log.wtf(TAG, "Linker runtime tests failed in this process");
+                assertForTesting(false);
             }
+
+            Log.i(TAG, "All linker tests passed");
         }
     }
 
@@ -472,7 +472,7 @@ public abstract class Linker {
      *
      * @param memoryDeviceConfig MEMORY_DEVICE_CONFIG_LOW or MEMORY_DEVICE_CONFIG_NORMAL.
      */
-    public void setMemoryDeviceConfigForTesting(int memoryDeviceConfig) {
+    public final void setMemoryDeviceConfigForTesting(int memoryDeviceConfig) {
         if (DEBUG) {
             Log.i(TAG, "setMemoryDeviceConfigForTesting(" + memoryDeviceConfig + ") called");
         }