Bug 1645114 - for screenshot capture, include exception in failure result rather than throwing. r=geckoview-reviewers,tthibaud,twhite

Also remove exception handling at call site in Android components - no need to catch exceptions for method that no longer throws.

Differential Revision: https://phabricator.services.mozilla.com/D206488

mobile/android/android-components/components/browser/engine-gecko/src/main/java/mozilla/components/browser/engine/gecko/GeckoEngineView.kt

@@ -205,29 +205,18 @@ class GeckoEngineView @JvmOverloads constructor(
         geckoView.activityContextDelegate = GeckoViewActivityContextDelegate(WeakReference(context))
     }
 
-    @Suppress("TooGenericExceptionCaught")
     override fun captureThumbnail(onFinish: (Bitmap?) -> Unit) {
-        try {
         val geckoResult = geckoView.capturePixels()
         geckoResult.then(
             { bitmap ->
                 onFinish(bitmap)
-                    GeckoResult<Void>()
+                GeckoResult()
             },
             {
                 onFinish(null)
                 GeckoResult<Void>()
             },
         )
-        } catch (e: Exception) {
-            // There's currently no reliable way for consumers of GeckoView to
-            // know whether or not the compositor is ready. So we have to add
-            // a catch-all here. In the future, GeckoView will invoke our error
-            // callback instead and this block can be removed:
-            // https://bugzilla.mozilla.org/show_bug.cgi?id=1645114
-            // https://github.com/mozilla-mobile/android-components/issues/6680
-            onFinish(null)
-        }
     }
 
     override fun clearSelection() {

mobile/android/android-components/components/browser/engine-gecko/src/test/java/mozilla/components/browser/engine/gecko/GeckoEngineViewTest.kt

@@ -90,15 +90,6 @@ class GeckoEngineViewTest {
         shadowOf(getMainLooper()).idle()
 
         assertNull(thumbnail)
-
-        // Test GeckoView throwing an exception
-        whenever(mockGeckoView.capturePixels()).thenThrow(IllegalStateException("Compositor not ready"))
-
-        thumbnail = mock()
-        engineView.captureThumbnail {
-            thumbnail = it
-        }
-        assertNull(thumbnail)
     }
 
     @Test

mobile/android/android-components/docs/changelog.md

@@ -10,6 +10,9 @@ permalink: /changelog/
   * Added `StartForegroundService` to safely start a foreground service, see [Bug 1839039](https://bugzilla.mozilla.org/show_bug.cgi?id=1839039) for crash reference.
   * Added `ProcessInfoProvider` and `BuildVersionProvider` to get information about the app process and the build version.
 
+* **browser-engine-gecko**
+    * For screenshot capture, include exception in failure result rather than throwing.
+
 # 126.0
 
 * **browser-menu**

mobile/android/geckoview/src/androidTest/java/org/mozilla/geckoview/test/ScreenshotTest.kt

@@ -12,7 +12,7 @@ import androidx.test.ext.junit.runners.AndroidJUnit4
 import androidx.test.filters.MediumTest
 import androidx.test.platform.app.InstrumentationRegistry
 import org.hamcrest.Matchers.* // ktlint-disable no-wildcard-imports
-import org.junit.Assert
+import org.junit.Assert.*
 import org.junit.Assume.assumeThat
 import org.junit.Test
 import org.junit.runner.RunWith
@@ -25,6 +25,7 @@ import org.mozilla.geckoview.GeckoSession.ContentDelegate
 import org.mozilla.geckoview.GeckoSession.ProgressDelegate
 import org.mozilla.geckoview.test.rule.GeckoSessionTestRule.AssertCalled
 import org.mozilla.geckoview.test.rule.GeckoSessionTestRule.WithDisplay
+import org.mozilla.geckoview.test.util.UiThreadUtils
 import java.lang.IllegalStateException
 import kotlin.math.absoluteValue
 import kotlin.math.max
@@ -142,6 +143,27 @@ class ScreenshotTest : BaseSessionTest() {
         }
     }
 
+    @WithDisplay(height = SCREEN_HEIGHT, width = SCREEN_WIDTH)
+    @Test
+    fun capturePixelsFailsWhenCompositorNotReady() {
+        sessionRule.display?.let { display ->
+            mainSession.close()
+            var exceptionListenerCalled = false
+            val result = display.capturePixels()
+            result.exceptionally { error: Throwable ->
+                assertTrue(error is IllegalStateException)
+                exceptionListenerCalled = true
+                result
+            }.accept {
+                fail("screenshot shouldn't complete successfully after session is closed")
+            }
+            UiThreadUtils.waitForCondition(
+                { exceptionListenerCalled },
+                sessionRule.env.defaultTimeoutMillis,
+            )
+        } ?: run { fail("no display found") }
+    }
+
     // This tests tries to catch problems like Bug 1644561.
     @WithDisplay(height = SCREEN_HEIGHT, width = SCREEN_WIDTH)
     @Test
@@ -430,7 +452,7 @@ class ScreenshotTest : BaseSessionTest() {
             .capture()
             .exceptionally(
                 OnExceptionListener<Throwable> { error: Throwable ->
-                    Assert.assertTrue(error is OutOfMemoryError)
+                    assertTrue(error is OutOfMemoryError)
                     fromException(error)
                 },
             )

mobile/android/geckoview/src/main/java/org/mozilla/geckoview/GeckoDisplay.java

@@ -453,13 +453,15 @@ public class GeckoDisplay {
      * <p>This function must be called on the UI thread.
      *
      * @return A {@link GeckoResult} that completes with a {@link Bitmap} containing the pixels and
-     *     size information of the requested portion of the visible web page.
+     *     size information of the requested portion of the visible web page, or returns a failure
+     *     {@link GeckoResult} including the reason why in an {@link Exception}
      */
     @UiThread
     public @NonNull GeckoResult<Bitmap> capture() {
       ThreadUtils.assertOnUiThread();
       if (!mSession.isCompositorReady()) {
-        throw new IllegalStateException("Compositor must be ready before pixels can be captured");
+        return GeckoResult.fromException(
+            new IllegalStateException("Compositor must be ready before pixels can be captured"));
       }
 
       final GeckoResult<Bitmap> result = new GeckoResult<>();