Work around dead activity issue in AppDetails
It seems like install() sometimes runs when the AppDetails activity is
finished or finishing. This results in the windows (dialogs) failing to
show, and a BadTokenException to fire:
android.view.WindowManager$BadTokenException: Unable to add window -- token android.os.BinderProxy@d6e3570 is not valid; is your activity running?
This seems to be the culprit:
at org.fdroid.fdroid.AppDetails.install(AppDetails.java:840)
at org.fdroid.fdroid.AppDetails$AppDetailsListFragment.install(AppDetails.java:1657)
at org.fdroid.fdroid.AppDetails$AppDetailsListFragment.onListItemClick(AppDetails.java:1721)
Apparently, you can check whether an activity/context is being finished:
https://stackoverflow.com/questions/7811993/error-binderproxy45d459c0-is-not-valid-is-your-activity-running
I cannot reproduce this issue, thus can't say whether this fixes it or
not. Either way, it can't hurt to try. This can be reverted if we see
ACRA reports of this in the future, and the issue reopened.
Fixes#565.
See merge request !204
It seems like install() sometimes runs when the AppDetails activity is
finished or finishing. This results in the windows (dialogs) failing to
show, and a BadTokenException to fire:
android.view.WindowManager$BadTokenException: Unable to add window -- token android.os.BinderProxy@d6e3570 is not valid; is your activity running?
This seems to be the culprit:
at org.fdroid.fdroid.AppDetails.install(AppDetails.java:840)
at org.fdroid.fdroid.AppDetails$AppDetailsListFragment.install(AppDetails.java:1657)
at org.fdroid.fdroid.AppDetails$AppDetailsListFragment.onListItemClick(AppDetails.java:1721)
Apparently, you can check whether an activity/context is being finished:
https://stackoverflow.com/questions/7811993/error-binderproxy45d459c0-is-not-valid-is-your-activity-running
I cannot reproduce this issue, thus can't say whether this fixes it or
not. Either way, it can't hurt to try. This can be reverted if we see
ACRA reports of this in the future, and the issue reopened.
Fixes#565.
Fix 555 content provider invalid uri
Was not correctly encoding "/" characters when searching. This caused the Uri used by the Content Providers to include a slash, which makes it look like a separate segment of the path which was wrong. Now correctly encodes "/" characters. Also noticed one other place incorrectly encoding characters, where they would've been double encoded when added as query parameters to a Uri.
See merge request !203
Refactor swap "peer finders" to use ReactiveX
*NOTE: This includes the commit specified by !197.*
In the old code, there is a _lot_ of procedual style "Is this peer finder running, if so, do this". In addition, the choice to do things on background threads or not is a little ad-hoc. Finally, the `SwapService` needs to know about both bluetooth and wifi peer finders, whereas really they are both only there to emit "Peers", regardless of the type.
As such, some improvements in this change are:
* The choice to run peer finding on a background thread is made once, at a higher level when starting the peer finder.
* No longer does the UI code ask "Am I searching for peers". It instead waits to be told whether it is or isn't.
* The addition of new types of peers in the future is the job of the Peer finder itself. It quietly aggregates all of the Peer Finders it knows about into a single observable that emits different types of peers.
This code doesn't fix any particular issue, but rather it is about making the entire swap workflow easier to reason about. I plan on migrating more of this workflow to this functional style in the future, and hopefully that will have benefits in terms of stability and code understanding.
See merge request !198
I misread the documentation when first using the `appendEncodedPath` method,
because it expects the path to already be encoded. This causes a bug because
if you search for a '/'. The result is a malformed URI that has the path
'/search//' instead of '/search/%2F'.
Using `appendPath` will always encode the string given to it, which is desirable.
Also check for empty strings, and return a URI that gives all apps. This was
not strictly neccesary, because the code which invokes it checks for empty
strings, but if somewhere else in the future starts to use this code, they
would've had to know to check for empty strings first.
Fixes#555.
Put null check around access of `R.id.header` fragment.
Please note I haven't reproduced the specific problem. Also, the stack
traces being reported are only marginally informative, because they are
in response to a content providers firing events, and thus don't have
any context about when or where the event was fired from.
However, my looking at the code seems to indicate that this will prevent
NPE when the Activity is no longer visible but an app is finished
installing. Also, the view should still update correctly on resuming the
Activity because the `onResumeFragments()` methods will be invoked
which invokes the `refreshHeaders()` method.
Fixes#286.
See merge request !202
Please note I haven't reproduced the specific problem. Also, the stack
traces being reported are only marginally informative, because they are
in response to a content providers firing events, and thus don't have
any context about when or where the event was fired from.
However, my looking at the code seems to indicate that this will prevent
NPE when the Activity is no longer visible but an app is finished
installing. Also, the view should still update correctly on resuming the
Activity because the `onResumeFragments()` methods will be invoked
which invokes the `refreshHeaders()` method.
Fixes#286.
The benefits of this are as follows:
No longer need to worry about how many types of `Peer`s exist.
There is a single publicly accessible `PeerFinder` which aggregates
the results of both the Bluetooth and Bonjour peer finders. In the
future if another is added, the consumer of the peer finder
(i.e. `StartSwapView`) doesn't need to be aware of this. Neither does
the `SwapService` or `SwapActivity` or any other code.
Never ask "Are we searching" but instead receive push notifications
from the peer finder when it stops searching.
Don't worry about receiving the same peer multiple times, it will
automatically get filtered out.
Less concern about doing things in `AsyncTasks` (and knowing what to
do in an `AsyncTask`). The RXJava + RXAndroid libraries deal with this
by allowing the client consuming the `PeerFinder` to specify which
thread to perform the background task on, and also that the found
`Peer`s should be emitted on the UI thread.
In the future, can play with caching the results of a particular
sequence of found peers. However right now using the `Observable.cache()`
method means we can no longer unsubscribe from the peer finders
and thus they run longer than they need to when we move on from
the initial swap screen.
Changing the search query is quite an expensive operation, so this does some rudimentary
checking to see if the two queries are meaningfully different. At present, it trims the
strings and does a case insensitive comparison.
The query is eventually exploded based on whitespace, so leading and trailing white
space is not important. Also, sqlite `LIKE` clauses are case insensitive, so case
is unimportant. Having said that, I'm not sure how someone will be able to change
the queries case without first deleting and then adding characters (thus inducing
meaningfull changse).
This means that instead of building invalid SQL such as `WHERE (() OR ())` it
will build `WHERE((1) OR (1))` which, while non-optimal, is at least valid.
Fixes issue #560.
The fix for the database locking bug was to have a singleton
`DBHelper` instance. This breaks tests because multiple tests
share the same database. The solution is to:
* Hack together a static method which clears the singleton,
then invoke it in the `setUp()` method for relevant test cases.
* Ensure the mock context provided to the providers during
the tests is able to provide a context via `getApplicationContext()`.
Without this, the mock context throws an `UnsupportedOperationException`
when invoking this method.
Using whatever `Activity` as the `Context` used to construct the first content
provider means that it will be help onto in memory until the application is GC'ed.
Previously, all of the various subclasses of FDroidProvider would create
their own database open helper in their respective `onCreate()` methods.
This seemed to be the cause of the multiple database locked exceptions.
Various online articles/SO posts/etc helped come to this conclusion:
* http://stackoverflow.com/a/3689883
* http://stackoverflow.com/a/8888606
* https://web.archive.org/web/20150709074733/http://www.dmytrodanylyk.com/pages/blog/concurrent-database.html
This should fix#265.
In the process, also did away with the two `read()` and `write()` methods
that returned a "readable" and "writeable" database respectively. It turns
out that it doesn't quite do what I originally thought. There is not much
benefit to specifying to the database helper that you want a readable/writeable
database. In fact, it is often the case that a call to `read()` would most
likely have returned the same instance that is returned by `write()`. The
semantics of them were therefore broken, and they've been replaced with
`db()`.
This is what used to happen before the recent refactor to the search UI.
Finishing the main activity means that it never comes back to handle the
"View this app" intent again. Instead, the main Activity just becomes an
entry point for redirecting the UI to the correct place.
Incidentally, I don't particularly like the current solution even though
it should work, and hope to come up with a better one in the future. The
reason is because the behaviour of some methods (`onCreate()` +
`handleIntent()`) is what defines the behaviour of other methods
(`checkForAddRepoIntent()`). This means it is hard to reason about the
state of the activity at any point in time. Developers need to be careful
when making changes to the `onCreate()` method because modifying it has
unintended consequences. That is what caused the problem in issue #541.
Fixes#541.
Before, both the swap activity and the fdroid activity would use
the "handled" key to check if an intent had been handled. This caused
problems, because by the time we got to handling the add repo intent,
it had already been "handled" by the swap activity so we bailed.
Before, it only happened in onResume(). Now, it happens in onNewIntent()
too (but ensures to only handle the same intent once).
Note the onResume() only ever handles the original intent, not the new
intent.
Fixes#524.
The approach to creating fragments in the constructor of the `AppListFragmentPagerAdapter`
was incorrect. Fragments are a special kind of magic, so this commit uses the approach
documented at http://stackoverflow.com/a/15261142 to make sure that the correct
fragment instances are always retrieved.
Fixes#521.
Use custom layout for ACRA report dialog, not default one.
Styling the default dialog was difficult and it doesn't obey some
of the guidelines provided by the Android design docs:
https://www.google.com/design/spec/components/dialogs.html#dialogs-specs
(see "Content Guidelines")
This change introduces a custom dialog extending the base ACRA reporting
activity. Specifically, it introduces a padding of 24dp around the dialog
contents.
I couldn't find design specs on the specific spacing between different elements _within_ the dialog, so I used the same spacing of 20dp suggested to use between the title and the main body of the dialog.
At the very least, this change should make it easier to update styles in the future if somebody suggests improvements to the layout of the dialog.
### Before
### After
See merge request !186
Partially revert f2212e33. Make some constructors public again.
There is some code in the Android SDK using reflection to find the constructor
of one of our custom views. As such, a previous change in f2212e33 broke this
code because it made constructors package local. This partially reverts the bit
of f2212e33 which pertains to the constructors accessed using reflection.
Fixes#519.
See merge request !187
There is some code in the Android SDK using reflection to find the constructor
of one of our custom views. As such, a previous change in f2212e33 broke this
code because it made constructors package local. This partially reverts the bit
of f2212e33 which pertains to the constructors accessed using reflection.
Styling the default dialog was difficult and it doesn't obey some
of the guidelines provided by the Android design docs:
https://www.google.com/design/spec/components/dialogs.html#dialogs-specs
(see "Content Guidelines")
This change introduces a custom dialog extending the base ACRA reporting
activity. Specifically, it introduces a padding of 24dp around the dialog
contents.
