1297836 - Minor optimizations for LayerView.Compositor

Status: RESOLVED FIXED

(Core Graveyard :: Widget: Android, defect, P3)

Comment 1

[(inactive) Jim Chen [:jchen] [:darchons]]

Attachment: Don't use WindowEvent for LayerViewSupport (v1)

Now that nsWindow doesn't own LayerViewSupport, we shouldn't be using
WindowEvent for LayerViewSupport calls. This patch converts the calls
that dispatch to proxy to dispatch directly to Gecko. For
SyncResumeResizeCompositor, it used a proxy to call OnResumedCompositor
on the Gecko thread; this patch makes SyncResumeResizeCompositor post an
event to call OnResumedCompositor directly, without going through the
proxy.

Comment 2

[(inactive) Jim Chen [:jchen] [:darchons]]

Attachment: Get surface when creating or resuming compositor (v1)

Get rid of LayerView.Compositor.getSurface and just pass in the surface
when creating or resuming the compositor. That also lets us get rid of
some synchronization required for getSurface.

Comment 3

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

Comment on attachment 8784561 [details] [diff] [review]
Get surface when creating or resuming compositor (v1)

Review of attachment 8784561 [details] [diff] [review]:
-----------------------------------------------------------------

I think you need to clear mSurface in SyncPauseCompositor(), otherwise we'll end up returning a stale one. Previously, LVS::GetSurface() would've obtained a null one and set it there.

That being said, we should never be trying to consume a null surface, so it's probably not a huge deal in practice.

Comment 4

[(inactive) Jim Chen [:jchen] [:darchons]]

(In reply to James Willcox (:snorp) ([e-mail]) from comment #3)
> Comment on attachment 8784561 [details] [diff] [review]
> Get surface when creating or resuming compositor (v1)
> 
> Review of attachment 8784561 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> I think you need to clear mSurface in SyncPauseCompositor(), otherwise we'll
> end up returning a stale one. Previously, LVS::GetSurface() would've
> obtained a null one and set it there.
> 
> That being said, we should never be trying to consume a null surface, so
> it's probably not a huge deal in practice.

Yeah, we hard crash in GLContextProviderEGL if we return a null surface. Other code calls SyncPauseCompositor expecting the surface to stay the same, so I didn't clear the surface in SyncPauseCompositor.

Comment 5

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/59fceff95c70
Don't use WindowEvent for LayerViewSupport; r=snorp
https://hg.mozilla.org/integration/mozilla-inbound/rev/db1826bc2da5
Get surface when creating or resuming compositor; r=snorp

Comment 6

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/59fceff95c70
https://hg.mozilla.org/mozilla-central/rev/db1826bc2da5