Folded: Various registration related UpdateState() calls and abort on failure to create a service worker. Set ServiceWorker instances state based on corresponding ServiceWorkerInfo state.

Comment on attachment 8539221 [details] [diff] [review] Fire statechange event on ServiceWorker instances Review of attachment 8539221 [details] [diff] [review]: ----------------------------------------------------------------- ::: dom/workers/ServiceWorkerManager.cpp @@ +1489,5 @@ > + // Refactor this iteration pattern across this and 2 other call-sites. > + nsTObserverArray<ServiceWorkerRegistration*>::ForwardIterator it(domainInfo->mServiceWorkerRegistrations); > + while (it.HasMore()) { > + nsRefPtr<ServiceWorkerRegistration> target = it.GetNext(); > + nsString regScope; nsAutoString :) @@ +1493,5 @@ > + nsString regScope; > + target->GetScope(regScope); > + MOZ_ASSERT(!regScope.IsEmpty()); > + > + nsCString utf8Scope = NS_ConvertUTF16toUTF8(regScope); NS_ConvertUTF16toUTF8 utf8Scope(regScope); ::: dom/workers/ServiceWorkerManager.h @@ +190,5 @@ > + const ServiceWorkerRegistrationInfo* mRegistration; > + nsCString mScriptSpec; > + ServiceWorkerState mState; > + > + ~ServiceWorkerInfo() private dtor? @@ +197,5 @@ > +public: > + NS_INLINE_DECL_REFCOUNTING(ServiceWorkerInfo) > + > + const nsCString& > + GetScriptSpec() const ScriptSpec() I know that you just moved the class but, in the meantime we can improve it changing something like this. Maybe? @@ +213,5 @@ > + } > + > + ServiceWorkerState > + GetState() const > + { State() Usually we use GetFoobar when it can fail. Or for string references such as GetFoobar(nsAString& bar); ::: dom/workers/ServiceWorkerRegistration.cpp @@ +276,5 @@ > + worker = mActiveWorker; > + } else { > + MOZ_CRASH("Invalid case"); > + } > + What about if worker is null? Can it be? what do we do in that case?

Comment on attachment 8539221 [details] [diff] [review] Fire statechange event on ServiceWorker instances Review of attachment 8539221 [details] [diff] [review]: ----------------------------------------------------------------- ::: dom/workers/ServiceWorkerRegistration.h @@ +58,5 @@ > void > InvalidateWorkerReference(WhichServiceWorker aWhichOnes); > > + void > + QueueStateChangeEvent(WhichServiceWorker aWhichOne, ServiceWorkerState aState) const; Missing #include "mozilla/dom/ServiceWorkerBinding.h"

(In reply to Yuan Pengfei from comment #5) > Missing #include "mozilla/dom/ServiceWorkerBinding.h" This exact issue broke my local m-c build, FWIW (using clang 3.6) with: { 0:10.50 In file included from /scratch/work/builds/mozilla-central/mozilla-central.12-12-07.08-38/obj/dom/bindings/UnifiedBindings18.cpp:14: 0:10.50 In file included from ./ServiceWorkerRegistrationBinding.cpp:14: 0:10.50 ../../dist/include/mozilla/dom/ServiceWorkerRegistration.h:62:55: error: unknown type name 'ServiceWorkerState'; did you mean 'workers::ServiceWorker'? 0:10.50 QueueStateChangeEvent(WhichServiceWorker aWhichOne, ServiceWorkerState aState) const; 0:10.50 ^~~~~~~~~~~~~~~~~~ 0:10.50 workers::ServiceWorker 0:10.50 ../../dist/include/mozilla/dom/ServiceWorkerRegistration.h:21:7: note: 'workers::ServiceWorker' declared here 0:10.50 class ServiceWorker; 0:10.51 ^ } (The stuff about workers::ServiceWorker is a red herring -- it's clang suspecting that the type name "ServiceWorkerState" might be a typo, and suggesting a recognized type name.) I was going to suggest a forward-declare for ServiceWorkerState, but it looks like it's an enum, which doesn't support forward-declarations. So we do indeed need this #include, as Yuan suggests. Here's a followup patch to add that. I've verified that this fixes my local build issue. Once this has r+, anyone can feel free to land this, if they get to it before me.

This has busted all of my local builds (on one machine at least) so I imagine it's affecting others too. (I'm guessing I'm affected but TBPL is not, due to how my particular constellation of mozconfig options interact with unified-build C++-file-grouping.) I'm landing the followup sooner rather than later, with "(no review, minor obviously-correct patch to fix local build bustage)", to get this fixed & minimize pain from other people updating their tree & hitting this. Landed: https://hg.mozilla.org/integration/mozilla-inbound/rev/114d75337bed

Fwiw, we've also seen that failure on our bsd buildbot, see http://buildbot.rhaalovely.net/builders/mozilla-central-netbsd-amd64/builds/22. Interestingly, it didnt break all bsds, only netbsd...

Comment on attachment 8554284 [details] [diff] [review] followup to add missing #include Review of attachment 8554284 [details] [diff] [review]: ----------------------------------------------------------------- Just for the record, looks good to me.