Ensure low-memory renderers retry failed loads correctly.

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
Committed: https://crrev.com/1f11873066a51ebe84e302128957ddc5ee594065
Cr-Commit-Position: refs/heads/master@{#349420}

[simonb (inactive), 2015-09-16 17:43:02]

simonb@chromium.org changed reviewers:
+ petrcermak@chromium.org, rmcilroy@chromium.org

[petrcermak, 2015-09-16 18:44:21]

LGTM with one comment and two description nits:

1) I think the correct preposition is "ON a low-memory target" rather than "IN a
low-memory target".
2) Either provide a bug number, or remove the "BUG=" line.

Thanks,
Petr

https://codereview.chromium.org/1349673003/diff/1/base/android/java/src/org/c...
File base/android/java/src/org/chromium/base/library_loader/Linker.java (right):

https://codereview.chromium.org/1349673003/diff/1/base/android/java/src/org/c...
base/android/java/src/org/chromium/base/library_loader/Linker.java:319: public
static final int getImplementationForTesting() {
Is the new "static" modifier intentional (I'm asking because you didn't mention
it in the description)? If so, please update all call sites to call
Linker.getImplementationForTesting() rather than
linker.getImplementationForTesting().

[simonb (inactive), 2015-09-17 10:36:15]

https://codereview.chromium.org/1349673003/diff/1/base/android/java/src/org/c...
File base/android/java/src/org/chromium/base/library_loader/Linker.java (right):

https://codereview.chromium.org/1349673003/diff/1/base/android/java/src/org/c...
base/android/java/src/org/chromium/base/library_loader/Linker.java:319: public
static final int getImplementationForTesting() {
On 2015/09/16 18:44:21, petrcermak wrote:
> Is the new "static" modifier intentional (I'm asking because you didn't
mention
> it in the description)? If so, please update all call sites to call
> Linker.getImplementationForTesting() rather than
> linker.getImplementationForTesting().

Removed static, and tightened later assertion to reflect that the caller
instance is always the singleton instance.

[rmcilroy, 2015-09-17 10:41:51]

lgtm, thanks.

[simonb (inactive), 2015-09-17 13:37:42]

The CQ bit was checked by simonb@chromium.org

[simonb (inactive), 2015-09-17 13:37:43]

The patchset sent to the CQ was uploaded after l-g-t-m from
petrcermak@chromium.org
Link to the patchset: https://codereview.chromium.org/1349673003/#ps20001
(title: "Update for review comments.")

[commit-bot: I haz the power, 2015-09-17 13:38:09]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1349673003/20001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1349673003/20001

[commit-bot: I haz the power, 2015-09-17 14:58:29]

Committed patchset #2 (id:20001)

[commit-bot: I haz the power, 2015-09-17 14:59:06]

Patchset 2 (id:??) landed as
https://crrev.com/1f11873066a51ebe84e302128957ddc5ee594065
Cr-Commit-Position: refs/heads/master@{#349420}