Making a way to create thread with a Java Looper for Android

base/android/java_thread.cc

+// Copyright (c) 2013 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include "base/android/java_thread.h"
+
+#include <jni.h>
+
+#include "base/android/jni_android.h"
+#include "base/android/jni_string.h"
+#include "base/message_loop.h"
+#include "base/synchronization/waitable_event.h"
+#include "jni/JavaThread_jni.h"
+
+namespace base {
+
+namespace android {
+
+  JavaThread::JavaThread(const char* name) : started_(false) {

[joth, 2013/07/10 20:33:48]

unindent

: started_(false) on its own line (unless entire c'tor body fits on one line...)

[Kristian Monsen, 2013/07/16 21:54:41]

Done.

+  JNIEnv* env = base::android::AttachCurrentThread();
+  DCHECK(env);

[joth, 2013/07/10 20:33:48]

spurious

[Kristian Monsen, 2013/07/16 21:54:41]

Done.

+
+  java_thread_.Reset(Java_JavaThread_create(
+      env, ConvertUTF8ToJavaString(env, name).Release()));
+}
+
+void JavaThread::Start() {
+  // Check the thread has not already been started

[joth, 2013/07/10 20:33:48]

nit: end sentence in . on all the comments

[Kristian Monsen, 2013/07/16 21:54:41]

Done.

+  DCHECK(!started_);
+
+  JNIEnv* env = base::android::AttachCurrentThread();
+  DCHECK(env);

[joth, 2013/07/10 20:33:48]

ditto

[Kristian Monsen, 2013/07/16 21:54:41]

Done.

+
+  waitable_event_ = new base::WaitableEvent(false, false);

[joth, 2013/07/10 20:33:48]

cleaner as:

base::WaitableEvent event(false, false);
base::AutoReset(&waitable_event_, &event);

could give waitable_event_ a more useful name :)  initialize_completed_event_ ?

[Kristian Monsen, 2013/07/16 21:54:41]

I made it a stack variable, see what you think?

+  Java_JavaThread_start(env, java_thread_.obj(), reinterpret_cast<jint>(this));

[joth, 2013/07/10 20:33:48]

slight tempted to pass &event through as an additional param here, and have it
passed back as a param into InitializeThread(), and then you can avoid the
pointer hanging around as a member for the rest of the time

[Kristian Monsen, 2013/07/16 21:54:41]

Done.

+  // Wait for thread to be initialized so it is ready to be used when Start
+  // returns
+  waitable_event_->Wait();
+
+  // Delete object only used in initialization
+  delete waitable_event_;
+  waitable_event_ = NULL;
+  started_ = true;
+}
+
+void JavaThread::Stop() {
+}
+
+void JavaThread::InitializeThread(JNIEnv* env, jobject obj) {
+  // TYPE_UI to get the Android java style message loop
+  message_loop_.reset(new base::MessageLoop(base::MessageLoop::TYPE_UI));
+  MessageLoopForUI::current()->Start();

[joth, 2013/07/10 20:33:48]

message_loop_->Start()  ? (/shrug)

[Kristian Monsen, 2013/07/16 21:54:41]

Sadly not possible, Start() is an #ifdef ANDROID in MeesageLoopForUI.

Actually, I want to remove that #ifdef and just call start in the CTOR. I don't
think it is needed. But that is another CL.

+  waitable_event_->Signal();
+}
+
+// static
+bool JavaThread::RegisterBindings(JNIEnv* env) {
+  return RegisterNativesImpl(env);
+}
+
+} // namespace android
+
+} // namespace base