Skip to content

Commit 1f977b2

Browse files
authored
fix: remove WebMessageListener before WebView.destroy() to prevent memory leaks (#431)
2 parents 246d6da + aa722fe commit 1f977b2

2 files changed

Lines changed: 40 additions & 0 deletions

File tree

sdk/forms/src/main/java/com/klaviyo/forms/webview/KlaviyoWebViewClient.kt

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,9 @@ import android.webkit.WebResourceResponse
1111
import android.webkit.WebView
1212
import android.webkit.WebViewClient as AndroidWebViewClient
1313
import androidx.core.net.toUri
14+
import androidx.webkit.WebViewCompat
15+
import androidx.webkit.WebViewFeature.WEB_MESSAGE_LISTENER
16+
import androidx.webkit.WebViewFeature.isFeatureSupported
1417
import com.klaviyo.core.Registry
1518
import com.klaviyo.core.config.Clock
1619
import com.klaviyo.core.utils.WeakReferenceDelegate
@@ -141,6 +144,18 @@ internal class KlaviyoWebViewClient() : AndroidWebViewClient(), WebViewClient, J
141144
webView?.let { webView ->
142145
Registry.threadHelper.runOnUiThread {
143146
Registry.log.verbose("Clear IAF WebView reference")
147+
val nativeBridge = Registry.get<NativeBridge>()
148+
149+
if (isFeatureSupported(WEB_MESSAGE_LISTENER)) {
150+
// Explicitly remove the listener: On some vendors' implementation of webview,
151+
// failure to remove the listener results in a memory leak.
152+
WebViewCompat.removeWebMessageListener(webView, nativeBridge.name)
153+
} else {
154+
// For completeness, remove the JS interface if WEB_MESSAGE_LISTENER
155+
// is not supported, although [destroy] should clean this up internally
156+
webView.removeJavascriptInterface(nativeBridge.name)
157+
}
158+
144159
webView.destroy()
145160
this.webView = null
146161
}

sdk/forms/src/test/java/com/klaviyo/forms/webview/KlaviyoWebViewClientTest.kt

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -151,6 +151,9 @@ class KlaviyoWebViewClientTest : BaseTest() {
151151

152152
mockkStatic(WebViewCompat::class)
153153
every { WebViewCompat.addWebMessageListener(any(), any(), any(), any()) } just runs
154+
every { WebViewCompat.removeWebMessageListener(any(), any()) } just runs
155+
156+
every { anyConstructed<KlaviyoWebView>().removeJavascriptInterface(any()) } just runs
154157
}
155158

156159
@After
@@ -345,6 +348,28 @@ class KlaviyoWebViewClientTest : BaseTest() {
345348
verifyDestroy()
346349
}
347350

351+
@Test
352+
fun `destroyWebView removes WebMessageListener when supported`() {
353+
val client = KlaviyoWebViewClient()
354+
client.initializeWebView()
355+
client.destroyWebView()
356+
357+
verify { WebViewCompat.removeWebMessageListener(any(), eq("MockNativeBridge")) }
358+
}
359+
360+
@Test
361+
fun `destroyWebView removes JavascriptInterface when WebMessageListener unsupported`() {
362+
every { WebViewFeature.isFeatureSupported(WebViewFeature.WEB_MESSAGE_LISTENER) } returns false
363+
every { anyConstructed<KlaviyoWebView>().addJavascriptInterface(any(), any()) } just runs
364+
365+
val client = KlaviyoWebViewClient()
366+
client.initializeWebView()
367+
client.destroyWebView()
368+
369+
verify { anyConstructed<KlaviyoWebView>().removeJavascriptInterface(eq("MockNativeBridge")) }
370+
verify(inverse = true) { WebViewCompat.removeWebMessageListener(any(), any()) }
371+
}
372+
348373
@Test
349374
fun `verify detachWebView fails on a null webview`() {
350375
val client = KlaviyoWebViewClient()

0 commit comments

Comments
 (0)