From b98895219aba9a21374fbb35a13b68039dea8247 Mon Sep 17 00:00:00 2001 From: Eliot the Cougar Date: Mon, 21 Sep 2026 09:32:15 +0300 Subject: [PATCH] Fix WebView Browser Dialer recovery after network handovers (#6229) * Confine WebView dialer lifecycle to its main looper Preserve synchronous start/stop completion and failure propagation for worker-thread network reloads. Supersede queued lifecycle requests and prevent callbacks from reviving destroyed views. Add JVM and real-WebView regression coverage for issue #6228. * Keep WebView device tests focused on framework thread ownership * Remove WebView dialer instrumentation tests --- .../v2ray/ang/service/DialerWebviewService.kt | 114 +++++++--- .../ang/service/DialerWebviewServiceTest.kt | 206 ++++++++++++++++++ 2 files changed, 286 insertions(+), 34 deletions(-) create mode 100644 V2rayNG/app/src/test/java/com/v2ray/ang/service/DialerWebviewServiceTest.kt diff --git a/V2rayNG/app/src/main/java/com/v2ray/ang/service/DialerWebviewService.kt b/V2rayNG/app/src/main/java/com/v2ray/ang/service/DialerWebviewService.kt index 86f48954c..42dd5bc18 100644 --- a/V2rayNG/app/src/main/java/com/v2ray/ang/service/DialerWebviewService.kt +++ b/V2rayNG/app/src/main/java/com/v2ray/ang/service/DialerWebviewService.kt @@ -8,18 +8,21 @@ import android.webkit.WebSettings import android.webkit.WebView import android.webkit.WebViewClient import com.v2ray.ang.contracts.IDialerService +import java.util.concurrent.ExecutionException +import java.util.concurrent.FutureTask +import java.util.concurrent.atomic.AtomicLong class DialerWebviewService : IDialerService { private var webView: WebView? = null private val handler = Handler(Looper.getMainLooper()) + private val lifecycleVersion = AtomicLong() private val keepAliveInterval = 30_000L // 30 seconds private val keepAliveRunnable = object : Runnable { override fun run() { - webView?.let { - it.resumeTimers() - it.onResume() - } + val view = webView ?: return + view.resumeTimers() + view.onResume() handler.postDelayed(this, keepAliveInterval) } } @@ -30,45 +33,88 @@ class DialerWebviewService : IDialerService { * @param context Service context */ @SuppressLint("SetJavaScriptEnabled") - override fun start(context: Context, dialerAddr: String) { - if (webView != null) stop() - if (dialerAddr.isEmpty()) return + override fun start(context: Context, dialerAddr: String) = updateOnMainThread { + stopOnMainThread() + if (dialerAddr.isEmpty()) return@updateOnMainThread val dialerUrl = "http://$dialerAddr/" - webView = WebView(context.applicationContext).apply { - settings.apply { - javaScriptEnabled = true - domStorageEnabled = true - // Allow JS to run even if not triggered by user - mediaPlaybackRequiresUserGesture = false - // Prevent aggressive caching issues - cacheMode = WebSettings.LOAD_DEFAULT - } - - webViewClient = object : WebViewClient() { - override fun onPageFinished(view: WebView?, url: String?) { - super.onPageFinished(view, url) - view?.onResume() - view?.resumeTimers() + val view = WebView(context.applicationContext) + webView = view + try { + view.apply { + settings.apply { + javaScriptEnabled = true + domStorageEnabled = true + // Allow JS to run even if not triggered by user + mediaPlaybackRequiresUserGesture = false + // Prevent aggressive caching issues + cacheMode = WebSettings.LOAD_DEFAULT } + + webViewClient = object : WebViewClient() { + override fun onPageFinished(view: WebView?, url: String?) { + if (view === webView) { + view?.onResume() + view?.resumeTimers() + } + } + } + + loadUrl(dialerUrl) } - - loadUrl(dialerUrl) + handler.post(keepAliveRunnable) + } catch (e: Exception) { + try { + stopOnMainThread() + } catch (cleanupError: Exception) { + e.addSuppressed(cleanupError) + } + throw e } - - handler.post(keepAliveRunnable) } - override fun stop() { + override fun stop() = updateOnMainThread { stopOnMainThread() } + + private fun stopOnMainThread() { handler.removeCallbacks(keepAliveRunnable) - webView?.apply { - stopLoading() - pauseTimers() - // Important to call onPause to stop internal Chromium threads properly - onPause() - destroy() - } + val view = webView ?: return webView = null + view.apply { + try { + stopLoading() + pauseTimers() + // Important to call onPause to stop internal Chromium threads properly + onPause() + } finally { + destroy() + } + } + } + + private fun updateOnMainThread(action: () -> Unit) { + val version = lifecycleVersion.incrementAndGet() + val update = { + // A stop or replacement can overtake a worker's queued start on the main thread. + if (lifecycleVersion.get() == version) action() + } + if (Looper.myLooper() == handler.looper) { + update() + return + } + + // WebView requires its creating looper on all Android versions. Keep the dialer's + // synchronous contract so reload waits for cleanup/setup and receives any failure. + val task = FutureTask(update) + check(handler.post(task)) { "WebView looper is shutting down" } + try { + task.get() + } catch (e: ExecutionException) { + throw e.cause ?: e + } catch (e: InterruptedException) { + task.cancel(false) + Thread.currentThread().interrupt() + throw e + } } } diff --git a/V2rayNG/app/src/test/java/com/v2ray/ang/service/DialerWebviewServiceTest.kt b/V2rayNG/app/src/test/java/com/v2ray/ang/service/DialerWebviewServiceTest.kt new file mode 100644 index 000000000..cffb83355 --- /dev/null +++ b/V2rayNG/app/src/test/java/com/v2ray/ang/service/DialerWebviewServiceTest.kt @@ -0,0 +1,206 @@ +package com.v2ray.ang.service + +import android.content.Context +import android.os.Handler +import android.os.Looper +import android.webkit.WebSettings +import android.webkit.WebView +import android.webkit.WebViewClient +import org.junit.Assert.assertEquals +import org.junit.Assert.assertFalse +import org.junit.Assert.assertSame +import org.junit.Assert.assertThrows +import org.junit.Assert.assertTrue +import org.junit.Test +import org.mockito.Mockito.mockConstruction +import org.mockito.Mockito.mockStatic +import org.mockito.kotlin.any +import org.mockito.kotlin.argumentCaptor +import org.mockito.kotlin.clearInvocations +import org.mockito.kotlin.doThrow +import org.mockito.kotlin.mock +import org.mockito.kotlin.never +import org.mockito.kotlin.times +import org.mockito.kotlin.verify +import org.mockito.kotlin.whenever +import java.util.concurrent.ExecutionException +import java.util.concurrent.Executors +import java.util.concurrent.Future +import java.util.concurrent.FutureTask +import java.util.concurrent.LinkedBlockingQueue +import java.util.concurrent.TimeUnit + +class DialerWebviewServiceTest { + @Test + fun workerStartWaitsForMainThreadSetup() = Fixture().use { f -> + val start = f.onWorker { f.dialer.start(f.context, "127.0.0.1:10001") } + val queued = f.nextUpdate() + assertFalse(start.isDone) + assertTrue(f.views.isEmpty()) + queued.run() + start.get(5, TimeUnit.SECONDS) + verify(f.views.single()).loadUrl("http://127.0.0.1:10001/") + } + + @Test + fun workerStopCompletesCleanupBeforeReturning() = Fixture().use { f -> + f.dialer.start(f.context, "127.0.0.1:10001") + val view = f.views.single() + val stop = f.onWorker { f.dialer.stop() } + val queued = f.nextUpdate() + assertFalse(stop.isDone) + verify(view, never()).destroy() + queued.run() + stop.get(5, TimeUnit.SECONDS) + verify(view).stopLoading() + verify(view).pauseTimers() + verify(view).onPause() + verify(view).destroy() + } + + @Test + fun restartReplacesTheViewAndRepeatedStopDestroysItOnce() = Fixture().use { f -> + f.dialer.start(f.context, "127.0.0.1:10001") + f.dialer.start(f.context, "127.0.0.1:10002") + verify(f.views.first()).destroy() + verify(f.views.last()).loadUrl("http://127.0.0.1:10002/") + f.dialer.stop() + f.dialer.stop() + f.views.forEach { verify(it, times(1)).destroy() } + } + + @Test + fun mainThreadStopInvalidatesAQueuedWorkerStart() = Fixture().use { f -> + val start = f.onWorker { f.dialer.start(f.context, "127.0.0.1:10001") } + val queued = f.nextUpdate() + f.dialer.stop() + queued.run() + start.get(5, TimeUnit.SECONDS) + assertTrue(f.views.isEmpty()) + } + + @Test + fun interruptedWorkerCannotLeaveAQueuedStartBehind() = Fixture().use { f -> + val start = f.onWorker { f.dialer.start(f.context, "127.0.0.1:10001") } + val queued = f.nextUpdate() + start.cancel(true) + // The single worker reaches this marker only after handling the interruption. + f.onWorker {}.get(5, TimeUnit.SECONDS) + queued.run() + assertTrue(f.views.isEmpty()) + } + + @Test + fun rejectedMainThreadDispatchFailsWithoutWaiting() = Fixture().use { f -> + whenever(f.handler.post(any())).thenReturn(false) + val start = f.onWorker { f.dialer.start(f.context, "127.0.0.1:10001") } + val thrown = assertThrows(ExecutionException::class.java) { start.get(5, TimeUnit.SECONDS) } + assertTrue(thrown.cause is IllegalStateException) + assertTrue(f.views.isEmpty()) + } + + @Test + fun oldQueuedStopCannotDestroyANewerStart() = Fixture().use { f -> + f.dialer.start(f.context, "127.0.0.1:10001") + val stop = f.onWorker { f.dialer.stop() } + val queued = f.nextUpdate() + f.dialer.start(f.context, "127.0.0.1:10002") + queued.run() + stop.get(5, TimeUnit.SECONDS) + verify(f.views.first()).destroy() + verify(f.views.last(), never()).destroy() + } + + @Test + fun setupFailureCleansUpAndReachesTheWorkerUnwrapped() = Fixture().use { f -> + val failure = IllegalStateException("WebView setup failed") + f.loadFailure = failure + val start = f.onWorker { f.dialer.start(f.context, "127.0.0.1:10001") } + f.nextUpdate().run() + val thrown = assertThrows(ExecutionException::class.java) { start.get(5, TimeUnit.SECONDS) } + assertSame(failure, thrown.cause) + verify(f.views.single()).destroy() + f.dialer.stop() + verify(f.views.single(), times(1)).destroy() + } + + @Test + fun cleanupFailureStillDestroysTheViewAndReachesTheCaller() = Fixture().use { f -> + f.dialer.start(f.context, "127.0.0.1:10001") + val view = f.views.single() + val failure = IllegalStateException("WebView stop failed") + doThrow(failure).whenever(view).stopLoading() + assertSame(failure, assertThrows(IllegalStateException::class.java) { f.dialer.stop() }) + f.dialer.stop() + verify(view, times(1)).destroy() + } + + @Test + fun callbacksCannotResumeADestroyedViewOrRescheduleKeepAlive(): Unit = Fixture().use { f -> + f.dialer.start(f.context, "127.0.0.1:10001") + val view = f.views.single() + val client = argumentCaptor() + verify(view).webViewClient = client.capture() + val keepAlive = f.keepAlives.single() + f.dialer.stop() + clearInvocations(view, f.handler) + client.firstValue.onPageFinished(view, "http://127.0.0.1:10001/") + keepAlive.run() + verify(view, never()).onResume() + verify(view, never()).resumeTimers() + verify(f.handler, never()).postDelayed(any(), any()) + } + + @Test + fun emptyAddressStopsThePreviousViewWithoutCreatingAnother() = Fixture().use { f -> + f.dialer.start(f.context, "127.0.0.1:10001") + f.dialer.start(f.context, "") + assertEquals(1, f.views.size) + verify(f.views.single()).destroy() + } + + private class Fixture : AutoCloseable { + private val mainLooper = mock() + private val loopers = mockStatic(Looper::class.java).apply { + `when` { Looper.getMainLooper() }.thenReturn(mainLooper) + `when` { Looper.myLooper() }.thenReturn(mainLooper) + } + private val updates = LinkedBlockingQueue() + private val worker = Executors.newSingleThreadExecutor() + val keepAlives = mutableListOf() + var loadFailure: Exception? = null + val context = mock().also { whenever(it.applicationContext).thenReturn(it) } + private val handlers = mockConstruction(Handler::class.java) { handler, _ -> + whenever(handler.looper).thenReturn(mainLooper) + whenever(handler.post(any())).thenAnswer { + val task = it.getArgument(0) + if (task is FutureTask<*>) updates.add(task) else keepAlives.add(task) + true + } + } + private val webViews = mockConstruction(WebView::class.java) { view, _ -> + whenever(view.settings).thenReturn(mock()) + loadFailure?.let { doThrow(it).whenever(view).loadUrl(any()) } + } + val dialer = DialerWebviewService() + val handler get() = handlers.constructed().single() + val views get() = webViews.constructed() + + fun onWorker(action: () -> Unit): Future<*> = worker.submit { + // Static mocks are thread-local: this worker has no Android looper. + mockStatic(Looper::class.java).use { action() } + } + + fun nextUpdate(): Runnable = checkNotNull(updates.poll(5, TimeUnit.SECONDS)) { + "Worker did not dispatch its WebView operation to the main looper" + } + + override fun close() { + worker.shutdownNow() + worker.awaitTermination(5, TimeUnit.SECONDS) + webViews.close() + handlers.close() + loopers.close() + } + } +}