Increase Observer of "network-activity-blip-[upload|download]" when screen on/off

RESOLVED FIXED in Firefox 26

Status

defect
--
critical
RESOLVED FIXED
6 years ago
6 years ago

People

(Reporter: jeremykimleo, Assigned: jeremykimleo)

Tracking

unspecified
1.1 QE5
ARM
Gonk (Firefox OS)
Dependency tree / graph

Firefox Tracking Flags

(blocking-b2g:leo+, firefox24 wontfix, firefox25 wontfix, firefox26 fixed, b2g18 verified, b2g18-v1.0.0 wontfix, b2g18-v1.0.1 wontfix, b2g-v1.1hd fixed)

Details

(Whiteboard: [MemShrink:P2])

Attachments

(2 attachments, 1 obsolete attachment)

Version Info>
project b2g/gaia/
45f6a739b09292e16717fb21003386c914ca29c2 Bug 901044 - Everything.me needs additional strings for the search bar. r=pike

project b2g/gecko/
8878e8a4636c58227856ee2b76fc7d95a4cc9709 Bug 739681 - Allow DumpHeapComplete to print unreachable objects (r=mccr8 a=leo+)

STR>
1. enable Usage
2. turn on/off screen by power key
3. if #2 is over 200 times, target becomes extremely slow.

window object has a many observer regarding as "network-activity-blip-[upload|download]" when screen on/off.
it caused overhead of processing when target is doing network operation.
Posted video video of repro
00:15" - push home-button. no response
00:31" - push power-key. no response of lockscreen
it can reproduce 100%. and if it is this situation. user can't do anything.
blocking-b2g: --- → leo+
This issue is originated from tester's normal behavior as end user.
- He used browser/youtube or other apps as end-user (not a stress test) for one or two days and then his devcie suddelny became slow.

And we found the sequence to reproduce as the above.
dear. mounir,  can you help this?
Flags: needinfo?(mounir)
Posted patch proposal V1 (obsolete) — Splinter Review
i recomands it should fix like this.

globla variabls blocks AddObserver to regist one more.
Flags: needinfo?(mounir)
to remove Observer of network-activity-blip-[upload/download], it should call nsGlobalWindow::DisableNetworkEvent(uint32_t aType)

but, it isn't called because nsEventListenerManager::RemoveEventListener() never has typeCount == 0
https://hg.mozilla.org/releases/mozilla-b2g18/file/895f4b058c60/content/events/src/nsEventListenerManager.cpp#l468
please check this.
Flags: needinfo?(mounir)
Whiteboard: [MemShrink]
I think this is the right change; it looks like we call EnableNetworkEvent every time someone adds a network-event eventlistener on the window (see nsEventListenerManager.cpp).  I think the code here was written with the (incorrect) assumption that we get one DisableNetworkEvent call for every EnableNetworkEvent call.

Thanks, Jeremy.
Flags: needinfo?(mounir)
Whiteboard: [MemShrink] → [MemShrink:P2]
Duplicate of this bug: 900923
I'll push this today.
Flags: needinfo?(justin.lebar+bug)
Trees are closed, let's just use checkin-needed.

This is the same as "proposal V1", except I changed the names of the variables and added commit headers.

There are merge conflicts when applying this to b2g18, but only in context.
That is to say, you should be able to uplift this without issue.
Attachment #786069 - Attachment is obsolete: true
Attachment #786968 - Flags: review+
Flags: needinfo?(justin.lebar+bug)
Keywords: checkin-needed
I filed bug 903290 on adding instrumentation which will make bugs like this easier to find.  Thanks again for finding this one.
Unable to reproduce on Leo Build ID: 20130816041203 verifying fixed on b2g18

Environmental Variables
Build ID: 20130816041203
Gecko: http://hg.mozilla.org/releases/mozilla-b2g18/rev/09a8c5b6b287
Gaia: 6040508a535b587d4155356a47b22d7bd9ea4de7
Platform Version: 18.1
RIL Version: 01.01.00.019.164
Someone with the IRC nick sireesha has pinged me a few times about this bug.  Unfortunately these pings have been coming at around 11:30p my time, so I haven't been able to respond.

> <sireesha> After this fix the Usage widget data updation become very slow
> <sireesha> Please refer comment 16 in https://bugzilla.mozilla.org/show_bug.cgi?id=900923
> <sireesha> So i need information related to "How frequently network events are send to system 
> application"

I guess sireesha is trying to weigh the upside of this patch, since she believes there is a downside to the patch.

It looks like the fix here might have exposed a bug in the cost control app, where the activity threshold was set too high, because we were over-firing network-activity-blip events.

This patch is an important main-process memory leak fix.  If you don't take this fix in Leo, we cannot be held responsible for bugs where the phone becomes unresponsive after it is used for a long time.

The appropriate thing to do at this time is to file a bug which blocks this one, describing the regression resulting from this patch.  Once we understand the situation, we can weigh our options as far as backing this patch out or fixing the regression.
Blocks: 910018
You need to log in before you can comment on or make changes to this bug.