Re: Feedback on Pull Request - RE: Session cancellation and reconnection

Hi Dominik,

On Wed, Sep 17, 2014 at 12:45 PM, Rottsches, Dominik <
dominik.rottsches@intel.com> wrote:

> Hi MarkFo, Jonas, Anton,
>
> thanks for the pull request, I agree it's good to have the distinction
> between join and startSession. Also, it's good to start the algorithms
> sections.
>
> > The outcome of this sub-thread was a proposal made by Anton and Jonas
> > that would make the interface look like the following:
> > partial interface NavigatorPresentation {
> >   Promise<PresentationSession> startSession(
> >      DOMString url, DOMString sessionId);
> >  Promise<PresentationSession> joinSession(
> >      DOMString url, DOMString sessionId);
> > }
>
> > Note, also, that the mozilla-dev@ thread expressing intent to implement
> > uses the start/join flavor of the API (without Promises);
>
> Before merging, I would like to briefly revisit the questions of the
> return type here.
>
> Looking at the new example code (thanks for updating it!) – mixing
> Promises and the onstatechange event on the PresentationSession looks
> perhaps a bit more complicated than it needs to be. The steps of how a
> PresentationSession gets connected and usable are now spread across a
> Promise and the onstatechange handler, the developer using our API has to
> track both.
>

This is true, and is a consequence of having two ways that a session can
become available to the page: resolution of a Promise, or a onstatechange
transition to "connected" for a session that the page has already accessed.


> Would it be more consistent to just return a session object, for which we
> could define an additional - for example - "Idle" state, or use the
> "Disconnected" state as well, then transition that to "Connected". This
> would still allow showing the user selection dialog asynchronously, and we
> would not need to map the transitions to Promise-resolution and
> PresentationSession state separately? If that sounds okay to you, I am
> happy to edit and update the pull request in this regard.


It is simpler, but I could see some drawbacks.

First, there was some feedback from Jonas and Anton that returning a
Session from a call to joinSession() for a presentation that doesn't exist
was counterintuitive, and a Promise that resolves to a valid Session or
rejects was more ergonomic.  Perhaps we could introduce an "unknown" state
for this case, unless we don't want to leak the existence or non-existence
of a session.

Also, the page would need to be careful to avoid a race condition where:

(1) It calls joinSession(url), which immediately returns a disconnected
session
(2) Since the page doesn't have a connected session, it offers the show()
button
(3) User clicks show() and page calls startSession(url) to generate a new
(disconnected?) session
(4) Session in #1 fires onstatechange and becomes connected.  Now page has
two sessions.  Does onstatechange fire for session in #3 as well?

We could mandate that the sessions returned in #1 and #3 end up being the
same objects so the page could detect this scenario and the event handler
only gets fired once.  I think working through a code sample with the
non-Promise API would probably clear this up :)

One particular overlap/redundancy I see for example: When the Promise is
> resolved successfully from startSession or joinSession, is the session
> object is immediately in "connected" state? So the only transition on the
> onstatechange handler we would ever see is the one from "connected" to
> "disconnected"?
>

Agreed.

Would it make sense for you fork my pull request and update it in a
separate branch, so we could refer to the two APIs and code samples
(Promises and non-Promises)?


> Dominik
>

Received on Thursday, 18 September 2014 00:26:57 UTC