1131255 - Use ICE connection state to better determine when a video call is successful

Status: RESOLVED DUPLICATE of bug 1018060

(Instantbird Graveyard :: Conversation, defect)

Description

[aleth [:aleth]]

The existing patch assumes a video call is established when we send the accept call stanza. In fact, we should only infer the call has been successful if the ICE connection process is successful.

Since ICE can take a while, there should also be a notification bar state for this waiting period.

Comment 1

[aleth [:aleth]]

Attachment: fixice.diff

Also tidies up some of the notification box stuff a bit.

Comment 2

[aleth [:aleth]]

Comment on attachment 8561638 [details] [diff] [review]
fixice.diff

Review of attachment 8561638 [details] [diff] [review]:
-----------------------------------------------------------------

clokep said he'd prefer flo would review this.

Comment 3

[Florian Quèze [:florian]]

Comment on attachment 8561638 [details] [diff] [review]
fixice.diff

Review of attachment 8561638 [details] [diff] [review]:
-----------------------------------------------------------------

r+ as this looks good overall, but I'm not entirely confident in my review because all the unrelated refactoring/cleanup changes were distracting me from the main point of the bug. I'm not going to waste your time by requiring you split them into separate patches though.

::: im/content/conversation.xml
@@ +356,5 @@
>            // Display error notification for call failure to the user.
> +          let notificationBox = this.getElt("convNotificationBox");
> +          notificationBox.appendNotification(
> +            this.convBundle.GetStringFromName("callError.label"),
> +            "video-call-error", null, notificationBox.PRIORITY_INFO_HIGH);

It seems this notification is never removed (both before and after this patch), we should probably remove it when the user attempts to call again.

Comment 4

[aleth [:aleth]]

(In reply to Florian Quèze [:florian] [:flo] from comment #3)
> r+ as this looks good overall, but I'm not entirely confident in my review
> because all the unrelated refactoring/cleanup changes were distracting me
> from the main point of the bug. 

Folded into the main bug.