Closed Bug 1009560 Opened 7 years ago Closed 7 years ago

Regression: Sharing an image crashes the browser @ at org.mozilla.gecko.GeckoAppShell.downloadImageForIntent(GeckoAppShell.java:2690)

Categories

(Firefox for Android Graveyard :: General, defect)

ARM
Android
defect
Not set
critical

Tracking

(firefox30+ verified, firefox31+ verified, firefox32 verified, fennec32+)

VERIFIED FIXED
Firefox 32
Tracking Status
firefox30 + verified
firefox31 + verified
firefox32 --- verified
fennec 32+ ---

People

(Reporter: aaronmt, Assigned: wesj)

References

Details

(Keywords: crash, regression, reproducible)

Crash Data

Attachments

(1 file)

W/dalvikvm( 5713): threadid=1: thread exiting with uncaught exception (group=0x41d13da0)
E/GeckoAppShell( 5713): >>> REPORTING UNCAUGHT EXCEPTION FROM THREAD 1 ("main")
E/GeckoAppShell( 5713): android.os.NetworkOnMainThreadException
E/GeckoAppShell( 5713): 	at android.os.StrictMode$AndroidBlockGuardPolicy.onNetwork(StrictMode.java:1166)
E/GeckoAppShell( 5713): 	at java.net.InetAddress.lookupHostByName(InetAddress.java:385)
E/GeckoAppShell( 5713): 	at java.net.InetAddress.getAllByNameImpl(InetAddress.java:236)
E/GeckoAppShell( 5713): 	at java.net.InetAddress.getAllByName(InetAddress.java:214)
E/GeckoAppShell( 5713): 	at com.android.okhttp.internal.Dns$1.getAllByName(Dns.java:28)
E/GeckoAppShell( 5713): 	at com.android.okhttp.internal.http.RouteSelector.resetNextInetSocketAddress(RouteSelector.java:216)
E/GeckoAppShell( 5713): 	at com.android.okhttp.internal.http.RouteSelector.next(RouteSelector.java:122)
E/GeckoAppShell( 5713): 	at com.android.okhttp.internal.http.HttpEngine.connect(HttpEngine.java:390)
E/GeckoAppShell( 5713): 	at com.android.okhttp.internal.http.HttpEngine.sendSocketRequest(HttpEngine.java:343)
E/GeckoAppShell( 5713): 	at com.android.okhttp.internal.http.HttpEngine.sendRequest(HttpEngine.java:289)
E/GeckoAppShell( 5713): 	at com.android.okhttp.internal.http.HttpURLConnectionImpl.execute(HttpURLConnectionImpl.java:345)
E/GeckoAppShell( 5713): 	at com.android.okhttp.internal.http.HttpURLConnectionImpl.getResponse(HttpURLConnectionImpl.java:296)
E/GeckoAppShell( 5713): 	at com.android.okhttp.internal.http.HttpURLConnectionImpl.getInputStream(HttpURLConnectionImpl.java:179)
E/GeckoAppShell( 5713): 	at java.net.URL.openStream(URL.java:470)
E/GeckoAppShell( 5713): 	at org.mozilla.gecko.GeckoAppShell.downloadImageForIntent(GeckoAppShell.java:2690)
E/GeckoAppShell( 5713): 	at org.mozilla.gecko.widget.GeckoActionProvider$Callbacks.chooseActivity(GeckoActionProvider.java:191)
E/GeckoAppShell( 5713): 	at org.mozilla.gecko.widget.GeckoActionProvider$Callbacks.access$100(GeckoActionProvider.java:182)
E/GeckoAppShell( 5713): 	at org.mozilla.gecko.widget.GeckoActionProvider.chooseActivity(GeckoActionProvider.java:176)
E/GeckoAppShell( 5713): 	at org.mozilla.gecko.prompts.PromptListAdapter$1$1.onIntentSelected$7a9ca511(PromptListAdapter.java:201)
E/GeckoAppShell( 5713): 	at org.mozilla.gecko.prompts.IntentChooserPrompt$1.onPromptFinished(IntentChooserPrompt.java:85)
E/GeckoAppShell( 5713): 	at org.mozilla.gecko.prompts.Prompt.notifyClosing(Prompt.java:424)
E/GeckoAppShell( 5713): 	at org.mozilla.gecko.prompts.Prompt.closeDialog(Prompt.java:409)
E/GeckoAppShell( 5713): 	at org.mozilla.gecko.prompts.Prompt.onClick(Prompt.java:200)
E/GeckoAppShell( 5713): 	at com.android.internal.app.AlertController$AlertParams$3.onItemClick(AlertController.java:954)
E/GeckoAppShell( 5713): 	at android.widget.AdapterView.performItemClick(AdapterView.java:308)
E/GeckoAppShell( 5713): 	at android.widget.AbsListView.performItemClick(AbsListView.java:1524)
E/GeckoAppShell( 5713): 	at android.widget.AbsListView$PerformClick.run(AbsListView.java:3531)
E/GeckoAppShell( 5713): 	at android.widget.AbsListView$3.run(AbsListView.java:4898)
E/GeckoAppShell( 5713): 	at android.os.Handler.handleCallback(Handler.java:733)
E/GeckoAppShell( 5713): 	at android.os.Handler.dispatchMessage(Handler.java:95)
E/GeckoAppShell( 5713): 	at android.os.Looper.loop(Looper.java:136)
E/GeckoAppShell( 5713): 	at android.app.ActivityThread.main(ActivityThread.java:5476)
E/GeckoAppShell( 5713): 	at java.lang.reflect.Method.invokeNative(Native Method)
E/GeckoAppShell( 5713): 	at java.lang.reflect.Method.invoke(Method.java:515)
E/GeckoAppShell( 5713): 	at com.android.internal.os.ZygoteInit$MethodAndArgsCaller.run(ZygoteInit.java:1268)
E/GeckoAppShell( 5713): 	at com.android.internal.os.ZygoteInit.main(ZygoteInit.java:1084)
E/GeckoAppShell( 5713): 	at dalvik.system.NativeStart.main(Native Method)
My crash report:

