Blink side of requestSyncEvents() function.

Blink side of requestSyncEvents() function.

Browser side: https://codereview.chromium.org/214383005/

BUG=354112

[jkarlin, 2014-04-03 19:02:55]

kinuko: Please review everything.

darin: Please review the web/ files.  Also, this CL is likely of interest to you
given today's discussion.

[jkarlin, 2014-04-03 19:04:35]

Kinuko: Please review everything

Darin: Please review web/ and I figured that this CL would generally be
interesting to you.

[jsbell, 2014-04-03 21:38:07]

If I'm reading the code correctly:

requestSyncEvents()

is considered the same as:

requestSyncEvents(true)

While that's expressible with WebIDL and prose, that's generally considered bad
design for a JS API and should be avoided. That's because:

requestSyncEvents()

should be equivalent to:

requestSyncEvents(undefined)

and sync undefined is falsy that would be coerced via the IDL rules to:

requestSyncEvents(false)

[jsbell, 2014-04-03 21:38:59]

On 2014/04/03 21:38:07, jsbell wrote:
> and sync undefined is falsy that would be coerced via the IDL rules to:

Er, "since undefined is falsy". (I type good.)

[michaeln, 2014-04-03 22:29:27]

I wonder if we want to tread more lightly in .idl for the time being. Thoughts?

https://codereview.chromium.org/224333003/diff/1/Source/modules/serviceworker...
File Source/modules/serviceworkers/ServiceWorkerGlobalScope.idl (right):

https://codereview.chromium.org/224333003/diff/1/Source/modules/serviceworker...
Source/modules/serviceworkers/ServiceWorkerGlobalScope.idl:39:
[CallWith=ExecutionContext] void requestSyncEvents(optional boolean requested,
optional Dictionary options);
The onsync event seems pretty solid but the registration/request mechanism is
less clear? Assuming we get a number of possible optional events, do we want
individual IDL to toggle each on/off or is there a generalized mechanism?

Until thats more clear, I wonder if we want to automatically toggle on/off based
on EventTarget::hasEventListeners('sync') at the end of script parse/eval?

I'm considering doing something like that for onfetch since we have some early
customers that don't care about that event either.

[jkarlin, 2014-04-04 00:27:48]

On 2014/04/03 22:29:27, michaeln wrote:
> I wonder if we want to tread more lightly in .idl for the time being.
Thoughts?
> 
>
https://codereview.chromium.org/224333003/diff/1/Source/modules/serviceworker...
> File Source/modules/serviceworkers/ServiceWorkerGlobalScope.idl (right):
> 
>
https://codereview.chromium.org/224333003/diff/1/Source/modules/serviceworker...
> Source/modules/serviceworkers/ServiceWorkerGlobalScope.idl:39:
> [CallWith=ExecutionContext] void requestSyncEvents(optional boolean requested,
> optional Dictionary options);
> The onsync event seems pretty solid but the registration/request mechanism is
> less clear? Assuming we get a number of possible optional events, do we want
> individual IDL to toggle each on/off or is there a generalized mechanism?
> 
> Until thats more clear, I wonder if we want to automatically toggle on/off
based
> on EventTarget::hasEventListeners('sync') at the end of script parse/eval?
> 
> I'm considering doing something like that for onfetch since we have some early
> customers that don't care about that event either.

The requestSyncEvents function will also be used to set the parameters of the
sync event, such as "sync when online, or sync when on battery, or sync within
the next two weeks" etc.  So we'll need it beyond just toggling on and off.  I
know very little about IDL and how things should be arranged, so I leave the
file that it belongs in to your discretion.

[jkarlin, 2014-04-04 00:28:44]

On 2014/04/03 21:38:07, jsbell wrote:
> If I'm reading the code correctly:
> 
> requestSyncEvents()
> 
> is considered the same as:
> 
> requestSyncEvents(true)
> 
> While that's expressible with WebIDL and prose, that's generally considered
bad
> design for a JS API and should be avoided. That's because:
> 
> requestSyncEvents()
> 
> should be equivalent to:
> 
> requestSyncEvents(undefined)
> 
> and sync undefined is falsy that would be coerced via the IDL rules to:
> 
> requestSyncEvents(false)

Excellent point, thank you!  I'll remove the option tomorrow.

[jkarlin, 2014-04-04 12:05:09]

On 2014/04/04 00:28:44, jkarlin wrote:
> On 2014/04/03 21:38:07, jsbell wrote:
> > If I'm reading the code correctly:
> > 
> > requestSyncEvents()
> > 
> > is considered the same as:
> > 
> > requestSyncEvents(true)
> > 
> > While that's expressible with WebIDL and prose, that's generally considered
> bad
> > design for a JS API and should be avoided. That's because:
> > 
> > requestSyncEvents()
> > 
> > should be equivalent to:
> > 
> > requestSyncEvents(undefined)
> > 
> > and sync undefined is falsy that would be coerced via the IDL rules to:
> > 
> > requestSyncEvents(false)
> 
> Excellent point, thank you!  I'll remove the option tomorrow.

Changed to '[Default=Undefined] optional bool' and the default behavior is now
false, as would be expected.  Tests updated to reflect that in the browser CL.

[jkarlin, 2014-04-04 12:05:29]

https://codereview.chromium.org/224333003/diff/1/Source/modules/serviceworker...
File Source/modules/serviceworkers/ServiceWorkerGlobalScope.idl (right):

https://codereview.chromium.org/224333003/diff/1/Source/modules/serviceworker...
Source/modules/serviceworkers/ServiceWorkerGlobalScope.idl:39:
[CallWith=ExecutionContext] void requestSyncEvents(optional boolean requested,
optional Dictionary options);
Sync needs to toggle on and off its event listener dynamically (to avoid waking
up a SW needlessly).  It also needs to be able to provide hints as to when it
wants the sync event to hire.  The toggling and the hints could be two separate
functions, or they could be one.  I had intended  to place both within the
requestSyncEvents function.


On 2014/04/03 22:29:27, michaeln wrote:
> The onsync event seems pretty solid but the registration/request mechanism is
> less clear? Assuming we get a number of possible optional events, do we want
> individual IDL to toggle each on/off or is there a generalized mechanism?
> 
> Until thats more clear, I wonder if we want to automatically toggle on/off
based
> on EventTarget::hasEventListeners('sync') at the end of script parse/eval?
> 
> I'm considering doing something like that for onfetch since we have some early
> customers that don't care about that event either.

[jkarlin, 2014-04-10 18:17:47]

Removing reviewers as this CL will change dramatically as per the design doc.