Last Comment Bug 619670 - onLowMemory may not work on Android
: onLowMemory may not work on Android
Product: Core
Classification: Components
Component: Widget: Android (show other bugs)
: Trunk
: All Android
-- normal (vote)
: ---
Assigned To: Makoto Kato [:m_kato]
: Jim Chen [:jchen] [:darchons]
Depends on:
Blocks: 736436
  Show dependency treegraph
Reported: 2010-12-16 05:46 PST by Makoto Kato [:m_kato]
Modified: 2012-04-11 10:59 PDT (History)
2 users (show)
See Also:
Crash Signature:
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---

fix v1 (2.85 KB, patch)
2010-12-21 00:01 PST, Makoto Kato [:m_kato]
doug.turner: review-
Details | Diff | Splinter Review
minor clean up from last patch to address my concern. (3.57 KB, patch)
2011-01-04 16:11 PST, Doug Turner (:dougt)
blassey.bugs: review+
Details | Diff | Splinter Review

Description User image Makoto Kato [:m_kato] 2010-12-16 05:46:37 PST
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.
Comment 1 User image Makoto Kato [:m_kato] 2010-12-21 00:01:13 PST
Created attachment 498970 [details] [diff] [review]
fix v1
Comment 2 User image Doug Turner (:dougt) 2011-01-03 12:23:23 PST
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.
Comment 3 User image Doug Turner (:dougt) 2011-01-04 16:11:51 PST
Created attachment 501197 [details] [diff] [review]
minor clean up from last patch to address my concern.
Comment 4 User image Doug Turner (:dougt) 2011-01-06 09:09:52 PST
Comment 5 User image Benoit Jacob [:bjacob] (mostly away) 2012-04-11 10:58:55 PDT
onLowMemory still doesn't work on this test case:

See bug 736436
Comment 6 User image Benoit Jacob [:bjacob] (mostly away) 2012-04-11 10:59:45 PDT
Hm, though, that is not our fault, it's android who fails to notify us. re-closing. will file a separate bug.

Note You need to log in before you can comment on or make changes to this bug.