Ladybird/Android: Explicitly schedule Core::EventLoop in main activity

Instead of having an annoying loop that constantly reschedules a
Core::EventLoop trigger, have the ALooperEventLoopManager do it itself
in the did_post_event() function.

We cannot simply re-use the Unix implementation directly because that
implementation expects to actually be called all the time in order to
service timers. If you don't call its' pump() method, timers do not get
triggered. So, we do still need the seconary thread for Timers that was
added earlier.

Author: ADKaster

Ladybird/Android/src/main/cpp/ALooperEventLoopImplementation.cpp

@@ -27,11 +27,12 @@ static ALooperEventLoopImplementation& current_impl()
     return verify_cast<ALooperEventLoopImplementation>(Core::EventLoop::current().impl());
 }
 
-ALooperEventLoopManager::ALooperEventLoopManager(JavaVM* vm, jobject timer_service)
-    : m_vm(vm)
-    , m_timer_service(timer_service)
+static int looper_callback(int fd, int events, void* data);
+
+ALooperEventLoopManager::ALooperEventLoopManager(jobject timer_service)
+    : m_timer_service(timer_service)
 {
-    JavaEnvironment env(m_vm);
+    JavaEnvironment env(global_vm);
 
     jclass timer_class = env.get()->FindClass("org/serenityos/ladybird/TimerExecutorService$Timer");
     if (!timer_class)
@@ -53,14 +54,30 @@ ALooperEventLoopManager::ALooperEventLoopManager(JavaVM* vm, jobject timer_servi
     if (!m_unregister_timer)
         TODO();
     env.get()->DeleteLocalRef(timer_service_class);
+
+    auto ret = pipe2(m_pipe, O_CLOEXEC | O_NONBLOCK);
+    VERIFY(ret == 0);
+
+    m_main_looper = ALooper_forThread();
+    VERIFY(m_main_looper);
+    ALooper_acquire(m_main_looper);
+
+    ret = ALooper_addFd(m_main_looper, m_pipe[0], ALOOPER_POLL_CALLBACK, ALOOPER_EVENT_INPUT, &looper_callback, this);
+    VERIFY(ret == 1);
 }
 
 ALooperEventLoopManager::~ALooperEventLoopManager()
 {
-    JavaEnvironment env(m_vm);
+    JavaEnvironment env(global_vm);
 
     env.get()->DeleteGlobalRef(m_timer_service);
     env.get()->DeleteGlobalRef(m_timer_class);
+
+    ALooper_removeFd(m_main_looper, m_pipe[0]);
+    ALooper_release(m_main_looper);
+
+    ::close(m_pipe[0]);
+    ::close(m_pipe[1]);
 }
 
 NonnullOwnPtr<Core::EventLoopImplementation> ALooperEventLoopManager::make_implementation()
@@ -70,7 +87,7 @@ NonnullOwnPtr<Core::EventLoopImplementation> ALooperEventLoopManager::make_imple
 
 int ALooperEventLoopManager::register_timer(Core::EventReceiver& receiver, int milliseconds, bool should_reload, Core::TimerShouldFireWhenNotVisible visibility)
 {
-    JavaEnvironment env(m_vm);
+    JavaEnvironment env(global_vm);
     auto& thread_data = EventLoopThreadData::the();
 
     auto timer = env.get()->NewObject(m_timer_class, m_timer_constructor, reinterpret_cast<long>(&current_impl()));
@@ -87,7 +104,7 @@ int ALooperEventLoopManager::register_timer(Core::EventReceiver& receiver, int m
 bool ALooperEventLoopManager::unregister_timer(int timer_id)
 {
     if (auto timer = EventLoopThreadData::the().timers.take(timer_id); timer.has_value()) {
-        JavaEnvironment env(m_vm);
+        JavaEnvironment env(global_vm);
         return env.get()->CallBooleanMethod(m_timer_service, m_unregister_timer, timer_id);
     }
     return false;
@@ -107,29 +124,34 @@ void ALooperEventLoopManager::unregister_notifier(Core::Notifier& notifier)
 
 void ALooperEventLoopManager::did_post_event()
 {
-    current_impl().poke();
+    int msg = 0xCAFEBABE;
+    (void)write(m_pipe[1], &msg, sizeof(msg));
+}
+
+int looper_callback(int fd, int events, void* data)
+{
+    auto& manager = *static_cast<ALooperEventLoopManager*>(data);
+
+    if (events & ALOOPER_EVENT_INPUT) {
+        int msg = 0;
+        while (read(fd, &msg, sizeof(msg)) == sizeof(msg)) {
+            // Do nothing, we don't actually care what the message was, just that it was posted
+        }
+        manager.on_did_post_event();
+    }
+    return 1;
 }
 
 ALooperEventLoopImplementation::ALooperEventLoopImplementation()
     : m_event_loop(ALooper_prepare(0))
     , m_thread_data(&EventLoopThreadData::the())
 {
-    auto ret = pipe2(m_pipe, O_CLOEXEC | O_NONBLOCK);
-    VERIFY(ret == 0);
-
     ALooper_acquire(m_event_loop);
-
-    ret = ALooper_addFd(m_event_loop, m_pipe[0], ALOOPER_POLL_CALLBACK, ALOOPER_EVENT_INPUT, &ALooperEventLoopImplementation::looper_callback, this);
-    VERIFY(ret == 1);
 }
 
 ALooperEventLoopImplementation::~ALooperEventLoopImplementation()
 {
-    ALooper_removeFd(m_event_loop, m_pipe[0]);
     ALooper_release(m_event_loop);
-
-    ::close(m_pipe[0]);
-    ::close(m_pipe[1]);
 }
 
 EventLoopThreadData& ALooperEventLoopImplementation::thread_data()
@@ -181,26 +203,6 @@ void ALooperEventLoopImplementation::post_event(Core::EventReceiver& receiver, N
         wake();
 }
 
-int ALooperEventLoopImplementation::looper_callback(int fd, int events, void* data)
-{
-    auto& impl = *static_cast<ALooperEventLoopImplementation*>(data);
-    (void)impl; // FIXME: Do we need to do anything with the instance here?
-
-    if (events & ALOOPER_EVENT_INPUT) {
-        int msg = 0;
-        while (read(fd, &msg, sizeof(msg)) == sizeof(msg)) {
-            // Do nothing, we don't actually care what the message was, just that it was posted
-        }
-    }
-    return 1;
-}
-
-void ALooperEventLoopImplementation::poke()
-{
-    int msg = 0xCAFEBABE;
-    (void)write(m_pipe[1], &msg, sizeof(msg));
-}
-
 static int notifier_callback(int fd, int, void* data)
 {
     auto& notifier = *static_cast<Core::Notifier*>(data);
@@ -211,7 +213,7 @@ static int notifier_callback(int fd, int, void* data)
     notifier.dispatch_event(event);
 
     // Wake up from ALooper_pollAll, and service this event on the event queue
-    current_impl().poke();
+    current_impl().wake();
 
     return 1;
 }

Ladybird/Android/src/main/cpp/ALooperEventLoopImplementation.h

@@ -19,7 +19,7 @@ namespace Ladybird {
 
 class ALooperEventLoopManager : public Core::EventLoopManager {
 public:
-    ALooperEventLoopManager(JavaVM*, jobject timer_service);
+    ALooperEventLoopManager(jobject timer_service);
     virtual ~ALooperEventLoopManager() override;
     virtual NonnullOwnPtr<Core::EventLoopImplementation> make_implementation() override;
 
@@ -31,12 +31,15 @@ class ALooperEventLoopManager : public Core::EventLoopManager {
 
     virtual void did_post_event() override;
 
+    Function<void()> on_did_post_event;
+
     // FIXME: These APIs only exist for obscure use-cases inside SerenityOS. Try to get rid of them.
     virtual int register_signal(int, Function<void(int)>) override { return 0; }
     virtual void unregister_signal(int) override { }
 
 private:
-    JavaVM* m_vm { nullptr };
+    int m_pipe[2] = {};
+    ALooper* m_main_looper { nullptr };
     jobject m_timer_service { nullptr };
     jmethodID m_register_timer { nullptr };
     jmethodID m_unregister_timer { nullptr };
@@ -74,22 +77,17 @@ class ALooperEventLoopImplementation : public Core::EventLoopImplementation {
     virtual bool was_exit_requested() const override { return false; }
     virtual void notify_forked_and_in_child() override { }
 
-    void poke();
-
     EventLoopThreadData& thread_data();
 
 private:
     friend class ALooperEventLoopManager;
 
     ALooperEventLoopImplementation();
 
-    static int looper_callback(int fd, int events, void* data);
-
     void register_notifier(Core::Notifier&);
     void unregister_notifier(Core::Notifier&);
 
     ALooper* m_event_loop { nullptr };
-    int m_pipe[2] {};
     int m_exit_code { 0 };
     Atomic<bool> m_exit_requested { false };
     EventLoopThreadData* m_thread_data { nullptr };

Ladybird/Android/src/main/cpp/JNIHelpers.h

@@ -41,3 +41,5 @@ class JavaEnvironment {
     JNIEnv* m_env = nullptr;
     bool m_did_attach_thread = false;
 };
+
+extern JavaVM* global_vm;

Ladybird/Android/src/main/cpp/LadybirdActivity.cpp

@@ -5,6 +5,7 @@
  */
 
 #include "ALooperEventLoopImplementation.h"
+#include "JNIHelpers.h"
 #include <AK/DeprecatedString.h>
 #include <AK/Format.h>
 #include <AK/HashMap.h>
@@ -22,10 +23,13 @@
 
 static ErrorOr<void> extract_tar_archive(String archive_file, DeprecatedString output_directory);
 
+JavaVM* global_vm;
 static OwnPtr<Core::EventLoop> s_main_event_loop;
+static jobject s_java_instance;
+static jmethodID s_schedule_event_loop_method;
 
 extern "C" JNIEXPORT void JNICALL
-Java_org_serenityos_ladybird_LadybirdActivity_initNativeCode(JNIEnv* env, jobject /* thiz */, jstring resource_dir, jstring tag_name, jobject timer_service)
+Java_org_serenityos_ladybird_LadybirdActivity_initNativeCode(JNIEnv* env, jobject thiz, jstring resource_dir, jstring tag_name, jobject timer_service)
 {
     char const* raw_resource_dir = env->GetStringUTFChars(resource_dir, nullptr);
     s_serenity_resource_root = raw_resource_dir;
@@ -46,18 +50,43 @@ Java_org_serenityos_ladybird_LadybirdActivity_initNativeCode(JNIEnv* env, jobjec
         dbgln("Hopefully no developer changed the asset files and expected them to be re-extracted!");
     }
 
+    env->GetJavaVM(&global_vm);
+    VERIFY(global_vm);
+
+    s_java_instance = env->NewGlobalRef(thiz);
+    jclass clazz = env->GetObjectClass(s_java_instance);
+    VERIFY(clazz);
+    s_schedule_event_loop_method = env->GetMethodID(clazz, "scheduleEventLoop", "()V");
+    VERIFY(s_schedule_event_loop_method);
+    env->DeleteLocalRef(clazz);
+
     jobject timer_service_ref = env->NewGlobalRef(timer_service);
-    JavaVM* vm = nullptr;
-    jint ret = env->GetJavaVM(&vm);
-    VERIFY(ret == 0);
-    Core::EventLoopManager::install(*new Ladybird::ALooperEventLoopManager(vm, timer_service_ref));
+
+    auto* event_loop_manager = new Ladybird::ALooperEventLoopManager(timer_service_ref);
+    event_loop_manager->on_did_post_event = [] {
+        JavaEnvironment env(global_vm);
+        env.get()->CallVoidMethod(s_java_instance, s_schedule_event_loop_method);
+    };
+    Core::EventLoopManager::install(*event_loop_manager);
     s_main_event_loop = make<Core::EventLoop>();
 }
 
 extern "C" JNIEXPORT void JNICALL
 Java_org_serenityos_ladybird_LadybirdActivity_execMainEventLoop(JNIEnv*, jobject /* thiz */)
 {
-    s_main_event_loop->pump(Core::EventLoop::WaitMode::PollForEvents);
+    if (s_main_event_loop) {
+        s_main_event_loop->pump(Core::EventLoop::WaitMode::PollForEvents);
+    }
+}
+
+extern "C" JNIEXPORT void JNICALL
+Java_org_serenityos_ladybird_LadybirdActivity_disposeNativeCode(JNIEnv* env, jobject /* thiz */)
+{
+    s_main_event_loop = nullptr;
+    s_schedule_event_loop_method = nullptr;
+    env->DeleteGlobalRef(s_java_instance);
+
+    delete &Core::EventLoopManager::the();
 }
 
 ErrorOr<void> extract_tar_archive(String archive_file, DeprecatedString output_directory)

Ladybird/Android/src/main/cpp/LadybirdServiceBase.h

@@ -10,5 +10,3 @@
 #include <jni.h>
 
 ErrorOr<int> service_main(int ipc_socket, int fd_passing_socket);
-
-extern JavaVM* global_vm;

Ladybird/Android/src/main/cpp/TimerExecutorService.cpp

@@ -29,5 +29,7 @@ Java_org_serenityos_ladybird_TimerExecutorService_00024Timer_nativeRun(JNIEnv*,
         event_loop_impl.post_event(*receiver, make<Core::TimerEvent>(id));
     }
     // Flush the event loop on this thread to keep any garbage from building up
-    s_event_loop.pump(Core::EventLoop::WaitMode::PollForEvents);
+    if (auto num_events = s_event_loop.pump(Core::EventLoop::WaitMode::PollForEvents); num_events != 0) {
+        dbgln("BUG: Processed {} events on Timer thread!", num_events);
+    }
 }

Ladybird/Android/src/main/cpp/WebViewImplementationNative.h

@@ -32,7 +32,6 @@ class WebViewImplementationNative : public WebView::ViewImplementation {
     static jclass global_class_reference;
     static jmethodID bind_webcontent_method;
     static jmethodID invalidate_layout_method;
-    static JavaVM* global_vm;
 
     jobject java_instance() const { return m_java_instance; }
 

Ladybird/Android/src/main/cpp/WebViewImplementationNativeJNI.cpp

@@ -12,15 +12,10 @@ using namespace Ladybird;
 jclass WebViewImplementationNative::global_class_reference;
 jmethodID WebViewImplementationNative::bind_webcontent_method;
 jmethodID WebViewImplementationNative::invalidate_layout_method;
-JavaVM* WebViewImplementationNative::global_vm;
 
 extern "C" JNIEXPORT void JNICALL
 Java_org_serenityos_ladybird_WebViewImplementation_00024Companion_nativeClassInit(JNIEnv* env, jobject /* thiz */)
 {
-    auto ret = env->GetJavaVM(&WebViewImplementationNative::global_vm);
-    if (ret != 0)
-        TODO();
-
     auto local_class = env->FindClass("org/serenityos/ladybird/WebViewImplementation");
     if (!local_class)
         TODO();

Ladybird/Android/src/main/java/org/serenityos/ladybird/LadybirdActivity.kt

@@ -16,6 +16,8 @@ class LadybirdActivity : AppCompatActivity() {
 
     private lateinit var binding: ActivityMainBinding
     private lateinit var resourceDir: String
+    private lateinit var view: WebView
+    private var timerService = TimerExecutorService()
 
     override fun onCreate(savedInstanceState: Bundle?) {
         super.onCreate(savedInstanceState)
@@ -28,10 +30,6 @@ class LadybirdActivity : AppCompatActivity() {
         setSupportActionBar(binding.toolbar)
         view = binding.webView
         view.initialize(resourceDir)
-
-        mainExecutor.execute {
-            callNativeEventLoopForever()
-        }
     }
 
     override fun onStart() {
@@ -44,31 +42,19 @@ class LadybirdActivity : AppCompatActivity() {
 
     override fun onDestroy() {
         view.dispose()
+        disposeNativeCode()
         super.onDestroy()
     }
 
-    private lateinit var view: WebView
-    private var timerService = TimerExecutorService()
-
-    /**
-     * A native method that is implemented by the 'ladybird' native library,
-     * which is packaged with this application.
-     */
-    private external fun initNativeCode(
-        resourceDir: String,
-        tag: String,
-        timerService: TimerExecutorService
-    )
-
-    // FIXME: Instead of doing this, can we push a message to the message queue of the java Looper
-    //        when an event is pushed to the main thread, and use that to clear out the
-    //        Core::ThreadEventQueues?
-    private fun callNativeEventLoopForever() {
-        execMainEventLoop()
-        mainExecutor.execute { callNativeEventLoopForever() }
+    private fun scheduleEventLoop() {
+        mainExecutor.execute {
+            execMainEventLoop()
+        }
     }
 
-    private external fun execMainEventLoop();
+    private external fun initNativeCode(resourceDir: String, tag: String, timerService: TimerExecutorService)
+    private external fun disposeNativeCode()
+    private external fun execMainEventLoop()
 
     companion object {
         // Used to load the 'ladybird' library on application startup.