From da8f450335935d0b0af81f6971e0401e0c7d47a4 Mon Sep 17 00:00:00 2001 From: Andrew Kaster Date: Fri, 15 Sep 2023 15:09:01 -0600 Subject: [PATCH] Ladybird/Android: Move common service functionality to a base class Create LadybirdServiceBase to hold the standard "set resource dir" and "init ipc sockets" service functionality that will be common between the WebContent, RequestServer, and WebSocket services. Refactor the handler class slightly to avoid the HandlerLeak lint by making the class a static class inside the companion object and use a WeakReference to the service instead of a strong one. --- ...ContentService.h => LadybirdServiceBase.h} | 2 +- ...viceJNI.cpp => LadybirdServiceBaseJNI.cpp} | 8 +- .../src/main/cpp/WebContentService.cpp | 4 +- .../ladybird/LadybirdServiceBase.kt | 96 +++++++++++++++++++ ...ection.kt => LadybirdServiceConnection.kt} | 18 ++-- .../serenityos/ladybird/WebContentService.kt | 76 +-------------- .../ladybird/WebViewImplementation.kt | 2 +- Ladybird/WebContent/CMakeLists.txt | 2 +- 8 files changed, 117 insertions(+), 91 deletions(-) rename Ladybird/Android/src/main/cpp/{WebContentService.h => LadybirdServiceBase.h} (68%) rename Ladybird/Android/src/main/cpp/{WebContentServiceJNI.cpp => LadybirdServiceBaseJNI.cpp} (75%) create mode 100644 Ladybird/Android/src/main/java/org/serenityos/ladybird/LadybirdServiceBase.kt rename Ladybird/Android/src/main/java/org/serenityos/ladybird/{WebContentServiceConnection.kt => LadybirdServiceConnection.kt} (81%) diff --git a/Ladybird/Android/src/main/cpp/WebContentService.h b/Ladybird/Android/src/main/cpp/LadybirdServiceBase.h similarity index 68% rename from Ladybird/Android/src/main/cpp/WebContentService.h rename to Ladybird/Android/src/main/cpp/LadybirdServiceBase.h index ebf22046de..1879f9a201 100644 --- a/Ladybird/Android/src/main/cpp/WebContentService.h +++ b/Ladybird/Android/src/main/cpp/LadybirdServiceBase.h @@ -8,4 +8,4 @@ #include -ErrorOr web_content_main(int ipc_socket, int fd_passing_socket); +ErrorOr service_main(int ipc_socket, int fd_passing_socket); diff --git a/Ladybird/Android/src/main/cpp/WebContentServiceJNI.cpp b/Ladybird/Android/src/main/cpp/LadybirdServiceBaseJNI.cpp similarity index 75% rename from Ladybird/Android/src/main/cpp/WebContentServiceJNI.cpp rename to Ladybird/Android/src/main/cpp/LadybirdServiceBaseJNI.cpp index 73d4e2591e..3a7c12bc73 100644 --- a/Ladybird/Android/src/main/cpp/WebContentServiceJNI.cpp +++ b/Ladybird/Android/src/main/cpp/LadybirdServiceBaseJNI.cpp @@ -4,17 +4,17 @@ * SPDX-License-Identifier: BSD-2-Clause */ -#include "WebContentService.h" +#include "LadybirdServiceBase.h" #include #include #include #include extern "C" JNIEXPORT void JNICALL -Java_org_serenityos_ladybird_WebContentService_nativeThreadLoop(JNIEnv*, jobject /* thiz */, jint ipc_socket, jint fd_passing_socket) +Java_org_serenityos_ladybird_LadybirdServiceBase_nativeThreadLoop(JNIEnv*, jobject /* thiz */, jint ipc_socket, jint fd_passing_socket) { dbgln("New binding received, sockets {} and {}", ipc_socket, fd_passing_socket); - auto ret = web_content_main(ipc_socket, fd_passing_socket); + auto ret = service_main(ipc_socket, fd_passing_socket); if (ret.is_error()) { warnln("Runtime Error: {}", ret.release_error()); } else { @@ -23,7 +23,7 @@ Java_org_serenityos_ladybird_WebContentService_nativeThreadLoop(JNIEnv*, jobject } extern "C" JNIEXPORT void JNICALL -Java_org_serenityos_ladybird_WebContentService_initNativeCode(JNIEnv* env, jobject /* thiz */, jstring resource_dir, jstring tag_name) +Java_org_serenityos_ladybird_LadybirdServiceBase_initNativeCode(JNIEnv* env, jobject /* thiz */, jstring resource_dir, jstring tag_name) { static Atomic s_initialized_flag { false }; if (s_initialized_flag.exchange(true) == true) { diff --git a/Ladybird/Android/src/main/cpp/WebContentService.cpp b/Ladybird/Android/src/main/cpp/WebContentService.cpp index 33da106083..919c707f6a 100644 --- a/Ladybird/Android/src/main/cpp/WebContentService.cpp +++ b/Ladybird/Android/src/main/cpp/WebContentService.cpp @@ -4,7 +4,7 @@ * SPDX-License-Identifier: BSD-2-Clause */ -#include "WebContentService.h" +#include "LadybirdServiceBase.h" #include #include #include @@ -38,7 +38,7 @@ class NullResourceConnector : public Web::ResourceLoaderConnector { } }; -ErrorOr web_content_main(int ipc_socket, int fd_passing_socket) +ErrorOr service_main(int ipc_socket, int fd_passing_socket) { Core::EventLoop event_loop; diff --git a/Ladybird/Android/src/main/java/org/serenityos/ladybird/LadybirdServiceBase.kt b/Ladybird/Android/src/main/java/org/serenityos/ladybird/LadybirdServiceBase.kt new file mode 100644 index 0000000000..15d28c6639 --- /dev/null +++ b/Ladybird/Android/src/main/java/org/serenityos/ladybird/LadybirdServiceBase.kt @@ -0,0 +1,96 @@ +/** + * Copyright (c) 2023, Andrew Kaster + * + * SPDX-License-Identifier: BSD-2-Clause + */ + +package org.serenityos.ladybird + +import android.app.Service +import android.content.Intent +import android.util.Log +import android.os.ParcelFileDescriptor +import android.os.Handler +import android.os.IBinder +import android.os.Looper +import android.os.Message +import android.os.Messenger +import java.lang.ref.WeakReference +import java.util.concurrent.Executors + +const val MSG_SET_RESOURCE_ROOT = 1 +const val MSG_TRANSFER_SOCKETS = 2 + +abstract class LadybirdServiceBase(protected val TAG: String) : Service() { + private val threadPool = Executors.newCachedThreadPool() + private lateinit var resourceDir: String + + override fun onCreate() { + super.onCreate() + Log.i(TAG, "Creating Service") + } + + override fun onDestroy() { + super.onDestroy() + Log.i(TAG, "Destroying Service") + } + + override fun onStartCommand(intent: Intent?, flags: Int, startId: Int): Int { + Log.i(TAG, "Start command received") + return super.onStartCommand(intent, flags, startId) + } + + private fun handleTransferSockets(msg: Message) { + val bundle = msg.data + // FIXME: Handle garbage messages from wierd clients + val ipcSocket = bundle.getParcelable("IPC_SOCKET")!! + val fdSocket = bundle.getParcelable("FD_PASSING_SOCKET")!! + createThread(ipcSocket, fdSocket) + } + + private fun handleSetResourceRoot(msg: Message) { + // FIXME: Handle this being already set, not being present, etc + resourceDir = msg.data.getString("PATH")!! + + initNativeCode(resourceDir, TAG) + } + + override fun onBind(p0: Intent?): IBinder? { + // FIXME: Check the intent to make sure it's legit + return Messenger(IncomingHandler(WeakReference(this))).binder + } + + + private fun createThread(ipcSocket: ParcelFileDescriptor, fdSocket: ParcelFileDescriptor) { + threadPool.execute { + nativeThreadLoop(ipcSocket.detachFd(), fdSocket.detachFd()) + } + } + + private external fun nativeThreadLoop(ipcSocket: Int, fdPassingSocket: Int) + private external fun initNativeCode(resourceDir: String, tagName: String); + + abstract fun handleServiceSpecificMessage(msg: Message): Boolean + + companion object { + + class IncomingHandler(private val service: WeakReference) : + Handler(Looper.getMainLooper()) { + override fun handleMessage(msg: Message) { + when (msg.what) { + MSG_TRANSFER_SOCKETS -> service.get()?.handleTransferSockets(msg) + ?: super.handleMessage(msg) + + MSG_SET_RESOURCE_ROOT -> service.get()?.handleSetResourceRoot(msg) + ?: super.handleMessage(msg) + + else -> { + val ret = service.get()?.handleServiceSpecificMessage(msg) + if (ret == null || !ret) + super.handleMessage(msg) + } + } + } + } + } +} diff --git a/Ladybird/Android/src/main/java/org/serenityos/ladybird/WebContentServiceConnection.kt b/Ladybird/Android/src/main/java/org/serenityos/ladybird/LadybirdServiceConnection.kt similarity index 81% rename from Ladybird/Android/src/main/java/org/serenityos/ladybird/WebContentServiceConnection.kt rename to Ladybird/Android/src/main/java/org/serenityos/ladybird/LadybirdServiceConnection.kt index ddb5f1c35d..4c0da0cd28 100644 --- a/Ladybird/Android/src/main/java/org/serenityos/ladybird/WebContentServiceConnection.kt +++ b/Ladybird/Android/src/main/java/org/serenityos/ladybird/LadybirdServiceConnection.kt @@ -13,15 +13,15 @@ import android.os.Message import android.os.Messenger import android.os.ParcelFileDescriptor -class WebContentServiceConnection( +class LadybirdServiceConnection( private var ipcFd: Int, private var fdPassingFd: Int, private var resourceDir: String ) : ServiceConnection { - var boundToWebContent: Boolean = false + var boundToService: Boolean = false var onDisconnect: () -> Unit = {} - private var webContentService: Messenger? = null + private var service: Messenger? = null override fun onServiceConnected(className: ComponentName, svc: IBinder) { // This is called when the connection with the service has been @@ -29,24 +29,24 @@ class WebContentServiceConnection( // interact with the service. We are communicating with the // service using a Messenger, so here we get a client-side // representation of that from the raw IBinder object. - webContentService = Messenger(svc) - boundToWebContent = true + service = Messenger(svc) + boundToService = true val init = Message.obtain(null, MSG_SET_RESOURCE_ROOT) init.data.putString("PATH", resourceDir) - webContentService!!.send(init) + service!!.send(init) val msg = Message.obtain(null, MSG_TRANSFER_SOCKETS) msg.data.putParcelable("IPC_SOCKET", ParcelFileDescriptor.adoptFd(ipcFd)) msg.data.putParcelable("FD_PASSING_SOCKET", ParcelFileDescriptor.adoptFd(fdPassingFd)) - webContentService!!.send(msg) + service!!.send(msg) } override fun onServiceDisconnected(className: ComponentName) { // This is called when the connection with the service has been // unexpectedly disconnected; that is, its process crashed. - webContentService = null - boundToWebContent = false + service = null + boundToService = false // Notify owner that the service is dead onDisconnect() diff --git a/Ladybird/Android/src/main/java/org/serenityos/ladybird/WebContentService.kt b/Ladybird/Android/src/main/java/org/serenityos/ladybird/WebContentService.kt index f8230d14b3..5b7c9fd670 100644 --- a/Ladybird/Android/src/main/java/org/serenityos/ladybird/WebContentService.kt +++ b/Ladybird/Android/src/main/java/org/serenityos/ladybird/WebContentService.kt @@ -6,83 +6,13 @@ package org.serenityos.ladybird -import android.app.Service -import android.content.Intent -import android.util.Log -import android.os.ParcelFileDescriptor -import android.os.Handler -import android.os.IBinder import android.os.Message -import android.os.Messenger -import java.util.concurrent.Executors -const val MSG_SET_RESOURCE_ROOT = 1 -const val MSG_TRANSFER_SOCKETS = 2 - -class WebContentService : Service() { - private val TAG = "WebContentService" - - private val threadPool = Executors.newCachedThreadPool() - private lateinit var resourceDir: String - - override fun onCreate() { - super.onCreate() - Log.i(TAG, "Creating Service") +class WebContentService : LadybirdServiceBase("WebContentService") { + override fun handleServiceSpecificMessage(msg: Message): Boolean { + return false } - override fun onDestroy() { - super.onDestroy() - Log.i(TAG, "Destroying Service") - } - - override fun onStartCommand(intent: Intent?, flags: Int, startId: Int): Int { - Log.i(TAG, "Start command received") - return super.onStartCommand(intent, flags, startId) - } - - private fun handleTransferSockets(msg: Message) { - val bundle = msg.data - // FIXME: Handle garbage messages from wierd clients - val ipcSocket = bundle.getParcelable("IPC_SOCKET")!! - val fdSocket = bundle.getParcelable("FD_PASSING_SOCKET")!! - createThread(ipcSocket, fdSocket) - } - - private fun handleSetResourceRoot(msg: Message) { - // FIXME: Handle this being already set, not being present, etc - resourceDir = msg.data.getString("PATH")!! - - initNativeCode(resourceDir, TAG) - } - - internal class IncomingHandler( - context: WebContentService, - private val owner: WebContentService = context - ) : Handler() { - override fun handleMessage(msg: Message) { - when (msg.what) { - MSG_TRANSFER_SOCKETS -> this.owner.handleTransferSockets(msg) - MSG_SET_RESOURCE_ROOT -> this.owner.handleSetResourceRoot(msg) - else -> super.handleMessage(msg) - } - } - } - - override fun onBind(p0: Intent?): IBinder? { - // FIXME: Check the intent to make sure it's legit - return Messenger(IncomingHandler(this)).binder - } - - private external fun nativeThreadLoop(ipcSocket: Int, fdPassingSocket: Int) - - private fun createThread(ipcSocket: ParcelFileDescriptor, fdSocket: ParcelFileDescriptor) { - threadPool.execute { - nativeThreadLoop(ipcSocket.detachFd(), fdSocket.detachFd()) - } - } - - private external fun initNativeCode(resourceDir: String, tagName: String); - companion object { init { System.loadLibrary("webcontent") diff --git a/Ladybird/Android/src/main/java/org/serenityos/ladybird/WebViewImplementation.kt b/Ladybird/Android/src/main/java/org/serenityos/ladybird/WebViewImplementation.kt index cc399f7c1d..ab0eff9e0c 100644 --- a/Ladybird/Android/src/main/java/org/serenityos/ladybird/WebViewImplementation.kt +++ b/Ladybird/Android/src/main/java/org/serenityos/ladybird/WebViewImplementation.kt @@ -51,7 +51,7 @@ class WebViewImplementation(private val view: WebView) { // Functions called from native code fun bindWebContentService(ipcFd: Int, fdPassingFd: Int) { - val connector = WebContentServiceConnection(ipcFd, fdPassingFd, resourceDir) + val connector = LadybirdServiceConnection(ipcFd, fdPassingFd, resourceDir) connector.onDisconnect = { // FIXME: Notify impl that service is dead and might need restarted Log.e("WebContentView", "WebContent Died! :(") diff --git a/Ladybird/WebContent/CMakeLists.txt b/Ladybird/WebContent/CMakeLists.txt index 40d20a8cf4..20c131dd72 100644 --- a/Ladybird/WebContent/CMakeLists.txt +++ b/Ladybird/WebContent/CMakeLists.txt @@ -55,7 +55,7 @@ else() if (ANDROID) target_sources(webcontent PRIVATE ../Android/src/main/cpp/WebContentService.cpp - ../Android/src/main/cpp/WebContentServiceJNI.cpp + ../Android/src/main/cpp/LadybirdServiceBaseJNI.cpp ) target_link_libraries(webcontent PRIVATE android) endif()