Closed
Bug 933070
Opened 11 years ago
Closed 11 years ago
Removing a listener from nsFrameMessageManager may prevent other listeners being called.
Categories
(Core :: DOM: Core & HTML, defect)
Core
DOM: Core & HTML
Tracking
()
People
(Reporter: markh, Assigned: smaug)
References
Details
(Whiteboard: [qa-])
Attachments
(2 files)
12.34 KB,
patch
|
Details | Diff | Splinter Review | |
13.55 KB,
patch
|
fabrice
:
review+
|
Details | Diff | Splinter Review |
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.
Updated•11 years ago
|
Component: IPC → DOM
Assignee | ||
Comment 1•11 years ago
|
||
Yup. We'll need to move to use ns(Auto)TObserverArray. (I was thinking about this last week.)
Assignee: nobody → bugs
Assignee | ||
Comment 2•11 years ago
|
||
untested. Will test tomorrow. https://tbpl.mozilla.org/?tree=Try&rev=7558760bd15f
Attachment #825047 -
Flags: review?(fabrice)
Assignee | ||
Comment 3•11 years ago
|
||
hrmm, a test failure
Comment 4•11 years ago
|
||
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)
Assignee | ||
Comment 5•11 years ago
|
||
So we had one test actually relying on the behavior :/ https://tbpl.mozilla.org/?tree=Try&rev=6f96c09bad57
Attachment #826743 -
Flags: review?(fabrice)
Assignee | ||
Comment 6•11 years ago
|
||
... but that looked like a bug in the test, so I just fixed it.
Assignee | ||
Comment 7•11 years ago
|
||
Try looks good now.
Updated•11 years ago
|
Attachment #826743 -
Flags: review?(fabrice) → review+
Assignee | ||
Comment 8•11 years ago
|
||
https://hg.mozilla.org/integration/mozilla-inbound/rev/070eda490577
Comment 9•11 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/070eda490577
Status: NEW → RESOLVED
Closed: 11 years ago
Flags: in-testsuite+
Resolution: --- → FIXED
Target Milestone: --- → mozilla28
Comment 10•10 years ago
|
||
We need to backport this to 1.2 to fix bug 937394.
Blocks: 937394
blocking-b2g: --- → koi?
Comment 12•10 years ago
|
||
https://hg.mozilla.org/releases/mozilla-b2g26_v1_2/rev/8f277371699d
status-b2g-v1.2:
--- → fixed
Updated•10 years ago
|
status-b2g-v1.3:
--- → fixed
status-firefox26:
--- → wontfix
status-firefox27:
--- → wontfix
status-firefox28:
--- → fixed
Comment 13•10 years ago
|
||
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-]
Updated•5 years ago
|
Component: DOM → DOM: Core & HTML
You need to log in
before you can comment on or make changes to this bug.
Description
•