Skip to content

Commit

Permalink
Avoid multiple calls to onPageFinished (#4247)
Browse files Browse the repository at this point in the history
<!--
Note: This checklist is a reminder of our shared engineering
expectations.
The items in Bold are required
If your PR involves UI changes:
1. Upload screenshots or screencasts that illustrate the changes before
/ after
2. Add them under the UI changes section (feel free to add more columns
if needed)
If your PR does not involve UI changes, you can remove the **UI
changes** section

At a minimum, make sure your changes are tested in API 23 and one of the
more recent API levels available.
-->

Task/Issue URL:
https://app.asana.com/0/488551667048375/1206636072031849/f

### Description
This PR prevents logic inside onPageFinished to be executed unless
progress is 100.

### Steps to test this PR
Smoke tests
  • Loading branch information
malmstein authored Mar 7, 2024
1 parent a483bea commit a0f2ebc
Show file tree
Hide file tree
Showing 2 changed files with 83 additions and 23 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -789,6 +789,64 @@ class BrowserWebViewClientTest {
assertEquals(10L, endArgumentCaptor.firstValue)
}

@UiThreadTest
@Test
fun whenOnPageFinishedCalledBeforeCompletionThenJsCodeNotInjected() {
val mockWebView = getImmediatelyInvokedMockWebView()
whenever(mockWebView.progress).thenReturn(10)
whenever(mockWebView.settings).thenReturn(mock())

assertEquals(0, jsPlugins.plugin.countFinished)
testee.onPageFinished(mockWebView, EXAMPLE_URL)
assertEquals(0, jsPlugins.plugin.countFinished)
assertEquals(0, jsPlugins.plugin.countStarted)
}

@Test
fun whenOnPageFinishedCalledBeforeCompleteThenVerifyVerificationCompletedNotCalled() {
val mockWebView = getImmediatelyInvokedMockWebView()
whenever(mockWebView.progress).thenReturn(10)
whenever(mockWebView.settings).thenReturn(mock())
whenever(mockWebView.safeCopyBackForwardList()).thenReturn(TestBackForwardList())

testee.onPageFinished(mockWebView, EXAMPLE_URL)
verifyNoInteractions(internalTestUserChecker)
}

@UiThreadTest
@Test
fun whenOnPageFinishedCalledBeforeCompleteThenNavigationStateNotInvoked() {
val mockWebView = getImmediatelyInvokedMockWebView()
whenever(mockWebView.progress).thenReturn(10)
whenever(mockWebView.settings).thenReturn(mock())
whenever(mockWebView.safeCopyBackForwardList()).thenReturn(TestBackForwardList())

testee.onPageFinished(mockWebView, EXAMPLE_URL)
verifyNoInteractions(listener)
}

@Test
fun whenOnPageFinishedCalledThenPrintInjectorInjected() {
val mockWebView = getImmediatelyInvokedMockWebView()
whenever(mockWebView.progress).thenReturn(100)
whenever(mockWebView.settings).thenReturn(mock())
whenever(mockWebView.safeCopyBackForwardList()).thenReturn(TestBackForwardList())

testee.onPageFinished(mockWebView, EXAMPLE_URL)
verify(printInjector).injectPrint(mockWebView)
}

@Test
fun whenOnPageFinishedBeforeCompleteThenPrintInjectorNotInjected() {
val mockWebView = getImmediatelyInvokedMockWebView()
whenever(mockWebView.progress).thenReturn(10)
whenever(mockWebView.settings).thenReturn(mock())
whenever(mockWebView.safeCopyBackForwardList()).thenReturn(TestBackForwardList())

testee.onPageFinished(mockWebView, EXAMPLE_URL)
verifyNoInteractions(printInjector)
}

private class TestWebView(context: Context) : WebView(context) {
override fun getOriginalUrl(): String {
return EXAMPLE_URL
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -302,29 +302,31 @@ class BrowserWebViewClient @Inject constructor(
webView: WebView,
url: String?,
) {
jsPlugins.getPlugins().forEach {
it.onPageFinished(webView, url, webViewClientListener?.getSite())
}
url?.let {
// We call this for any url but it will only be processed for an internal tester verification url
internalTestUserChecker.verifyVerificationCompleted(it)
}
Timber.v("onPageFinished webViewUrl: ${webView.url} URL: $url")
val navigationList = webView.safeCopyBackForwardList() ?: return
webViewClientListener?.run {
navigationStateChanged(WebViewNavigationState(navigationList))
url?.let { prefetchFavicon(url) }
}
flushCookies()
printInjector.injectPrint(webView)

url?.let {
start?.let { safeStart ->
val progress = webView.progress
// See https://app.asana.com/0/0/1206159443951489/f (WebView limitations)
if (url != ABOUT_BLANK && progress == 100) {
shouldSendPageLoadedPixel(it, safeStart, currentTimeProvider.getTimeInMillis())
start = null
Timber.v("onPageFinished webViewUrl: ${webView.url} URL: $url progress: ${webView.progress}")
if (webView.progress == 100) {
jsPlugins.getPlugins().forEach {
it.onPageFinished(webView, url, webViewClientListener?.getSite())
}
url?.let {
// We call this for any url but it will only be processed for an internal tester verification url
internalTestUserChecker.verifyVerificationCompleted(it)
}
val navigationList = webView.safeCopyBackForwardList() ?: return
webViewClientListener?.run {
navigationStateChanged(WebViewNavigationState(navigationList))
url?.let { prefetchFavicon(url) }
}
flushCookies()
printInjector.injectPrint(webView)

url?.let {
start?.let { safeStart ->
val progress = webView.progress
// See https://app.asana.com/0/0/1206159443951489/f (WebView limitations)
if (url != ABOUT_BLANK) {
shouldSendPageLoadedPixel(it, safeStart, currentTimeProvider.getTimeInMillis())
start = null
}
}
}
}
Expand Down

0 comments on commit a0f2ebc

Please sign in to comment.