[Android] Fix a few issues related to old lib deletion.

[Android] Fix a few issues related to old lib deletion.
    
Discovered following issues:
1. after each native library load, the code starts a thread
   to delete old libs;
2. render process also tries to delete old libs;
3. Context.getDir(...) creates the directory if not
   existing, so existence test is always successful,
   and assertion may fail;
    
The CL fixes these issues. LibraryLoader.ensureInitialized
and LibraryLoader.loadNow have a second boolean parameter,
inBrowserProcess, used by caller to indicate whether the code should try to delete old libs.

BUG=357655
Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=261382

[Yaron, 2014-04-02 18:37:59]

lgtm

This seems to actually break layering in some sense. base/ shouldn't be aware of
browser process or not. That said, Linker.java is already abusing this and it
seems this falls into the same boat.

https://codereview.chromium.org/217283005/diff/50001/base/android/java/src/or...
File base/android/java/src/org/chromium/base/library_loader/LibraryLoader.java
(right):

https://codereview.chromium.org/217283005/diff/50001/base/android/java/src/or...
base/android/java/src/org/chromium/base/library_loader/LibraryLoader.java:63:
throws ProcessInitException {
nit: should fit above

https://codereview.chromium.org/217283005/diff/50001/base/android/java/src/or...
File
base/android/java/src/org/chromium/base/library_loader/LibraryLoaderHelper.java
(right):

https://codereview.chromium.org/217283005/diff/50001/base/android/java/src/or...
base/android/java/src/org/chromium/base/library_loader/LibraryLoaderHelper.java:215:
File libDir = getWorkaroundLibDir(context);
nit: replace both with call to deleteWorkaroundLibrariesSynchronously. Avoids
copy-pasting and ensures same logic, just depends on which thread it's on

https://codereview.chromium.org/217283005/diff/50001/base/android/java/src/or...
base/android/java/src/org/chromium/base/library_loader/LibraryLoaderHelper.java:231:
private static void deleteLibFiles(File dir) {
why rename? the functionality didn't change and isn't specific to "Lib"

[sky, 2014-04-02 20:02:01]

mojo LGTM

[Feng Qian, 2014-04-02 20:38:38]

The CQ bit was checked by feng@chromium.org

[commit-bot: I haz the power, 2014-04-02 20:40:10]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/feng@chromium.org/217283005/60001

[commit-bot: I haz the power, 2014-04-02 21:05:08]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2014-04-02 21:05:08]

Retried try job too often on chromium_presubmit for step(s) presubmit
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=chromium_p...

[cjhopman, 2014-04-02 22:24:23]

This change description really doesn't seem to match the change.

https://codereview.chromium.org/217283005/diff/60001/base/android/java/src/or...
File base/android/java/src/org/chromium/base/library_loader/LibraryLoader.java
(right):

https://codereview.chromium.org/217283005/diff/60001/base/android/java/src/or...
base/android/java/src/org/chromium/base/library_loader/LibraryLoader.java:123: *
  the browser process.
It's unclear (from this documentation) what this flag does. How about renaming
it to something like shouldDeleteOldWorkaroundLibraries (or something shorter).

Then it also won't feel so much like this "knows about" the browser process.

https://codereview.chromium.org/217283005/diff/60001/base/android/java/src/or...
base/android/java/src/org/chromium/base/library_loader/LibraryLoader.java:172:
workaroundLibrariesDeleted = true;
Since we only want this to happen once, how about moving it outside of the loop
over libraries. sNativeLibraryHackWasUsed is probably a better signal for
whether or not we should delete the workaround libraries anyway.

[Feng Qian, 2014-04-03 00:47:30]

Updated description to match the CL description.

Add boliu@ as reviewer for AwBrowserProcess

[Feng Qian, 2014-04-03 00:50:41]

https://codereview.chromium.org/217283005/diff/60001/base/android/java/src/or...
File base/android/java/src/org/chromium/base/library_loader/LibraryLoader.java
(right):

https://codereview.chromium.org/217283005/diff/60001/base/android/java/src/or...
base/android/java/src/org/chromium/base/library_loader/LibraryLoader.java:123: *
  the browser process.
ok, I am fine with renaming.

On 2014/04/02 22:24:24, cjhopman wrote:
> It's unclear (from this documentation) what this flag does. How about renaming
> it to something like shouldDeleteOldWorkaroundLibraries (or something
shorter).
> 
> Then it also won't feel so much like this "knows about" the browser process.

https://codereview.chromium.org/217283005/diff/60001/base/android/java/src/or...
base/android/java/src/org/chromium/base/library_loader/LibraryLoader.java:172:
workaroundLibrariesDeleted = true;
The code is not just run when sNativeLibraryHackWasUsed is true.
But you bring up a good point that, even if the content linker is used for this
build, the previous build might used the hack, so the code should always run.

On 2014/04/02 22:24:24, cjhopman wrote:
> Since we only want this to happen once, how about moving it outside of the
loop
> over libraries. sNativeLibraryHackWasUsed is probably a better signal for
> whether or not we should delete the workaround libraries anyway.

[boliu, 2014-04-03 01:03:22]

aw lgtm

A comment in LibraryLoader but I have no power over there anyway...

https://codereview.chromium.org/217283005/diff/60001/base/android/java/src/or...
File base/android/java/src/org/chromium/base/library_loader/LibraryLoader.java
(right):

https://codereview.chromium.org/217283005/diff/60001/base/android/java/src/or...
base/android/java/src/org/chromium/base/library_loader/LibraryLoader.java:168:
&& inBrowserProcess
Webview technically only has the browser process, so "inBrowserProcess" is
misleading. Can we rename this to what it actually does, something like
"skipDeleteWorkaroundLibrary"?

Or just set it to true in webview, since context is NULL in webview anyway...

[Feng Qian, 2014-04-03 01:55:07]

The CQ bit was checked by feng@chromium.org

[commit-bot: I haz the power, 2014-04-03 02:01:19]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/feng@chromium.org/217283005/80001

[commit-bot: I haz the power, 2014-04-03 08:35:31]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2014-04-03 08:35:32]

Commit queue rejected this change because the description was changed
between the time the change entered the commit queue and the time it
was ready to commit. You can safely check the commit box again.

[Feng Qian, 2014-04-03 13:53:24]

The CQ bit was checked by feng@chromium.org

[commit-bot: I haz the power, 2014-04-03 13:53:26]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/feng@chromium.org/217283005/80001

[commit-bot: I haz the power, 2014-04-03 14:42:25]

Change committed as 261382