Test: fix video PiP and fullscreen

.github/workflows/build-video-downloader.yml

@@ -1,14 +1,17 @@
-name: Build Nova Video Downloader (feature branch)
+name: Build Nova PiP Video Test (feature branch)
+# PiP UI/fullscreen fix test build
 
 on:
   workflow_dispatch:
     inputs:
       version_name:
-        description: 'Version name (e.g. 3.1.0-video)'
+        description: 'Version name for the PiP test build'
         required: true
-        default: '3.1.0-video'
+        default: '1.4.6-pip-test'
+  pull_request:
+    branches: [ main ]
   push:
-    branches: [ feature/video-downloader ]
+    branches: [ feature/video-downloader, feature/pip-video-test ]
     paths-ignore:
       - 'README.md'
       - 'screenshots/**'
@@ -23,7 +26,7 @@ concurrency:
 
 jobs:
   build:
-    name: Build Nova Video Downloader APK
+    name: Build Nova PiP Video Test APK
     runs-on: ubuntu-latest
     timeout-minutes: 360
     steps:
@@ -45,9 +48,9 @@ jobs:
 
       - name: Create version name
         run: |
-          VERSION_NAME="${{ github.event.inputs.version_name || '3.1.0-video' }}"
+          VERSION_NAME="${{ github.event.inputs.version_name || '1.4.6-pip-test' }}"
           echo "VERSION_NAME=$VERSION_NAME" >> $GITHUB_ENV
-          echo "Building video-downloader test APK, version $VERSION_NAME"
+          echo "Building Nova PiP test APK, version $VERSION_NAME"
 
       - name: Build forkRelease variant
         timeout-minutes: 90
@@ -79,14 +82,14 @@ jobs:
             --ks-key-alias "$ALIAS" \
             --ks-pass env:KEY_STORE_PASSWORD \
             --key-pass env:KEY_PASSWORD \
-            --out nova-video-downloader.apk "$APK_PATH"
-          $HOME/android-sdk/android-sdk-linux/build-tools/$BUILD_TOOL_VERSION/apksigner verify --print-certs nova-video-downloader.apk
-          ls -alh nova-video-downloader.apk
-          $HOME/android-sdk/android-sdk-linux/build-tools/$BUILD_TOOL_VERSION/aapt dump badging nova-video-downloader.apk | head -3
+            --out nova-pip-video-test.apk "$APK_PATH"
+          $HOME/android-sdk/android-sdk-linux/build-tools/$BUILD_TOOL_VERSION/apksigner verify --print-certs nova-pip-video-test.apk
+          ls -alh nova-pip-video-test.apk
+          $HOME/android-sdk/android-sdk-linux/build-tools/$BUILD_TOOL_VERSION/aapt dump badging nova-pip-video-test.apk | head -3
 
       - name: Upload test APK as artifact (no release)
         uses: actions/upload-artifact@v4
         with:
-          name: nova-video-downloader.apk
-          path: nova-video-downloader.apk
+          name: nova-pip-video-test.apk
+          path: nova-pip-video-test.apk
           retention-days: 30

android-components/components/feature/media/src/main/java/mozilla/components/feature/media/fullscreen/MediaSessionFullscreenFeature.kt

