Remove separate thread for the shell on Android.

Remove separate thread for the shell on Android.

The shell is now run on the main (UI) thread along with the NativeViewport stuff.

R=abarth@chromium.org
http://crbug.com/324637

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

[Ben Goodger (Google), 2013-12-03 18:58:18]

This appears to work, but let me know if I missed anything.

[abarth-chromium, 2013-12-03 19:04:44]

LGTM!

We should also merge the two haves of NativeViewportAndroid now that
they're on the same thread, but we can do that in the next CL.


On Tuesday, December 3, 2013, wrote:

> Reviewers: abarth,
>
> Message:
> This appears to work, but let me know if I missed anything.
>
> Description:
> Remove separate thread for the shell on Android.
>
> The shell is now run on the main (UI) thread along with the NativeViewport
> stuff.
>
> R=abarth@chromium.org
> http://crbug.com/324637
>
> Please review this at https://codereview.chromium.org/100623003/
>
> SVN Base: svn://svn.chromium.org/chrome/trunk/src
>
> Affected files (+8, -47 lines):
>   M mojo/services/native_viewport/native_viewport_android.cc
>   M mojo/shell/android/mojo_main.cc
>   M mojo/shell/task_runners.h
>
>
> Index: mojo/services/native_viewport/native_viewport_android.cc
> diff --git a/mojo/services/native_viewport/native_viewport_android.cc
> b/mojo/services/native_viewport/native_viewport_android.cc
> index 5c03a0afcb3da8116e6961d6dfe22c7715c5c4ab..
> bc8071662d81c0eee8f9a3e5a0267f5e833b3d1a 100644
> --- a/mojo/services/native_viewport/native_viewport_android.cc
> +++ b/mojo/services/native_viewport/native_viewport_android.cc
> @@ -71,10 +71,7 @@ void NativeViewportAndroid::Init() {
>    init->ui_runner = context_->task_runners()->ui_runner();
>    init->native_viewport = GetWeakPtr();
>
> -  context_->task_runners()->java_runner()->PostTask(FROM_HERE,
> -      base::Bind(MojoViewport::CreateForActivity,
> -                 context_->activity(),
> -                 init));
> +  MojoViewport::CreateForActivity(context_->activity(), init);
>  }
>
>  void NativeViewportAndroid::Close() {
> Index: mojo/shell/android/mojo_main.cc
> diff --git a/mojo/shell/android/mojo_main.cc b/mojo/shell/android/mojo_
> main.cc
> index 55b1c53cf7bcc6fca8d9b0aa6d87325ded738bae..
> 18dc5f60864e52f70b1fcae237d24a79ed7749ff 100644
> --- a/mojo/shell/android/mojo_main.cc
> +++ b/mojo/shell/android/mojo_main.cc
> @@ -10,7 +10,6 @@
>  #include "base/command_line.h"
>  #include "base/lazy_instance.h"
>  #include "base/logging.h"
> -#include "base/threading/thread.h"
>  #include "jni/MojoMain_jni.h"
>  #include "mojo/shell/init.h"
>  #include "mojo/shell/run.h"
> @@ -27,28 +26,9 @@ base::AtExitManager* g_at_exit = 0;
>  LazyInstance<scoped_ptr<base::MessageLoop> > g_java_message_loop =
>      LAZY_INSTANCE_INITIALIZER;
>
> -LazyInstance<scoped_ptr<base::Thread> > g_shell_thread =
> -    LAZY_INSTANCE_INITIALIZER;
> -
>  LazyInstance<scoped_ptr<shell::Context> > g_context =
>      LAZY_INSTANCE_INITIALIZER;
>
> -struct ShellInit {
> -  scoped_refptr<base::SingleThreadTaskRunner> java_runner;
> -  base::android::ScopedJavaGlobalRef<jobject> activity;
> -};
> -
> -void StartOnShellThread(ShellInit* init) {
> -  shell::Context* context = new shell::Context();
> -
> -  context->set_activity(init->activity.obj());
> -  context->task_runners()->set_java_runner(init->java_runner.get());
> -  delete init;
> -
> -  g_context.Get().reset(context);
> -  shell::Run(context);
> -}
> -
>  }  // namspace
>
>  static void Init(JNIEnv* env, jclass clazz, jobject context) {
> @@ -83,16 +63,13 @@ static void Start(JNIEnv* env, jclass clazz, jobject
> context, jstring jurl) {
>      CommandLine::ForCurrentProcess()->InitFromArgv(argv);
>    }
>
> -  ShellInit* init = new ShellInit();
> -  init->java_runner = base::MessageLoopForUI::
> current()->message_loop_proxy();
> -  init->activity.Reset(env, context);
> -
> -  g_shell_thread.Get().reset(new base::Thread("shell_thread"));
> -  g_shell_thread.Get()->Start();
> -  g_shell_thread.Get()->message_loop()->PostTask(FROM_HERE,
> -      base::Bind(StartOnShellThread, init));
> +  base::android::ScopedJavaGlobalRef<jobject> activity;
> +  activity.Reset(env, context);
>
> -  // TODO(abarth): Currently we leak g_shell_thread.
> +  shell::Context* shell_context = new shell::Context();
> +  shell_context->set_activity(activity.obj());
> +  g_context.Get().reset(shell_context);
> +  shell::Run(shell_context);
>  }
>
>  bool RegisterMojoMain(JNIEnv* env) {
> Index: mojo/shell/task_runners.h
> diff --git a/mojo/shell/task_runners.h b/mojo/shell/task_runners.h
> index 509c5ae8df3009d79228eb2446110301687c96f8..
> 7311cad21acb5c9aa7fbe57474e31d3a5342e6fe 100644
> --- a/mojo/shell/task_runners.h
> +++ b/mojo/shell/task_runners.h
> @@ -34,26 +34,13 @@ class TaskRunners {
>      return cache_thread_->message_loop_proxy();
>    }
>
> -#if defined(OS_ANDROID)
> -  void set_java_runner(base::SingleThreadTaskRunner* java_runner) {
> -    java_runner_ = java_runner;
> -  }
> -
> -  base::SingleThreadTaskRunner* java_runner() const {
> -    return java_runner_.get();
> -  }
> -#endif // defined(OS_ANDROID)
> -
>   private:
> +  // TODO(beng): should this be named shell_runner_?
>    scoped_refptr<base::SingleThreadTaskRunner> ui_runner_;
>    scoped_ptr<base::Thread> cache_thread_;
>    scoped_ptr<base::Thread> io_thread_;
>    scoped_ptr<base::Thread> file_thread_;
>
> -#if defined(OS_ANDROID)
> -  scoped_refptr<base::SingleThreadTaskRunner> java_runner_;
> -#endif // defined(OS_ANDROID)
> -
>    DISALLOW_COPY_AND_ASSIGN(TaskRunners);
>  };
>
>
>
>

To unsubscribe from this group and stop receiving emails from it, send an email
to chromium-reviews+unsubscribe@chromium.org.

[Ben Goodger (Google), 2013-12-03 23:15:59]

Committed patchset #2 manually as r238474 (presubmit successful).