This patch starts up services/notifications. I don't know what I'm doing inside Firefox, so the structure and style are probably not great. Some parts I copy/pasted. Please be harsh. I think the NotificationSvc singleton should eventually become something like the app-startup WeaveService. Here's the server API: http://push.rtfd.org#client-http-api From the README: Here lies most of the moving parts for push notifcations in the browser. DOM and UI bindings will live elsewhere; these files deal with talking to the API, storing messages, and creating persistent connections to the notification server. Structure: services.js::Service This is a singleton that manages API calls and message storage. It's an instance of the NotificationSvc class. Messages and state are persisted to a JSON file on disk.

Comment on attachment 622930 [details] [diff] [review] [PATCH] create services/notifications Review of attachment 622930 [details] [diff] [review]: ----------------------------------------------------------------- Looking pretty good! The main piece lacking is the manifest registration magic. It's confusing, especially for the uninitiated. I can help you piece this together. Also, cancelling review from mconnor. I can handle reviews for you (unless mconnor really wants to review this). ::: services/notifications/Makefile.in @@ +18,5 @@ > +source_modules = $(foreach module,$(modules),$(srcdir)/$(module)) > +module_dir = $(FINAL_TARGET)/modules/services-notifications > + > +libs:: > + $(NSINSTALL) -D $(module_dir) The new convention (as of about 2 weeks ago) is: GENERATED_DIRS += $(module_dir) libs:: $(NSINSTALL) $(source_modules) $(module_dir) ::: services/notifications/service.js @@ +1,1 @@ > +const EXPORTED_SYMBOLS = ["Service"]; MPL header. ::: services/notifications/tests/unit/head_helpers.js @@ +1,1 @@ > +const {classes: Cc, interfaces: Ci, results: Cr, utils: Cu} = Components; I don't believe you need this. I could be wrong. ::: services/notifications/tests/unit/xpcshell.ini @@ +1,2 @@ > +[DEFAULT] > +head = head_helpers.js You probably want to pull in head_global.js from services/common. See sync's xpcshell.ini for an example. Yes, it is ugly. What that file does is set up some magical XUL registration voodoo. More advanced xpcshell tests won't work without it. ::: services/sync/SyncComponents.manifest @@ +21,4 @@ > resource services-sync resource:///modules/services-sync/ > resource services-common resource:///modules/services-common/ > resource services-crypto resource:///modules/services-crypto/ > +resource services-notifications resource:///modules/services-notifications/ You'll want to write your own .manifest file. I can help you with this.

You may also want to create a new Bugzilla component for notifications. See bug 743333 for inspiration.

Now with my own manifest, GENERATED_DIRS, and MPL header. > ::: services/notifications/tests/unit/head_helpers.js > @@ +1,1 @@ > > +const {classes: Cc, interfaces: Ci, results: Cr, utils: Cu} = Components; > > I don't believe you need this. I could be wrong. I need it until I pull in do_load_httpd_js() in the next patch. > ::: services/notifications/tests/unit/xpcshell.ini > @@ +1,2 @@ > > +[DEFAULT] > > +head = head_helpers.js > > You probably want to pull in head_global.js from services/common. See sync's > xpcshell.ini for an example. Yes, it is ugly. What that file does is set up > some magical XUL registration voodoo. More advanced xpcshell tests won't > work without it. I don't understand it so so I'm leaving it out for now, until I know why I want it. Trying to keep copy/paste to a minimum.

Comment on attachment 622946 [details] [diff] [review] [PATCH] create services/notifications v2 Review of attachment 622946 [details] [diff] [review]: ----------------------------------------------------------------- You are still missing a lot of xpcom glue around service registration and loading. The patch in https://bugzilla.mozilla.org/attachment.cgi?id=623924&action=diff is basically what you need to do. Be sure to generate new UUIDs! If you want to land things as-is just to get some code landed, I'm cool with that. For the record, I'm only acceptable of it because it isn't registered as a service so it can't break the browser or incur a start-up penalty. I would also like to see a try build of this work before landing. You may want to use https://wiki.mozilla.org/ReleaseEngineering:Autoland. ::: services/notifications/NotificationsComponents.manifest @@ +1,2 @@ > +# Register resource aliases > +resource services-notifications resource:///modules/services-notifications/ This file is missing xpcom registration. Not sure if you'll want that some day. ::: services/notifications/tests/unit/test_service_start.js @@ +2,5 @@ > +function run_test() { > + _("When imported, Service.onStartup is called."); > + Cu.import("resource://services-notifications/service.js"); > + > + do_check_eq(Service.serverURL, "https://notifications.mozilla.org/"); Did you actually test this? services-notifications.js (the prefs file) seems to be empty. So, I'm not sure where this value is coming from.

(In reply to Gregory Szorc [:gps] from comment #4) > Did you actually test this? services-notifications.js (the prefs file) seems > to be empty. So, I'm not sure where this value is coming from. It's in there: +pref("services.notifications.serverURL", "https://notifications.mozilla.org/");

(In reply to Jeff Balogh (:jbalogh) from comment #5) > (In reply to Gregory Szorc [:gps] from comment #4) > > Did you actually test this? services-notifications.js (the prefs file) seems > > to be empty. So, I'm not sure where this value is coming from. > > It's in there: > > +pref("services.notifications.serverURL", > "https://notifications.mozilla.org/"); I blame splinter.

Less bitrot, more fun!

This broke s-c because of build system foo. The patch introduced a new build system variable, MOZ_SERVICES_NOTIFICATIONS, which gated having things loaded. However, the patch missed some plumbing necessary to hook things up. I have a Try build at http://tbpl.mozilla.org/?tree=Try&rev=fbce2f402d81. It may take a few iterations to get working...

New Try: http://tbpl.mozilla.org/?tree=Try&rev=0b40306c8106 I'm fairly confident this one will work. It looks like we were missing the preferences file in package-manifest.in.

Typo in my last push. New Try: http://tbpl.mozilla.org/?tree=Try&rev=d9ea8718680b

I decided to combine the packaging fixes with AITC changes to avoid bit rot. See bug 765294.

Backed out because of xpcshell breakage. https://hg.mozilla.org/services/services-central/rev/591fd1c696f0 Will reland once bug 765294 is reviewed.