Start message loop on creation on Android
The message loop has to be started to post tasks on Android. Previously
the message loop is started on PreEarlyInitialization(). Now we need to
post tasks earlier than that. This change starts the message loop when
it being created on Android.
For files:
android_webview/browser/aw_browser_main_parts.cc,
chrome/browser/chrome_browser_main_android.cc,
chromecast/browser/cast_browser_main_parts.cc,
components/cronet/android/cronet_library_loader.cc,
content/shell/browser/shell_browser_main_parts.cc,
remoting/client/chromoting_client_runtime.cc
[email protected],[email protected],[email protected],[email protected],[email protected],[email protected]
Bug: 848615, 729596
Cq-Include-Trybots: master.tryserver.chromium.android:android_cronet_tester;master.tryserver.chromium.mac:ios-simulator-cronet
Change-Id: I4b2a2afb570096cc56339978ef8e4ba3f3f9ecc7
Reviewed-on: https://chromium-review.googlesource.com/1112958
Commit-Queue: Ran Ji <[email protected]>
Reviewed-by: Ran Ji <[email protected]>
Reviewed-by: Gabriel Charette <[email protected]>
Reviewed-by: Xi Han <[email protected]>
Cr-Commit-Position: refs/heads/master@{#573447}diff --git a/android_webview/browser/aw_browser_main_parts.cc b/android_webview/browser/aw_browser_main_parts.cc
index 33ed26a..712f0748 100644
--- a/android_webview/browser/aw_browser_main_parts.cc
+++ b/android_webview/browser/aw_browser_main_parts.cc
@@ -80,7 +80,6 @@
DCHECK(!main_message_loop_.get());
if (!base::MessageLoopCurrent::IsSet())
main_message_loop_.reset(new base::MessageLoopForUI);
- base::MessageLoopCurrentForUI::Get()->Start();
return service_manager::RESULT_CODE_NORMAL_EXIT;
}
diff --git a/base/android/java_handler_thread.cc b/base/android/java_handler_thread.cc
index 3b3f3b6..8147764f 100644
--- a/base/android/java_handler_thread.cc
+++ b/base/android/java_handler_thread.cc
@@ -64,7 +64,7 @@
jlong event) {
// TYPE_JAVA to get the Android java style message loop.
message_loop_ = new base::MessageLoop(base::MessageLoop::TYPE_JAVA);
- StartMessageLoop();
+ Init();
reinterpret_cast<base::WaitableEvent*>(event)->Signal();
}
@@ -79,11 +79,6 @@
CleanUp();
}
-void JavaHandlerThread::StartMessageLoop() {
- static_cast<MessageLoopForUI*>(message_loop_)->Start();
- Init();
-}
-
void JavaHandlerThread::StopMessageLoop() {
base::RunLoop::QuitCurrentWhenIdleDeprecated();
}
diff --git a/base/android/java_handler_thread.h b/base/android/java_handler_thread.h
index df90f66..24c7d2e 100644
--- a/base/android/java_handler_thread.h
+++ b/base/android/java_handler_thread.h
@@ -54,7 +54,6 @@
void StopThread(JNIEnv* env, const JavaParamRef<jobject>& obj);
void OnLooperStopped(JNIEnv* env, const JavaParamRef<jobject>& obj);
- virtual void StartMessageLoop();
virtual void StopMessageLoop();
void StopMessageLoopForTesting();
diff --git a/base/message_loop/message_loop.cc b/base/message_loop/message_loop.cc
index f4cd32c..0ad28fa 100644
--- a/base/message_loop/message_loop.cc
+++ b/base/message_loop/message_loop.cc
@@ -263,6 +263,12 @@
&sequence_local_storage_map_);
RunLoop::RegisterDelegateForCurrentThread(this);
+
+#if defined(OS_ANDROID)
+ // On Android, attach to the native loop when there is one.
+ if (type_ == TYPE_UI || type_ == TYPE_JAVA)
+ static_cast<MessagePumpForUI*>(pump_.get())->Attach(this);
+#endif
}
std::string MessageLoop::GetThreadName() const {
@@ -596,11 +602,6 @@
#endif // defined(OS_IOS)
#if defined(OS_ANDROID)
-void MessageLoopForUI::Start() {
- // No Histogram support for UI message loop as it is managed by Java side
- static_cast<MessagePumpForUI*>(pump_.get())->Start(this);
-}
-
void MessageLoopForUI::Abort() {
static_cast<MessagePumpForUI*>(pump_.get())->Abort();
}
diff --git a/base/message_loop/message_loop.h b/base/message_loop/message_loop.h
index a1c5c40..64838a9 100644
--- a/base/message_loop/message_loop.h
+++ b/base/message_loop/message_loop.h
@@ -363,11 +363,6 @@
#endif
#if defined(OS_ANDROID)
- // On Android, the UI message loop is handled by Java side. So Run() should
- // never be called. Instead use Start(), which will forward all the native UI
- // events to the Java message loop.
- void Start();
-
// In Android there are cases where we want to abort immediately without
// calling Quit(), in these cases we call Abort().
void Abort();
diff --git a/base/message_loop/message_loop_current.cc b/base/message_loop/message_loop_current.cc
index 5554e9f..4959b70 100644
--- a/base/message_loop/message_loop_current.cc
+++ b/base/message_loop/message_loop_current.cc
@@ -168,10 +168,6 @@
#endif // defined(OS_IOS)
#if defined(OS_ANDROID)
-void MessageLoopCurrentForUI::Start() {
- static_cast<MessageLoopForUI*>(current_)->Start();
-}
-
void MessageLoopCurrentForUI::Abort() {
static_cast<MessageLoopForUI*>(current_)->Abort();
}
diff --git a/base/message_loop/message_loop_current.h b/base/message_loop/message_loop_current.h
index 9e19a78..61d1607 100644
--- a/base/message_loop/message_loop_current.h
+++ b/base/message_loop/message_loop_current.h
@@ -225,12 +225,6 @@
#endif
#if defined(OS_ANDROID)
- // Forwards to MessageLoopForUI::Start().
- // TODO(https://crbug.com/825327): Plumb the actual MessageLoopForUI* to
- // callers and remove ability to access this method from
- // MessageLoopCurrentForUI.
- void Start();
-
// Forwards to MessageLoopForUI::Abort().
// TODO(https://crbug.com/825327): Plumb the actual MessageLoopForUI* to
// callers and remove ability to access this method from
diff --git a/base/message_loop/message_pump_android.cc b/base/message_loop/message_pump_android.cc
index 8c5bb57..5f8981d 100644
--- a/base/message_loop/message_pump_android.cc
+++ b/base/message_loop/message_pump_android.cc
@@ -80,7 +80,7 @@
" test_stub_android.h";
}
-void MessagePumpForUI::Start(Delegate* delegate) {
+void MessagePumpForUI::Attach(Delegate* delegate) {
DCHECK(!quit_);
delegate_ = delegate;
run_loop_ = std::make_unique<RunLoop>();
diff --git a/base/message_loop/message_pump_android.h b/base/message_loop/message_pump_android.h
index d09fdde..21f0315 100644
--- a/base/message_loop/message_pump_android.h
+++ b/base/message_loop/message_pump_android.h
@@ -36,7 +36,9 @@
void ScheduleWork() override;
void ScheduleDelayedWork(const TimeTicks& delayed_work_time) override;
- virtual void Start(Delegate* delegate);
+ // Attaches |delegate| to this native MessagePump. |delegate| will from then
+ // on be invoked by the native loop to process application tasks.
+ virtual void Attach(Delegate* delegate);
// We call Abort when there is a pending JNI exception, meaning that the
// current thread will crash when we return to Java.
diff --git a/base/test/test_support_android.cc b/base/test/test_support_android.cc
index 33a6628..411ccf02 100644
--- a/base/test/test_support_android.cc
+++ b/base/test/test_support_android.cc
@@ -75,10 +75,10 @@
class MessagePumpForUIStub : public base::MessagePumpForUI {
~MessagePumpForUIStub() override {}
- void Start(base::MessagePump::Delegate* delegate) override {
- NOTREACHED() << "The Start() method shouldn't be called in test, using"
- " Run() method should be used.";
- }
+ // In tests, there isn't a native thread, as such RunLoop::Run() should be
+ // used to run the loop instead of attaching and delegating to the native
+ // loop. As such, this override ignores the Attach() request.
+ void Attach(base::MessagePump::Delegate* delegate) override {}
void Run(base::MessagePump::Delegate* delegate) override {
// The following was based on message_pump_glib.cc, except we're using a
diff --git a/chrome/browser/chrome_browser_main_android.cc b/chrome/browser/chrome_browser_main_android.cc
index a71265dc..6f63f97 100644
--- a/chrome/browser/chrome_browser_main_android.cc
+++ b/chrome/browser/chrome_browser_main_android.cc
@@ -104,8 +104,8 @@
// Android specific MessageLoop.
DCHECK(!main_message_loop_.get());
- // Create and start the MessageLoop if doesn't yet exist.
- // This is a critical point in the startup process.
+ // Create the MessageLoop if doesn't yet exist (and bind it to the native Java
+ // loop). This is a critical point in the startup process.
{
TRACE_EVENT0("startup",
"ChromeBrowserMainPartsAndroid::PreEarlyInitialization:CreateUiMsgLoop");
@@ -113,12 +113,6 @@
main_message_loop_ = std::make_unique<base::MessageLoopForUI>();
}
- {
- TRACE_EVENT0("startup",
- "ChromeBrowserMainPartsAndroid::PreEarlyInitialization:StartUiMsgLoop");
- base::MessageLoopCurrentForUI::Get()->Start();
- }
-
// In order for SetLoadSecondaryLocalePaks() to work ResourceBundle must
// not have been created yet.
DCHECK(!ui::ResourceBundle::HasSharedInstance());
diff --git a/chromecast/browser/cast_browser_main_parts.cc b/chromecast/browser/cast_browser_main_parts.cc
index d1e0bc2..640b7247 100644
--- a/chromecast/browser/cast_browser_main_parts.cc
+++ b/chromecast/browser/cast_browser_main_parts.cc
@@ -396,10 +396,6 @@
void CastBrowserMainParts::PostMainMessageLoopStart() {
// Ensure CastMetricsHelper initialized on UI thread.
metrics::CastMetricsHelper::GetInstance();
-
-#if defined(OS_ANDROID)
- base::MessageLoopCurrentForUI::Get()->Start();
-#endif // defined(OS_ANDROID)
}
void CastBrowserMainParts::ToolkitInitialized() {
diff --git a/components/cronet/android/cronet_library_loader.cc b/components/cronet/android/cronet_library_loader.cc
index fcb3484..9359844 100644
--- a/components/cronet/android/cronet_library_loader.cc
+++ b/components/cronet/android/cronet_library_loader.cc
@@ -101,7 +101,6 @@
DCHECK(!g_init_message_loop);
g_init_message_loop =
new base::MessageLoop(base::MessageLoop::Type::TYPE_JAVA);
- static_cast<base::MessageLoopForUI*>(g_init_message_loop)->Start();
DCHECK(!g_network_change_notifier);
if (!net::NetworkChangeNotifier::GetFactory()) {
diff --git a/content/public/test/nested_message_pump_android.cc b/content/public/test/nested_message_pump_android.cc
index 8e0b8df..48d4d17 100644
--- a/content/public/test/nested_message_pump_android.cc
+++ b/content/public/test/nested_message_pump_android.cc
@@ -117,7 +117,7 @@
state_ = previous_state;
}
-void NestedMessagePumpAndroid::Start(
+void NestedMessagePumpAndroid::Attach(
base::MessagePump::Delegate* delegate) {
JNIEnv* env = base::android::AttachCurrentThread();
DCHECK(env);
diff --git a/content/public/test/nested_message_pump_android.h b/content/public/test/nested_message_pump_android.h
index 91c68a0..4f5cebe 100644
--- a/content/public/test/nested_message_pump_android.h
+++ b/content/public/test/nested_message_pump_android.h
@@ -22,7 +22,7 @@
void Quit() override;
void ScheduleWork() override;
void ScheduleDelayedWork(const base::TimeTicks& delayed_work_time) override;
- void Start(Delegate* delegate) override;
+ void Attach(Delegate* delegate) override;
protected:
~NestedMessagePumpAndroid() override;
diff --git a/content/shell/browser/shell_browser_main_parts.cc b/content/shell/browser/shell_browser_main_parts.cc
index 4d7e94e..f855437 100644
--- a/content/shell/browser/shell_browser_main_parts.cc
+++ b/content/shell/browser/shell_browser_main_parts.cc
@@ -114,10 +114,6 @@
#endif
void ShellBrowserMainParts::PostMainMessageLoopStart() {
-#if defined(OS_ANDROID)
- base::MessageLoopCurrentForUI::Get()->Start();
-#endif
-
#if defined(OS_CHROMEOS)
chromeos::DBusThreadManager::Initialize();
bluez::BluezDBusManager::Initialize(
diff --git a/remoting/client/chromoting_client_runtime.cc b/remoting/client/chromoting_client_runtime.cc
index 220ba30..7bd10f7 100644
--- a/remoting/client/chromoting_client_runtime.cc
+++ b/remoting/client/chromoting_client_runtime.cc
@@ -11,6 +11,7 @@
#include "base/message_loop/message_loop.h"
#include "base/message_loop/message_loop_current.h"
#include "base/task_scheduler/task_scheduler.h"
+#include "build/build_config.h"
#include "remoting/base/chromium_url_request.h"
#include "remoting/base/telemetry_log_writer.h"
#include "remoting/base/url_request_context_getter.h"
@@ -36,13 +37,10 @@
VLOG(1) << "Starting main message loop";
ui_loop_.reset(new base::MessageLoopForUI());
-#if defined(OS_ANDROID)
- // On Android, the UI thread is managed by Java, so we need to attach and
- // start a special type of message loop to allow Chromium code to run tasks.
- ui_loop_->Start();
-#elif defined(OS_IOS)
+#if defined(OS_IOS)
+ // TODO(ranj): Attach on BindToCurrentThread().
ui_loop_->Attach();
-#endif // OS_ANDROID, OS_IOS
+#endif
#if defined(DEBUG)
net::URLFetcher::SetIgnoreCertificateRequests(true);
