Closed Bug 794642 Opened 7 years ago Closed 7 years ago

New fennec UpdateService service creates unnecessary components

Categories

(Firefox for Android :: General, defect)

All
Android
defect
Not set

Tracking

()

RESOLVED FIXED
Firefox 18
Tracking Status
firefox18 --- fixed

People

(Reporter: kats, Assigned: snorp)

References

Details

Attachments

(3 files)

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.
Attached patch Add missing elseSplinter Review
Also while I was poking around in UpdateService I noticed a (benign) missing else. See attached.
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.
(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.
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+
https://hg.mozilla.org/mozilla-central/rev/71e1ca0f2da3
Assignee: nobody → snorp
Status: NEW → RESOLVED
Closed: 7 years ago
Flags: in-testsuite-
Resolution: --- → FIXED
Target Milestone: --- → Firefox 18
This code is sitting *before* GeckoAppShell.registerGlobalExceptionHandler();

Doesn't this mean that if it is buggy, it will never show up in crash reports?
(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.
(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.
You need to log in before you can comment on or make changes to this bug.