Closed Bug 1031429 Opened 5 years ago Closed 5 years ago

crash in java.lang.NullPointerException: at org.mozilla.gecko.FilePickerResultHandler$FileLoaderCallbacks.onTabChanged(FilePickerResultHandler.java)

Categories

(Firefox for Android :: General, defect, critical)

31 Branch
All
Android
defect
Not set
critical

Tracking

()

RESOLVED FIXED
Firefox 38
Tracking Status
firefox31 --- affected
fennec + ---

People

(Reporter: aaronmt, Assigned: swaroop.rao, Mentored)

References

Details

(Keywords: crash, reproducible, Whiteboard: [lang=java][good first bug])

Crash Data

Attachments

(1 file)

This bug was filed from the Socorro interface and is 
report bp-d19344c9-964e-498c-bad7-04cd62140626.
=============================================================

java.lang.NullPointerException
	at org.mozilla.gecko.FilePickerResultHandler$FileLoaderCallbacks.onTabChanged(FilePickerResultHandler.java:248)
	at org.mozilla.gecko.Tabs$5.run(Tabs.java:596)
	at android.os.Handler.handleCallback(Handler.java:733)
	at android.os.Handler.dispatchMessage(Handler.java:95)
	at android.os.Looper.loop(Looper.java:146)
	at android.app.ActivityThread.main(ActivityThread.java:5602)
	at java.lang.reflect.Method.invokeNative(Native Method)
	at java.lang.reflect.Method.invoke(Method.java:515)
	at com.android.internal.os.ZygoteInit$MethodAndArgsCaller.run(ZygoteInit.java:1283)
	at com.android.internal.os.ZygoteInit.main(ZygoteInit.java:1099)
	at dalvik.system.NativeStart.main(Native Method)
