1250088 - Closing a https://apprtc.appspot.com/ tab takes a long time

Status: RESOLVED DUPLICATE of bug 1103323

(Firefox :: General, defect)

Description

[Marco Castelluccio [:marco]]

Steps to reproduce:
1) Open https://apprtc.appspot.com/
2) Join a room
3) If possible, make someone else join your room (this makes the problem more noticeable)
4) Close the tab

https://cleopatra.io/#report=2d97430775a0f27d4def2ef8bfab29397130f593

In the profile, I see a lot of time spent in 'poll', which is similar to bug 1224845.

mozregression tells me bug 967873 is the culprit: https://hg.mozilla.org/integration/mozilla-inbound/pushloghtml?fromchange=961911623a6f2ec1d036c7b12a5117ebbeff45d8&tochange=8e0bc70119606b70d74f1aa19d84e697ac4793c7.

Comment 1

[Mike Conley (:mconley) (:⚙️)]

Hey marco, what happens if you try to do this in single-process mode? Does it also hang the browser?

I ask because it looks like the app is doing a synchronous XHR, which should block up the main process in single-process mode as well.

Comment 2

[Marco Castelluccio [:marco]]

> Hey marco, what happens if you try to do this in single-process mode? Does it also hang the browser?

It also happens on Firefox Beta on Android, so yes.

Comment 3

[Mike Conley (:mconley) (:⚙️)]

Alright, so I think it's what I suspected - sync XHR on unload, which is kind of an anti-pattern sites use to ensure their servers get hit with a request of some kind when you go away.

This isn't e10s specific. Minusing.

Comment 4

[Marco Castelluccio [:marco]]

(In reply to Mike Conley (:mconley) - Needinfo me! from comment #3)
> Alright, so I think it's what I suspected - sync XHR on unload, which is
> kind of an anti-pattern sites use to ensure their servers get hit with a
> request of some kind when you go away.
> 
> This isn't e10s specific. Minusing.

It is "kind of" e10-specific, since the bug that caused the regression (bug 967873) was needed for e10s.

Comment 5

[Lawrence Mandel [:lmandel] (use needinfo)]

We're not going to fix this in 46 and without a fix ready we're not realistically going to fix this in 47.

Comment 6

[Sylvestre Ledru [:Sylvestre]]

Same with 48, is it still occurring with 49?

Comment 7

[Liz Henry (:lizzard) (relman/hg->git project)]

Andrei, can your team try to reproduce this? Thanks!

Comment 8

[Bogdan Maris, Desktop Test Engineering]

I could reproduce this using Latest Nightly and 48.0.2 on Ubuntu 14.04 64-bit:

48.0.2 - https://cleopatra.io/#report=2a43026b1a0ee20364834993af169ecea5c8a6d2
Latest Nightly - https://cleopatra.io/#report=8a56f8bbd5b8fb0727b3f98f6745f3b6adfa0986

Comment 9

[Mike Conley (:mconley) (:⚙️)]

I filed https://github.com/webrtc/apprtc/issues/368

Comment 10

[Mike Conley (:mconley) (:⚙️)]

Hey RyanVM, according to both marco and the folks in https://github.com/webrtc/apprtc/issues/368, Firefox behaved better in the past for the scenario in comment 0.

The bug that marco identified in comment 0 is, I believe, what gave Firefox+e10s the ability to deal with onbeforeunload events, so it's not surprising that things seem worse after that patch; though with e10s _disabled_, I'd expect us to have always performed poorly. However, this seems to contrary to what the developer in the GitHub issue experienced.

Would it be possible to have somebody from SV use mozregression and test the scenario in comment 0 with e10s disabled (they'll probably need to use the --pref "browser.tabs.remote.autostart:false" argument to make sure that e10s is off) to see when the regression might have been introduced?

Note that I'm not 100% convinced yet that this is anything new - I can't see how we would have been better at this in the past. A sync XHR in onbeforeunload is an anti-pattern that I suspect Firefox has never reacted well to, but I think we'd better make sure.

Comment 11

[Ryan VanderMeulen [:RyanVM]]

Sure, we can try!

Comment 12

[Ciprian Muresan [:cmuresan], Ecosystem QA]

Hi guys,

I have used mozregression to narrow the regression window for this but I can only go as far as Nightly 39 where the issue is still reproducible. On builds older than this, the Gecko Profiler is not supported.

https://cleopatra.io/#report=f0ec71d318831910689d22897ea6a148d356d621 - FF 39
https://cleopatra.io/#report=9c265b21675f0d222a9afdd13db70621df5fb88f - FF 41

As far as I can tell, it seems Firefox never did perform better in the past.
Let me know if there's anything else I can do.

Comment 13

[:Gijs (out for now; he/him)]

Is this effectively just a dupe of bug 1103323?

Comment 14

[Marco Castelluccio [:marco]]

(In reply to :Gijs Kruitbosch from comment #13)
> Is this effectively just a dupe of bug 1103323?

Yes, I think so.