From fd2ce39ca248b379e2ab8b3d5e0a8ed7635ab253 Mon Sep 17 00:00:00 2001 From: "codrut.topliceanu" Date: Wed, 8 Sep 2021 12:21:24 +0300 Subject: [PATCH] For #10867 - Dismisses user prompt when GV calls `onPromptDismiss` --- .../gecko/prompt/GeckoPromptDelegate.kt | 6 ++- .../prompt/PromptInstanceDismissDelegate.kt | 21 ++++++++++ .../gecko/prompt/GeckoPromptDelegateTest.kt | 20 ++++++++++ .../PromptInstanceDismissDelegateTest.kt | 38 +++++++++++++++++++ .../browser/state/engine/EngineObserver.kt | 6 +++ .../concept/engine/EngineSession.kt | 5 +++ .../feature/prompts/PromptFeature.kt | 12 +++--- .../feature/prompts/PromptFeatureTest.kt | 30 +++++++++++++++ 8 files changed, 132 insertions(+), 6 deletions(-) create mode 100644 components/browser/engine-gecko/src/main/java/mozilla/components/browser/engine/gecko/prompt/PromptInstanceDismissDelegate.kt create mode 100644 components/browser/engine-gecko/src/test/java/mozilla/components/browser/engine/gecko/prompt/PromptInstanceDismissDelegateTest.kt diff --git a/components/browser/engine-gecko/src/main/java/mozilla/components/browser/engine/gecko/prompt/GeckoPromptDelegate.kt b/components/browser/engine-gecko/src/main/java/mozilla/components/browser/engine/gecko/prompt/GeckoPromptDelegate.kt index c2c51660f1f..e8b129f94ab 100644 --- a/components/browser/engine-gecko/src/main/java/mozilla/components/browser/engine/gecko/prompt/GeckoPromptDelegate.kt +++ b/components/browser/engine-gecko/src/main/java/mozilla/components/browser/engine/gecko/prompt/GeckoPromptDelegate.kt @@ -125,7 +125,11 @@ internal class GeckoPromptDelegate(private val geckoEngineSession: GeckoEngineSe logins = prompt.options.map { it.value.toLogin() }, onConfirm = onConfirmSave, onDismiss = onDismiss - ) + ).also { + prompt.delegate = PromptInstanceDismissDelegate( + geckoEngineSession, it + ) + } ) } return geckoResult diff --git a/components/browser/engine-gecko/src/main/java/mozilla/components/browser/engine/gecko/prompt/PromptInstanceDismissDelegate.kt b/components/browser/engine-gecko/src/main/java/mozilla/components/browser/engine/gecko/prompt/PromptInstanceDismissDelegate.kt new file mode 100644 index 00000000000..f80ddfb5e75 --- /dev/null +++ b/components/browser/engine-gecko/src/main/java/mozilla/components/browser/engine/gecko/prompt/PromptInstanceDismissDelegate.kt @@ -0,0 +1,21 @@ +/* This Source Code Form is subject to the terms of the Mozilla Public + * License, v. 2.0. If a copy of the MPL was not distributed with this + * file, You can obtain one at http://mozilla.org/MPL/2.0/. */ + +package mozilla.components.browser.engine.gecko.prompt + +import mozilla.components.browser.engine.gecko.GeckoEngineSession +import mozilla.components.concept.engine.prompt.PromptRequest +import org.mozilla.geckoview.GeckoSession + +internal class PromptInstanceDismissDelegate( + private val geckoSession: GeckoEngineSession, + private val promptRequest: PromptRequest +) : GeckoSession.PromptDelegate.PromptInstanceDelegate { + + override fun onPromptDismiss(prompt: GeckoSession.PromptDelegate.BasePrompt) { + geckoSession.notifyObservers { + onPromptDismissed(promptRequest) + } + } +} diff --git a/components/browser/engine-gecko/src/test/java/mozilla/components/browser/engine/gecko/prompt/GeckoPromptDelegateTest.kt b/components/browser/engine-gecko/src/test/java/mozilla/components/browser/engine/gecko/prompt/GeckoPromptDelegateTest.kt index a66fba6eda3..bd09e13cb4e 100644 --- a/components/browser/engine-gecko/src/test/java/mozilla/components/browser/engine/gecko/prompt/GeckoPromptDelegateTest.kt +++ b/components/browser/engine-gecko/src/test/java/mozilla/components/browser/engine/gecko/prompt/GeckoPromptDelegateTest.kt @@ -682,6 +682,26 @@ class GeckoPromptDelegateTest { assertFalse(onLoginSaved) } + @Test + fun `Calling onLoginSave must set a PromptInstanceDismissDelegate`() { + val mockSession = GeckoEngineSession(runtime) + var loginSaveRequest: PromptRequest.SaveLoginPrompt = mock() + val promptDelegate = spy(GeckoPromptDelegate(mockSession)) + mockSession.register(object : EngineSession.Observer { + override fun onPromptRequest(promptRequest: PromptRequest) { + loginSaveRequest = promptRequest as PromptRequest.SaveLoginPrompt + } + }) + val login = createLogin() + val saveOption = Autocomplete.LoginSaveOption(login.toLoginEntry()) + val saveLoginPrompt = spy(geckoLoginSavePrompt(arrayOf(saveOption))) + + promptDelegate.onLoginSave(mock(), saveLoginPrompt) + + assertNotNull(loginSaveRequest) + assertNotNull(saveLoginPrompt.delegate) + } + @Test fun `Calling onLoginSelect must provide an SelectLoginPrompt PromptRequest`() { val mockSession = GeckoEngineSession(runtime) diff --git a/components/browser/engine-gecko/src/test/java/mozilla/components/browser/engine/gecko/prompt/PromptInstanceDismissDelegateTest.kt b/components/browser/engine-gecko/src/test/java/mozilla/components/browser/engine/gecko/prompt/PromptInstanceDismissDelegateTest.kt new file mode 100644 index 00000000000..22b33bea1d2 --- /dev/null +++ b/components/browser/engine-gecko/src/test/java/mozilla/components/browser/engine/gecko/prompt/PromptInstanceDismissDelegateTest.kt @@ -0,0 +1,38 @@ +/* This Source Code Form is subject to the terms of the Mozilla Public + * License, v. 2.0. If a copy of the MPL was not distributed with this + * file, You can obtain one at http://mozilla.org/MPL/2.0/. */ +package mozilla.components.browser.engine.gecko.prompt + +import androidx.test.ext.junit.runners.AndroidJUnit4 +import mozilla.components.browser.engine.gecko.GeckoEngineSession +import mozilla.components.concept.engine.EngineSession +import mozilla.components.concept.engine.prompt.PromptRequest +import mozilla.components.support.test.mock +import org.junit.Assert.assertTrue +import org.junit.Test +import org.junit.runner.RunWith +import org.mozilla.geckoview.Autocomplete +import org.mozilla.geckoview.GeckoSession + +@RunWith(AndroidJUnit4::class) +class PromptInstanceDismissDelegateTest { + + @Test + fun `GIVEN delegate with promptRequest WHEN onPromptDismiss called from geckoview THEN notifyObservers the prompt is dismissed`() { + val mockSession = GeckoEngineSession(mock()) + var onDismissWasCalled = false + mockSession.register(object : EngineSession.Observer { + override fun onPromptDismissed(promptRequest: PromptRequest) { + super.onPromptDismissed(promptRequest) + onDismissWasCalled = true + } + }) + val basePrompt: GeckoSession.PromptDelegate.AutocompleteRequest = mock() + val prompt: PromptRequest = mock() + val delegate = PromptInstanceDismissDelegate(mockSession, prompt) + + delegate.onPromptDismiss(basePrompt) + + assertTrue(onDismissWasCalled) + } +} diff --git a/components/browser/state/src/main/java/mozilla/components/browser/state/engine/EngineObserver.kt b/components/browser/state/src/main/java/mozilla/components/browser/state/engine/EngineObserver.kt index ea415d22379..81e52ce6699 100644 --- a/components/browser/state/src/main/java/mozilla/components/browser/state/engine/EngineObserver.kt +++ b/components/browser/state/src/main/java/mozilla/components/browser/state/engine/EngineObserver.kt @@ -256,6 +256,12 @@ internal class EngineObserver( ) } + override fun onPromptDismissed(promptRequest: PromptRequest) { + store.dispatch( + ContentAction.ConsumePromptRequestAction(tabId, promptRequest) + ) + } + override fun onRepostPromptCancelled() { store.dispatch(ContentAction.UpdateRefreshCanceledStateAction(tabId, true)) } diff --git a/components/concept/engine/src/main/java/mozilla/components/concept/engine/EngineSession.kt b/components/concept/engine/src/main/java/mozilla/components/concept/engine/EngineSession.kt index 345c5afdb22..643c39bd46c 100644 --- a/components/concept/engine/src/main/java/mozilla/components/concept/engine/EngineSession.kt +++ b/components/concept/engine/src/main/java/mozilla/components/concept/engine/EngineSession.kt @@ -74,6 +74,11 @@ abstract class EngineSession( fun onCancelContentPermissionRequest(permissionRequest: PermissionRequest) = Unit fun onPromptRequest(promptRequest: PromptRequest) = Unit + /** + * The engine has requested a prompt be dismissed. + */ + fun onPromptDismissed(promptRequest: PromptRequest) = Unit + /** * User cancelled a repost prompt. Page will not be reloaded. */ diff --git a/components/feature/prompts/src/main/java/mozilla/components/feature/prompts/PromptFeature.kt b/components/feature/prompts/src/main/java/mozilla/components/feature/prompts/PromptFeature.kt index 92187e46734..e9a090ca5d2 100644 --- a/components/feature/prompts/src/main/java/mozilla/components/feature/prompts/PromptFeature.kt +++ b/components/feature/prompts/src/main/java/mozilla/components/feature/prompts/PromptFeature.kt @@ -298,12 +298,14 @@ class PromptFeature private constructor( arrayOf(it?.content?.promptRequests, it?.content?.loading) } .collect { state -> - state?.content?.let { - if (it.promptRequests.lastOrNull() != activePromptRequest) { + state?.content?.let { content -> + if (content.promptRequests.lastOrNull() != activePromptRequest) { // Dismiss any active select login or credit card prompt if it does // not match the current prompt request for the session. if (activePromptRequest is SelectLoginPrompt) { loginPicker?.dismissCurrentLoginSelect(activePromptRequest as SelectLoginPrompt) + } else if (activePromptRequest is SaveLoginPrompt) { + (activePrompt?.get() as? SaveLoginDialogFragment)?.dismissAllowingStateLoss() } else if (activePromptRequest is SelectCreditCard) { creditCardPicker?.dismissSelectCreditCardRequest( activePromptRequest as SelectCreditCard @@ -311,13 +313,13 @@ class PromptFeature private constructor( } onPromptRequested(state) - } else if (!it.loading) { + } else if (!content.loading) { promptAbuserDetector.resetJSAlertAbuseState() - } else if (it.loading) { + } else if (content.loading) { dismissSelectPrompts() } - activePromptRequest = it.promptRequests.lastOrNull() + activePromptRequest = content.promptRequests.lastOrNull() } } } diff --git a/components/feature/prompts/src/test/java/mozilla/components/feature/prompts/PromptFeatureTest.kt b/components/feature/prompts/src/test/java/mozilla/components/feature/prompts/PromptFeatureTest.kt index 9931e2672b9..6a032a31849 100644 --- a/components/feature/prompts/src/test/java/mozilla/components/feature/prompts/PromptFeatureTest.kt +++ b/components/feature/prompts/src/test/java/mozilla/components/feature/prompts/PromptFeatureTest.kt @@ -300,6 +300,36 @@ class PromptFeatureTest { assertEquals(true, result) } + @Test + fun `GIVEN saveLoginPrompt is visible WHEN prompt is removed from state THEN dismiss saveLoginPrompt`() { + // given + val saveLoginPrompt: SaveLoginDialogFragment = mock() + val promptRequest: PromptRequest.SaveLoginPrompt = mock() + + store.dispatch(ContentAction.UpdatePromptRequestAction(tabId, promptRequest)) + .joinBlocking() + store.waitUntilIdle() + + val feature = spy( + PromptFeature( + mock(), + store, + fragmentManager = fragmentManager + ) { } + ) + + feature.start() + feature.activePrompt = WeakReference(saveLoginPrompt) + feature.activePromptRequest = promptRequest + + // when + store.dispatch(ContentAction.ConsumePromptRequestAction(tabId, promptRequest)) + .joinBlocking() + + // then + verify(saveLoginPrompt).dismissAllowingStateLoss() + } + @Test fun `GIVEN loginPickerView is not visible WHEN dismissSelectPrompts THEN dismissCurrentLoginSelect called and false returned`() { // given