Closed Bug 872470 Opened 11 years ago Closed 11 years ago

add EventSource to frameworker


(Firefox Graveyard :: SocialAPI, defect)

22 Branch
Not set


(firefox23 fixed, firefox24 fixed)

Firefox 24
Tracking Status
firefox23 --- fixed
firefox24 --- fixed


(Reporter: mixedpuppy, Assigned: mixedpuppy)



(Whiteboard: [qa-])


(1 file, 1 obsolete file)

Attached patch add eventsource to frameworker (obsolete) — Splinter Review
EventSource is a simple mechanism for handling server push events as dom events, and is defined to be a part of WorkerUtils per the W3C draft.  The WebRTC Apps group needs use of EventSource.
Attachment #749816 - Flags: review?(felipc)
Comment on attachment 749816 [details] [diff] [review]
add eventsource to frameworker

>diff --git a/toolkit/components/social/test/browser/worker_eventsource.js b/toolkit/components/social/test/browser/worker_eventsource.js

>+function fn_onmessage(e) {
>+  fn_onmessage.msg_ok = true;

>+function doTest() {

>+  setTimeout(function() {
>+    ok(fn_onmessage.msg_ok, "eventsource onmessage test");
>+    ok(esListener.msg_ok, "listener ok");
>+    es.close();
>+    port.postMessage({topic: "pong"});
>+  }, 3000);

Why a 3 second timeout? Looks fishy. Can't fn_onmessage just end the test when it is called?
feedback implemented.  anyone can review, it's simple.
Assignee: nobody → mixedpuppy
Attachment #749816 - Attachment is obsolete: true
Attachment #749816 - Flags: review?(felipc)
Attachment #750380 - Flags: review?(mhammond)
Attachment #750380 - Flags: review?(
Attachment #750380 - Flags: review?(felipc)
Are there any plans to add EventSource to real workers?
Comment on attachment 750380 [details] [diff] [review]
add eventsource to frameworker

Review of attachment 750380 [details] [diff] [review]:

This looks OK to me.  It seems EventSource and WebSockets are a logical pair, both have real use-cases inside workers, neither are in the worker spec as it currently stands but both are supported in workers for chrome.

So I'll take the easy way out - r+ as the impl and tests looks fine, but leave the final decision on *whether* it should be done to gavin :)  Shane, it might help if you indicated if you have a real and current use-case for this (eg, WebRTC or a provider)

::: toolkit/components/social/test/browser/browser_frameworker.js
@@ +635,5 @@
> +
> +  testEventSource: function(cbnext) {
> +    let worker = getFrameWorkerHandle("", undefined, "testEventSource");
> +    worker.port.onmessage = function(e) {
> +      let m =;

even though 'm' is defined here, the block still uses in a couple of places - it should use 'm'
Attachment #750380 - Flags: superreview?(
Attachment #750380 - Flags: review?(mhammond)
Attachment #750380 - Flags: review?(
Attachment #750380 - Flags: review?(felipc)
Attachment #750380 - Flags: review+
Comment on attachment 750380 [details] [diff] [review]
add eventsource to frameworker

If I understand correctly, Webkit/Chrome already support EventSource in workers.

From asking on IRC, seems like we don't. Can you file a bug on supporting it?
Attachment #750380 - Flags: superreview?( → superreview+
bug 876498 added for supporting eventsource in real workers.
Comment on attachment 750380 [details] [diff] [review]
add eventsource to frameworker

[Approval Request Comment]
Bug caused by (feature/regressing bug #): none
User impact if declined: feature needed for talkilla
Testing completed (on m-c, etc.): on m-c
Risk to taking this patch (and alternatives if risky): low if any, only affects social
String or IDL/UUID changes made by this patch: none
Attachment #750380 - Flags: approval-mozilla-aurora?
Closed: 11 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 24
Attachment #750380 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Looks like this landed with tests so I'm deprioritizing fix verification. If there's some testing needed please drop the [qa-] tag and add the verifyme keyword. Thank you.
Flags: in-testsuite+
Whiteboard: [qa-]
Product: Firefox → Firefox Graveyard
You need to log in before you can comment on or make changes to this bug.