1357872 - Hundreds of ghost windows from touch events

Status: RESOLVED FIXED

(Core :: DOM: Core & HTML, defect)

Description

[Benjamin Smedberg]

Attachment: 2017-04-19-memory-report.json.gz

I had some really bad pauses today. I'm not sure which site(s) might be causing this, and I had trouble reading the profile so I'm going to file this and hopefully get some help understanding the profile.

https://perfht.ml/2oPN69Z <- profile
about:memory attached

Comment 1

[Benjamin Smedberg]

Questions about profile reading:
There are some really long pauses identified in the upper section with hover titles like "8.NNN second event processing delay on tab thread". But it's not clear to me how to figure out which of the four content processes that is and then how to get the profile of that process but not the rest.

Comment 2

[(no longer active)]

Talked with Benjamin about this on IRC, these are CC pauses...

Comment 3

[Olli Pettay [:smaug][[e-mail]]]

Based on the memory report, there are 60 ghost windows.
Gecko profiler profiles aren't really useful with leaks.
CC/GC logs from about:memory might reveal the issue.

Comment 4

[Andrew McCreight [:mccr8]]

Yeah, you have hundreds of ghost windows across your various processes. Odds are there is some addon that is causing this.

Comment 5

[Andrew McCreight [:mccr8]]

Are you on Firefox 55? What addons do you have installed?

Comment 6

[Benjamin Smedberg]

This was very recent nightly 55 win64. At the time I was running the following:

* Mass Password Reset
* User Agent Switcher
* Moo Later
* Mind the Time

I have since:

installed "Gecko Profiler"
uninstalled "Moo Later" and "Mind the Time"

I will report as I browse today whether I see more ghost windows or similar symptoms.

Comment 7

[Andrew McCreight [:mccr8]]

Honestly, with that many ghost windows, you were probably leaking every single one you opened. So just browsing for a few minutes and checking about:memory should tell you something. I think I've never seen so many ghost windows in a single memory report, and I've seen a lot of memory reports. ;)

Comment 8

[Benjamin Smedberg]

Attachment: 2017-04-20-memory-report.json.gz

Browsing today with the shorter addon list, still seeing ghost windows. memory dump attached

Comment 9

[Andrew McCreight [:mccr8]]

Which "User Agent Switcher" addon do you use? I see 4 with that name and various hypenations.
The most popular one is https://addons.mozilla.org/en-US/firefox/addon/user-agent-switcher/?src=search so I'll try that.

Comment 10

[Kan-Ru Chen [:kanru] (UTC+9)]

Attachment: 2017-04-21-memory-report-kanru.json.gz

I'm not sure if it's same issue but I've got 400+ ghost windows

Comment 11

[Andrew McCreight [:mccr8]]