@@ -58,20 +58,35 @@ class MediaSessionFullscreenFeature(
 
     @Suppress("SourceLockedOrientationActivity") // We deliberately want to lock the orientation here.
     private fun setOrientationForTabState(activeTabState: SessionState) {
-        when (activeTabState.mediaSessionState?.elementMetadata?.portrait) {
-            true ->
-                activity.requestedOrientation =
-                    ActivityInfo.SCREEN_ORIENTATION_USER_PORTRAIT
+        val metadata = activeTabState.mediaSessionState?.elementMetadata
 
-            false ->
-                if (activity.isInPictureInPictureMode) {
-                    activity.requestedOrientation = ActivityInfo.SCREEN_ORIENTATION_UNSPECIFIED
+        if (activity.isInPictureInPictureMode) {
+            activity.requestedOrientation = ActivityInfo.SCREEN_ORIENTATION_UNSPECIFIED
+            return
+        }
+
+        when {
+            // Prefer the actual media dimensions. This avoids a temporary
+            // portrait lock while a landscape player is still reporting its
+            // metadata during fullscreen entry.
+            metadata != null && metadata.width > 0L && metadata.height > 0L -> {
+                activity.requestedOrientation = if (metadata.height > metadata.width) {
+                    ActivityInfo.SCREEN_ORIENTATION_USER_PORTRAIT
                 } else {
-                    activity.requestedOrientation =
-                        ActivityInfo.SCREEN_ORIENTATION_SENSOR_LANDSCAPE
+                    ActivityInfo.SCREEN_ORIENTATION_SENSOR_LANDSCAPE
                 }
+            }
 
-            null -> activity.requestedOrientation = ActivityInfo.SCREEN_ORIENTATION_USER
+            // A video fullscreen session without dimensions should still
+            // enter landscape instead of briefly switching to portrait.
+            metadata?.videoTrackCount?.let { it > 0 } == true -> {
+                activity.requestedOrientation =
+                    ActivityInfo.SCREEN_ORIENTATION_SENSOR_LANDSCAPE
+            }
+
+            else -> {
+                activity.requestedOrientation = ActivityInfo.SCREEN_ORIENTATION_USER
+            }
         }
     }
 

android-components/components/feature/session/src/main/java/mozilla/components/feature/session/PictureInPictureFeature.kt

@@ -7,7 +7,9 @@ package mozilla.components.feature.session
 import android.app.Activity
 import android.app.PictureInPictureParams
 import android.content.pm.PackageManager
+import android.os.Build
 import mozilla.components.browser.state.action.ContentAction
+import mozilla.components.browser.state.state.SessionState
 import mozilla.components.browser.state.selector.findTabOrCustomTabOrSelectedTab
 import mozilla.components.browser.state.selector.selectedTab
 import mozilla.components.browser.state.store.BrowserStore
@@ -29,26 +31,44 @@ class PictureInPictureFeature(
     private val activity: Activity,
     private val crashReporting: CrashReporting? = null,
     private val tabId: String? = null,
+    private val isEnabled: () -> Boolean = { true },
 ) {
     internal val logger = Logger("PictureInPictureFeature")
 
     private val hasSystemFeature =
         activity.packageManager.hasSystemFeature(PackageManager.FEATURE_PICTURE_IN_PICTURE)
 
     fun onHomePressed(): Boolean {
-        if (!hasSystemFeature) {
+        // PiP is deliberately user-initiated from the in-video button. Do not
+        // intercept Home and unexpectedly move a video into PiP.
+        return false
+    }
+
+    /**
+     * Enters PiP from the dedicated in-video control.
+     */
+    fun enterPipMode(): Boolean {
+        if (!isEnabled() || !hasSystemFeature || Build.VERSION.SDK_INT < Build.VERSION_CODES.O) {
             return false
         }
 
         val session = store.state.findTabOrCustomTabOrSelectedTab(tabId)
-        val fullScreenMode = session?.content?.fullScreen == true
-        val contentIsPlaying = session?.mediaSessionState?.playbackState == MediaSession.PlaybackState.PLAYING
-        return fullScreenMode && contentIsPlaying && try {
-            enterPipModeCompat()
+        val mediaSession = session?.mediaSessionState
+        val canEnter =
+            mediaSession?.playbackState in listOf(
+                MediaSession.PlaybackState.PLAYING,
+                MediaSession.PlaybackState.PAUSED,
+            ) &&
+            mediaSession?.elementMetadata?.videoTrackCount?.let { it > 0 } == true
+
+        if (!canEnter) {
+            return false
+        }
+
+        return try {
+            updatePipParams(session)
+            activity.enterPictureInPictureMode(buildPipParams(session))
         } catch (e: IllegalStateException) {
-            // On certain Samsung devices, if accessibility mode is enabled, this will throw an
-            // IllegalStateException even if we check for the system feature beforehand. So let's
-            // catch it, log it, and not enter PiP. See https://stackoverflow.com/q/55288858
             logger.warn("Entering PipMode failed", e)
             crashReporting?.submitCaughtException(e)
             false
@@ -58,13 +78,89 @@ class PictureInPictureFeature(
     /**
      * Enter Picture-in-Picture mode.
      */
-    fun enterPipModeCompat() = when {
+    fun enterPipModeCompat(session: SessionState? = null) = when {
         !hasSystemFeature -> false
-        else -> enterPipModeForO()
+        else -> enterPipModeForO(session)
+    }
+
+    fun updatePipParams(session: SessionState?) {
+        if (!hasSystemFeature) {
+            return
+        }
+
+        val mediaSession = session?.mediaSessionState
+        val isVideoPlaying =
+            mediaSession?.playbackState in listOf(
+                MediaSession.PlaybackState.PLAYING,
+                MediaSession.PlaybackState.PAUSED,
+            ) &&
+            mediaSession?.elementMetadata?.videoTrackCount?.let { it > 0 } == true
+
+        val builder = PictureInPictureParams.Builder()
+
+        if (isVideoPlaying) {
+            val metadata = mediaSession?.elementMetadata
+            val width = metadata?.width ?: 0L
+            val height = metadata?.height ?: 0L
+
+            if (width > 0L && height > 0L) {
+                val ratio = width.toDouble() / height.toDouble()
+                if (ratio in (1.0 / 2.39)..2.39) {
+                    builder.setAspectRatio(
+                        android.util.Rational(
+                            width.coerceAtMost(Int.MAX_VALUE.toLong()).toInt(),
+                            height.coerceAtMost(Int.MAX_VALUE.toLong()).toInt(),
+                        ),
+                    )
+                }
+            }
+
+
+            if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.S) {
+                builder.setAutoEnterEnabled(false)
+                builder.setSeamlessResizeEnabled(true)
+            }
+        } else if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.S) {
+            builder.setAutoEnterEnabled(false)
+        }
+
+        activity.setPictureInPictureParams(builder.build())
     }
 
-    private fun enterPipModeForO() =
-        activity.enterPictureInPictureMode(PictureInPictureParams.Builder().build())
+    private fun enterPipModeForO(session: SessionState?) =
+        activity.enterPictureInPictureMode(buildPipParams(session))
+
+    private fun buildPipParams(session: SessionState?): PictureInPictureParams {
+        val builder = PictureInPictureParams.Builder()
+        val metadata = session?.mediaSessionState?.elementMetadata
+        val width = metadata?.width ?: 0L
+        val height = metadata?.height ?: 0L
+
+        if (width > 0L && height > 0L) {
+            val ratio = width.toDouble() / height.toDouble()
+            if (ratio in (1.0 / 2.39)..2.39) {
+                builder.setAspectRatio(
+                    android.util.Rational(
+                        width.coerceAtMost(Int.MAX_VALUE.toLong()).toInt(),
+                        height.coerceAtMost(Int.MAX_VALUE.toLong()).toInt(),
+                    ),
+                )
+            }
+        }
+
+        playerView?.let { view ->
+            val sourceRect = Rect()
+            if (view.getGlobalVisibleRect(sourceRect)) {
+                builder.setSourceRectHint(sourceRect)
+            }
+        }
+
+        if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.S) {
+            builder.setSeamlessResizeEnabled(true)
+        }
+
+        return builder.build()
+    }
 
     /**
      * Should be called when the system informs you of changes to and from picture-in-picture mode.