This bug was filed from the Socorro interface and is report bp-9fd0d253-3aa2-4652-ab90-086372140408. ============================================================= java.lang.IllegalStateException: Already registered Webapps:Preinstall at org.mozilla.gecko.EventDispatcher.registerListener(EventDispatcher.java:57) at org.mozilla.gecko.EventDispatcher.registerEventListener(EventDispatcher.java:132) at org.mozilla.gecko.webapp.EventListener.registerEventListener(EventListener.java:57) at org.mozilla.gecko.webapp.EventListener.registerEvents(EventListener.java:65) at org.mozilla.gecko.GeckoApp.onWindowFocusChanged(GeckoApp.java:2028) at com.android.internal.policy.impl.PhoneWindow$DecorView.onWindowFocusChanged(PhoneWindow.java:2462) at android.view.View.dispatchWindowFocusChanged(View.java:7578) at android.view.ViewGroup.dispatchWindowFocusChanged(ViewGroup.java:960) at android.view.ViewRootImpl$ViewRootHandler.handleMessage(ViewRootImpl.java:3115) at android.os.Handler.dispatchMessage(Handler.java:99) at android.os.Looper.loop(Looper.java:137) at android.app.ActivityThread.main(ActivityThread.java:5103) at java.lang.reflect.Method.invokeNative(Native Method) at java.lang.reflect.Method.invoke(Method.java:525) at com.android.internal.os.ZygoteInit$MethodAndArgsCaller.run(ZygoteInit.java:737) at com.android.internal.os.ZygoteInit.main(ZygoteInit.java:553) at dalvik.system.NativeStart.main(Native Method)

Reproducible (Nightly 04/15, Nexus 5 (Android 4.4.2). Steps: i) Installed Twitter ii) Launched Twitter from the 'PackageManager' installation complete dialog iii) Re-launched Twitter again from my application listing on my device

About ~15 crash reports as of today (04/21)

I can reliably reproduce this, however it only causes an exception in the logs, rather than crashing the app or Fennec.

