Closed Bug 1227604 Opened 4 years ago Closed 4 years ago

Compositor events can be in the wrong order

Categories

(Core :: Widget: Android, defect)

All
Android
defect
Not set

Tracking

()

RESOLVED FIXED
mozilla45
Tracking Status
firefox44 --- fixed
firefox45 --- fixed
b2g-v2.5 --- fixed

People

(Reporter: jchen, Assigned: jchen)

Details

Attachments

(1 file)

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.
When the queue only contains compositor events, a compositor event
should go to the back of the queue to maintain order.
Attachment #8691448 - Flags: review?(snorp)
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...
Attachment #8691448 - Flags: review?(snorp) → review-
(In reply to James Willcox (:snorp) (jwillcox@mozilla.com) 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.
Attachment #8691448 - Flags: review- → review?(snorp)
Attachment #8691448 - Flags: review?(snorp) → review+
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
Attachment #8691448 - Flags: approval-mozilla-aurora?
https://hg.mozilla.org/mozilla-central/rev/3b624179e13b
Status: ASSIGNED → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla45
Comment on attachment 8691448 [details] [diff] [review]
Fix compositor event order (v1)

Aurora44+
Attachment #8691448 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
You need to log in before you can comment on or make changes to this bug.