Closed Bug 1458258 Opened 3 years ago Closed 3 years ago

Switching away from GeckoView and back again results in black screen

Categories

(Core :: Graphics: Layers, defect, P1)

59 Branch
Unspecified
Android
defect

Tracking

()

RESOLVED FIXED
mozilla62
Tracking Status
firefox-esr52 --- wontfix
firefox-esr60 --- wontfix
firefox59 --- wontfix
firefox60 --- wontfix
firefox61 --- wontfix
firefox62 --- fixed

People

(Reporter: snorp, Assigned: snorp)

Details

(Whiteboard: [geckoview:e10s] [geckoview:klar:p1] [geckoview:fenix] )

Attachments

(1 file)

STR

1) Start GeckoViewExample
2) Wait for page to load
3) Hit the home button
4) Switch back to GeckoViewExample
5) Observe black screen, which is resolved if you touch

This doesn't happen in Fennec or in GeckoViewExample without e10s.
I've determined that this is caused by CompositorBridgeParent::Invalidate() passing an empty region to LayerManager::AddInvalidRegion().
Component: GeckoView → Graphics: Layers
Product: Firefox for Android → Core
Version: Firefox 59 → 59 Branch
OS: Unspecified → Android
Kats if there is someone better for this review, feel free to redirect. Also, I am not at all sure why this patch is required, as GetLocalVisibleRegion() seems to be the thing that gets used pretty much everywhere. It does seem to work in both e10s and non, however.
Assignee: nobody → snorp
Attachment #8972431 - Flags: review?(bugmail) → review?(matt.woodrow)
Comment on attachment 8972431 [details]
Bug 1458258 - Fix compositor invalidatation after resume with e10s

https://reviewboard.mozilla.org/r/241030/#review246836

::: commit-message-d3643:1
(Diff revision 1)
> +Bug 1458258 - Fix compositor invalidatation after resume with e10s r=kats

Matt would be a better reviewer. Also you have a typo "invalidation"
Comment on attachment 8972431 [details]
Bug 1458258 - Fix compositor invalidatation after resume with e10s

https://reviewboard.mozilla.org/r/241030/#review246982

The normal visible region is what layout computed, the shadow visible region is that, plus takes into account any async changes (like APZ, async transform animations).

I don't really understand why this is needed, I guess we recomputed the shadow visible to be empty, but I'm not sure why.

Can we just instead just add HostLayerManager::InvalidateAll? For both impls, I think we can just use mRenderBounds and set the invalid region to that. I think that makes the intent a bit clearer.
Priority: -- → P1
Whiteboard: [geckoview:e10s] [geckoview:klar] → [geckoview:e10s] [geckoview:klar] [geckoview:fenix]
Whiteboard: [geckoview:e10s] [geckoview:klar] [geckoview:fenix] → [geckoview:e10s] [geckoview:klar:p1] [geckoview:fenix]
Matt I updated the patch but not sure if you got a notification, so NI here.
Flags: needinfo?(matt.woodrow)
Comment on attachment 8972431 [details]
Bug 1458258 - Fix compositor invalidatation after resume with e10s

https://reviewboard.mozilla.org/r/241030/#review249852
Attachment #8972431 - Flags: review?(matt.woodrow) → review+
(In reply to James Willcox (:snorp) (jwillcox@mozilla.com) from comment #7)
> Matt I updated the patch but not sure if you got a notification, so NI here.

I indeed missed it, thanks for the reminder!
Flags: needinfo?(matt.woodrow)
Pushed by jwillcox@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/b51a237fe66d
Fix compositor invalidatation after resume with e10s r=mattwoodrow
https://hg.mozilla.org/mozilla-central/rev/b51a237fe66d
Status: NEW → RESOLVED
Closed: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla62
You need to log in before you can comment on or make changes to this bug.