Closed
Bug 794642
Opened 13 years ago
Closed 13 years ago
New fennec UpdateService service creates unnecessary components
Categories
(Firefox for Android Graveyard :: General, defect)
Tracking
(firefox18 fixed)
RESOLVED
FIXED
Firefox 18
Tracking | Status | |
---|---|---|
firefox18 | --- | fixed |
People
(Reporter: kats, Assigned: snorp)
References
Details
Attachments
(3 files)
9.09 KB,
text/plain
|
Details | |
1.22 KB,
patch
|
Details | Diff | Splinter Review | |
4.12 KB,
patch
|
kats
:
review+
|
Details | Diff | Splinter Review |
Right now the UpdateService service runs in a separate process from Fennec. This means that it instantiates its own copy of GeckoApplication, including all of the things that GeckoApplication instantiates (i.e. GeckoConnectivityReceiver, GeckoBatteryManager, GeckoNetworkManager, and MemoryMonitor). It might also instantiate other things that I'm not aware of.
This seems unnecessary, and in fact can cause the UpdateService to get killed by ANR in the following way:
1) Low memory notification fires
2) MemoryMonitor catches it and tries to send a low-memory event to gecko
3) MemoryMonitor calls GeckoAppShell.geckoEventSync() to wait for the response from gecko
4) No response is coming (since gecko isn't actually running in this process) and so android kills the UpdateService for getting locked up.
I'm attaching an /data/anr/traces.txt where I saw this happen. Note that the "Cmd line" at the top is that of the UpdateService, but the first stack trace clearly shows the MemoryMonitor blocked on gecko.
Reporter | ||
Comment 1•13 years ago
|
||
Also while I was poking around in UpdateService I noticed a (benign) missing else. See attached.
Assignee | ||
Comment 2•13 years ago
|
||
Attachment #665440 -
Flags: review?(bugmail.mozilla)
Reporter | ||
Comment 3•13 years ago
|
||
Is the mInited guard needed? According to the pretty picture at http://developer.android.com/reference/android/app/Activity.html#ActivityLifecycle the onCreate should ever be called once.
Assignee | ||
Comment 4•13 years ago
|
||
(In reply to Kartikaya Gupta (:kats) (away sep28-oct08) from comment #3)
> Is the mInited guard needed? According to the pretty picture at
> http://developer.android.com/reference/android/app/Activity.
> html#ActivityLifecycle the onCreate should ever be called once.
Within that Activity instance this is true. But it may be possible that another instance of the activity could be created within the same Application lifetime, I believe.
Reporter | ||
Comment 5•13 years ago
|
||
Comment on attachment 665440 [details] [diff] [review]
Avoid unnecessary GeckoApplication initialization in services
Review of attachment 665440 [details] [diff] [review]:
-----------------------------------------------------------------
If we end up with two instances of GeckoApp I think we'll have much bigger problems. But I guess this won't hurt.
::: mobile/android/base/GeckoApplication.java
@@ +28,5 @@
> GeckoNetworkManager.getInstance().init(getApplicationContext());
> MemoryMonitor.getInstance().init(getApplicationContext());
> + mInited = true;
> +
> + android.util.Log.i("SNORP", "Initialized GeckoApplication");
Take out log line
Attachment #665440 -
Flags: review?(bugmail.mozilla) → review+
Assignee | ||
Comment 6•13 years ago
|
||
status-firefox18:
--- → fixed
Comment 7•13 years ago
|
||
Assignee: nobody → snorp
Status: NEW → RESOLVED
Closed: 13 years ago
Flags: in-testsuite-
Resolution: --- → FIXED
Target Milestone: --- → Firefox 18
Comment 8•13 years ago
|
||
This code is sitting *before* GeckoAppShell.registerGlobalExceptionHandler();
Doesn't this mean that if it is buggy, it will never show up in crash reports?
Assignee | ||
Comment 9•13 years ago
|
||
(In reply to Gian-Carlo Pascutto (:gcp) from comment #8)
> This code is sitting *before* GeckoAppShell.registerGlobalExceptionHandler();
>
> Doesn't this mean that if it is buggy, it will never show up in crash
> reports?
Probably so. I was just trying to maintain the current flow, I guess.
Reporter | ||
Comment 10•13 years ago
|
||
(In reply to Gian-Carlo Pascutto (:gcp) from comment #8)
> This code is sitting *before* GeckoAppShell.registerGlobalExceptionHandler();
>
> Doesn't this mean that if it is buggy, it will never show up in crash
> reports?
Filed bug 799472 for this.
Updated•5 years ago
|
Product: Firefox for Android → Firefox for Android Graveyard
You need to log in
before you can comment on or make changes to this bug.
Description
•