1458258 - Switching away from GeckoView and back again results in black screen

Status: RESOLVED FIXED

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

Description

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

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.

Comment 1

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

I've determined that this is caused by CompositorBridgeParent::Invalidate() passing an empty region to LayerManager::AddInvalidRegion().

Comment 2

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

Attachment: Bug 1458258 - Fix compositor invalidatation after resume with e10s

Review commit: https://reviewboard.mozilla.org/r/241030/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/241030/

Comment 3

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

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.

Comment 4

[Kartikaya Gupta (email:[e-mail])]

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 5

[Matt Woodrow (:mattwoodrow)]

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.

Comment 6

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

Comment on attachment 8972431 [details]
Bug 1458258 - Fix compositor invalidatation after resume with e10s

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/241030/diff/1-2/

Comment 7

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

Matt I updated the patch but not sure if you got a notification, so NI here.

Comment 8

[Matt Woodrow (:mattwoodrow)]

Comment on attachment 8972431 [details]
Bug 1458258 - Fix compositor invalidatation after resume with e10s

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

Comment 9

[Matt Woodrow (:mattwoodrow)]

(In reply to James Willcox (:snorp) ([e-mail]) 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!

Comment 10

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/b51a237fe66d
Fix compositor invalidatation after resume with e10s r=mattwoodrow

Comment 11

[Eliza Balazs [:ebalazs_]]

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