Re: Alarm API Feedback

On Tue, Jan 15, 2013 at 3:14 AM, Dumez, Christophe
<christophe.dumez@intel.com> wrote:
> Hi,
>
> Thank you for the feedback. We should indeed discuss / address the issues
> you mentioned. Please find my comments inline.
>
> On Mon, Jan 14, 2013 at 6:58 PM, Mandyam, Giridhar <mandyam@quicinc.com>
> wrote:
>>
>> Hello,
>> Qualcomm Innovation Center, a member of the SysApps Working Group, objects
>> to the publication of the Alarm API as an FPWD.
>>
>> Some items we would like clarified in any subsequent modifications of the
>> Editor's Draft are:
>>
>> a) Section 4.2:  The error codes returned for add() and remove() are not
>> defined.  This is not consistent with other device-level API's defined in
>> the W3C that would likely be used in a SysApps deployment (e.g. see the
>> Geolocation API,
>> http://dev.w3.org/geo/api/spec-source.html#posittheseion_error_interface).

Mandyam: Are these things that really need to hold up a FPWD? FPWD is
just the first stage of many and there is no expectation that the
final specification will look *anything* like the version published as
FPWD. So there will be ample time to address issues after the FPWD.

The only reason to hold up FPWD is generally if you think that the
specification is going in the entirely wrong direction. But if you
feel like there are significant problems and that the spec is going in
entirely the wrong direction, then objecting to FPWD is definitely the
right thing to do. Based on the comments that you have that doesn't
seem to be the case though?

The stage when the group feels that it's approaching something that
will be the final specification is the Last Call. I definitely agree
that these types of issues need to be addressed well before Last Call
though.

> Yes, you are right. The error types need to be defined. I am proposing the
> following:
> - getAll()
>   * "SecurityError" if the application does not have the permission to
> perform this operation.

I think that if a page doesn't have permission to use the alarm API,
then we should return null for navigator.alarms. And if it does have
permission to use the alarm API, it should have the permission to use
all of it. That significantly simplifies the model for developers
since they wouldn't have to constantly check for security errors.

We also need to exactly define under which conditions a page has
access to use the alarm API, and under which conditions it does not.
But I think we need more support from the security model specification
before we can do that.

>   * "TimeoutError" if the operation could not be performed in a reasonable
> time.

I'm not sure that we need to explicitly define this. Timeouts are
always very tricky. Under what circumstances would you want to time
out an operation if the operation could succeed by waiting another
second? It would be bad if an implementation decides that a 5 second
timeout is long enough and this causes intermittent issues if too much
file IO is happening in other applications, causing saving of the
alarm to sometimes take 6 seconds.

>   * "UnknownError" in any other error case.

We should probably add some IO error as well, or maybe that is best
handled through UnknownError.

> - add()
>   * "InvalidStateError" if the alarm's date is set in the past.

This makes sense. We *could* throw this as an exception rather than
asynchronously fire this as an error (I assume that the errors we are
talking about here are errors events fired on the returned DOMRequest,
right?)

> - remove()
>   * "NotFoundError" if there is no alarm with the given identifier.

Firing this as an error will generally just make things harder for the
developer. Generally a better solution for handling "remove" requests
of something that doesn't exist is to return a return value indicating
if something was removed or not. This since the end result has
actually been fulfilled since the item requested to be removed doesn't
exist in the backing database, and so treating this as an error means
that the developer will have to handle both success callbacks as well
as some error callbacks as "things went ok".

So if needed we can set the .result to true/false to indicate if an
alarm was found and removed. In IndexedDB we decided to not do even
that as a performance optimization. Though IndexedDB is of course more
performance critical than this API.

>> b) Section 4.2: The data object in add() is has to be in valid JSON
>> format.  Why?   Why not another textual format?
>
> I believe the idea is that this data would not need parsing if it is in JSON
> format. If we use another textual format then the data will need parsing
> when the alarm is fired.
> Of course, it is possible to do but why do we need to support another
> textual format? Did you have any particular format in mind? What is the use
> case?

Note that the API doesn't take a *string* which has to be valid JSON.

The API takes an *object* which has to be serializable *into* JSON.
The language in the spec can probably improved to make this more
clear.

Taking an object which is serializable into JSON is a superset of
taking a string, since strings serialize into JSON just fine. So
passing

"hello world"

is just as valid as passing

true

or

{ someProp: "hello", someOtherProp: "world" }

The reason we need to serialize the object is so that we can store the
data to disk so that if the device is restarted, we can still properly
fire the alarm. There are currently two common serialization formats
for JS objects: JSON and structured clones (defined in HTML5). JSON is
definitely a simpler format though, but either format is fine with me
to support.

>> d) Section 4.2.  Why is TimeZoneDirective an enum (as opposed to a
>> Boolean)?  If there are only two options for the method argument, I don't
>> see the point in defining an enum.
>
>
> The point was to make the resulting code more readable and less bug-prone.
> My personal opinion is that:
> alarmManager.add(date, "respectTimezone");
> is more readable / understandable than
> alarmManager.add(date, true);
>
> I know for example that this coding style is used quite commonly in WebKit
> code.
> Of course, it should be discussed if we really want to adopt this style in
> our SysApps APIs or not.

APIs that take booleans is something we've generally started avoiding
in web standards because it makes code unclear. Not only is hard to
remember what the second argument means for .add(x, true), it's even
harder to remember if true was for "does honor the timezone" or if it
means "does not honor the timezone".

/ Jonas

Received on Friday, 18 January 2013 08:28:27 UTC