I'm not sure I fully understand what you're trying to do? You want to replace PROCESS_LAUNCHER thread with Android's UI thread on Java side? What are the advantages of doing this?

No, it's not replacing launcher thread with UI thread, it's replacing launcher thread with a thread that's created by java code, and has a looper (android's equivalent to message loop). Advantage of course is having that java looper, in particular, for java code to use before the native library is loaded.

UI thread on android is set up like this.

And do we ever need to launch processes from Java code before initializing the native library?

Yeah, chrome start up launches a "warm up" connection to a renderer and load the native library in parallel. Right now warm up runs in an java AsyncTask, which runs on a java background thread pool.

There's also a few other places in android's ChildProcessLauncher that needs to post to a task, and since it's not safe to assume native side is ready, they either also use AsyncTask, or just starts off a one-off thread.

I want to unify these.

Would it be possible to defer posting / queue these tasks until the native Chrome library is loaded?

Seems like in the end, the native library has to do some work. Either you queue implicitly (which sounds like what you're doing now via async tasks) or explicitly.

What work does this "warmup" connection perform? Does it just get the process started before loading its own library?

"child" process on android is a misnomer. It's actually android services, and those can definitely be started without any native code, which is what warm up does. So queuing warm up until native library is loaded effectively defeats the whole point of warm up.

Gotcha. So this is an optimization for the Android environment, right?

Yep.


fwiw, the alternative is just keep using the AsyncTask thread pool, which makes andorid's ChildProcessLauncher a bit more complex, but not by that much. But I wanted to check first if I can simplify it more, and just make everything use the launcher thread instead.

I just wrote about a specific case of complexity here: https://bugs.chromium.org/p/chromium/issues/detail?id=689758#c27

Guess there aren't any objections, so I'm gonna do this.. "soon"

Over time we would like as many tasks as possible to go through base/task_scheduler (BrowserThread::PROCESS_LAUNCHER will soon be migrated to it). Having as few tasks environments as possible that are platform specific will make monitoring and scheduling improvements easier later.

But I guess given process launching is always high priority, if you're sure java looper will always be responsive under heavy system load: it's fine given the java looper has to exist anyways for the warmup.

All in all, I'd favor code simplicity so if making this code platform specific makes it cleaner go for it (though, without specific Android knowledge, that sounds backwards to me at first).

> Over time we would like as many tasks as possible to go through base/task_scheduler

Does that include java things too? A bit off topic for this bug, but lots of java code uses the AsyncTask thread pool. I'm sure there's lots of potential for unification there too.

> But I guess given process launching is always high priority, if you're sure java looper will always be responsive under heavy system load: it's fine given the java looper has to exist anyways for the warmup.

It's just its own thread. Not better or worse the current behavior.

It doesn't include Java things for now but it ideally eventually would.

Le ven. 24 mars 2017 13:26, boliu via monorail <
monorail+v2.1779540367@chromium.org> a écrit :

Started working on this here: https://codereview.chromium.org/2774363003/

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/61687ec5b7b5dce1c9c86ae46f552c0c34473555

commit 61687ec5b7b5dce1c9c86ae46f552c0c34473555
Author: boliu <boliu@chromium.org>
Date: Wed Mar 29 20:09:34 2017

android: Java-based launcher thread

A major part of android's ChildProcessLauncher is implemented in Java
so having an easy access to the launcher thread is important.

Refactor JavaHandlerThread in base so that that java part can be created
first, and then connected with native peer after consutrction. This is
needed because the launcher thread can be used before native library is
loaded.

Add LauncherThread in content which is a simple wrapper around
JavaHandlerThread. Then for android, override the launcher thread
message loop. Note this means the launcher thread will no longer be
joined on shutdown, but this is not a problem in practice since android
never does clean shutdowns.

Convert two cases of random background threads in ChildProcessLauncher
to use LauncherThread. There are more in the future.

BUG= 701442 

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

[modify] https://crrev.com/61687ec5b7b5dce1c9c86ae46f552c0c34473555/base/BUILD.gn
[modify] https://crrev.com/61687ec5b7b5dce1c9c86ae46f552c0c34473555/base/android/java/src/org/chromium/base/JavaHandlerThread.java
[modify] https://crrev.com/61687ec5b7b5dce1c9c86ae46f552c0c34473555/base/android/java_handler_thread.cc
[modify] https://crrev.com/61687ec5b7b5dce1c9c86ae46f552c0c34473555/base/android/java_handler_thread.h
[modify] https://crrev.com/61687ec5b7b5dce1c9c86ae46f552c0c34473555/base/message_loop/message_loop_unittest.cc
[modify] https://crrev.com/61687ec5b7b5dce1c9c86ae46f552c0c34473555/base/test/BUILD.gn
[add] https://crrev.com/61687ec5b7b5dce1c9c86ae46f552c0c34473555/base/test/android/java/src/org/chromium/base/JavaHandlerThreadTest.java
[modify] https://crrev.com/61687ec5b7b5dce1c9c86ae46f552c0c34473555/base/test/android/java_handler_thread_for_testing.cc
[modify] https://crrev.com/61687ec5b7b5dce1c9c86ae46f552c0c34473555/base/test/android/java_handler_thread_for_testing.h
[modify] https://crrev.com/61687ec5b7b5dce1c9c86ae46f552c0c34473555/content/browser/BUILD.gn
[add] https://crrev.com/61687ec5b7b5dce1c9c86ae46f552c0c34473555/content/browser/android/launcher_thread.cc
[add] https://crrev.com/61687ec5b7b5dce1c9c86ae46f552c0c34473555/content/browser/android/launcher_thread.h
[modify] https://crrev.com/61687ec5b7b5dce1c9c86ae46f552c0c34473555/content/browser/browser_main_loop.cc
[modify] https://crrev.com/61687ec5b7b5dce1c9c86ae46f552c0c34473555/content/browser/browser_process_sub_thread.cc
[modify] https://crrev.com/61687ec5b7b5dce1c9c86ae46f552c0c34473555/content/browser/browser_process_sub_thread.h
[modify] https://crrev.com/61687ec5b7b5dce1c9c86ae46f552c0c34473555/content/public/android/BUILD.gn
[modify] https://crrev.com/61687ec5b7b5dce1c9c86ae46f552c0c34473555/content/public/android/java/src/org/chromium/content/browser/ChildProcessLauncher.java
[add] https://crrev.com/61687ec5b7b5dce1c9c86ae46f552c0c34473555/content/public/android/java/src/org/chromium/content/browser/LauncherThread.java

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/311458dc726c28f8d091aa496ca6ce70778269ef

commit 311458dc726c28f8d091aa496ca6ce70778269ef
Author: boliu <boliu@chromium.org>
Date: Thu Apr 13 18:01:37 2017

android: Fix java launcher thread

This is a follow up fix for refs/heads/master@{#460509} which is not
correct:
* content::android::LauncherThread never starts the thread, so
  GetMessageLoop just returns null.
* This of course causes an actual native thread to be started for
  launcher thread in  BrowserMainLoop::CreateThreads.

Fix does this:
* content::android::LauncherThread calls Start on construction so
  GetMessageLoop no longer returns null.
* Make sure Start is not called on BrowserProcessSubThread for launcher
  thread. This matches behavior in BrowserMainLoop::InitializeMainThread
  for the UI thread.

Luckily we have not removed any thread safety code in this time, so
looks like this mistake has not caused huge issues.

BUG= 701442 

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

[modify] https://crrev.com/311458dc726c28f8d091aa496ca6ce70778269ef/content/browser/android/launcher_thread.cc
[modify] https://crrev.com/311458dc726c28f8d091aa496ca6ce70778269ef/content/browser/browser_main_loop.cc