(In reply to Kan-Ru Chen [:kanru] (UTC+8) from comment #10)
> I'm not sure if it's same issue but I've got 400+ ghost windows

Do you have any of the same addons as listed in comment 6? If not, please file a new bug and needinfo me.

Comment 12

[Andrew McCreight [:mccr8]]

(In reply to Andrew McCreight [:mccr8] from comment #9)
> Which "User Agent Switcher" addon do you use? I see 4 with that name and
> various hypenations.

Also, do you have any rules or whatever set up for it? I tried browsing a little with the user agent switcher and mass password reset addons installed but I didn't get any leaks. Though I should try it with the profiler, too.

Comment 13

[Benjamin Smedberg]

I have the one with ID {e968fc70-8f95-4ab9-9e79-304de2a71ee1}

I'm not actually sure how to tell whether I have any site-specific overrides set up.

Also I don't think I'm using this addon currently, so let me try disabling it as well.

Comment 14

[Olli Pettay [:smaug][[e-mail]]]

I'm seeing ghosts locally. I don't know how to reproduce the issue, but atm
the regression range looks to be 
https://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=c697e756f738ce37abc56f31bfbc48f55625d617&tochange=bb38d935d699e0529f9e0bb35578d381026415c4

I got some ghosts with 2017-04-18-10-02-13-mozilla-central (bb38d935d699e0529f9e0bb35578d381026415c4) but so far haven't got using 
2017-04-17-10-02-36-mozilla-central (c697e756f738ce37abc56f31bfbc48f55625d617)

Comment 15

[Olli Pettay [:smaug][[e-mail]]]

telemetry seems to hint about something similar. There is a spike in 04-18.

https://telemetry.mozilla.org/new-pipeline/evo.html#!aggregates=median!mean!5th-percentile!25th-percentile!75th-percentile!95th-percentile&cumulative=0&end_date=2017-04-10&keys=!__none__!__none__&max_channel_version=nightly%252F55&measure=GHOST_WINDOWS&min_channel_version=nightly%252F55&os=Windows_NT&processType=*&product=Firefox&sanitize=1&sort_keys=submissions&start_date=2017-03-08&trim=1&use_submission_date=0

Comment 16

[Olli Pettay [:smaug][[e-mail]]]

Also https://telemetry.mozilla.org/new-pipeline/evo.html#!aggregates=median!mean!5th-percentile!25th-percentile!75th-percentile!95th-percentile&cumulative=0&end_date=2017-04-10&keys=!__none__!__none__&max_channel_version=nightly%252F55&measure=CYCLE_COLLECTOR_MAX_PAUSE&min_channel_version=nightly%252F55&os=Windows_NT&processType=*&product=Firefox&sanitize=1&sort_keys=submissions&start_date=2017-03-08&trim=1&use_submission_date=0 seems to hint there is a regression between 04-17 and 04-18.

Comment 17

[Olli Pettay [:smaug][[e-mail]]]

a374c3546993 does seem to leak.

Comment 18

[Dave Townsend [:mossop]]

I think I'm seeing this too, is there any information I can provide to help?

Comment 19

[Olli Pettay [:smaug][[e-mail]]]

STR would be really nice here. I can occasionally reproduce this locally using my main FF profile.
https://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=c697e756f738ce37abc56f31bfbc48f55625d617&tochange=bb38d935d699e0529f9e0bb35578d381026415c4 is probably the right regression range.

Oddly enough https://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=cac374fb0de1&tochange=a374c3546993 seems to be more precise range, but that doesn't quite make sense.
Could be that I just haven't seen the leak.

Bug 1358979 doesn't seem to help here.

Comment 20

[Asa Dotzler [:asa]]

I'm seeing this with no addons so updating the summary.

Comment 21

[(no longer active)]

(In reply to Olli Pettay [:smaug] from comment #19)
> Oddly enough
> https://hg.mozilla.org/mozilla-central/
> pushloghtml?fromchange=cac374fb0de1&tochange=a374c3546993 seems to be more
> precise range, but that doesn't quite make sense.

Could bug 1357069 be a candidate?

Comment 22

[Olli Pettay [:smaug][[e-mail]]]

I haven't opened any pdfs, but perhaps pdf.js accesses all the pages somehow.

Comment 23

[Andrew Overholt [:overholt]]

Attachment: memory-report.json.gz

I have been experiencing this a lot and also have no addons other than the profiler enabled.

Here's my about:memory (you can see some closed-earlier-today Google Slides and Sheets URLs) and my profile is: https://perfht.ml/2ptKnWU

Comment 24

[Olli Pettay [:smaug][[e-mail]]]

I'm going through changesets now one by one. Just need to browse random pages after a build is ready, since I still don't have STR, so this takes time.
If anyone has some STR (which might just work locally with a certain FF profile), help would be appreciated.

Comment 25

[Benjamin Smedberg]

With my main profile, STR is still:
* launch profile with restored session consisting of 7 pinned tabs
* open a new tab and load something
* close the new tab

Comment 26

[Andrew McCreight [:mccr8]]

(In reply to Olli Pettay [:smaug] from comment #24)
> I'm going through changesets now one by one. Just need to browse random
> pages after a build is ready, since I still don't have STR, so this takes
> time.

Have you run find_roots.py on the CC edges file yet? What does that turn up?

[Tracking Requested - why for this release]: bad leaks many people are seeing

Comment 27

[Olli Pettay [:smaug][[e-mail]]]

I think I found it. r=smaug patch :p
/me tests.

Comment 28

[(no longer active)]

(In reply to Olli Pettay [:smaug] from comment #27)
> I think I found it. r=smaug patch :p
> /me tests.

Ugh, nsDocShell::GetPresContext() addrefs!

Comment 29

[Olli Pettay [:smaug][[e-mail]]]

Attachment: patch

Comment 30

[Andrew McCreight [:mccr8]]

[Tracking Requested - why for this release]:

Comment 31

[Andrew McCreight [:mccr8]]

Thanks for tracking this down, Olli!

Comment 32

[Ryan VanderMeulen [:RyanVM]]

Aha, maybe this bug *does* explain why I've been seeing 54.0b1 doing the same thing also then! I really think this should block 54.0b2, Gerry. My experience over the weekend is that this was giving me an extremely janky, nearly unusable browser.

Comment 33

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/363b1e7eccc3
nsPresContext is refcounted, so use it so, r=kats

Comment 34

[Olli Pettay [:smaug][[e-mail]]]

Comment on attachment 8861185 [details] [diff] [review]
patch

(Since I don't know whether one is supposed to ask approval for aurora or beta, asking both.)

Approval Request Comment
[Feature/Bug causing the regression]: bug 1356695
[User impact if declined]: Major leaks
[Is this code covered by automated tests?]: NA
[Has the fix been verified in Nightly?]: NA
[Needs manual test from QE? If yes, steps to reproduce]: Load pages and then
about:memory Measure and look for ghosts
[Is the change risky?]:Not risky
[Why is the change risky/not risky?]: just using refcounted object correctly
[String changes made/needed]:NA

Comment 35

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

Comment on attachment 8861185 [details] [diff] [review]
patch

Let's get this on m-b now, so that we may still be able to build beta 2 tonight.

Comment 36

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-beta/rev/ca3c6131b6ea

Comment 37

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/363b1e7eccc3

Comment 38

[Andrei Vaida [:avaida]]

Flagging this for manual testing, instructions in Comment 34.

Comment 39

[Andrew McCreight [:mccr8]]

(In reply to Andrei Vaida [:avaida], Desktop Release QA – please ni? me from comment #38)
> Flagging this for manual testing, instructions in Comment 34.

The testing probably needs to be done on a computer with a touch screen, and it requires using the touch screen.

Comment 40

[Olli Pettay [:smaug][[e-mail]]]

Does need touchscreen, but doesn't need using it. It just needs to be enabled and pages need to do something to try to use touch events or something.