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

RESOLVED FIXED in Firefox 28

Status

()

RESOLVED FIXED
5 years ago
5 years ago

People

(Reporter: markh, Assigned: smaug)

Tracking

Trunk
mozilla28
Points:
---
Dependency tree / graph
Bug Flags:
in-testsuite +

Firefox Tracking Flags

(blocking-b2g:koi+, firefox26 wontfix, firefox27 wontfix, firefox28 fixed, b2g-v1.2 fixed, b2g-v1.3 fixed)

Details

(Whiteboard: [qa-])

Attachments

(2 attachments)

(Reporter)

Description

5 years ago
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.
Component: IPC → DOM
Yup. We'll need to move to use ns(Auto)TObserverArray.
(I was thinking about this last week.)
Assignee: nobody → bugs
Created attachment 825047 [details] [diff] [review]
wip

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)
Created attachment 826743 [details] [diff] [review]
patch

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
Last Resolved: 5 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+
status-b2g-v1.3: --- → fixed
status-firefox26: --- → wontfix
status-firefox27: --- → wontfix
status-firefox28: --- → fixed
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-]
You need to log in before you can comment on or make changes to this bug.