1227604 - Compositor events can be in the wrong order

Status: RESOLVED FIXED

(Core Graveyard :: Widget: Android, defect)

Description

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

I found a bug in nsAppShell::LegacyGeckoEvent::PostTo, where a compositor event can be inadvertently added to the front of the event queue, when there are existing compositor events already in the queue.

Comment 1

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

Attachment: Fix compositor event order (v1)

When the queue only contains compositor events, a compositor event
should go to the back of the queue to maintain order.

Comment 2

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

Comment on attachment 8691448 [details] [diff] [review]
Fix compositor event order (v1)

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

This seems to put it at the back of the queue in the case where we have no other compositor events, which does not match your comment. Is that what you intended? Seems wrong to me...

Comment 3

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

(In reply to James Willcox (:snorp) ([e-mail]) from comment #2)
> Comment on attachment 8691448 [details] [diff] [review]
> Fix compositor event order (v1)
> 
> Review of attachment 8691448 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> This seems to put it at the back of the queue in the case where we have no
> other compositor events, which does not match your comment. Is that what you
> intended? Seems wrong to me...

It's kind of confusing, but |event| is null when the queue only has compositor events. In that case, we want the new event to go to the back of the queue to maintain order.

If the queue only has non-compositor events, |event| will point to the first element in the queue, and we correctly insert the new event in front of it.

Comment 4

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

Try: https://treeherder.mozilla.org/#/jobs?repo=try&revision=473a369fecbf&selectedJob=14106296

Comment 5

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/3b624179e13b

Comment 6

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

https://hg.mozilla.org/integration/mozilla-inbound/rev/3b624179e13b

Comment 7

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

Comment on attachment 8691448 [details] [diff] [review]
Fix compositor event order (v1)

Approval Request Comment
[Feature/regressing bug #]: Bug 1197976
[User impact if declined]: Possible glitches when showing or hiding the app.
[Describe test coverage new/current, TreeHerder]: Locally
[Risks and why]: Very small; patch corrects a single-line mistake and doesn't affect anything else.
[String/UUID change made/needed]: None

Comment 8

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/3b624179e13b

Comment 9

[Ritu Kothari (:ritu) (Inactive, please n-i to RyanVM, jcristau, or pascal)]

Comment on attachment 8691448 [details] [diff] [review]
Fix compositor event order (v1)

Aurora44+

Comment 10

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/a0e9399f1eef

Comment 11

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/releases/mozilla-b2g44_v2_5/rev/a0e9399f1eef