Commit 1fed2331 authored by MozLando's avatar MozLando
Browse files

Merge #5219 

5219: Closes #5189: Make sure compositor is ready before capturing pixels r=Amejia481 a=csadilek

GeckoView used to return a `GeckoResult.fromException` when the compositor wasn't ready which we ignored (and tried again later). It recently started throwing the exception instead: https://hg.mozilla.org/integration/autoland/rev/2a8ed7fb920c#l11.59

We now need to either ignore the exception or better check if the session ever had a "contenfulPaint". That's also what FxR does: https://github.com/MozillaReality/FirefoxReality/blob/master/app/src/common/shared/org/mozilla/vrbrowser/browser/engine/Session.java#L472



So, we only capture the pixels now if content was ever rendered for the session.

Co-authored-by: default avatarChristian Sadilek <christian.sadilek@gmail.com>
parents 7291f30d 849f99cd

components/browser/engine-gecko-nightly/src/main/java/mozilla/components/browser/engine/gecko/GeckoEngineSession.kt

+10 −0
Original line number Diff line number Diff line
@@ -65,6 +65,11 @@ class GeckoEngineSession( 
    internal lateinit var geckoSession: GeckoSession

    internal var currentUrl: String? = null

    internal var scrollY: Int = 0



    // This is set once the first content paint has occurred and can be used to

    // decide if it's safe to call capturePixels on the view.

    internal var firstContentfulPaint = false



    internal var job: Job = Job()

    private var lastSessionState: GeckoSession.SessionState? = null

    private var stateBeforeCrash: GeckoSession.SessionState? = null
@@ -314,6 +319,7 @@ class GeckoEngineSession( 
        super.close()

        job.cancel()

        geckoSession.close()

        firstContentfulPaint = false

    }



    /**
@@ -559,6 +565,10 @@ class GeckoEngineSession( 
    internal fun createContentDelegate() = object : GeckoSession.ContentDelegate {

        override fun onFirstComposite(session: GeckoSession) = Unit



        override fun onFirstContentfulPaint(session: GeckoSession) {

            firstContentfulPaint = true

        }



        override fun onContextMenu(

            session: GeckoSession,

            screenX: Int,

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

+12 −8
Original line number Diff line number Diff line
@@ -146,6 +146,9 @@ class GeckoEngineView @JvmOverloads constructor( 
    }



    override fun captureThumbnail(onFinish: (Bitmap?) -> Unit) {

        // Do not capture pixels if no content has been rendered for this session yet. GeckoView

        // might otherwise throw an IllegalStateException if the compositor isn't ready.

        if (currentSession?.firstContentfulPaint == true) {

            val geckoResult = currentGeckoView.capturePixels()

            geckoResult.then({ bitmap ->

                onFinish(bitmap)
@@ -156,3 +159,4 @@ class GeckoEngineView @JvmOverloads constructor( 
            })

        }

    }

}

components/browser/engine-gecko-nightly/src/test/java/mozilla/components/browser/engine/gecko/GeckoEngineSessionTest.kt

+15 −0
Original line number Diff line number Diff line
@@ -2064,6 +2064,21 @@ class GeckoEngineSessionTest { 
        assertEquals(WindowRequest.Type.CLOSE, receivedWindowRequest!!.type)

    }



    @Test

    fun managesStateOfFirstContenfulPaint() {

        val engineSession = GeckoEngineSession(mock(),

                geckoSessionProvider = geckoSessionProvider)



        captureDelegates()



        assertFalse(engineSession.firstContentfulPaint)

        contentDelegate.value.onFirstContentfulPaint(geckoSession)

        assertTrue(engineSession.firstContentfulPaint)



        engineSession.close()

        assertFalse(engineSession.firstContentfulPaint)

    }



    private fun mockGeckoSession(): GeckoSession {

        val session = mock<GeckoSession>()

        whenever(session.settings).thenReturn(

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

+13 −0
Original line number Diff line number Diff line
@@ -49,6 +49,7 @@ class GeckoEngineViewTest { 


    @Test

    fun captureThumbnail() {

        val geckoSession: GeckoEngineSession = mock()

        val engineView = GeckoEngineView(context)

        val mockGeckoView = mock<NestedGeckoView>()

        var thumbnail: Bitmap? = null
@@ -60,7 +61,19 @@ class GeckoEngineViewTest { 
        engineView.captureThumbnail {

            thumbnail = it

        }

        verify(mockGeckoView, never()).capturePixels()



        engineView.currentSession = geckoSession

        engineView.captureThumbnail {

            thumbnail = it

        }

        verify(mockGeckoView, never()).capturePixels()



        whenever(geckoSession.firstContentfulPaint).thenReturn(true)

        engineView.captureThumbnail {

            thumbnail = it

        }

        verify(mockGeckoView).capturePixels()

        geckoResult.complete(mock())

        assertNotNull(thumbnail)