568818 - sendSyncMessage fails with "SyntaxError: JSON.parse" if one of the listeners did not return a value

Status: RESOLVED FIXED

(Core :: DOM: Events, defect)

Description

[Vivien Nicolas (:vingtetun) (:21) - (NOT reading bugmails, needinfo? please)]

Attachment: Patch

If you have multiple listeners on one message and if one of them doesn't return a value, sendSyncMessage throw an error.

The attached patch (2 lines) ignore the the retval if it has a length of zero which allow listener to return nothing and/or use "return;" to break the listener code if needed.

Comment 1

[Vivien Nicolas (:vingtetun) (:21) - (NOT reading bugmails, needinfo? please)]

do I need a sr? for this patch?

Comment 2

[Olli Pettay [:smaug][[e-mail]]]

No.

I'll add some tests for this.

Comment 3

[Mark Finkle (:mfinkle) (use needinfo?)]

This bug is causing us some grief. Any idea when we can get it landed?

Comment 4

[Vivien Nicolas (:vingtetun) (:21) - (NOT reading bugmails, needinfo? please)]

(In reply to comment #3)
> This bug is causing us some grief. Any idea when we can get it landed?

http://hg.mozilla.org/mozilla-central/rev/d1c910f2ec4d

Comment 5

[(not currently active) Ted Mielczarek]

I backed this out because it seemed to be causing a lot more crashes in the messageManager tests:
http://hg.mozilla.org/mozilla-central/rev/ef715f78e927

Comment 6

[Olli Pettay [:smaug][[e-mail]]]

Huh. What kind of crashes?

Comment 7

[Vivien Nicolas (:vingtetun) (:21) - (NOT reading bugmails, needinfo? please)]

I've re-runned all the tests yesterday and everything seems fine (http://ftp.mozilla.org/pub/mozilla.org/firefox/tryserver-builds/[e-mail]-e2346fa5faaa/).

I really think the crashes were from an other point, and now that messageManager is very stable, if we start to see crashes again we can consider this bug as the root cause but it is unlikely.

http://hg.mozilla.org/mozilla-central/rev/adaf0592cdfa

Comment 8

[Dão Gottwald [:dao]]

backed out because of test_bug549682.xul failures

Comment 9

[Vivien Nicolas (:vingtetun) (:21) - (NOT reading bugmails, needinfo? please)]

Ok, one line change to prevent the return value to have crazy pointer that cause 
the crash.

http://hg.mozilla.org/mozilla-central/rev/5252602a7e19