From 18dcdc0e64d7c1d09c9edd1ab64a874a8208e052 Mon Sep 17 00:00:00 2001 From: Benoit Marty Date: Fri, 23 Aug 2024 16:17:57 +0200 Subject: [PATCH 1/7] Communicate with Element Call about PiP status. Also only use eventSink to communicate with the Presenter, instead of having public methods. Change WeakReference to an Activity to a listener and update tests. --- .../call/impl/pip/PictureInPictureEvents.kt | 4 + .../impl/pip/PictureInPicturePresenter.kt | 94 +++++++++--------- .../features/call/impl/pip/PipActivity.kt | 23 +++++ .../features/call/impl/ui/CallScreenView.kt | 3 + .../call/impl/ui/ElementCallActivity.kt | 51 +++++++++- .../features/call/impl/utils/WebPipApi.kt | 23 +++++ .../call/impl/utils/WebViewWebPipApi.kt | 41 ++++++++ .../features/call/impl/pip/FakePipActivity.kt | 29 ++++++ .../features/call/impl/pip/FakeWebPipApi.kt | 32 +++++++ .../impl/pip/PictureInPicturePresenterTest.kt | 96 ++++++++++++++----- 10 files changed, 317 insertions(+), 79 deletions(-) create mode 100644 features/call/impl/src/main/kotlin/io/element/android/features/call/impl/pip/PipActivity.kt create mode 100644 features/call/impl/src/main/kotlin/io/element/android/features/call/impl/utils/WebPipApi.kt create mode 100644 features/call/impl/src/main/kotlin/io/element/android/features/call/impl/utils/WebViewWebPipApi.kt create mode 100644 features/call/impl/src/test/kotlin/io/element/android/features/call/impl/pip/FakePipActivity.kt create mode 100644 features/call/impl/src/test/kotlin/io/element/android/features/call/impl/pip/FakeWebPipApi.kt diff --git a/features/call/impl/src/main/kotlin/io/element/android/features/call/impl/pip/PictureInPictureEvents.kt b/features/call/impl/src/main/kotlin/io/element/android/features/call/impl/pip/PictureInPictureEvents.kt index da3c08da32..be376aa079 100644 --- a/features/call/impl/src/main/kotlin/io/element/android/features/call/impl/pip/PictureInPictureEvents.kt +++ b/features/call/impl/src/main/kotlin/io/element/android/features/call/impl/pip/PictureInPictureEvents.kt @@ -16,6 +16,10 @@ package io.element.android.features.call.impl.pip +import io.element.android.features.call.impl.utils.WebPipApi + sealed interface PictureInPictureEvents { + data class SetupWebPipApi(val webPipApi: WebPipApi) : PictureInPictureEvents data object EnterPictureInPicture : PictureInPictureEvents + data class OnPictureInPictureModeChanged(val isInPip: Boolean) : PictureInPictureEvents } diff --git a/features/call/impl/src/main/kotlin/io/element/android/features/call/impl/pip/PictureInPicturePresenter.kt b/features/call/impl/src/main/kotlin/io/element/android/features/call/impl/pip/PictureInPicturePresenter.kt index 2c974382d0..78d347d715 100644 --- a/features/call/impl/src/main/kotlin/io/element/android/features/call/impl/pip/PictureInPicturePresenter.kt +++ b/features/call/impl/src/main/kotlin/io/element/android/features/call/impl/pip/PictureInPicturePresenter.kt @@ -16,17 +16,17 @@ package io.element.android.features.call.impl.pip -import android.app.Activity -import android.app.PictureInPictureParams -import android.os.Build -import android.util.Rational -import androidx.annotation.RequiresApi import androidx.compose.runtime.Composable +import androidx.compose.runtime.getValue import androidx.compose.runtime.mutableStateOf +import androidx.compose.runtime.remember +import androidx.compose.runtime.rememberCoroutineScope +import androidx.compose.runtime.setValue +import io.element.android.features.call.impl.utils.WebPipApi import io.element.android.libraries.architecture.Presenter import io.element.android.libraries.core.log.logger.LoggerTag +import kotlinx.coroutines.launch import timber.log.Timber -import java.lang.ref.WeakReference import javax.inject.Inject private val loggerTag = LoggerTag("PiP") @@ -35,71 +35,69 @@ class PictureInPicturePresenter @Inject constructor( pipSupportProvider: PipSupportProvider, ) : Presenter { private val isPipSupported = pipSupportProvider.isPipSupported() - private var isInPictureInPicture = mutableStateOf(false) - private var hostActivity: WeakReference? = null + private var pipActivity: PipActivity? = null @Composable override fun present(): PictureInPictureState { + val coroutineScope = rememberCoroutineScope() + var isInPictureInPicture by remember { mutableStateOf(false) } + var webPipApi by remember { mutableStateOf(null) } + fun handleEvent(event: PictureInPictureEvents) { when (event) { - PictureInPictureEvents.EnterPictureInPicture -> switchToPip() + is PictureInPictureEvents.SetupWebPipApi -> { + webPipApi = event.webPipApi + } + PictureInPictureEvents.EnterPictureInPicture -> { + coroutineScope.launch { + switchToPip(webPipApi) + } + } + is PictureInPictureEvents.OnPictureInPictureModeChanged -> { + Timber.tag(loggerTag.value).d("onPictureInPictureModeChanged: ${event.isInPip}") + isInPictureInPicture = event.isInPip + if (event.isInPip) { + webPipApi?.enterPip() + } else { + webPipApi?.exitPip() + } + } } } return PictureInPictureState( supportPip = isPipSupported, - isInPictureInPicture = isInPictureInPicture.value, + isInPictureInPicture = isInPictureInPicture, eventSink = ::handleEvent, ) } - fun onCreate(activity: Activity) { + fun setPipActivity(pipActivity: PipActivity?) { if (isPipSupported) { - Timber.tag(loggerTag.value).d("onCreate: Setting PiP params") - hostActivity = WeakReference(activity) - hostActivity?.get()?.setPictureInPictureParams(getPictureInPictureParams()) + Timber.tag(loggerTag.value).d("Setting PiP params") + this.pipActivity = pipActivity + pipActivity?.setPipParams() } else { Timber.tag(loggerTag.value).d("onCreate: PiP is not supported") } } - fun onDestroy() { - Timber.tag(loggerTag.value).d("onDestroy") - hostActivity?.clear() - hostActivity = null - } - - @RequiresApi(Build.VERSION_CODES.O) - private fun getPictureInPictureParams(): PictureInPictureParams { - return PictureInPictureParams.Builder() - // Portrait for calls seems more appropriate - .setAspectRatio(Rational(3, 5)) - .apply { - if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.S) { - setAutoEnterEnabled(true) - } - } - .build() - } - /** - * Enters Picture-in-Picture mode. + * Enters Picture-in-Picture mode, if allowed by Element Call. */ - private fun switchToPip() { + private suspend fun switchToPip(webPipApi: WebPipApi?) { if (isPipSupported) { - Timber.tag(loggerTag.value).d("Switch to PiP mode") - hostActivity?.get()?.enterPictureInPictureMode(getPictureInPictureParams()) - ?.also { Timber.tag(loggerTag.value).d("Switch to PiP mode result: $it") } + if (webPipApi == null) { + Timber.tag(loggerTag.value).w("webPipApi is not available") + } + if (webPipApi == null || webPipApi.canEnterPip()) { + Timber.tag(loggerTag.value).d("Switch to PiP mode") + pipActivity?.enterPipMode() + ?.also { Timber.tag(loggerTag.value).d("Switch to PiP mode result: $it") } + } else { + Timber.tag(loggerTag.value).w("Cannot enter PiP mode, hangup the call") + pipActivity?.hangUp() + } } } - - fun onPictureInPictureModeChanged(isInPictureInPictureMode: Boolean) { - Timber.tag(loggerTag.value).d("onPictureInPictureModeChanged: $isInPictureInPictureMode") - isInPictureInPicture.value = isInPictureInPictureMode - } - - fun onUserLeaveHint() { - Timber.tag(loggerTag.value).d("onUserLeaveHint") - switchToPip() - } } diff --git a/features/call/impl/src/main/kotlin/io/element/android/features/call/impl/pip/PipActivity.kt b/features/call/impl/src/main/kotlin/io/element/android/features/call/impl/pip/PipActivity.kt new file mode 100644 index 0000000000..6b86988ae2 --- /dev/null +++ b/features/call/impl/src/main/kotlin/io/element/android/features/call/impl/pip/PipActivity.kt @@ -0,0 +1,23 @@ +/* + * Copyright (c) 2024 New Vector Ltd + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package io.element.android.features.call.impl.pip + +interface PipActivity { + fun setPipParams() + fun enterPipMode(): Boolean + fun hangUp() +} diff --git a/features/call/impl/src/main/kotlin/io/element/android/features/call/impl/ui/CallScreenView.kt b/features/call/impl/src/main/kotlin/io/element/android/features/call/impl/ui/CallScreenView.kt index 70ab2e30c2..e81d386dc5 100644 --- a/features/call/impl/src/main/kotlin/io/element/android/features/call/impl/ui/CallScreenView.kt +++ b/features/call/impl/src/main/kotlin/io/element/android/features/call/impl/ui/CallScreenView.kt @@ -40,6 +40,7 @@ import io.element.android.features.call.impl.pip.PictureInPictureEvents import io.element.android.features.call.impl.pip.PictureInPictureState import io.element.android.features.call.impl.pip.PictureInPictureStateProvider import io.element.android.features.call.impl.pip.aPictureInPictureState +import io.element.android.features.call.impl.utils.WebViewWebPipApi import io.element.android.features.call.impl.utils.WebViewWidgetMessageInterceptor import io.element.android.libraries.architecture.AsyncData import io.element.android.libraries.designsystem.components.ProgressDialog @@ -108,6 +109,8 @@ internal fun CallScreenView( onWebViewCreate = { webView -> val interceptor = WebViewWidgetMessageInterceptor(webView) state.eventSink(CallScreenEvents.SetupMessageChannels(interceptor)) + val webPipApi = WebViewWebPipApi(webView) + pipState.eventSink(PictureInPictureEvents.SetupWebPipApi(webPipApi)) } ) when (state.urlState) { diff --git a/features/call/impl/src/main/kotlin/io/element/android/features/call/impl/ui/ElementCallActivity.kt b/features/call/impl/src/main/kotlin/io/element/android/features/call/impl/ui/ElementCallActivity.kt index 3af6c7cf95..aace025bb4 100644 --- a/features/call/impl/src/main/kotlin/io/element/android/features/call/impl/ui/ElementCallActivity.kt +++ b/features/call/impl/src/main/kotlin/io/element/android/features/call/impl/ui/ElementCallActivity.kt @@ -17,6 +17,7 @@ package io.element.android.features.call.impl.ui import android.Manifest +import android.app.PictureInPictureParams import android.content.Intent import android.content.res.Configuration import android.media.AudioAttributes @@ -24,11 +25,13 @@ import android.media.AudioFocusRequest import android.media.AudioManager import android.os.Build import android.os.Bundle +import android.util.Rational import android.view.WindowManager import android.webkit.PermissionRequest import androidx.activity.compose.setContent import androidx.activity.result.ActivityResultLauncher import androidx.activity.result.contract.ActivityResultContracts +import androidx.annotation.RequiresApi import androidx.appcompat.app.AppCompatActivity import androidx.compose.runtime.mutableStateOf import androidx.core.content.IntentCompat @@ -36,7 +39,9 @@ import androidx.lifecycle.Lifecycle import io.element.android.features.call.api.CallType import io.element.android.features.call.impl.DefaultElementCallEntryPoint import io.element.android.features.call.impl.di.CallBindings +import io.element.android.features.call.impl.pip.PictureInPictureEvents import io.element.android.features.call.impl.pip.PictureInPicturePresenter +import io.element.android.features.call.impl.pip.PipActivity import io.element.android.features.call.impl.services.CallForegroundService import io.element.android.features.call.impl.utils.CallIntentDataParser import io.element.android.libraries.architecture.bindings @@ -45,7 +50,10 @@ import io.element.android.libraries.preferences.api.store.AppPreferencesStore import timber.log.Timber import javax.inject.Inject -class ElementCallActivity : AppCompatActivity(), CallScreenNavigator { +class ElementCallActivity : + AppCompatActivity(), + CallScreenNavigator, + PipActivity { @Inject lateinit var callIntentDataParser: CallIntentDataParser @Inject lateinit var presenterFactory: CallScreenPresenter.Factory @Inject lateinit var appPreferencesStore: AppPreferencesStore @@ -66,6 +74,7 @@ class ElementCallActivity : AppCompatActivity(), CallScreenNavigator { private val webViewTarget = mutableStateOf(null) private var eventSink: ((CallScreenEvents) -> Unit)? = null + private var pipEventSink: ((PictureInPictureEvents) -> Unit)? = null override fun onCreate(savedInstanceState: Bundle?) { super.onCreate(savedInstanceState) @@ -86,13 +95,14 @@ class ElementCallActivity : AppCompatActivity(), CallScreenNavigator { updateUiMode(resources.configuration) } - pictureInPicturePresenter.onCreate(this) + pictureInPicturePresenter.setPipActivity(this) audioManager = getSystemService(AUDIO_SERVICE) as AudioManager requestAudioFocus() setContent { val pipState = pictureInPicturePresenter.present() + pipEventSink = pipState.eventSink ElementThemeApp(appPreferencesStore) { val state = presenter.present() eventSink = state.eventSink @@ -115,7 +125,7 @@ class ElementCallActivity : AppCompatActivity(), CallScreenNavigator { override fun onPictureInPictureModeChanged(isInPictureInPictureMode: Boolean, newConfig: Configuration) { super.onPictureInPictureModeChanged(isInPictureInPictureMode, newConfig) - pictureInPicturePresenter.onPictureInPictureModeChanged(isInPictureInPictureMode) + pipEventSink?.invoke(PictureInPictureEvents.OnPictureInPictureModeChanged(isInPictureInPictureMode)) if (!isInPictureInPictureMode && !lifecycle.currentState.isAtLeast(Lifecycle.State.STARTED)) { Timber.d("Exiting PiP mode: Hangup the call") @@ -142,14 +152,14 @@ class ElementCallActivity : AppCompatActivity(), CallScreenNavigator { override fun onUserLeaveHint() { super.onUserLeaveHint() - pictureInPicturePresenter.onUserLeaveHint() + pipEventSink?.invoke(PictureInPictureEvents.EnterPictureInPicture) } override fun onDestroy() { super.onDestroy() releaseAudioFocus() CallForegroundService.stop(this) - pictureInPicturePresenter.onDestroy() + pictureInPicturePresenter.setPipActivity(null) } override fun finish() { @@ -249,6 +259,37 @@ class ElementCallActivity : AppCompatActivity(), CallScreenNavigator { } } } + + override fun setPipParams() { + if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.O) { + setPictureInPictureParams(getPictureInPictureParams()) + } + } + + override fun enterPipMode(): Boolean { + return if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.O) { + enterPictureInPictureMode(getPictureInPictureParams()) + } else { + false + } + } + + @RequiresApi(Build.VERSION_CODES.O) + private fun getPictureInPictureParams(): PictureInPictureParams { + return PictureInPictureParams.Builder() + // Portrait for calls seems more appropriate + .setAspectRatio(Rational(3, 5)) + .apply { + if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.S) { + setAutoEnterEnabled(true) + } + } + .build() + } + + override fun hangUp() { + eventSink?.invoke(CallScreenEvents.Hangup) + } } internal fun mapWebkitPermissions(permissions: Array): List { diff --git a/features/call/impl/src/main/kotlin/io/element/android/features/call/impl/utils/WebPipApi.kt b/features/call/impl/src/main/kotlin/io/element/android/features/call/impl/utils/WebPipApi.kt new file mode 100644 index 0000000000..af1cc6b3f9 --- /dev/null +++ b/features/call/impl/src/main/kotlin/io/element/android/features/call/impl/utils/WebPipApi.kt @@ -0,0 +1,23 @@ +/* + * Copyright (c) 2024 New Vector Ltd + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package io.element.android.features.call.impl.utils + +interface WebPipApi { + suspend fun canEnterPip(): Boolean + fun enterPip() + fun exitPip() +} diff --git a/features/call/impl/src/main/kotlin/io/element/android/features/call/impl/utils/WebViewWebPipApi.kt b/features/call/impl/src/main/kotlin/io/element/android/features/call/impl/utils/WebViewWebPipApi.kt new file mode 100644 index 0000000000..43b7dfadde --- /dev/null +++ b/features/call/impl/src/main/kotlin/io/element/android/features/call/impl/utils/WebViewWebPipApi.kt @@ -0,0 +1,41 @@ +/* + * Copyright (c) 2024 New Vector Ltd + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package io.element.android.features.call.impl.utils + +import android.webkit.WebView +import kotlin.coroutines.resume +import kotlin.coroutines.suspendCoroutine + +class WebViewWebPipApi( + private val webView: WebView, +) : WebPipApi { + override suspend fun canEnterPip(): Boolean { + return suspendCoroutine { continuation -> + webView.evaluateJavascript("controls.canEnterPip()") { result -> + continuation.resume(result == "true") + } + } + } + + override fun enterPip() { + webView.evaluateJavascript("controls.enablePip()", null) + } + + override fun exitPip() { + webView.evaluateJavascript("controls.disablePip()", null) + } +} diff --git a/features/call/impl/src/test/kotlin/io/element/android/features/call/impl/pip/FakePipActivity.kt b/features/call/impl/src/test/kotlin/io/element/android/features/call/impl/pip/FakePipActivity.kt new file mode 100644 index 0000000000..8a3089453e --- /dev/null +++ b/features/call/impl/src/test/kotlin/io/element/android/features/call/impl/pip/FakePipActivity.kt @@ -0,0 +1,29 @@ +/* + * Copyright (c) 2024 New Vector Ltd + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package io.element.android.features.call.impl.pip + +import io.element.android.tests.testutils.lambda.lambdaError + +class FakePipActivity( + private val setPipParamsResult: () -> Unit = { lambdaError() }, + private val enterPipModeResult: () -> Boolean = { lambdaError() }, + private val handUpResult: () -> Unit = { lambdaError() } +) : PipActivity { + override fun setPipParams() = setPipParamsResult() + override fun enterPipMode(): Boolean = enterPipModeResult() + override fun hangUp() = handUpResult() +} diff --git a/features/call/impl/src/test/kotlin/io/element/android/features/call/impl/pip/FakeWebPipApi.kt b/features/call/impl/src/test/kotlin/io/element/android/features/call/impl/pip/FakeWebPipApi.kt new file mode 100644 index 0000000000..ca752cd8ce --- /dev/null +++ b/features/call/impl/src/test/kotlin/io/element/android/features/call/impl/pip/FakeWebPipApi.kt @@ -0,0 +1,32 @@ +/* + * Copyright (c) 2024 New Vector Ltd + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package io.element.android.features.call.impl.pip + +import io.element.android.features.call.impl.utils.WebPipApi +import io.element.android.tests.testutils.lambda.lambdaError + +class FakeWebPipApi( + private val canEnterPipResult: () -> Boolean = { lambdaError() }, + private val enterPipResult: () -> Unit = { lambdaError() }, + private val exitPipResult: () -> Unit = { lambdaError() }, +) : WebPipApi { + override suspend fun canEnterPip(): Boolean = canEnterPipResult() + + override fun enterPip() = enterPipResult() + + override fun exitPip() = exitPipResult() +} diff --git a/features/call/impl/src/test/kotlin/io/element/android/features/call/impl/pip/PictureInPicturePresenterTest.kt b/features/call/impl/src/test/kotlin/io/element/android/features/call/impl/pip/PictureInPicturePresenterTest.kt index 895505c278..2343f2cb70 100644 --- a/features/call/impl/src/test/kotlin/io/element/android/features/call/impl/pip/PictureInPicturePresenterTest.kt +++ b/features/call/impl/src/test/kotlin/io/element/android/features/call/impl/pip/PictureInPicturePresenterTest.kt @@ -16,23 +16,16 @@ package io.element.android.features.call.impl.pip -import android.os.Build.VERSION_CODES import app.cash.molecule.RecompositionMode import app.cash.molecule.moleculeFlow import app.cash.turbine.test import com.google.common.truth.Truth.assertThat -import io.element.android.features.call.impl.ui.ElementCallActivity +import io.element.android.tests.testutils.lambda.lambdaRecorder import kotlinx.coroutines.test.runTest import org.junit.Test -import org.junit.runner.RunWith -import org.robolectric.Robolectric -import org.robolectric.RobolectricTestRunner -import org.robolectric.annotation.Config -@RunWith(RobolectricTestRunner::class) class PictureInPicturePresenterTest { @Test - @Config(sdk = [VERSION_CODES.O, VERSION_CODES.S]) fun `when pip is not supported, the state value supportPip is false`() = runTest { val presenter = createPictureInPicturePresenter(supportPip = false) moleculeFlow(RecompositionMode.Immediate) { @@ -41,68 +34,119 @@ class PictureInPicturePresenterTest { val initialState = awaitItem() assertThat(initialState.supportPip).isFalse() } - presenter.onDestroy() + presenter.setPipActivity(null) } @Test - @Config(sdk = [VERSION_CODES.O, VERSION_CODES.S]) fun `when pip is supported, the state value supportPip is true`() = runTest { - val presenter = createPictureInPicturePresenter(supportPip = true) + val presenter = createPictureInPicturePresenter( + supportPip = true, + pipActivity = FakePipActivity(setPipParamsResult = { }), + ) moleculeFlow(RecompositionMode.Immediate) { presenter.present() }.test { val initialState = awaitItem() assertThat(initialState.supportPip).isTrue() } - presenter.onDestroy() } @Test - @Config(sdk = [VERSION_CODES.S]) fun `when entering pip is supported, the state value isInPictureInPicture is true`() = runTest { - val presenter = createPictureInPicturePresenter(supportPip = true) + val enterPipModeResult = lambdaRecorder { true } + val presenter = createPictureInPicturePresenter( + supportPip = true, + pipActivity = FakePipActivity( + setPipParamsResult = { }, + enterPipModeResult = enterPipModeResult, + ), + ) moleculeFlow(RecompositionMode.Immediate) { presenter.present() }.test { val initialState = awaitItem() assertThat(initialState.isInPictureInPicture).isFalse() initialState.eventSink(PictureInPictureEvents.EnterPictureInPicture) - presenter.onPictureInPictureModeChanged(true) + enterPipModeResult.assertions().isCalledOnce() + initialState.eventSink(PictureInPictureEvents.OnPictureInPictureModeChanged(true)) val pipState = awaitItem() assertThat(pipState.isInPictureInPicture).isTrue() // User stops pip - presenter.onPictureInPictureModeChanged(false) + initialState.eventSink(PictureInPictureEvents.OnPictureInPictureModeChanged(false)) val finalState = awaitItem() assertThat(finalState.isInPictureInPicture).isFalse() } - presenter.onDestroy() } @Test - @Config(sdk = [VERSION_CODES.S]) - fun `when onUserLeaveHint is called, the state value isInPictureInPicture becomes true`() = runTest { - val presenter = createPictureInPicturePresenter(supportPip = true) + fun `with webPipApi, when entering pip is supported, but web deny it, the call is finished`() = runTest { + val handUpResult = lambdaRecorder { } + val presenter = createPictureInPicturePresenter( + supportPip = true, + pipActivity = FakePipActivity( + setPipParamsResult = { }, + handUpResult = handUpResult + ), + ) moleculeFlow(RecompositionMode.Immediate) { presenter.present() }.test { val initialState = awaitItem() - assertThat(initialState.isInPictureInPicture).isFalse() - presenter.onUserLeaveHint() - presenter.onPictureInPictureModeChanged(true) + initialState.eventSink(PictureInPictureEvents.SetupWebPipApi(FakeWebPipApi(canEnterPipResult = { false }))) + initialState.eventSink(PictureInPictureEvents.EnterPictureInPicture) + handUpResult.assertions().isCalledOnce() + } + } + + @Test + fun `with webPipApi, when entering pip is supported, and web allows it, the state value isInPictureInPicture is true`() = runTest { + val enterPipModeResult = lambdaRecorder { true } + val enterPipResult = lambdaRecorder { } + val exitPipResult = lambdaRecorder { } + val presenter = createPictureInPicturePresenter( + supportPip = true, + pipActivity = FakePipActivity( + setPipParamsResult = { }, + enterPipModeResult = enterPipModeResult + ), + ) + moleculeFlow(RecompositionMode.Immediate) { + presenter.present() + }.test { + val initialState = awaitItem() + initialState.eventSink( + PictureInPictureEvents.SetupWebPipApi( + FakeWebPipApi( + canEnterPipResult = { true }, + enterPipResult = enterPipResult, + exitPipResult = exitPipResult, + ) + ) + ) + initialState.eventSink(PictureInPictureEvents.EnterPictureInPicture) + enterPipModeResult.assertions().isCalledOnce() + enterPipResult.assertions().isNeverCalled() + initialState.eventSink(PictureInPictureEvents.OnPictureInPictureModeChanged(true)) val pipState = awaitItem() assertThat(pipState.isInPictureInPicture).isTrue() + enterPipResult.assertions().isCalledOnce() + // User stops pip + exitPipResult.assertions().isNeverCalled() + initialState.eventSink(PictureInPictureEvents.OnPictureInPictureModeChanged(false)) + val finalState = awaitItem() + assertThat(finalState.isInPictureInPicture).isFalse() + exitPipResult.assertions().isCalledOnce() } - presenter.onDestroy() } private fun createPictureInPicturePresenter( supportPip: Boolean = true, + pipActivity: PipActivity? = FakePipActivity() ): PictureInPicturePresenter { - val activity = Robolectric.buildActivity(ElementCallActivity::class.java) return PictureInPicturePresenter( pipSupportProvider = FakePipSupportProvider(supportPip), ).apply { - onCreate(activity.get()) + setPipActivity(pipActivity) } } } From 368db3feb48c5a2b5c9aca9aa5b1ffadc2b25154 Mon Sep 17 00:00:00 2001 From: Benoit Marty Date: Fri, 23 Aug 2024 16:28:13 +0200 Subject: [PATCH 2/7] Allow entering Pip mode when `controls.canEnterPip()` cannot be evaluated. --- .../android/features/call/impl/utils/WebViewWebPipApi.kt | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/features/call/impl/src/main/kotlin/io/element/android/features/call/impl/utils/WebViewWebPipApi.kt b/features/call/impl/src/main/kotlin/io/element/android/features/call/impl/utils/WebViewWebPipApi.kt index 43b7dfadde..baf682d6b3 100644 --- a/features/call/impl/src/main/kotlin/io/element/android/features/call/impl/utils/WebViewWebPipApi.kt +++ b/features/call/impl/src/main/kotlin/io/element/android/features/call/impl/utils/WebViewWebPipApi.kt @@ -26,7 +26,8 @@ class WebViewWebPipApi( override suspend fun canEnterPip(): Boolean { return suspendCoroutine { continuation -> webView.evaluateJavascript("controls.canEnterPip()") { result -> - continuation.resume(result == "true") + // Note if the method is not available, it will return "null" + continuation.resume(result == "true" || result == "null") } } } From a4b6d4c5d77b570dae4dc4f5df721527a54a7716 Mon Sep 17 00:00:00 2001 From: Benoit Marty Date: Fri, 23 Aug 2024 16:34:44 +0200 Subject: [PATCH 3/7] Simplify code. --- .../features/call/impl/ui/ElementCallActivity.kt | 12 ++++-------- 1 file changed, 4 insertions(+), 8 deletions(-) diff --git a/features/call/impl/src/main/kotlin/io/element/android/features/call/impl/ui/ElementCallActivity.kt b/features/call/impl/src/main/kotlin/io/element/android/features/call/impl/ui/ElementCallActivity.kt index aace025bb4..8c758c2ee5 100644 --- a/features/call/impl/src/main/kotlin/io/element/android/features/call/impl/ui/ElementCallActivity.kt +++ b/features/call/impl/src/main/kotlin/io/element/android/features/call/impl/ui/ElementCallActivity.kt @@ -260,18 +260,14 @@ class ElementCallActivity : } } + @RequiresApi(Build.VERSION_CODES.O) override fun setPipParams() { - if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.O) { - setPictureInPictureParams(getPictureInPictureParams()) - } + setPictureInPictureParams(getPictureInPictureParams()) } + @RequiresApi(Build.VERSION_CODES.O) override fun enterPipMode(): Boolean { - return if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.O) { - enterPictureInPictureMode(getPictureInPictureParams()) - } else { - false - } + return enterPictureInPictureMode(getPictureInPictureParams()) } @RequiresApi(Build.VERSION_CODES.O) From 0b2edcb6d14fe3dd5d0b2c41cb8af3ea9d987f2b Mon Sep 17 00:00:00 2001 From: Benoit Marty Date: Fri, 23 Aug 2024 16:40:48 +0200 Subject: [PATCH 4/7] Fix UI tests. --- .../android/features/call/impl/ui/CallScreenViewTest.kt | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/features/call/impl/src/test/kotlin/io/element/android/features/call/impl/ui/CallScreenViewTest.kt b/features/call/impl/src/test/kotlin/io/element/android/features/call/impl/ui/CallScreenViewTest.kt index 6d15e5001c..e92e9cc4e6 100644 --- a/features/call/impl/src/test/kotlin/io/element/android/features/call/impl/ui/CallScreenViewTest.kt +++ b/features/call/impl/src/test/kotlin/io/element/android/features/call/impl/ui/CallScreenViewTest.kt @@ -37,7 +37,7 @@ class CallScreenViewTest { @Test fun `clicking on back when pip is not supported hangs up`() { val eventsRecorder = EventsRecorder() - val pipEventsRecorder = EventsRecorder(expectEvents = false) + val pipEventsRecorder = EventsRecorder() rule.setCallScreenView( aCallScreenState( eventSink = eventsRecorder @@ -51,6 +51,8 @@ class CallScreenViewTest { eventsRecorder.assertSize(2) eventsRecorder.assertTrue(0) { it is CallScreenEvents.SetupMessageChannels } eventsRecorder.assertTrue(1) { it == CallScreenEvents.Hangup } + pipEventsRecorder.assertSize(1) + pipEventsRecorder.assertTrue(0) { it is PictureInPictureEvents.SetupWebPipApi } } @Test @@ -69,7 +71,9 @@ class CallScreenViewTest { rule.pressBack() eventsRecorder.assertSize(1) eventsRecorder.assertTrue(0) { it is CallScreenEvents.SetupMessageChannels } - pipEventsRecorder.assertSingle(PictureInPictureEvents.EnterPictureInPicture) + pipEventsRecorder.assertSize(2) + pipEventsRecorder.assertTrue(0) { it is PictureInPictureEvents.SetupWebPipApi } + pipEventsRecorder.assertTrue(1) { it == PictureInPictureEvents.EnterPictureInPicture } } } From 306043876f4de997cd15d2abafdc304ebeb627ed Mon Sep 17 00:00:00 2001 From: Benoit Marty Date: Mon, 26 Aug 2024 16:50:35 +0200 Subject: [PATCH 5/7] Rename `WebPipApi` to `PipController` --- .../call/impl/pip/PictureInPictureEvents.kt | 4 ++-- .../impl/pip/PictureInPicturePresenter.kt | 20 +++++++++---------- .../features/call/impl/ui/CallScreenView.kt | 12 +++++------ .../utils/{WebPipApi.kt => PipController.kt} | 2 +- ...ewWebPipApi.kt => WebViewPipController.kt} | 4 ++-- ...{FakeWebPipApi.kt => FakePipController.kt} | 6 +++--- .../impl/pip/PictureInPicturePresenterTest.kt | 6 +++--- .../call/impl/ui/CallScreenViewTest.kt | 4 ++-- 8 files changed, 29 insertions(+), 29 deletions(-) rename features/call/impl/src/main/kotlin/io/element/android/features/call/impl/utils/{WebPipApi.kt => PipController.kt} (96%) rename features/call/impl/src/main/kotlin/io/element/android/features/call/impl/utils/{WebViewWebPipApi.kt => WebViewPipController.kt} (96%) rename features/call/impl/src/test/kotlin/io/element/android/features/call/impl/pip/{FakeWebPipApi.kt => FakePipController.kt} (90%) diff --git a/features/call/impl/src/main/kotlin/io/element/android/features/call/impl/pip/PictureInPictureEvents.kt b/features/call/impl/src/main/kotlin/io/element/android/features/call/impl/pip/PictureInPictureEvents.kt index be376aa079..4d29f757ea 100644 --- a/features/call/impl/src/main/kotlin/io/element/android/features/call/impl/pip/PictureInPictureEvents.kt +++ b/features/call/impl/src/main/kotlin/io/element/android/features/call/impl/pip/PictureInPictureEvents.kt @@ -16,10 +16,10 @@ package io.element.android.features.call.impl.pip -import io.element.android.features.call.impl.utils.WebPipApi +import io.element.android.features.call.impl.utils.PipController sealed interface PictureInPictureEvents { - data class SetupWebPipApi(val webPipApi: WebPipApi) : PictureInPictureEvents + data class SetPipController(val pipController: PipController) : PictureInPictureEvents data object EnterPictureInPicture : PictureInPictureEvents data class OnPictureInPictureModeChanged(val isInPip: Boolean) : PictureInPictureEvents } diff --git a/features/call/impl/src/main/kotlin/io/element/android/features/call/impl/pip/PictureInPicturePresenter.kt b/features/call/impl/src/main/kotlin/io/element/android/features/call/impl/pip/PictureInPicturePresenter.kt index 78d347d715..e6422f2dd6 100644 --- a/features/call/impl/src/main/kotlin/io/element/android/features/call/impl/pip/PictureInPicturePresenter.kt +++ b/features/call/impl/src/main/kotlin/io/element/android/features/call/impl/pip/PictureInPicturePresenter.kt @@ -22,7 +22,7 @@ import androidx.compose.runtime.mutableStateOf import androidx.compose.runtime.remember import androidx.compose.runtime.rememberCoroutineScope import androidx.compose.runtime.setValue -import io.element.android.features.call.impl.utils.WebPipApi +import io.element.android.features.call.impl.utils.PipController import io.element.android.libraries.architecture.Presenter import io.element.android.libraries.core.log.logger.LoggerTag import kotlinx.coroutines.launch @@ -41,25 +41,25 @@ class PictureInPicturePresenter @Inject constructor( override fun present(): PictureInPictureState { val coroutineScope = rememberCoroutineScope() var isInPictureInPicture by remember { mutableStateOf(false) } - var webPipApi by remember { mutableStateOf(null) } + var pipController by remember { mutableStateOf(null) } fun handleEvent(event: PictureInPictureEvents) { when (event) { - is PictureInPictureEvents.SetupWebPipApi -> { - webPipApi = event.webPipApi + is PictureInPictureEvents.SetPipController -> { + pipController = event.pipController } PictureInPictureEvents.EnterPictureInPicture -> { coroutineScope.launch { - switchToPip(webPipApi) + switchToPip(pipController) } } is PictureInPictureEvents.OnPictureInPictureModeChanged -> { Timber.tag(loggerTag.value).d("onPictureInPictureModeChanged: ${event.isInPip}") isInPictureInPicture = event.isInPip if (event.isInPip) { - webPipApi?.enterPip() + pipController?.enterPip() } else { - webPipApi?.exitPip() + pipController?.exitPip() } } } @@ -85,12 +85,12 @@ class PictureInPicturePresenter @Inject constructor( /** * Enters Picture-in-Picture mode, if allowed by Element Call. */ - private suspend fun switchToPip(webPipApi: WebPipApi?) { + private suspend fun switchToPip(pipController: PipController?) { if (isPipSupported) { - if (webPipApi == null) { + if (pipController == null) { Timber.tag(loggerTag.value).w("webPipApi is not available") } - if (webPipApi == null || webPipApi.canEnterPip()) { + if (pipController == null || pipController.canEnterPip()) { Timber.tag(loggerTag.value).d("Switch to PiP mode") pipActivity?.enterPipMode() ?.also { Timber.tag(loggerTag.value).d("Switch to PiP mode result: $it") } diff --git a/features/call/impl/src/main/kotlin/io/element/android/features/call/impl/ui/CallScreenView.kt b/features/call/impl/src/main/kotlin/io/element/android/features/call/impl/ui/CallScreenView.kt index e81d386dc5..5c3d02b8ac 100644 --- a/features/call/impl/src/main/kotlin/io/element/android/features/call/impl/ui/CallScreenView.kt +++ b/features/call/impl/src/main/kotlin/io/element/android/features/call/impl/ui/CallScreenView.kt @@ -40,7 +40,7 @@ import io.element.android.features.call.impl.pip.PictureInPictureEvents import io.element.android.features.call.impl.pip.PictureInPictureState import io.element.android.features.call.impl.pip.PictureInPictureStateProvider import io.element.android.features.call.impl.pip.aPictureInPictureState -import io.element.android.features.call.impl.utils.WebViewWebPipApi +import io.element.android.features.call.impl.utils.WebViewPipController import io.element.android.features.call.impl.utils.WebViewWidgetMessageInterceptor import io.element.android.libraries.architecture.AsyncData import io.element.android.libraries.designsystem.components.ProgressDialog @@ -96,9 +96,9 @@ internal fun CallScreenView( } CallWebView( modifier = Modifier - .padding(padding) - .consumeWindowInsets(padding) - .fillMaxSize(), + .padding(padding) + .consumeWindowInsets(padding) + .fillMaxSize(), url = state.urlState, userAgent = state.userAgent, onPermissionsRequest = { request -> @@ -109,8 +109,8 @@ internal fun CallScreenView( onWebViewCreate = { webView -> val interceptor = WebViewWidgetMessageInterceptor(webView) state.eventSink(CallScreenEvents.SetupMessageChannels(interceptor)) - val webPipApi = WebViewWebPipApi(webView) - pipState.eventSink(PictureInPictureEvents.SetupWebPipApi(webPipApi)) + val pipController = WebViewPipController(webView) + pipState.eventSink(PictureInPictureEvents.SetPipController(pipController)) } ) when (state.urlState) { diff --git a/features/call/impl/src/main/kotlin/io/element/android/features/call/impl/utils/WebPipApi.kt b/features/call/impl/src/main/kotlin/io/element/android/features/call/impl/utils/PipController.kt similarity index 96% rename from features/call/impl/src/main/kotlin/io/element/android/features/call/impl/utils/WebPipApi.kt rename to features/call/impl/src/main/kotlin/io/element/android/features/call/impl/utils/PipController.kt index af1cc6b3f9..01e23ab727 100644 --- a/features/call/impl/src/main/kotlin/io/element/android/features/call/impl/utils/WebPipApi.kt +++ b/features/call/impl/src/main/kotlin/io/element/android/features/call/impl/utils/PipController.kt @@ -16,7 +16,7 @@ package io.element.android.features.call.impl.utils -interface WebPipApi { +interface PipController { suspend fun canEnterPip(): Boolean fun enterPip() fun exitPip() diff --git a/features/call/impl/src/main/kotlin/io/element/android/features/call/impl/utils/WebViewWebPipApi.kt b/features/call/impl/src/main/kotlin/io/element/android/features/call/impl/utils/WebViewPipController.kt similarity index 96% rename from features/call/impl/src/main/kotlin/io/element/android/features/call/impl/utils/WebViewWebPipApi.kt rename to features/call/impl/src/main/kotlin/io/element/android/features/call/impl/utils/WebViewPipController.kt index baf682d6b3..2a90965c8c 100644 --- a/features/call/impl/src/main/kotlin/io/element/android/features/call/impl/utils/WebViewWebPipApi.kt +++ b/features/call/impl/src/main/kotlin/io/element/android/features/call/impl/utils/WebViewPipController.kt @@ -20,9 +20,9 @@ import android.webkit.WebView import kotlin.coroutines.resume import kotlin.coroutines.suspendCoroutine -class WebViewWebPipApi( +class WebViewPipController( private val webView: WebView, -) : WebPipApi { +) : PipController { override suspend fun canEnterPip(): Boolean { return suspendCoroutine { continuation -> webView.evaluateJavascript("controls.canEnterPip()") { result -> diff --git a/features/call/impl/src/test/kotlin/io/element/android/features/call/impl/pip/FakeWebPipApi.kt b/features/call/impl/src/test/kotlin/io/element/android/features/call/impl/pip/FakePipController.kt similarity index 90% rename from features/call/impl/src/test/kotlin/io/element/android/features/call/impl/pip/FakeWebPipApi.kt rename to features/call/impl/src/test/kotlin/io/element/android/features/call/impl/pip/FakePipController.kt index ca752cd8ce..086feecd39 100644 --- a/features/call/impl/src/test/kotlin/io/element/android/features/call/impl/pip/FakeWebPipApi.kt +++ b/features/call/impl/src/test/kotlin/io/element/android/features/call/impl/pip/FakePipController.kt @@ -16,14 +16,14 @@ package io.element.android.features.call.impl.pip -import io.element.android.features.call.impl.utils.WebPipApi +import io.element.android.features.call.impl.utils.PipController import io.element.android.tests.testutils.lambda.lambdaError -class FakeWebPipApi( +class FakePipController( private val canEnterPipResult: () -> Boolean = { lambdaError() }, private val enterPipResult: () -> Unit = { lambdaError() }, private val exitPipResult: () -> Unit = { lambdaError() }, -) : WebPipApi { +) : PipController { override suspend fun canEnterPip(): Boolean = canEnterPipResult() override fun enterPip() = enterPipResult() diff --git a/features/call/impl/src/test/kotlin/io/element/android/features/call/impl/pip/PictureInPicturePresenterTest.kt b/features/call/impl/src/test/kotlin/io/element/android/features/call/impl/pip/PictureInPicturePresenterTest.kt index 2343f2cb70..a5fb1f8beb 100644 --- a/features/call/impl/src/test/kotlin/io/element/android/features/call/impl/pip/PictureInPicturePresenterTest.kt +++ b/features/call/impl/src/test/kotlin/io/element/android/features/call/impl/pip/PictureInPicturePresenterTest.kt @@ -92,7 +92,7 @@ class PictureInPicturePresenterTest { presenter.present() }.test { val initialState = awaitItem() - initialState.eventSink(PictureInPictureEvents.SetupWebPipApi(FakeWebPipApi(canEnterPipResult = { false }))) + initialState.eventSink(PictureInPictureEvents.SetPipController(FakePipController(canEnterPipResult = { false }))) initialState.eventSink(PictureInPictureEvents.EnterPictureInPicture) handUpResult.assertions().isCalledOnce() } @@ -115,8 +115,8 @@ class PictureInPicturePresenterTest { }.test { val initialState = awaitItem() initialState.eventSink( - PictureInPictureEvents.SetupWebPipApi( - FakeWebPipApi( + PictureInPictureEvents.SetPipController( + FakePipController( canEnterPipResult = { true }, enterPipResult = enterPipResult, exitPipResult = exitPipResult, diff --git a/features/call/impl/src/test/kotlin/io/element/android/features/call/impl/ui/CallScreenViewTest.kt b/features/call/impl/src/test/kotlin/io/element/android/features/call/impl/ui/CallScreenViewTest.kt index e92e9cc4e6..ec75b9a1fa 100644 --- a/features/call/impl/src/test/kotlin/io/element/android/features/call/impl/ui/CallScreenViewTest.kt +++ b/features/call/impl/src/test/kotlin/io/element/android/features/call/impl/ui/CallScreenViewTest.kt @@ -52,7 +52,7 @@ class CallScreenViewTest { eventsRecorder.assertTrue(0) { it is CallScreenEvents.SetupMessageChannels } eventsRecorder.assertTrue(1) { it == CallScreenEvents.Hangup } pipEventsRecorder.assertSize(1) - pipEventsRecorder.assertTrue(0) { it is PictureInPictureEvents.SetupWebPipApi } + pipEventsRecorder.assertTrue(0) { it is PictureInPictureEvents.SetPipController } } @Test @@ -72,7 +72,7 @@ class CallScreenViewTest { eventsRecorder.assertSize(1) eventsRecorder.assertTrue(0) { it is CallScreenEvents.SetupMessageChannels } pipEventsRecorder.assertSize(2) - pipEventsRecorder.assertTrue(0) { it is PictureInPictureEvents.SetupWebPipApi } + pipEventsRecorder.assertTrue(0) { it is PictureInPictureEvents.SetPipController } pipEventsRecorder.assertTrue(1) { it == PictureInPictureEvents.EnterPictureInPicture } } } From 9fab13c50be261f7da406188a4950b6a245381f5 Mon Sep 17 00:00:00 2001 From: Benoit Marty Date: Mon, 26 Aug 2024 17:11:41 +0200 Subject: [PATCH 6/7] Avoid keeping a reference to the eventSink in a separate value --- .../call/impl/ui/ElementCallActivity.kt | 52 +++++++++++++------ 1 file changed, 35 insertions(+), 17 deletions(-) diff --git a/features/call/impl/src/main/kotlin/io/element/android/features/call/impl/ui/ElementCallActivity.kt b/features/call/impl/src/main/kotlin/io/element/android/features/call/impl/ui/ElementCallActivity.kt index 8c758c2ee5..abeafad4d3 100644 --- a/features/call/impl/src/main/kotlin/io/element/android/features/call/impl/ui/ElementCallActivity.kt +++ b/features/call/impl/src/main/kotlin/io/element/android/features/call/impl/ui/ElementCallActivity.kt @@ -33,14 +33,21 @@ import androidx.activity.result.ActivityResultLauncher import androidx.activity.result.contract.ActivityResultContracts import androidx.annotation.RequiresApi import androidx.appcompat.app.AppCompatActivity +import androidx.compose.runtime.Composable +import androidx.compose.runtime.DisposableEffect +import androidx.compose.runtime.getValue import androidx.compose.runtime.mutableStateOf +import androidx.compose.runtime.rememberUpdatedState +import androidx.core.app.PictureInPictureModeChangedInfo import androidx.core.content.IntentCompat +import androidx.core.util.Consumer import androidx.lifecycle.Lifecycle import io.element.android.features.call.api.CallType import io.element.android.features.call.impl.DefaultElementCallEntryPoint import io.element.android.features.call.impl.di.CallBindings import io.element.android.features.call.impl.pip.PictureInPictureEvents import io.element.android.features.call.impl.pip.PictureInPicturePresenter +import io.element.android.features.call.impl.pip.PictureInPictureState import io.element.android.features.call.impl.pip.PipActivity import io.element.android.features.call.impl.services.CallForegroundService import io.element.android.features.call.impl.utils.CallIntentDataParser @@ -74,7 +81,6 @@ class ElementCallActivity : private val webViewTarget = mutableStateOf(null) private var eventSink: ((CallScreenEvents) -> Unit)? = null - private var pipEventSink: ((PictureInPictureEvents) -> Unit)? = null override fun onCreate(savedInstanceState: Bundle?) { super.onCreate(savedInstanceState) @@ -102,7 +108,7 @@ class ElementCallActivity : setContent { val pipState = pictureInPicturePresenter.present() - pipEventSink = pipState.eventSink + ListenToAndroidEvents(pipState) ElementThemeApp(appPreferencesStore) { val state = presenter.present() eventSink = state.eventSink @@ -118,21 +124,38 @@ class ElementCallActivity : } } + @Composable + private fun ListenToAndroidEvents(pipState: PictureInPictureState) { + val pipEventSink by rememberUpdatedState(pipState.eventSink) + DisposableEffect(Unit) { + val onUserLeaveHintListener = Runnable { + pipEventSink(PictureInPictureEvents.EnterPictureInPicture) + } + addOnUserLeaveHintListener(onUserLeaveHintListener) + onDispose { + removeOnUserLeaveHintListener(onUserLeaveHintListener) + } + } + DisposableEffect(Unit) { + val onPictureInPictureModeChangedListener = Consumer { _: PictureInPictureModeChangedInfo -> + pipEventSink(PictureInPictureEvents.OnPictureInPictureModeChanged(isInPictureInPictureMode)) + if (!isInPictureInPictureMode && !lifecycle.currentState.isAtLeast(Lifecycle.State.STARTED)) { + Timber.d("Exiting PiP mode: Hangup the call") + eventSink?.invoke(CallScreenEvents.Hangup) + } + } + addOnPictureInPictureModeChangedListener(onPictureInPictureModeChangedListener) + onDispose { + removeOnPictureInPictureModeChangedListener(onPictureInPictureModeChangedListener) + } + } + } + override fun onConfigurationChanged(newConfig: Configuration) { super.onConfigurationChanged(newConfig) updateUiMode(newConfig) } - override fun onPictureInPictureModeChanged(isInPictureInPictureMode: Boolean, newConfig: Configuration) { - super.onPictureInPictureModeChanged(isInPictureInPictureMode, newConfig) - pipEventSink?.invoke(PictureInPictureEvents.OnPictureInPictureModeChanged(isInPictureInPictureMode)) - - if (!isInPictureInPictureMode && !lifecycle.currentState.isAtLeast(Lifecycle.State.STARTED)) { - Timber.d("Exiting PiP mode: Hangup the call") - eventSink?.invoke(CallScreenEvents.Hangup) - } - } - override fun onNewIntent(intent: Intent) { super.onNewIntent(intent) setCallType(intent) @@ -150,11 +173,6 @@ class ElementCallActivity : } } - override fun onUserLeaveHint() { - super.onUserLeaveHint() - pipEventSink?.invoke(PictureInPictureEvents.EnterPictureInPicture) - } - override fun onDestroy() { super.onDestroy() releaseAudioFocus() From 7f4b84638f2591093b4fd2a0cfca35420765a673 Mon Sep 17 00:00:00 2001 From: Benoit Marty Date: Mon, 26 Aug 2024 17:15:32 +0200 Subject: [PATCH 7/7] Rename `PipActivity` to `PipView` --- .../call/impl/pip/PictureInPicturePresenter.kt | 14 +++++++------- .../call/impl/pip/{PipActivity.kt => PipView.kt} | 2 +- .../features/call/impl/ui/ElementCallActivity.kt | 8 ++++---- .../pip/{FakePipActivity.kt => FakePipView.kt} | 4 ++-- .../call/impl/pip/PictureInPicturePresenterTest.kt | 14 +++++++------- 5 files changed, 21 insertions(+), 21 deletions(-) rename features/call/impl/src/main/kotlin/io/element/android/features/call/impl/pip/{PipActivity.kt => PipView.kt} (96%) rename features/call/impl/src/test/kotlin/io/element/android/features/call/impl/pip/{FakePipActivity.kt => FakePipView.kt} (96%) diff --git a/features/call/impl/src/main/kotlin/io/element/android/features/call/impl/pip/PictureInPicturePresenter.kt b/features/call/impl/src/main/kotlin/io/element/android/features/call/impl/pip/PictureInPicturePresenter.kt index e6422f2dd6..ab0b8f49b9 100644 --- a/features/call/impl/src/main/kotlin/io/element/android/features/call/impl/pip/PictureInPicturePresenter.kt +++ b/features/call/impl/src/main/kotlin/io/element/android/features/call/impl/pip/PictureInPicturePresenter.kt @@ -35,7 +35,7 @@ class PictureInPicturePresenter @Inject constructor( pipSupportProvider: PipSupportProvider, ) : Presenter { private val isPipSupported = pipSupportProvider.isPipSupported() - private var pipActivity: PipActivity? = null + private var pipView: PipView? = null @Composable override fun present(): PictureInPictureState { @@ -72,13 +72,13 @@ class PictureInPicturePresenter @Inject constructor( ) } - fun setPipActivity(pipActivity: PipActivity?) { + fun setPipView(pipView: PipView?) { if (isPipSupported) { Timber.tag(loggerTag.value).d("Setting PiP params") - this.pipActivity = pipActivity - pipActivity?.setPipParams() + this.pipView = pipView + pipView?.setPipParams() } else { - Timber.tag(loggerTag.value).d("onCreate: PiP is not supported") + Timber.tag(loggerTag.value).d("setPipView: PiP is not supported") } } @@ -92,11 +92,11 @@ class PictureInPicturePresenter @Inject constructor( } if (pipController == null || pipController.canEnterPip()) { Timber.tag(loggerTag.value).d("Switch to PiP mode") - pipActivity?.enterPipMode() + pipView?.enterPipMode() ?.also { Timber.tag(loggerTag.value).d("Switch to PiP mode result: $it") } } else { Timber.tag(loggerTag.value).w("Cannot enter PiP mode, hangup the call") - pipActivity?.hangUp() + pipView?.hangUp() } } } diff --git a/features/call/impl/src/main/kotlin/io/element/android/features/call/impl/pip/PipActivity.kt b/features/call/impl/src/main/kotlin/io/element/android/features/call/impl/pip/PipView.kt similarity index 96% rename from features/call/impl/src/main/kotlin/io/element/android/features/call/impl/pip/PipActivity.kt rename to features/call/impl/src/main/kotlin/io/element/android/features/call/impl/pip/PipView.kt index 6b86988ae2..998d36da5c 100644 --- a/features/call/impl/src/main/kotlin/io/element/android/features/call/impl/pip/PipActivity.kt +++ b/features/call/impl/src/main/kotlin/io/element/android/features/call/impl/pip/PipView.kt @@ -16,7 +16,7 @@ package io.element.android.features.call.impl.pip -interface PipActivity { +interface PipView { fun setPipParams() fun enterPipMode(): Boolean fun hangUp() diff --git a/features/call/impl/src/main/kotlin/io/element/android/features/call/impl/ui/ElementCallActivity.kt b/features/call/impl/src/main/kotlin/io/element/android/features/call/impl/ui/ElementCallActivity.kt index abeafad4d3..2ae53d571f 100644 --- a/features/call/impl/src/main/kotlin/io/element/android/features/call/impl/ui/ElementCallActivity.kt +++ b/features/call/impl/src/main/kotlin/io/element/android/features/call/impl/ui/ElementCallActivity.kt @@ -48,7 +48,7 @@ import io.element.android.features.call.impl.di.CallBindings import io.element.android.features.call.impl.pip.PictureInPictureEvents import io.element.android.features.call.impl.pip.PictureInPicturePresenter import io.element.android.features.call.impl.pip.PictureInPictureState -import io.element.android.features.call.impl.pip.PipActivity +import io.element.android.features.call.impl.pip.PipView import io.element.android.features.call.impl.services.CallForegroundService import io.element.android.features.call.impl.utils.CallIntentDataParser import io.element.android.libraries.architecture.bindings @@ -60,7 +60,7 @@ import javax.inject.Inject class ElementCallActivity : AppCompatActivity(), CallScreenNavigator, - PipActivity { + PipView { @Inject lateinit var callIntentDataParser: CallIntentDataParser @Inject lateinit var presenterFactory: CallScreenPresenter.Factory @Inject lateinit var appPreferencesStore: AppPreferencesStore @@ -101,7 +101,7 @@ class ElementCallActivity : updateUiMode(resources.configuration) } - pictureInPicturePresenter.setPipActivity(this) + pictureInPicturePresenter.setPipView(this) audioManager = getSystemService(AUDIO_SERVICE) as AudioManager requestAudioFocus() @@ -177,7 +177,7 @@ class ElementCallActivity : super.onDestroy() releaseAudioFocus() CallForegroundService.stop(this) - pictureInPicturePresenter.setPipActivity(null) + pictureInPicturePresenter.setPipView(null) } override fun finish() { diff --git a/features/call/impl/src/test/kotlin/io/element/android/features/call/impl/pip/FakePipActivity.kt b/features/call/impl/src/test/kotlin/io/element/android/features/call/impl/pip/FakePipView.kt similarity index 96% rename from features/call/impl/src/test/kotlin/io/element/android/features/call/impl/pip/FakePipActivity.kt rename to features/call/impl/src/test/kotlin/io/element/android/features/call/impl/pip/FakePipView.kt index 8a3089453e..d07eb52c90 100644 --- a/features/call/impl/src/test/kotlin/io/element/android/features/call/impl/pip/FakePipActivity.kt +++ b/features/call/impl/src/test/kotlin/io/element/android/features/call/impl/pip/FakePipView.kt @@ -18,11 +18,11 @@ package io.element.android.features.call.impl.pip import io.element.android.tests.testutils.lambda.lambdaError -class FakePipActivity( +class FakePipView( private val setPipParamsResult: () -> Unit = { lambdaError() }, private val enterPipModeResult: () -> Boolean = { lambdaError() }, private val handUpResult: () -> Unit = { lambdaError() } -) : PipActivity { +) : PipView { override fun setPipParams() = setPipParamsResult() override fun enterPipMode(): Boolean = enterPipModeResult() override fun hangUp() = handUpResult() diff --git a/features/call/impl/src/test/kotlin/io/element/android/features/call/impl/pip/PictureInPicturePresenterTest.kt b/features/call/impl/src/test/kotlin/io/element/android/features/call/impl/pip/PictureInPicturePresenterTest.kt index a5fb1f8beb..e433a09378 100644 --- a/features/call/impl/src/test/kotlin/io/element/android/features/call/impl/pip/PictureInPicturePresenterTest.kt +++ b/features/call/impl/src/test/kotlin/io/element/android/features/call/impl/pip/PictureInPicturePresenterTest.kt @@ -34,14 +34,14 @@ class PictureInPicturePresenterTest { val initialState = awaitItem() assertThat(initialState.supportPip).isFalse() } - presenter.setPipActivity(null) + presenter.setPipView(null) } @Test fun `when pip is supported, the state value supportPip is true`() = runTest { val presenter = createPictureInPicturePresenter( supportPip = true, - pipActivity = FakePipActivity(setPipParamsResult = { }), + pipView = FakePipView(setPipParamsResult = { }), ) moleculeFlow(RecompositionMode.Immediate) { presenter.present() @@ -56,7 +56,7 @@ class PictureInPicturePresenterTest { val enterPipModeResult = lambdaRecorder { true } val presenter = createPictureInPicturePresenter( supportPip = true, - pipActivity = FakePipActivity( + pipView = FakePipView( setPipParamsResult = { }, enterPipModeResult = enterPipModeResult, ), @@ -83,7 +83,7 @@ class PictureInPicturePresenterTest { val handUpResult = lambdaRecorder { } val presenter = createPictureInPicturePresenter( supportPip = true, - pipActivity = FakePipActivity( + pipView = FakePipView( setPipParamsResult = { }, handUpResult = handUpResult ), @@ -105,7 +105,7 @@ class PictureInPicturePresenterTest { val exitPipResult = lambdaRecorder { } val presenter = createPictureInPicturePresenter( supportPip = true, - pipActivity = FakePipActivity( + pipView = FakePipView( setPipParamsResult = { }, enterPipModeResult = enterPipModeResult ), @@ -141,12 +141,12 @@ class PictureInPicturePresenterTest { private fun createPictureInPicturePresenter( supportPip: Boolean = true, - pipActivity: PipActivity? = FakePipActivity() + pipView: PipView? = FakePipView() ): PictureInPicturePresenter { return PictureInPicturePresenter( pipSupportProvider = FakePipSupportProvider(supportPip), ).apply { - setPipActivity(pipActivity) + setPipView(pipView) } } }