Avoid loading collection if finishing() == true

[mikehardy]

This was proposed as a fix for #4987 but that issue is being fixed
in a defense-in-depth fashion as this is a threading change and
harder to reason about

This PR is a result of discussion here - 96ed621#r30675818

This was the reasoning for the change:

1. Previewer.onStart() (and other AnkiActivity subclasses, in certain conditions) immediately realizes it inherited an invalid state in it's intent and calls finish()/finishWithoutAnimation(). Activity.onFinishing() is now true but Lifecycle.getCurrentState().isAtLeast(CREATED) is still true
2. You must call super.onStart() though, or the framework throws an exception - it is a hard requirement, so you can't call return you must call super.onStart()
   AnkiActivity.onStart() goes off UI thread to load the collection
3. Framework does not call onResume() or onCreateOptionsMenu() or anything, because Activity.onFinishing() is true - onCreate() executes fully everywhere but other than that you are tearing down. Many member variables are uninitialized, AND you were given an invalid state in onCreate(), the reason you called finish() in the first place
4. AnkiActivity.loadCollection() thread completes work and does callbacks
   Previewer and AbstractFlashCardViewer.onLoadCollection() start to do all sorts of UI updates but nothing is in a state to display and you had an invalid state - this is literally the Crash in AbstractFlashCardViewer.onCollectionLoaded() ArrayIndexOutOfBoundsException #4987 crash stack trace, fully reproducible - in current alpha just get the CardBrowser to an empty list state and pick Preview from options menu

So here instead of loading the collection in to an Activity that signaled they were finishing (implying it's time to get to work even though we said we don't want to work), we demur and e.g. Previewer safely goes away. I could have protected Previewer.onLoadCollection() and AbstractFlashCardViewer.onLoadCollection() so they were safe even if finish() was called, but there may be things like this in any AnkiActivity subclass and since I think the general idea of "isFinishing()?, okay, forget it" seemed correct it would protect everyone at once

[timrae]

I feel our Activity startup logic is a bit hard to follow here... What do you think about the following alternative?

• move the call to startLoadingCollection() to the concrete classes (Reviewer, Previewer)
• move super.onCreate() to the top of the onCreate() method (or as close to the top as makes sense)
• After finishWithoutAnimation() is called in Previewer we just return

The first two are just necessary because it's illegal to not call super.onCreate(), and we'd probably have undefined behavior if we returned without calling it.

[mikehardy]

These things take a while to reason about - I will come back to this one but I still have a little of the putting-out-fires feelings on crashes in 2.8/2.9 and making sure analytics is really solid for 2.9 so I'm going to let this sit for a moment so I can really think through your suggestion

[mikehardy]

Took a moment to look through this - if I'm understanding correctly the goal here is to have equivalent logic (don't load if we're finishing) but have it in a way that's more coherent (not as async-y and superclass-disjointed).

I like it, just re-pushed

[timrae]

Not really related to the asynchronicity, just a more readable control flow really