try https://tbpl.mozilla.org/?tree=Try&rev=d7ecca911faa

Comment on attachment 710124 [details] [diff] [review] Implement SmsEvent using event generator >diff --git a/dom/sms/tests/marionette/test_outgoing.js b/dom/sms/tests/marionette/test_outgoing.js >--- a/dom/sms/tests/marionette/test_outgoing.js >+++ b/dom/sms/tests/marionette/test_outgoing.js >@@ -99,18 +99,17 @@ function doSendMessageAndCheckSuccess(re >- ok(event instanceof MozSmsEvent, >- "event is instanceof " + event.constructor); >+ ok(event, "event is valid"); Test whether the event is an instance of MozSmsEvent throws an exception and fails the test case. The command for e in `cat js/xpconnect/src/event_impl_gen.conf.in | \ grep Event | \ cut -d\' -f 2`; do \ grep -nR --exclude-dir=.git "instanceof $e"; \ done gives a list of generated event types which is then tested with instanceof operator in some test script. I got only one -- ProgressEvent. However, ProgressEvent is actually defined in dom/workers/Events.cpp:799 and is not a typical generated event as I've ever seen. Is there any way I can keep the original test case?

(In reply to Vicamo Yang [:vicamo][:vyang] from comment #2) > Is there any way I can keep the original test case? The same thing happened in bug 813596 comment #5, test cases for CellBroadcastEvent.

ProgressEvent in workers is a different thing. For main thread progressevent is implemented using codegen.

Comment on attachment 710124 [details] [diff] [review] Implement SmsEvent using event generator Why do you change the uuid of nsIDOMMozSmsMessage? I don't understand why the changes to test_outgoing.js are needed.

I mean bug 813596 comment #5 talks about using ok(event, ...) because instanceof crashes. Does it really crash? If so, we need to fix that crash, not just test something else. If JS can crash the browser, it means webpages can crash it. Not good. As an example (new ProgressEvent("foo")) instanceof ProgressEvent.constructor certainly doesn't crash on desktop.

(In reply to Olli Pettay [:smaug] from comment #5) > Why do you change the uuid of nsIDOMMozSmsMessage? I renamed nsIDOMSmsMessage.idl to nsIDOMMozSmsMessage.idl. Do I have to change uuid in this case?

fix https://tbpl.mozilla.org/?tree=Try&rev=2b61804a81bd

We did received onsending notification in line 713, but nothing happens then. It's because testing whether the received event is an instance of MozSmsEvent triggers an exception "'prototype' property of MozSmsEvent is not an object" and skips all following handling code. Soon in line 745, the test script fails.

(In reply to Vicamo Yang [:vicamo][:vyang] from comment #7) > I renamed nsIDOMSmsMessage.idl to nsIDOMMozSmsMessage.idl. Do I have to > change uuid in this case? Well, the interface itself stays the same, not uuid change isn't needed. (In reply to Vicamo Yang [:vicamo][:vyang] from comment #9) > We did received onsending notification in line 713, but nothing happens > then. It's because testing whether the received event is an instance of > MozSmsEvent triggers an exception "'prototype' property of MozSmsEvent is > not an object" and skips all following handling code. Soon in line 745, the > test script fails. Do you know why we get that exception. On desktop I can certainly do the instanceof check for generated events.

(In reply to Olli Pettay [:smaug] from comment #10) > Do you know why we get that exception. On desktop I can > certainly do the instanceof check for generated events. I noticed you test following pattern in comment #6, (new ProgressEvent("foo")) instanceof ProgressEvent.constructor but that doesn't work for me, either. I got: invalid 'instanceof' operand ProgressEvent.constructor And test (new ProgressEvent("foo")) instanceof ProgressEvent results in the same exception: 'prototype' property of ProgressEvent is not an object

bug 838542 filed, will address this in test scripts comments.

mention bug 838542 in comments.

Comment on attachment 710628 [details] [diff] [review] v3 Throwing is odd, but not about this bug.