onLowMemory may not work on Android

RESOLVED FIXED

Status

()

Core
Widget: Android
RESOLVED FIXED
7 years ago
5 years ago

People

(Reporter: m_kato, Assigned: m_kato)

Tracking

Trunk
All
Android
Points:
---

Firefox Tracking Flags

(fennec2.0+)

Details

(URL)

Attachments

(1 attachment, 1 obsolete attachment)

Java_org_mozilla_gecko_GeckoAppShell_onLowMemory is called from DalvikVM thread. But NotifyObservers only works on XUL main thread (in other word, NS_IsMainThread() == TRUE).  So onLowMemory may not work on Android.

Updated

7 years ago
tracking-fennec: --- → 2.0+
Created attachment 498970 [details] [diff] [review]
fix v1
Assignee: nobody → m_kato
Status: NEW → ASSIGNED
(Assignee)

Updated

6 years ago
Attachment #498970 - Flags: review?(doug.turner)

Comment 2

6 years ago
Comment on attachment 498970 [details] [diff] [review]
fix v1

The behavior of nsAppShell::NotifyObservers() is going to be different depending on if you are on the main thread or not.  If you are on the main thread, you will directly call nsIObserverService::NotifyObservers.  This call will enumerate all listeners and synchronously call Observe.  In the non-main-thread case, nsAppShell::NotifyObservers() will simple post an event and immediately return.

I think I would prefer that you make nsAppShell::NotifyObservers just post events regardless of which thread it is called on.

Otherwise looks fine.
Attachment #498970 - Flags: review?(doug.turner) → review-

Comment 3

6 years ago
Created attachment 501197 [details] [diff] [review]
minor clean up from last patch to address my concern.
Attachment #498970 - Attachment is obsolete: true
Attachment #501197 - Flags: review?(blassey.bugs)
Attachment #501197 - Flags: review?(blassey.bugs) → review+

Comment 4

6 years ago
http://hg.mozilla.org/mozilla-central/rev/a31e58c375b6
Status: ASSIGNED → RESOLVED
Last Resolved: 6 years ago
Resolution: --- → FIXED
onLowMemory still doesn't work on this test case:

   http://crazybugs.ivank.net/

See bug 736436
Blocks: 736436
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Hm, though, that is not our fault, it's android who fails to notify us. re-closing. will file a separate bug.
Status: REOPENED → RESOLVED
Last Resolved: 6 years ago5 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.