The Google IO app tends to serve as an example of Android best practices for the technology that was just released. The source code for the 2014 IO conference was just released so I decided to take a look. Within two minutes, I noticed something weird. I see something that appears to be a bug! This can't be.
The problem is on line 99 of the PartnersFragment. You can see the source code here: PartnersFragment.java. On that line, you will notice a call to getActivity(). The problem with that line is you shouldn't be calling getActivity() from onCreate(). That lifecycle method is too early! There is no guarantee that the activity will be available. Even if the activity is available, there is no guarantee it is a valid activity. In the worst case, this will result in passing in a null to the ImageLoader constructer, which might throw a NullPointerException.
You might be wondering, if this is a true bug, why hasn't it been reported already. The bug does not occur all the time. In theory, it should only appear sporadically. Most of the time, the activity gets created first, then the fragment gets created. This means during the happy path, the activity is always available to the fragment. If the activity and fragment need to be recreated from their Bundles, then their is no guarantee that the activity will be available to the fragment until the Fragment.onActivityCreated() method gets called. I have seen this experimentally and in my company's Android app. Here is a Stackoverflow question about the same topic. It even includes the diagram showing you the guaranteed ordering of activity and fragment callbacks.
For all I know, Google did something else that mitigates the bug so that it never actually happens. It could be safe to pass in a null into the ImageLoader constructor. I haven't reviewed much other code yet. That doesn't mean it isn't a bug, though. If something does mitigate the crash, that is logic that could break, which would cause this bug to appear. Also, if this is supposed to serve as example code, then other developers will use Fragment.getActivity() inside of Fragment.onCreate(), not realizing that they could be crashing.
No comments:
Post a Comment
Note: Only a member of this blog may post a comment.