Latest spec support |close| and |terminate| semantics to allow only closing session transport or terminating the session.

Hi Josh, Could you help mark 2.5+ to this one?

Hi Sean, Can you provide update for the current status? Thanks!

There are two tasks for this bug: 1. for supporting terminate: We already implement the behavior in |PresentationSession.close| but need to modify the function name to |PresentationSession.terminate| in WebIDL. 2. for supporting close: Need to send control message between session end points over the app-to-app transport channel. |close| semantic is an advanced functionality for power saving, but it's not planned to be used in any of the TV 2.5 user story. I'll suggest that we do task #1 in TV 2.5 scope and leave task #2 post 2.5.

According to comment 3, |close| hasn't been fully implemented in this patch. (NS_ERROR_NOT_IMPLEMENTED will be thrown when |close| gets called.)

Sorry about the delay. I'll try to review the patches tomorrow.

Comment on attachment 8668357 [details] [diff] [review] Part 1 - WebIDL & implementation changes, v1 >+PresentationSession::Close(ErrorResult& aRv) > { >- // Closing does nothing if the session is already terminated. >- if (NS_WARN_IF(mState == PresentationSessionState::Terminated)) { >+ // It only works when the state is CONNECTED. >+ if (NS_WARN_IF(mState != PresentationSessionState::Connected)) { >+ aRv.Throw(NS_ERROR_DOM_INVALID_STATE_ERR); >+ return; >+ } This method should not throw here. Per spec it just queues a task, and even that task "If the presentation connection state of connection is not connected, then abort these steps. " So just return early if the state isn't connected. >+PresentationSession::Terminate(ErrorResult& aRv) >+{ >+ // It only works when the state is CONNECTED. >+ if (NS_WARN_IF(mState != PresentationSessionState::Connected)) { >+ aRv.Throw(NS_ERROR_DOM_INVALID_STATE_ERR); > return; > } Same here. >+++ b/dom/presentation/interfaces/nsIPresentationService.idl >@@ -58,21 +58,28 @@ interface nsIPresentationService : nsISu update uuid >diff --git a/dom/webidl/PresentationSession.webidl b/dom/webidl/PresentationSession.webidl >--- a/dom/webidl/PresentationSession.webidl >+++ b/dom/webidl/PresentationSession.webidl >@@ -5,65 +5,70 @@ > */ > > enum PresentationSessionState This seems to be PresentationConnectionState in the current spec and .. > [Pref="dom.presentation.enabled", > CheckAnyPermissions="presentation"] > interface PresentationSession : EventTarget { ... this interface is called PresentationConnection in the current spec. Could you file a followup bug to change the name. > /* >- * Both the requesting page and the presenting page can close the session by >- * calling terminate(). Then, the session is destroyed and its state is >- * truned into "terminated". After getting into the state of "terminated", >- * resumeSession() is incapable of re-establishing the connection. >+ * Both the controlling and receving browsing context can close the session. >+ * Then, the session state should turn into "closed". > * >- * This function does nothing if the state has already been "terminated". >+ * This function only works when the state is not "connected". > */ >+ [Throws] > void close(); Could we rather just comment this out for now so that we don't expose unimplemented functions to JS. The C++ implementation can stay as it is.

Updating based on r+ comments.

The latest try-run. FYI. https://treeherder.mozilla.org/#/jobs?repo=try&revision=9a752af6d809

Hi Sean, i got: remote: *************************** ERROR *************************** remote: Push rejected because the following IDL interfaces were remote: modified without changing the UUID: remote: - nsIPresentationSessionListener in changeset 986ac78a1ea2 remote: remote: To update the UUID for all of the above interfaces and their remote: descendants, run: remote: ./mach update-uuids nsIPresentationSessionListener remote: remote: If you intentionally want to keep the current UUID, include remote: 'IGNORE IDL' in the commit message. remote: ************************************************************* when i tried to apply this. Is this expected ?

Updating UUID.

(In reply to Carsten Book [:Tomcat] from comment #11) > Hi Sean, i got: > > remote: *************************** ERROR *************************** > remote: Push rejected because the following IDL interfaces were > remote: modified without changing the UUID: > remote: - nsIPresentationSessionListener in changeset 986ac78a1ea2 > remote: > remote: To update the UUID for all of the above interfaces and their > remote: descendants, run: > remote: ./mach update-uuids nsIPresentationSessionListener > remote: > remote: If you intentionally want to keep the current UUID, include > remote: 'IGNORE IDL' in the commit message. > remote: ************************************************************* > > when i tried to apply this. Is this expected ? Sorry, I just updated the UUID in the new patch.

https://hg.mozilla.org/integration/mozilla-inbound/rev/7acef81a1e90 https://hg.mozilla.org/integration/mozilla-inbound/rev/3cb22083aaf0

https://hg.mozilla.org/mozilla-central/rev/7acef81a1e90 https://hg.mozilla.org/mozilla-central/rev/3cb22083aaf0