1227539 - Hello icon remains active (green) after sharing a window and exiting the conversation

Status: VERIFIED FIXED

(Hello (Loop) :: Client, defect, P1)

Description

[Bogdan Maris, Desktop QA]

Affected builds:
- latest Aurora 44.0a2
- latest Nighty 45.0a1

Affected OS`s:
- Windows 7 64-bit
- Ubuntu 14.04 32-bit
- Mac OS X 10.10

STR:
1. Start Firefox
2. Click Hello icon
3. Click Get started
4. Start a conversation
5. Share a window
6. Close conversation

Expected results: Hello icon changes color to grey since the conversation was closed.

Actual results: Hello icon is still green, meaning that the sharing is still active.

Notes:
- This is a recent regression caused by bug 1048850.

m-i:

Last good revision: 341b35834bac999ac2d00400efd114933a65ae88
First bad revision: e321d5a4e45bd7c85b4e5eb8f1d8ef8c4d1498bb

https://hg.mozilla.org/integration/mozilla-inbound/pushloghtml?fromchange=341b35834bac999ac2d00400efd114933a65ae88&tochange=e321d5a4e45bd7c85b4e5eb8f1d8ef8c4d1498bb

Bug 1048850 - [e10s] Use RemotePageManager to communicate between chrome and content, replacing document.mozLoop

Comment 1

[Bogdan Maris, Desktop QA]

(In reply to Bogdan Maris, QA [:bogdan_maris] from comment #0)
> Affected builds:
> - latest Aurora 44.0a2
> - latest Nighty 45.0a1

Ignore this, only Nightly is affected.

Comment 2

[Mike de Boer [:mikedeboer]]

Attachment: Patch v1: close a chat window properly, including stopping all sharing and leaving the room

Comment 3

[Mike de Boer [:mikedeboer]]

Attachment: Patch v2: close a chat window properly, including stopping all sharing and leaving the room

Comment 4

[Mike de Boer [:mikedeboer]]

Comment on attachment 8691937 [details] [diff] [review]
Patch v2: close a chat window properly, including stopping all sharing and leaving the room

Argh. This completely fails for undocked conversation windows, of course.

Comment 5

[Mike de Boer [:mikedeboer]]

Attachment: Patch v3: close a chat window properly, including stopping all sharing and leaving the room

Comment 6

[Mark Banner (:standard8)]

Comment on attachment 8692067 [details] [diff] [review]
Patch v3: close a chat window properly, including stopping all sharing and leaving the room

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

So in general this looks good, but there's two things that concern me:

- If the connection to tokbox drops or is aborted, then we display an error page and offer retry. Since the _leaveRoom now never does a leave, then the user wouldn't be able to rejoin the room.

Maybe we could have something that sends to the backend LoopRooms an optional leave? Maybe some tracking in LoopRooms about what has/hasn't been joined?

- If we're not doing the server comms in _leaveRoom, then we need to add some documentation that they are defined in a different place.

- We should also get tests for the new leave functionality in MozLoopService (or wherever it ends up).

Comment 7

[Mike de Boer [:mikedeboer]]

Attachment: Patch 1: allow a single argument to be passed to push message subscribers

Comment 8

[Mike de Boer [:mikedeboer]]

Attachment: Patch 1.1: allow a single argument to be passed to push message subscribers

Comment 9

[Mark Banner (:standard8)]

Comment on attachment 8693593 [details] [diff] [review]
Patch 1.1: allow a single argument to be passed to push message subscribers

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

Looks good. Thanks for splitting this out.

Comment 10

[Mike de Boer [:mikedeboer]]

https://hg.mozilla.org/integration/fx-team/rev/7905f57f7fa120a49fd15097b01680bcd9f2b0a6
Bug 1227539: allow a single argument to be passed to push message subscribers. r=Standard8

Comment 12

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/7905f57f7fa1

Comment 13

[Mike de Boer [:mikedeboer]]

Attachment: Patch 2.1: close a chat window properly, including stopping all sharing and leaving the room

Comment 14

[Mike de Boer [:mikedeboer]]

Attachment: Patch 2.1: close a chat window properly, including stopping all sharing and leaving the room

Comment 15

[Mark Banner (:standard8)]

I haven't had time to review this fully yet, but I did notice that leaving the room is now broken on standalone. We need to implement the hangupnow option for there (and check it happens sync for window close & things).

Comment 16

[Mike de Boer [:mikedeboer]]

Attachment: Patch 3: implement the new HangupNow action on standalone too

Seems I 'fixed' all the tests in part 2 the wrong way and should've done this instead.

Comment 17

[Mike de Boer [:mikedeboer]]

Comment on attachment 8695874 [details] [diff] [review]
Patch 3: implement the new HangupNow action on standalone too

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

::: browser/extensions/loop/test/shared/activeRoomStore_test.js
@@ +1823,5 @@
> +      sinon.assert.calledOnce(requestStubs["HangupNow"]);
> +      sinon.assert.calledWith(requestStubs["HangupNow"], "fakeToken", "1627384950");
> +    });
> +
> +    it("should call 'HangupNow' Loop API when _isDesktop is true", function() {

Changed to 'should not call'

Comment 18

[Mark Banner (:standard8)]

Comment on attachment 8695236 [details] [diff] [review]
Patch 2.1: close a chat window properly, including stopping all sharing and leaving the room

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

This seems to be working better for the leave. However, its still not removing the screen share bar and green icon when I leave the room.

Comment 19

[Mike de Boer [:mikedeboer]]

(In reply to Mark Banner (:standard8) from comment #18)
> This seems to be working better for the leave. However, its still not
> removing the screen share bar and green icon when I leave the room.

How strange. I can not reproduce this locally. Did you see any (error-)messages in your console?

Comment 21

[Mike de Boer [:mikedeboer]]

Attachment: Patch 2.2: close a chat window properly, including stopping all sharing and leaving the room

Comment 22

[Mark Banner (:standard8)]

Comment on attachment 8698457 [details] [diff] [review]
Patch 2.2: close a chat window properly, including stopping all sharing and leaving the room

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

Ok, this seems to work now. Lets get it landed ;-)

Comment 23

[Mike de Boer [:mikedeboer]]

https://hg.mozilla.org/integration/fx-team/rev/0fbd239d0c377c63b0f0ed4d70b373e4d4cdc1b5
Bug 1227539: Part 1 - close a chat window properly, including stopping all sharing and leaving the room. r=Standard8

https://hg.mozilla.org/integration/fx-team/rev/48d56e89c4828d82a255c37bb045710d4c1bab25
Bug 1227539: Part 2 - implement the new HangupNow action on standalone too so that leaving a room will still work. r=Standard8

Comment 24

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

Backed out in https://hg.mozilla.org/integration/fx-team/rev/cf17f3b3be6c for

1) Win 7 M(bc4) failures:
https://treeherder.mozilla.org/logviewer.html#?job_id=6225689&repo=fx-team
08:27:44     INFO -  244 INFO TEST-UNEXPECTED-FAIL | browser/extensions/loop/test/mochitest/browser_mozLoop_chat.js | Condition didn't pass. -
08:27:44     INFO -  Stack trace:
08:27:44     INFO -  chrome://mochitests/content/browser/browser/extensions/loop/test/mochitest/head.js:waitForCondition/interval<:73
08:27:44     INFO -  Not taking screenshot here: see the one that was previously logged
08:27:44     INFO -  245 INFO TEST-UNEXPECTED-FAIL | browser/extensions/loop/test/mochitest/browser_mozLoop_chat.js | chat window should have been closed - false == true - JS frame :: chrome://mochitests/content/browser/browser/extensions/loop/test/mochitest/browser_mozLoop_chat.js :: test_mozLoop_hangupAllChatWindows :: line 38
08:27:44     INFO -  Stack trace:
08:27:44     INFO -      chrome://mochitests/content/browser/browser/extensions/loop/test/mochitest/browser_mozLoop_chat.js:test_mozLoop_hangupAllChatWindows:38
08:27:44     INFO -      Handler.prototype.process@resource://gre/modules/Promise.jsm -> resource://gre/modules/Promise-backend.js:933:23
08:27:44     INFO -      this.PromiseWalker.walkerLoop@resource://gre/modules/Promise.jsm -> resource://gre/modules/Promise-backend.js:812:7
08:27:44     INFO -      Promise*this.PromiseWalker.scheduleWalkerLoop@resource://gre/modules/Promise.jsm -> resource://gre/modules/Promise-backend.js:743:11
08:27:44     INFO -      this.PromiseWalker.schedulePromise@resource://gre/modules/Promise.jsm -> resource://gre/modules/Promise-backend.js:775:7
08:27:44     INFO -      Promise.prototype.then@resource://gre/modules/Promise.jsm -> resource://gre/modules/Promise-backend.js:450:5
08:27:44     INFO -      this.PromiseWalker.completePromise@resource://gre/modules/Promise.jsm -> resource://gre/modules/Promise-backend.js:701:1
08:27:44     INFO -      TaskImpl_run@resource://gre/modules/Task.jsm:320:13
08:27:44     INFO -      TaskImpl@resource://gre/modules/Task.jsm:276:3
08:27:44     INFO -      createAsyncFunction/asyncFunction@resource://gre/modules/Task.jsm:250:14
08:27:44     INFO -      Tester_execTest/<@chrome://mochikit/content/browser-test.js:761:21
08:27:44     INFO -      TaskImpl_run@resource://gre/modules/Task.jsm:315:40
08:27:44     INFO -      TaskImpl@resource://gre/modules/Task.jsm:276:3
08:27:44     INFO -      createAsyncFunction/asyncFunction@resource://gre/modules/Task.jsm:250:14
08:27:44     INFO -      Task_spawn@resource://gre/modules/Task.jsm:164:12
08:27:44     INFO -      Tester_execTest@chrome://mochikit/content/browser-test.js:756:9
08:27:44     INFO -      Tester.prototype.nextTest</<@chrome://mochikit/content/browser-test.js:676:7
08:27:44     INFO -      SimpleTest.waitForFocus/waitForFocusInner/focusedOrLoaded/<@chrome://mochikit/content/tests/SimpleTest/SimpleTest.js:735:59
08:27:44     INFO -  246 INFO Leaving test test_mozLoop_hangupAllChatWindows
08:27:44     INFO -  247 INFO Entering test test_mozLoop_hangupOnClose
08:27:44     INFO -  console.error: Loop:
08:27:44     INFO -    invalid preference type getting loop.ot.guid
08:27:45     INFO -  console.error: Loop:
08:27:45     INFO -    invalid preference type getting loop.ot.guid
08:27:45     INFO -  Not taking screenshot here: see the one that was previously logged
08:27:45     INFO -  248 INFO TEST-UNEXPECTED-FAIL | browser/extensions/loop/test/mochitest/browser_mozLoop_chat.js | HangupNow handler should be called once - 3 === 1 - JS frame :: chrome://mochitests/content/browser/browser/extensions/loop/test/mochitest/browser_mozLoop_chat.js :: test_mozLoop_hangupOnClose :: line 59
08:27:45     INFO -  Stack trace:
08:27:45     INFO -      chrome://mochitests/content/browser/browser/extensions/loop/test/mochitest/browser_mozLoop_chat.js:test_mozLoop_hangupOnClose:59
08:27:45     INFO -      Handler.prototype.process@resource://gre/modules/Promise.jsm -> resource://gre/modules/Promise-backend.js:933:23
08:27:45     INFO -      this.PromiseWalker.walkerLoop@resource://gre/modules/Promise.jsm -> resource://gre/modules/Promise-backend.js:812:7
08:27:45     INFO -      Promise*this.PromiseWalker.scheduleWalkerLoop@resource://gre/modules/Promise.jsm -> resource://gre/modules/Promise-backend.js:743:11
08:27:45     INFO -      this.PromiseWalker.schedulePromise@resource://gre/modules/Promise.jsm -> resource://gre/modules/Promise-backend.js:775:7
08:27:45     INFO -      this.PromiseWalker.completePromise@resource://gre/modules/Promise.jsm -> resource://gre/modules/Promise-backend.js:710:7
08:27:45     INFO -      TaskImpl_run@resource://gre/modules/Task.jsm:320:13
08:27:45     INFO -      promise callback*TaskImpl_handleResultValue@resource://gre/modules/Task.jsm:391:7
08:27:45     INFO -      TaskImpl_run@resource://gre/modules/Task.jsm:323:13
08:27:45     INFO -      TaskImpl@resource://gre/modules/Task.jsm:276:3
08:27:45     INFO -      createAsyncFunction/asyncFunction@resource://gre/modules/Task.jsm:250:14
08:27:45     INFO -      Task_spawn@resource://gre/modules/Task.jsm:164:12
08:27:45     INFO -      TaskImpl_handleResultValue@resource://gre/modules/Task.jsm:384:1
08:27:45     INFO -      TaskImpl_run@resource://gre/modules/Task.jsm:323:13
08:27:45     INFO -      Handler.prototype.process@resource://gre/modules/Promise.jsm -> resource://gre/modules/Promise-backend.js:933:23
08:27:45     INFO -      this.PromiseWalker.walkerLoop@resource://gre/modules/Promise.jsm -> resource://gre/modules/Promise-backend.js:812:7
08:27:45     INFO -      Promise*this.PromiseWalker.scheduleWalkerLoop@resource://gre/modules/Promise.jsm -> resource://gre/modules/Promise-backend.js:743:11
08:27:45     INFO -      this.PromiseWalker.schedulePromise@resource://gre/modules/Promise.jsm -> resource://gre/modules/Promise-backend.js:775:7
08:27:45     INFO -      Promise.prototype.then@resource://gre/modules/Promise.jsm -> resource://gre/modules/Promise-backend.js:450:5
08:27:45     INFO -      this.PromiseWalker.completePromise@resource://gre/modules/Promise.jsm -> resource://gre/modules/Promise-backend.js:701:1
08:27:45     INFO -      TaskImpl_run@resource://gre/modules/Task.jsm:320:13
08:27:45     INFO -      TaskImpl@resource://gre/modules/Task.jsm:276:3
08:27:45     INFO -      createAsyncFunction/asyncFunction@resource://gre/modules/Task.jsm:250:14
08:27:45     INFO -      Tester_execTest/<@chrome://mochikit/content/browser-test.js:761:21
08:27:45     INFO -      TaskImpl_run@resource://gre/modules/Task.jsm:315:40
08:27:45     INFO -      TaskImpl@resource://gre/modules/Task.jsm:276:3
08:27:45     INFO -      createAsyncFunction/asyncFunction@resource://gre/modules/Task.jsm:250:14
08:27:45     INFO -      Task_spawn@resource://gre/modules/Task.jsm:164:12
08:27:45     INFO -      Tester_execTest@chrome://mochikit/content/browser-test.js:756:9
08:27:45     INFO -      Tester.prototype.nextTest</<@chrome://mochikit/content/browser-test.js:676:7
08:27:45     INFO -      SimpleTest.waitForFocus/waitForFocusInner/focusedOrLoaded/<@chrome://mochikit/content/tests/SimpleTest/SimpleTest.js:735:59
08:27:45     INFO -  249 INFO TEST-PASS | browser/extensions/loop/test/mochitest/browser_mozLoop_chat.js | Messages should be the same - {"name":"HangupNow","data":["fake1234","fake1234"]} deepEqual {"name":"HangupNow","data":["fake1234","fake1234"]} -
08:27:45     INFO -  250 INFO Leaving test test_mozLoop_hangupOnClose

Further down
INFO TEST-UNEXPECTED-FAIL | browser/extensions/loop/test/mochitest/browser_mozLoop_chat.js | chat window should have been closed - false == true - JS frame :: chrome://mochitests/content/browser/browser/extensions/loop/test/mochitest/browser_mozLoop_chat.js :: test_mozLoop_hangupAllChatWindows :: line 

2) Win 7 pgo and Win XP opt M(bc6) failures:
11:42:22     INFO -  254 INFO TEST-UNEXPECTED-FAIL | browser/extensions/loop/test/mochitest/browser_mozLoop_chat.js | HangupNow handler should be called once - 2 === 1 - JS frame :: chrome://mochitests/content/browser/browser/extensions/loop/test/mochitest/browser_mozLoop_chat.js :: test_mozLoop_hangupOnClose :: line 59
11:42:22     INFO -  Stack trace:
11:42:22     INFO -      chrome://mochitests/content/browser/browser/extensions/loop/test/mochitest/browser_mozLoop_chat.js:test_mozLoop_hangupOnClose:59
11:42:22     INFO -      Handler.prototype.process@resource://gre/modules/Promise.jsm -> resource://gre/modules/Promise-backend.js:933:23
11:42:22     INFO -      this.PromiseWalker.walkerLoop@resource://gre/modules/Promise.jsm -> resource://gre/modules/Promise-backend.js:812:7
11:42:22     INFO -      Promise*this.PromiseWalker.scheduleWalkerLoop@resource://gre/modules/Promise.jsm -> resource://gre/modules/Promise-backend.js:743:11
11:42:22     INFO -      this.PromiseWalker.walkerLoop@resource://gre/modules/Promise.jsm -> resource://gre/modules/Promise-backend.js:805:7
11:42:22     INFO -      Promise*this.PromiseWalker.scheduleWalkerLoop@resource://gre/modules/Promise.jsm -> resource://gre/modules/Promise-backend.js:743:11
11:42:22     INFO -      this.PromiseWalker.schedulePromise@resource://gre/modules/Promise.jsm -> resource://gre/modules/Promise-backend.js:775:7
11:42:22     INFO -      Promise.prototype.then@resource://gre/modules/Promise.jsm -> resource://gre/modules/Promise-backend.js:450:5
11:42:22     INFO -      MozLoopServiceInternal.hawkRequest/<@chrome://loop/content/modules/MozLoopService.jsm:692:7
11:42:22     INFO -      Promise@resource://gre/modules/Promise.jsm -> resource://gre/modules/Promise-backend.js:384:5
11:42:22     INFO -      MozLoopServiceInternal.hawkRequest@chrome://loop/content/modules/MozLoopService.jsm:691:1
11:42:22     INFO -      this.MozLoopService.hawkRequest@chrome://loop/content/modules/MozLoopService.jsm:1886:12
11:42:22     INFO -      _postToRoom@chrome://loop/content/modules/LoopRooms.jsm:759:5
11:42:22     INFO -      LoopRoomsInternal.leave@chrome://loop/content/modules/LoopRooms.jsm:838:1
11:42:22     INFO -      this.LoopRooms.leave@chrome://loop/content/modules/LoopRooms.jsm:1079:12
11:42:22     INFO -      kMessageHandlers.HangupNow@chrome://loop/content/modules/MozLoopAPI.jsm:729:5
11:42:22     INFO -      this.LoopAPI<.sendMessageToHandler@chrome://loop/content/modules/MozLoopAPI.jsm:1414:7
11:42:22     INFO -      socialFrameChanged@chrome://loop/content/modules/MozLoopService.jsm:956:1
11:42:22     INFO -      _remove@chrome://browser/content/socialchat.xml:559:11
11:42:22     INFO -      remove@chrome://browser/content/socialchat.xml:544:11
11:42:22     INFO -      close@chrome://browser/content/socialchat.xml:199:11
11:42:22     INFO -      _mozSocialDOMWindowClose@resource://gre/modules/MozSocialAPI.jsm:264:9
11:42:22     INFO -      WindowCloseMixin.closeWindow@chrome://loop/content/shared/js/mixins.js:81:7
11:42:22     INFO -      ConversationAppStore.prototype<.LoopHangupNowHandler@chrome://loop/content/panels/js/conversationAppStore.js:154:13
11:42:22     INFO -      hangupAllChatWindows@chrome://loop/content/modules/MozLoopService.jsm:881:1
11:42:22     INFO -      hangupAllChatWindows@chrome://loop/content/modules/MozLoopService.jsm:1418:12
11:42:22     INFO -      test_mozLoop_hangupAllChatWindows/<@chrome://mochitests/content/browser/browser/extensions/loop/test/mochitest/browser_mozLoop_chat.js:34:5
11:42:22     INFO -      waitForCondition/interval<@chrome://mochitests/content/browser/browser/extensions/loop/test/mochitest/head.js:78:25
11:42:22     INFO -      setInterval handler*waitForCondition@chrome://mochitests/content/browser/browser/extensions/loop/test/mochitest/head.js:71:18
11:42:22     INFO -      promiseWaitForCondition/<@chrome://mochitests/content/browser/browser/extensions/loop/test/mochitest/head.js:93:5
11:42:22     INFO -      promiseWaitForCondition@chrome://mochitests/content/browser/browser/extensions/loop/test/mochitest/head.js:92:1
11:42:22     INFO -      test_mozLoop_hangupAllChatWindows@chrome://mochitests/content/browser/browser/extensions/loop/test/mochitest/browser_mozLoop_chat.js:33:9
11:42:22     INFO -      TaskImpl_run@resource://gre/modules/Task.jsm:315:40
11:42:22     INFO -      TaskImpl@resource://gre/modules/Task.jsm:276:3
11:42:22     INFO -      createAsyncFunction/asyncFunction@resource://gre/modules/Task.jsm:250:14
11:42:22     INFO -      Task_spawn@resource://gre/modules/Task.jsm:164:12
11:42:22     INFO -      TaskImpl_handleResultValue@resource://gre/modules/Task.jsm:384:1
11:42:22     INFO -      TaskImpl_run@resource://gre/modules/Task.jsm:323:13
11:42:22     INFO -      Handler.prototype.process@resource://gre/modules/Promise.jsm -> resource://gre/modules/Promise-backend.js:933:23
11:42:22     INFO -      this.PromiseWalker.walkerLoop@resource://gre/modules/Promise.jsm -> resource://gre/modules/Promise-backend.js:812:7
11:42:22     INFO -      Promise*this.PromiseWalker.scheduleWalkerLoop@resource://gre/modules/Promise.jsm -> resource://gre/modules/Promise-backend.js:743:11
11:42:22     INFO -      this.PromiseWalker.schedulePromise@resource://gre/modules/Promise.jsm -> resource://gre/modules/Promise-backend.js:775:7
11:42:22     INFO -      Promise.prototype.then@resource://gre/modules/Promise.jsm -> resource://gre/modules/Promise-backend.js:450:5
11:42:22     INFO -      this.PromiseWalker.completePromise@resource://gre/modules/Promise.jsm -> resource://gre/modules/Promise-backend.js:701:1
11:42:22     INFO -      TaskImpl_run@resource://gre/modules/Task.jsm:320:13
11:42:22     INFO -      TaskImpl@resource://gre/modules/Task.jsm:276:3
11:42:22     INFO -      createAsyncFunction/asyncFunction@resource://gre/modules/Task.jsm:250:14
11:42:22     INFO -      Tester_execTest/<@chrome://mochikit/content/browser-test.js:761:21
11:42:22     INFO -      TaskImpl_run@resource://gre/modules/Task.jsm:315:40
11:42:22     INFO -      TaskImpl@resource://gre/modules/Task.jsm:276:3
11:42:22     INFO -      createAsyncFunction/asyncFunction@resource://gre/modules/Task.jsm:250:14
11:42:22     INFO -      Task_spawn@resource://gre/modules/Task.jsm:164:12
11:42:22     INFO -      Tester_execTest@chrome://mochikit/content/browser-test.js:756:9
11:42:22     INFO -      Tester.prototype.nextTest</<@chrome://mochikit/content/browser-test.js:676:7
11:42:22     INFO -      SimpleTest.waitForFocus/waitForFocusInner/focusedOrLoaded/<@chrome://mochikit/content/tests/SimpleTest/SimpleTest.js:735:59

Comment 25

[Mike de Boer [:mikedeboer]]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=4d50688ee0b3

Comment 26

[Mike de Boer [:mikedeboer]]

https://hg.mozilla.org/integration/fx-team/rev/860d77fbf406524dd7ad67066d92e051e9555fba
Bug 1227539: Part 1 - close a chat window properly, including stopping all sharing and leaving the room. r=Standard8

https://hg.mozilla.org/integration/fx-team/rev/30b9e8aa695d29d815baa2b1f8c05ff2fa10df6a
Bug 1227539: Part 2 - implement the new HangupNow action on standalone too so that leaving a room will still work. r=Standard8

Comment 27

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

Backed out in https://hg.mozilla.org/integration/fx-team/rev/b92cd30cf34f for win pgo bc6 failures like https://treeherder.mozilla.org/logviewer.html#?job_id=6329807&repo=fx-team

Comment 28

[Mike de Boer [:mikedeboer]]

https://hg.mozilla.org/integration/fx-team/rev/a25f919304364b6ac19e7a52ea935070a372b65f
Bug 1227539: Part 1 - close a chat window properly, including stopping all sharing and leaving the room. r=Standard8

https://hg.mozilla.org/integration/fx-team/rev/1ef0360a8bb223c64972e32f6a0ae3f460add33d
Bug 1227539: Part 2 - implement the new HangupNow action on standalone too so that leaving a room will still work. r=Standard8

Comment 29

[Mike de Boer [:mikedeboer]]

Apologies, Wes. I made a slight adjustment to fix the PGO build assertion failures. On the upside, this will most likely also fix bug 1229195.

Comment 30

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/a25f91930436
https://hg.mozilla.org/mozilla-central/rev/1ef0360a8bb2

Comment 31

[Mark Banner (:standard8)]

I think we left the leave-open here by accident.

Comment 32

[Mihai Boldan, Desktop QA [:mboldan]]

I managed to reproduce this issue with the STR from Comment 0 using Firefox 45.0a1(2015-11-24) and on Windows 10x86.
I've also tested this issue on Firefox 46.0a1(2015-12-31) and on FIrefox 46.0a2(2016-02-03) across platforms[1]. 
While testing I've encountered the same actual result as in Comment 0, if closing the first conversation from a clean profile. The Hello icon is still green after closing the conversation and the 'Leave Feedback' window is displayed(see the attached screenshot).
In my opinion the Hello icon should be grey while the message from the Hello window inform the user that the conversation ended.
Mike, can you please take a look over this and tell me if this is an expected result?

[1]Mac OS X 10.11.1, Windows 10 x86, Ubuntu 14.04 x86

Comment 33

[Mihai Boldan, Desktop QA [:mboldan]]

Attachment: hello.png

Comment 34

[Mark Banner (:standard8)]

(In reply to Mihai Boldan, QA [:mboldan] from comment #32)
> I managed to reproduce this issue with the STR from Comment 0 using Firefox
> 45.0a1(2015-11-24) and on Windows 10x86.
> I've also tested this issue on Firefox 46.0a1(2015-12-31) and on FIrefox
> 46.0a2(2016-02-03) across platforms[1]. 
> While testing I've encountered the same actual result as in Comment 0, if
> closing the first conversation from a clean profile. The Hello icon is still
> green after closing the conversation and the 'Leave Feedback' window is
> displayed(see the attached screenshot).
> In my opinion the Hello icon should be grey while the message from the Hello
> window inform the user that the conversation ended.
> Mike, can you please take a look over this and tell me if this is an
> expected result?

Yes, we should revert the icon I think to save confusing the user. Please can you file a follow up for that?

Comment 35

[Mihai Boldan, Desktop QA [:mboldan]]

Logged Bug 1247620 for the issue mentioned above.
I'm also marking this issue Verified Fixed.