[telepathy-spec/master] fd.o #21093: fix race between ChannelLost/Finished and connecting to those signals

[Simon McVittie]

---
 spec/Channel_Dispatch_Operation.xml |   46 +++++++++++++++++++++++++++---
 spec/Client_Approver.xml            |   52 ++++++++++++++++++++++++++---------
 2 files changed, 80 insertions(+), 18 deletions(-)

diff --git a/spec/Channel_Dispatch_Operation.xml b/spec/Channel_Dispatch_Operation.xml
index c3ba06a..98536a1 100644
--- a/spec/Channel_Dispatch_Operation.xml
+++ b/spec/Channel_Dispatch_Operation.xml
@@ -161,11 +161,29 @@
     </property>
 
     <signal name="ChannelLost" tp:name-for-bindings="Channel_Lost">
-      <tp:docstring>
-        A channel has closed before it could be claimed or handled. If this
-        is emitted for the last remaining channel in a channel dispatch
-        operation, it MUST immediately be followed by
-        <tp:member-ref>Finished</tp:member-ref>.
+      <tp:docstring xmlns="http://www.w3.org/1999/xhtml">
+        <p>A channel has closed before it could be claimed or handled. If
+          this is emitted for the last remaining channel in a channel
+          dispatch operation, it MUST immediately be followed by
+          <tp:member-ref>Finished</tp:member-ref>.</p>
+
+        <p>This signal MUST NOT be emitted until all Approvers that were
+          invoked have returned (successfully or with an error) from
+          their <tp:dbus-ref
+            namespace="org.freedesktop.Telepathy.Client.Approver.DRAFT">AddDispatchOperation</tp:dbus-ref>
+          method.</p>
+
+        <tp:rationale>
+          <p>This means that Approvers can connect to the ChannelLost signal
+            in a race-free way. Non-approver processes that discover
+            a channel dispatch operation in some way (such as observers)
+            will have to follow the usual "connect to signals then recover
+            state" model - first connect to ChannelLost and
+            <tp:member-ref>Finished</tp:member-ref>,
+            then download <tp:member-ref>Channels</tp:member-ref> (and
+            on error, perhaps assume that the operation has already
+            Finished).</p>
+        </tp:rationale>
       </tp:docstring>
 
       <arg name="Channel" type="o">
@@ -364,6 +382,24 @@
             <tp:member-ref>Claim</tp:member-ref> on a new dispatch operation
             instead of the one they intended to handle.</p>
         </tp:rationale>
+
+        <p>This signal MUST NOT be emitted until all Approvers that were
+          invoked have returned (successfully or with an error) from
+          their <tp:dbus-ref
+            namespace="org.freedesktop.Telepathy.Client.Approver.DRAFT">AddDispatchOperation</tp:dbus-ref>
+          method.</p>
+
+        <tp:rationale>
+          <p>This means that Approvers can connect to the ChannelLost signal
+            in a race-free way. Non-approver processes that discover
+            a channel dispatch operation in some way (such as observers)
+            will have to follow the usual "connect to signals then recover
+            state" model - first connect to
+            <tp:member-ref>ChannelLost</tp:member-ref> and
+            Finished, then download <tp:member-ref>Channels</tp:member-ref>
+            (and on error, perhaps assume that the operation has already
+            Finished).</p>
+        </tp:rationale>
       </tp:docstring>
     </signal>
 
diff --git a/spec/Client_Approver.xml b/spec/Client_Approver.xml
index bf43731..5c8cb21 100644
--- a/spec/Client_Approver.xml
+++ b/spec/Client_Approver.xml
@@ -130,6 +130,35 @@
         </tp:rationale>
       </tp:docstring>
 
+      <arg name="Channels" direction="in"
+        type="a(oa{sv})" tp:type="Channel_Details[]">
+        <tp:docstring xmlns="http://www.w3.org/1999/xhtml">
+          <p>The initial value of the <tp:dbus-ref
+              namespace="org.freedesktop.Telepathy">ChannelDispatchOperation.DRAFT.Channels</tp:dbus-ref>
+            property, containing the <tp:dbus-ref
+              namespace="org.freedesktop.Telepathy">Channel</tp:dbus-ref>s
+            to be dispatched and their properties.</p>
+
+          <tp:rationale>
+            <p>This can't be signalled to the approver through the Properties
+              parameter of this method, because Channels is not an immutable
+              property.</p>
+          </tp:rationale>
+
+          <p>The actual channels to be dispatched may reduce as channels are
+            closed: this is signalled by <tp:dbus-ref
+              namespace="org.freedesktop.Telepathy">ChannelDispatchOperation.DRAFT.ChannelLost</tp:dbus-ref>.
+          </p>
+
+          <p>Approvers SHOULD connect to ChannelLost and <tp:dbus-ref
+              namespace="org.freedesktop.Telepathy">ChannelDispatchOperation.DRAFT.Finished</tp:dbus-ref>.
+            (if desired) before returning from AddDispatchOperation, since
+            those signals are guaranteed not to be emitted until after all
+            AddDispatchOperation calls have returned (with success or failure)
+            or timed out.</p>
+        </tp:docstring>
+      </arg>
+
       <arg name="DispatchOperation" type="o" direction="in">
         <tp:docstring xmlns="http://www.w3.org/1999/xhtml">
           <p>The
@@ -141,19 +170,16 @@
       <arg name="Properties" type="a{sv}"
         tp:type="Qualified_Property_Value_Map" direction="in">
         <tp:docstring>
-          Properties of the channel dispatch operation. This MUST NOT include
-          properties that could change, SHOULD include as many properties as
-          possible given that constraint, and MUST include at least the
-          <tp:dbus-ref
-            namespace="org.freedesktop.Telepathy.ChannelDispatchOperation.DRAFT">Account</tp:dbus-ref>,
-          <tp:dbus-ref
-            namespace="org.freedesktop.Telepathy.ChannelDispatchOperation.DRAFT">Connection</tp:dbus-ref>,
-          <tp:dbus-ref
-            namespace="org.freedesktop.Telepathy.ChannelDispatchOperation.DRAFT">Channels</tp:dbus-ref>
-          and
-          <tp:dbus-ref
-            namespace="org.freedesktop.Telepathy.ChannelDispatchOperation.DRAFT">PossibleHandlers</tp:dbus-ref>
-          properties.
+          <p>Properties of the channel dispatch operation. This MUST NOT
+            include properties that could change, SHOULD include as many
+            properties as possible given that constraint, and MUST include at
+            least the <tp:dbus-ref
+              namespace="org.freedesktop.Telepathy.ChannelDispatchOperation.DRAFT">Account</tp:dbus-ref>,
+            <tp:dbus-ref
+              namespace="org.freedesktop.Telepathy.ChannelDispatchOperation.DRAFT">Connection</tp:dbus-ref>
+            and <tp:dbus-ref
+              namespace="org.freedesktop.Telepathy.ChannelDispatchOperation.DRAFT">PossibleHandlers</tp:dbus-ref>
+            properties.</p>
         </tp:docstring>
       </arg>
     </method>
-- 
1.5.6.5