My STR (which unlike Comment 1, don't require you to reinstall Twitter each time): 0. (Have twitter Open Web App installed) 1. Launch Twitter Open Web App (e.g. via launcher on home screen) 2. In android Settings | Apps | Downloaded | Twitter, hit "Force Stop" button. 3. Re-launch Twitter Open Web App. ACTUAL RESULTS: Crash report dialog appears, on startup. bp-ad2e03f4-fd4e-4cdb-a7aa-5a9b92140424 bp-02d06f8f-14a5-4c76-a2cd-d0baa2140424

This happened and I got lolcat during it: http://pastebin.mozilla.org/4936485

:ozten - what device/OS version are you using? And is fennec hard crashing for you?

Nexus 4 Android 4.4.2 Yes, these are my crashes: https://crash-stats.mozilla.com/report/index/278fae3a-16c5-48f1-adc3-3216a2140423 https://crash-stats.mozilla.com/report/index/eaf8379c-3565-466e-95ef-0de342140423 https://crash-stats.mozilla.com/report/index/cdb18f81-e9ea-4c03-9286-0544f2140424

The issue is that we are registering this event in InstallHelper.java and also in EventListener.java. It seems that this error is preventing the 'Webapps:AutoInstall' message from being broadcast from within the install function in InstallHelper. Commenting out the 'registerGeckoListener();' line in the install function in InstallHelper allows this event to be broadcast.

Comment on attachment 8421065 [details] [diff] [review] Fixed the stacktrace, although unable to recreate the crash. Review of attachment 8421065 [details] [diff] [review]: ----------------------------------------------------------------- I don't have feedback to provide about the core of the patch. It looks reasonable, and it resembles the way other GeckoEventListeners work, but I'm not familiar enough with that class to know why our current approach is buggy. So it'd be useful to get some context, i.e. an explanation of why we implemented it that way originally and why this change is correct. ::: mobile/android/base/GeckoApp.java @@ +396,4 @@ > onPreparePanel(featureId, mMenuPanel, mMenu); > } > > + return mMenuPanel; Avoid these unrelated whitespace changes, which makes the patch harder to review and pollutes history/blame. @@ +2253,4 @@ > } > } > > + @Override Avoid these unrelated (and unnecessary) changes. ::: mobile/android/base/webapp/WebappImpl.java @@ +260,4 @@ > // Don't show the titlebar for about:blank, which we load > // into the initial tab we create while waiting for the app > // to load. > + if (urlString == null || urlString.equals("about:blank")) { Why remove the titlebar when urlString is null? At the very least, the comment here needs updating to explain why this now removes the titlebar under that additional circumstance.

I think Myk covered everything I wanted to say.

Grr. Whitespaces/@Overrides. I thought I'd removed them. Approach has been to register one EventListener per GeckoApp. Previously, this was using a static EventListener, which was unclear as to which GeckApp the EventListener was communicating with, and caused this problem. The problem arose when multiple GeckoApp instances were being initialized, but only one EventListener was being used. The possible solutions to this were: 1. move the initialization check in to EventListener, so one EventListener could be used for multiple GeckoApps. This is the smallest change to make. 2. Make EventListener non-static, and initialize it at the same time as GeckoApp. 3. Investigate why two GeckoApp instances are being initialized. The STR would indicate that changing launch flags in Dispatcher may fix the problem, but I couldn't find a combination that would work. My suspicion has been that using 3. would fix the bug once and for all, but in the absence of being able to reproduce, fixing the stack trace moves this problem forward. > // Don't show the titlebar for about:blank, which we load > // into the initial tab we create while waiting for the app > // to load. > + if (urlString == null || urlString.equals("about:blank")) { urlString != null is assumed fairly soon after this line, but null urlString isn't otherwise being dealt with.

Comment on attachment 8421065 [details] [diff] [review] Fixed the stacktrace, although unable to recreate the crash. Let's get this in Nightly and see if it fixes the issues, since other can reproduce it. r+ with Myk and Martyn feedback addressed

Comment on attachment 8424880 [details] [diff] [review] Addressed nits from mfinkle's r+ jhugman: it'd be better to leave the "review" flag off this patch, so the sheriff who commits it doesn't think it's r=jhugman. Thus I'm removing the flag and will also un-obsolete your previous patch, so it shows up in the list of attachments with mfinkle's review, and the sheriff can see that mfinkle is the peer who gave the review. Also, note that your username in this patch is "jhugman <jhugman@jhugman-10147.local>", whereas it's been "James Hugman <jhugman@mozilla.com>" in the past. Presumably the latter is the correct value, in which case you might want to upload a new patch that corrects it!

Comment on attachment 8421065 [details] [diff] [review] Fixed the stacktrace, although unable to recreate the crash. Please leave obsolete patches marked as such. :)

James: is this problem present on Aurora and/or Beta as well; and if so, can you request uplift for it? It's a pretty bad problem, so it's worth uplifting it if it affects those branches.

Comment on attachment 8424880 [details] [diff] [review] Addressed nits from mfinkle's r+ [Approval Request Comment] Bug caused by (feature/regressing bug #): User impact if declined: Under fairly common webapp circumstances, Gecko actualy crashes. Testing completed (on m-c, etc.): On m-c. Risk to taking this patch (and alternatives if risky): None. String or IDL/UUID changes made by this patch: None.

Needs a branch patch for Aurora uplift.

Comment on attachment 8424880 [details] [diff] [review] Addressed nits from mfinkle's r+ Removing the approval per jhugman. He'll re-request once he has a branch patch.

[Approval Request Comment] Bug caused by (feature/regressing bug #): User impact if declined: Testing completed (on m-c, etc.): On M-C for a week. Risk to taking this patch (and alternatives if risky): String or IDL/UUID changes made by this patch: This was previously approved, but without a branch patch. This patch is that branch patch.

Verified as fixed using the steps from comment 1 and comment 4. Builds: Firefox for Android 31 Beta 6; Firefox for Android 32.0a1 (2014-07-01); Devices: Nexus 4 (Android 4.4.2); Asus Transformer Pad TF300T (Android 4.2.1).