add error IPC handling to dispatcher

add error IPC handling to dispatcher

BUG=285976

[alecflett, 2013-10-09 23:38:25]

micahaeln/kinuko - here is the error handling I was planning to do.

It's pretty minimal right now, combining both register and unregister errors
together, because everything is just hooked up to WebCallbacks::onError() I
figure we can split it later if we actually need separate dispatchers, but I
suspect if the to-be-spec'ed API is consistent, we won't need to separate them.

[kinuko, 2013-10-10 07:57:22]

https://codereview.chromium.org/26631006/diff/1/content/child/service_worker/...
File content/child/service_worker/service_worker_dispatcher.cc (right):

https://codereview.chromium.org/26631006/diff/1/content/child/service_worker/...
content/child/service_worker/service_worker_dispatcher.cc:43:
OnRegistrationError)
Any particular reason we use a separate IPC for errors? There's a pros/cons
though either way works.

Say, if we're going to add a new IPC request which would just return success or
fail in this family (I had imagined that unregisterSW might fall into this
category), are we going to add OnSuccess IPC?

https://codereview.chromium.org/26631006/diff/1/content/child/service_worker/...
content/child/service_worker/service_worker_dispatcher.cc:139:
callbacks->onError(NULL);
Looks like this method expects WebSW ptr type for now, while this should
probably be evolved to pass something like WebServiceWorkerErrorReason?

https://github.com/slightlyoff/ServiceWorker/blob/master/service_worker.ts#L29

[michaeln, 2013-10-11 01:29:11]

can you update the other cl to also define the protocol for replying with an
error and the messagemap/onhandler plumbing for it?

i'm waiting on the webkit patch to solidify to really look closely again at this
one

https://codereview.chromium.org/26631006/diff/1/content/child/service_worker/...
File content/child/service_worker/service_worker_dispatcher.cc (right):

https://codereview.chromium.org/26631006/diff/1/content/child/service_worker/...
content/child/service_worker/service_worker_dispatcher.cc:43:
OnRegistrationError)
On 2013/10/10 07:57:22, kinuko wrote:
> Any particular reason we use a separate IPC for errors? There's a 
> pros/cons though either way works.

Either way does work. I was expecting register/registercomplete +
unregister/unregistercomplete pairing of messages. The pros to me are that it's
more obvious whats going on at each step of the way... 
- when looking at the ipc message .h
- when looking at the onxxxcomplete handler, you see all possible complete paths
- its more clear what to do in the "please do it" handler what needs to be done
to complete the call
- its more clear what to do when adding a new request/response type

Fair to say i have a preference for foo/fooresult, bar/barresult pairs.

[michaeln, 2013-10-11 01:49:50]

> i'm waiting on the webkit patch to solidify to really look closely again at
this
> one

actually, what i meant by "this one" was the other CL that defines the
register/unregister roundtripping...
https://codereview.chromium.org/25008006/

[alecflett, 2013-10-11 19:23:42]

On 2013/10/11 01:29:11, michaeln wrote:
> can you update the other cl to also define the protocol for replying with an
> error and the messagemap/onhandler plumbing for it?
> 
> i'm waiting on the webkit patch to solidify to really look closely again at
this
> one
> 
>
https://codereview.chromium.org/26631006/diff/1/content/child/service_worker/...
> File content/child/service_worker/service_worker_dispatcher.cc (right):
> 
>
https://codereview.chromium.org/26631006/diff/1/content/child/service_worker/...
> content/child/service_worker/service_worker_dispatcher.cc:43:
> OnRegistrationError)
> On 2013/10/10 07:57:22, kinuko wrote:
> > Any particular reason we use a separate IPC for errors? There's a 
> > pros/cons though either way works.
> 
> Either way does work. I was expecting register/registercomplete +
> unregister/unregistercomplete pairing of messages. The pros to me are that
it's
> more obvious whats going on at each step of the way... 
> - when looking at the ipc message .h
> - when looking at the onxxxcomplete handler, you see all possible complete
paths
> - its more clear what to do in the "please do it" handler what needs to be
done
> to complete the call
> - its more clear what to do when adding a new request/response type
> 
> Fair to say i have a preference for foo/fooresult, bar/barresult pairs.

Yes and I guess I feel differently, hence this patch.

The philosophy that is driving me is the higher level goal to separate the error
path from the success path. When you have a series of asynchronous operations
that may or may not generate errors, that once you enter the error path, it
should be easy and obvious to propagate the error path, and you must be explicit
about returning to the success path. 

This is the model of exceptions in synchronous programming languages that have
them. (which sadly, C++ does not have them in any practical sense, so we must
model them explicitly, but that's another story since this is for async
operations :))

Since this patch is built on the other
(https://codereview.chromium.org/25008006/) I'll just add the browser-side
dispatching to this patch too.. I just threw this together in a few minutes as a
proof of concept, but I'm happy to flush it out.

[alecflett, 2013-10-15 16:37:39]

Further updates: In order to implement a browser-side check to see if service
workers are enabled in the first place (abarth@'s suggestion, because for
security reasons we don't want a compromised renderer to be able to activate
service workers before we even ship them!)

So for that I had to add browser-side support for turning service workers
on/off:
https://codereview.chromium.org/27238005/

I'll be updating this patch to be dependent on that one, and have the
browser-side do the check and return the error.

Note that this will also depend on https://codereview.chromium.org/27278002/
(blink-side) to propagate a real error code + message.

[michaeln, 2013-11-12 22:22:17]

i think this is obsolete... is that right, rmv'ing myself from the list... if
its still relevant please lmk