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

RESOLVED FIXED in Firefox 28



5 years ago
5 years ago


(Reporter: markh, Assigned: smaug)


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)


(Whiteboard: [qa-])


(2 attachments)



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]

untested. Will test tomorrow.
Attachment #825047 - Flags: review?(fabrice)
hrmm, a test failure
Comment on attachment 825047 [details] [diff] [review]

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]

So we had one test actually relying on the behavior :/
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+
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.