Compositor events can be in the wrong order

RESOLVED FIXED in Firefox 44, Firefox OS v2.5

Status

()

Core
Widget: Android
RESOLVED FIXED
2 years ago
2 years ago

People

(Reporter: jchen, Assigned: jchen)

Tracking

unspecified
mozilla45
All
Android
Points:
---

Firefox Tracking Flags

(firefox44 fixed, firefox45 fixed, b2g-v2.5 fixed)

Details

Attachments

(1 attachment)

(Assignee)

Description

2 years ago
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.
(Assignee)

Comment 1

2 years ago
Created attachment 8691448 [details] [diff] [review]
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.
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-
(Assignee)

Comment 3

2 years ago
(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.
(Assignee)

Updated

2 years ago
Attachment #8691448 - Flags: review- → review?(snorp)
Attachment #8691448 - Flags: review?(snorp) → review+
(Assignee)

Comment 7

2 years ago
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?

Comment 8

2 years ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/3b624179e13b
Status: ASSIGNED → RESOLVED
Last Resolved: 2 years ago
status-firefox45: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla45

Comment 9

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

Aurora44+
Attachment #8691448 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+

Updated

2 years ago
status-firefox44: --- → affected

Comment 10

2 years ago
bugherderuplift
https://hg.mozilla.org/releases/mozilla-aurora/rev/a0e9399f1eef
status-firefox44: affected → fixed
You need to log in before you can comment on or make changes to this bug.