bp-06a18e34-79ce-4805-9f15-23e122140513

Signature:

[@ android.os.NetworkOnMainThreadException: at android.os.StrictMode$AndroidBlockGuardPolicy.onNetwork(StrictMode.java)]
Crash Signature: [@ android.os.NetworkOnMainThreadException: at android.os.StrictMode$AndroidBlockGuardPolicy.onNetwork(StrictMode.java)]
As per bug 990642 comment 25, wesj did you get a chance to look at this?
Assignee: nobody → wjohnston
tracking-fennec: ? → 32+
Attached patch Patch v1Splinter Review
I'm not able to reproduce this crash on my phone, but I do see some strict mode warnings. This fires the intent (and downloads the image) from a background thread.

I purposefully left notifying the target that something was selected on the callers thread. That's used to update the menu UI. It should happen quickly. If the intent firing fails, it will still be called I think that's likely OK, and not much different than the previous behaviour (which also doesn't care if you successfully shared, just cares that you tried to share with an app).

There's a build here:

http://people.mozilla.com/~wjohnston/shareCrash.apk

can QA test it?
Attachment #8425628 - Flags: review?(michael.l.comella)
Flags: needinfo?(aaron.train)
I can confirm that sharing images is indeed working correctly in the try build and that there's no crashes.
Same here, WFM. I just was sharing the main Google image header to Google+ as a test.
Flags: needinfo?(aaron.train)
Comment on attachment 8425628 [details] [diff] [review]
Patch v1

Review of attachment 8425628 [details] [diff] [review]:
-----------------------------------------------------------------

Seems to provide the same functionality, but puts networking on the background thread, so lgtm.

::: mobile/android/base/widget/GeckoActionProvider.java
@@ +189,5 @@
> +                // This may cause a download to happen. Make sure we're on the background thread.
> +                ThreadUtils.postToBackgroundThread(new Runnable() {
> +                    @Override
> +                    public void run() {
> +                        // Share image syncrhonously downloads the image before sharing it.

nit: synchronously
Attachment #8425628 - Flags: review?(michael.l.comella) → review+
https://hg.mozilla.org/mozilla-central/rev/501b00809f10
Status: NEW → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 32
Verified fixed on:
Device: Samsung Galaxy Nexus (Android 4.2)
Build: Firefox for Android 32.0a1 (2014-05-22)
Comment on attachment 8425628 [details] [diff] [review]
Patch v1

[Approval Request Comment]
Bug caused by (feature/regressing bug #): 990642
User impact if declined: Crash sharing images on newer devices because of network on the main thread.
Risk to taking this patch (and alternatives if risky): Pretty low risk. Just pushes this onto a background thread like it previously was. Alternative to this is to hold the share patches up I think. We're regressed from 29 at that point, but still have feature parity with other browsers.
String or IDL/UUID changes made by this patch: None.
Attachment #8425628 - Flags: approval-mozilla-beta?
Attachment #8425628 - Flags: approval-mozilla-aurora?
Comment on attachment 8425628 [details] [diff] [review]
Patch v1

Since this is verified and we have two mobile betas left (so we can backout if we find we need to) - let's take it and not regress from 29.
Attachment #8425628 - Flags: approval-mozilla-beta?
Attachment #8425628 - Flags: approval-mozilla-beta+
Attachment #8425628 - Flags: approval-mozilla-aurora?
Attachment #8425628 - Flags: approval-mozilla-aurora+
Comment on attachment 8425628 [details] [diff] [review]
Patch v1

Hold the phone (pun intended).  This is marked unaffected on 31 and no status on 30 - can I get someone to confirm this presents on those branches?
Attachment #8425628 - Flags: approval-mozilla-beta?
Attachment #8425628 - Flags: approval-mozilla-beta+
Attachment #8425628 - Flags: approval-mozilla-aurora?
Attachment #8425628 - Flags: approval-mozilla-aurora+
Comment on attachment 8425628 [details] [diff] [review]
Patch v1

Nevermind. It's Friday.  I see that bug 990642 is approved to land all the way up to beta and this is needed in that case. I will update the flags accordingly.
Attachment #8425628 - Flags: approval-mozilla-beta?
Attachment #8425628 - Flags: approval-mozilla-beta+
Attachment #8425628 - Flags: approval-mozilla-aurora?
Attachment #8425628 - Flags: approval-mozilla-aurora+
Marking 30/31 affected for once bug 990642 lands to branches, they will be.
Sharing images is working correctly on Aurora 31.0a2 (2014-05-28) and Firefox 30 Beta 8
Status: RESOLVED → VERIFIED
Product: Firefox for Android → Firefox for Android Graveyard
You need to log in before you can comment on or make changes to this bug.