1157712 - Multiple OS X notifications when someone joins

Status: RESOLVED FIXED

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

Description

[Erik Rose [:erik][:erikrose]]

1. Click Hello icon.
2. Start Conversation.
3. Copy Link.
4. Paste it to a friend.
5. Friend clicks it.
6. 68 OS X notifications are posted, saying "Someone has joined the conversation!"

It happens every time.

Comment 1

[Mike de Boer [:mikedeboer]]

68?!? Wow. That's totally worth investigating, but your STR looks quite generic. Which Fx version are you using?

Comment 2

[Erik Rose [:erik][:erikrose]]

I'm on 37.0.2, mbrandt's on 38. We're both on OS X.

Comment 3

[Mark Banner (:standard8)]

From an email thread, Erik mention he is putting his computer to sleep occasionally. Also abr said he could reproduce this.

If it is being put to sleep, I wonder if something strange is happening with listeners when push servers re-connect.

Comment 4

[Adam Roach [:abr]]

I've rebooted, and am back down to one notification when someone joins. I've tried several sleep/resume cycles, and still get one. I've restarted the browser (which I end up doing somewhat frequently in normal use), and still get one.

So I'm not sure what triggers this, but it's not simple sleep/resume. I suspect we're going to have to find this one by code inspection, unless you have other theories on STR.

Comment 5

[Mark Banner (:standard8)]

I happened to notice this on my browser today. We get one extra notification per a window opening.

On windows, it seems like we just don't get any notification (apart from the background sound) if a new window is opened - it seems to just give up and not display it.

I'm going to fix the Mac bug here, and get that into nightly. We can then ask for some QA effort to take a look at the Windows issue.

Comment 6

[Mark Banner (:standard8)]

Attachment: Fix multiple OS X notifications when someone joins a Loop room. Ensure we don't attempt to re-initialise twice.

Ok, this prevents the multiple notifications by stopping us calling initialise multiple times. I've also added a few comments here and there and tidied up a couple of things - the promise is for test-only, so I've also made browser-loop catch the result and not log it if its one of our expected errors.

Once this is landed, I'll look at retesting notifications on windows a bit more and see if we can see anything strange going on.

Comment 7

[Mike de Boer [:mikedeboer]]

Comment on attachment 8608874 [details] [diff] [review]
Fix multiple OS X notifications when someone joins a Loop room. Ensure we don't attempt to re-initialise twice.

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

I like it! Thanks :)

Comment 8

[Pulsebot]

https://hg.mozilla.org/integration/fx-team/rev/bc4fc60d5759

Comment 9

[Phil Ringnalda (:philor)]

https://hg.mozilla.org/mozilla-central/rev/bc4fc60d5759

Comment 10

[Mark Banner (:standard8)]

Comment on attachment 8608874 [details] [diff] [review]
Fix multiple OS X notifications when someone joins a Loop room. Ensure we don't attempt to re-initialise twice.

Approval Request Comment
[Feature/regressing bug #]: Firefox Hello conversations - notifications of user joining rooms
[User impact if declined]: If the user opens more than one browser window, the user can get multiple notifications of a third party joining one of their rooms. OS X hides this a bit, I'm not sure what Linux would do - Windows appears to stop notifications altogether in my testing (which may end up becoming another bug).
[Describe test coverage new/current, TreeHerder]: Landed in m-c. Initialisation flow has existing and new tests.
[Risks and why]: Low, simple change to initialisation flow. The process is covered by existing tests.
[String/UUID change made/needed]: None

Comment 11

[Sylvestre Ledru [:Sylvestre]]

Comment on attachment 8608874 [details] [diff] [review]
Fix multiple OS X notifications when someone joins a Loop room. Ensure we don't attempt to re-initialise twice.

Has tests, localized in Hello, taking it.

Comment 12

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/e27219e4b619

Comment 13

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-beta/rev/928e51389b65