1660546 - Calling GeckoView.setSession() with an existing session sometimes fails to draw new session

Status: RESOLVED FIXED

(GeckoView :: General, defect, P1)

Description

[James Willcox (:snorp) ([e-mail]) (he/him)]

It appears that if you have an existing GeckoView with an existing session on the screen, then swap that out to a new session via GeckoView.setSession(), the compositor in the new session is sometimes not correctly initialized and the GeckoView displays the content of the old session. We see the following in logs:

2020-08-21 15:48:19.232 628-2439/? E/BufferQueueProducer: [SurfaceView - org.mozilla.fenix.debug/org.mozilla.fenix.debug.App#0] connect: already connected (cur=1 req=1)
2020-08-21 15:48:19.233 4369-4546/org.mozilla.fenix.debug E/libEGL: eglCreateWindowSurface: native_window_api_connect (win=0x76e7e9f010) failed (0xffffffea) (already connected to another API?)
2020-08-21 15:48:19.233 4369-4546/org.mozilla.fenix.debug E/libEGL: eglCreateWindowSurfaceTmpl:730 error 3003 (EGL_BAD_ALLOC)
2020-08-21 15:48:19.814 788-4684/? E/ResolverController: No valid NAT64 prefix (305, <unspecified>/0)
2020-08-21 15:48:31.663 2378-2486/? E/FirebaseMessaging: Topic operation failed: SERVICE_NOT_AVAILABLE. Will retry Topic operation.

This seems to indicate that we are not destroying the EGLSurface for the outgoing session before the incoming one is initialized.

Comment 1

[Chenxia Liu [:liuche]]

Talked to AC and this sounds like the most critical bug for 81, and would need to be uplifted into GV81 for the September release. Fenix 81 is planned to release to Beta just over a week from now 9/7 or so.

Comment 2

[James Willcox (:snorp) ([e-mail]) (he/him)]

I don't know if this is the cause of all of the problems, but at least one thing I'm seeing seems to be:

1. Create a window (session), attach to a surface.
2. Do some stuff with the window (load pages, etc)
3. Close the session
4. Create a new session and attach it to the prior surface
5. New session fails to create the EGLSurface because there is some async compositor shutdown happening in step 3 and it hasn't happened yet. Consequently, The first session still has an EGLSurface bound to the same Surface we're using here.

Comment 3

[Kevin Brosnan [Ex-Mozilla]]

[Tracking Requested - why for this release]: We want to make sure we address for 81

Comment 4

[James Willcox (:snorp) ([e-mail]) (he/him)]

Attachment: Bug 1660546 - Ensure Surface is released when closing GeckoSession

Without this, the asynchronous nature of compositor shutdown causes
races if the Android surface is immediately reused with another
GeckoSession instance.

Comment 5

[James Willcox (:snorp) ([e-mail]) (he/him)]

Comment on attachment 9173441 [details]
Bug 1660546 - Ensure Surface is released when closing GeckoSession

Beta/Release Uplift Approval Request

• User impact if declined: Blank tabs occassionally
• Is this code covered by automated tests?: No
• Has the fix been verified in Nightly?: No
• Needs manual test from QE?: No
• If yes, steps to reproduce:
• List of other uplifts needed: None
• Risk to taking this patch: Low
• Why is the change risky/not risky? (and alternatives if risky): One line change that is presumed safe
• String changes made/needed: None

Comment 6

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/b1a38140966e
Ensure Surface is released when closing GeckoSession r=geckoview-reviewers,aklotz

Comment 7

[James Willcox (:snorp) ([e-mail]) (he/him)]

(In reply to James Willcox (:snorp) ([e-mail]) (he/him) from comment #5)

• List of other uplifts needed: None

Whoops, we also need Bug 1662528

Comment 8

[Noemi Erli[:noemi_erli]]

https://hg.mozilla.org/mozilla-central/rev/b1a38140966e

Comment 9

[Ryan VanderMeulen [:RyanVM]]

(In reply to James Willcox (:snorp) ([e-mail]) (he/him) from comment #7)

Whoops, we also need Bug 1662528

Please request approval for this in the other bug.

Comment 10

[Ryan VanderMeulen [:RyanVM]]

Comment on attachment 9173441 [details]
Bug 1660546 - Ensure Surface is released when closing GeckoSession

Approved for 81.0b6.

Comment 11

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-beta/rev/dd92809b4d2f