Closed Bug 933070 Opened 8 years ago Closed 8 years ago

Removing a listener from nsFrameMessageManager may prevent other listeners being called.

Categories

(Core :: DOM: Core & HTML, defect)

defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla28
blocking-b2g koi+
Tracking Status
firefox26 --- wontfix
firefox27 --- wontfix
firefox28 --- fixed
b2g-v1.2 --- fixed
b2g-v1.3 --- fixed

People

(Reporter: markh, Assigned: smaug)

References

Details

(Whiteboard: [qa-])

Attachments

(2 files)

If a nsFrameMessageManager has multiple listeners, and one of the listeners removes itself while handling a message, another listener for the same message may not be called.

I believe this is due to removeMessageListener removing the element from the array while messages are being dispatched, thus moving all other listeners in the array down by one.  The loop dispatching messages doesn't handle this, and thus skips the message listener that is now at the index of the one that was removed.

I'll try and work up a test case next week.
Yup. We'll need to move to use ns(Auto)TObserverArray.
(I was thinking about this last week.)
Assignee: nobody → bugs
Attached patch wipSplinter Review
untested. Will test tomorrow.
https://tbpl.mozilla.org/?tree=Try&rev=7558760bd15f
Attachment #825047 - Flags: review?(fabrice)
hrmm, a test failure
Comment on attachment 825047 [details] [diff] [review]
wip

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

Just clearing until you fix the test failures.
Attachment #825047 - Flags: review?(fabrice)
Attached patch patchSplinter Review
So we had one test actually relying on the behavior :/

https://tbpl.mozilla.org/?tree=Try&rev=6f96c09bad57
Attachment #826743 - Flags: review?(fabrice)
... but that looked like a bug in the test, so I just fixed it.
Try looks good now.
Attachment #826743 - Flags: review?(fabrice) → review+
https://hg.mozilla.org/mozilla-central/rev/070eda490577
Status: NEW → RESOLVED
Closed: 8 years ago
Flags: in-testsuite+
Resolution: --- → FIXED
Target Milestone: --- → mozilla28
We need to backport this to 1.2 to fix bug 937394.
Blocks: 937394
blocking-b2g: --- → koi?
koi+ blocking a blocker
blocking-b2g: koi? → koi+
Given in-testsuite coverage this fix will not be manually verified by QA. If you believe this warrants extra QA attention please nominate for testing by removing this whiteboard tag and adding the verifyme keyword. Please also provide any details you have that may inform our testing.
Whiteboard: [qa-]
Component: DOM → DOM: Core & HTML
You need to log in before you can comment on or make changes to this bug.