I can reproduce this crash with the following steps:
1. Load: http://validator.w3.org/#validate_by_upload
2. Click Browse
3. Select file
Device: Samsung S5 (Android 4.4)
Build: Firefox for Android 36.0a1 (2014-10-19)
tracking-fennec: --- → ?
Keywords: reproducible
This is now exposed again via bug https://bugzilla.mozilla.org/show_bug.cgi?id=1085158
See Also: → 1085158
This is now exposed again via bug https://bugzilla.mozilla.org/show_bug.cgi?id=1085158
Duplicate of this bug: 1080709
Mentor: bnicholson
tracking-fennec: ? → +
Whiteboard: [lang=java]
On the face of it, this seems like a simple enough fix. There appear to be only two places where an NPE could occur (viz., when an object's attribute is being accessed). I can do a NULL check in those two places and have a patch ready. If there's more to it than that, let me know and I'd be happy to take this up. I have pulled the latest code base into my local repository and updated my working set. I have a build in progress which should complete in the next hour. Once the build has finished, I'll try to reproduce the issue in my Nexus 5.
Flags: needinfo?(bnicholson)
The NPE is happening here: http://mxr.mozilla.org/mozilla-central/source/mobile/android/base/FilePickerResultHandler.java#253

The tab can be null for the TabEvents.RESTORED event. Since we're calling "tab.getId()" before checking the event type, we can end up here for a RESTORED event, resulting in a NPE. Adding a simple "tab == null" condition to this line should be all that's necessary to fix this.

Swaroop, are you still interested in taking this?
Flags: needinfo?(bnicholson) → needinfo?(swaroop.rao)
Whiteboard: [lang=java] → [lang=java][good first bug]
Yes, please. I have the fix ready, so I will try and upload a patch later tonight (Indian Standard Time).
Flags: needinfo?(swaroop.rao)
I'm attaching a proposed fix for this defect. Please review and let me know if it looks good.
Attachment #8533795 - Flags: review?(bnicholson)
Comment on attachment 8533795 [details] [diff] [review]
bug-1031429.patch

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

Looks good. Thanks!
Attachment #8533795 - Flags: review?(bnicholson) → review+
Assignee: nobody → swaroop.rao
Swaroop, can you push this to try server? After that, you can set the checkin-needed flag and the sheriffs will land this for you.
Yes, I don't have an account on try, so I just finished reviewing the steps to apply for one. Will do that in the next few minutes.
I have filed a bug to request requesting a repository account (Level 1) and attached my public key to it. I have also sent an email to marcia@mozilla.com with the completed Committers' Agreement.
The bug I filed to request a repository account is 1110178 (https://bugzilla.mozilla.org/show_bug.cgi?id=1110178).
Brian, can you please act as a voucher for my request for repository access?
Flags: needinfo?(bnicholson)
Yep, done.
Flags: needinfo?(bnicholson)
(In reply to Brian Nicholson (:bnicholson) (on PTO through December 26) from comment #10)
> Swaroop, can you push this to try server? After that, you can set the
> checkin-needed flag and the sheriffs will land this for you.

Brian, I was finally able to push the change to try and ran a build/tests. I didn't know what tests/OSes to select so I selected a bunch of Android versions. "Android 4.2 x86" passed but "Android 4.0 API10" was busted. Can I still go ahead and set the checkin-needed flag?

Merry Christmas and happy holidays!
Flags: needinfo?(bnicholson)
Swaroop, do you have a link to the Try push?
Flags: needinfo?(swaroop.rao)
Richard, if you mean a link to the results, here's the link that I got in an email.

https://treeherder.mozilla.org/#/jobs?repo=try&revision=a3a0019d1b80

The other link I received in the same email is: https://ftp-ssl.mozilla.org/pub/mozilla.org/firefox/try-builds/swaroop.rao@gmail.com-a3a0019d1b80
Flags: needinfo?(swaroop.rao)
You probably want a Try commit message like:

try: -b o -p android-api-9,android-api-11 -u mochitest-1,robocop-1,robocop-2,robocop-3,robocop-4,robocop-5 -t none
Did that. The try page for this is at:
https://treeherder.mozilla.org/#/jobs?repo=try&revision=d662484b71fb

The error in the failure log is always the same ("ERROR 404: Not Found"). I'm not sure what it is referring to or how I should go about fixing it (if necessary).
Flags: needinfo?(bnicholson)
Pinging Brian / Richard for inputs regarding next steps, if any...
Flags: needinfo?(rnewman)
Flags: needinfo?(bnicholson)
Swaroop, it looks like the parent of your commit is f1f48ccb2d4e, which was back on Dec 8. There were some split APK-related TBPL changes on Dec 9: https://bugzilla.mozilla.org/show_bug.cgi?id=1073772#c82, meaning builds created from commits before that time will not work.

Please try pulling the latest code from mozilla-central, rebasing your changes onto that, then pushing it to try. Thanks!
Flags: needinfo?(swaroop.rao)
Flags: needinfo?(rnewman)
Flags: needinfo?(bnicholson)
Hey Brian,if swaroop is not working on this one can i do it it seems simple enough .This will be my first bug 

Thanks!!
Sorry, Brian. I had some problems with my VM which forced me to put this aside for a while. I will work on your changes tonight and let you know how it went. Ronak, if I have trouble with what Brian suggested, I will let you know and you can take it up.
Flags: needinfo?(swaroop.rao)
Thank you swaroop,still looking for an easy first bug
Brian, the situation is as follows. I had worked on two Fennec bugs a couple of months ago. One of them was approved after code reviews while the other one fizzled out because an alternate approach was proposed. For the bug that was approved, I sent out a patch file and didn't submit it to the try server. So, the changes I made for this defect are on top of the earlier changes. How do I go about getting the latest code and rebasing? I'm not very familiar with Mercurial, unfortunately. Basically, I need to get a fresh version of the code, apply the patch corresponding to one of the earlier bugs, discard the other bug and then apply the changes for the current bug. I would appreciate help either in the form of links to documentation or steps to follow. Thanks in advance.
Flags: needinfo?(bnicholson)
Brian,

Please review the results from try at https://treeherder.mozilla.org/#/jobs?repo=try&revision=989b9cef978d and let me know what the next step is. There were no failures this time.
Flags: needinfo?(bnicholson)
adding back a mistakenly cleared needinfo see comment 26 and 27
Flags: needinfo?(bnicholson)
(In reply to swaroop.rao from comment #27)
> Brian,
> 
> Please review the results from try at
> https://treeherder.mozilla.org/#/jobs?repo=try&revision=989b9cef978d and let
> me know what the next step is. There were no failures this time.

Thanks, that should be all that's needed! After uploading a patch that gets r+'d and submitting the patch to try, the last step is to add checkin-needed as a keyword on the bug so that a tree sheriff will land your changes. I'll go ahead and mark checkin-needed for you now.
Flags: needinfo?(bnicholson)
Keywords: checkin-needed
(In reply to ronak khandelwal from comment #25)
> Thank you swaroop,still looking for an easy first bug

Ronak, thanks for your interest in contributing! You can check out the list of unowned good first bugs here: http://www.joshmatthews.net/bugsahoy/?mobileandroid=1&unowned=1&simple=1. Also, feel free to ask any questions in #mobile on IRC -- you'll get answers much more quickly :)
https://hg.mozilla.org/integration/fx-team/rev/f526a385989a

Thanks for the patch, Swaroop! One request, please make sure that future patches include your email address as well in the commit information. Thanks!
Keywords: checkin-needed
Whiteboard: [lang=java][good first bug] → [lang=java][good first bug][fixed-in-fx-team]
https://hg.mozilla.org/mozilla-central/rev/f526a385989a
Status: NEW → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Whiteboard: [lang=java][good first bug][fixed-in-fx-team] → [lang=java][good first bug]
Target Milestone: --- → Firefox 38
You need to log in before you can comment on or make changes to this bug.