Moved the deletion to a CoroutineScope(IO) to decouple from fragment lifecycle
Used local val tempStorage to keep reference to bookmarkStorage of outside class
Ran deletions with awaitAll to avoid concurrency issues on storage
This was added to the state object as a top level param because it could reasonably coexist with any value of `tree` or `mode`. Even if we don't now, we may someday want to display a loading indicator while also showing cached bookmarks.
For now, we set isLoading to false whenever we receive any bookmarks
* For #4396 - Rename BookmarkInteractor methods
Following the naming model used in other Interactors this too will use reactive
method names in the form of "on..." instead of the previous imperative model.
Kept the imperative naming model for the methods from `SelectionInteractor` as
they are a new addition and I'm not sure about the future direction.
* For #4396 - Add a BookmarkController
It abstracts the Fragment behavior in a contract through which various
Interactors can inform about the specific View changes and can ask for
modifications in their container Fragment.
This contract and it's implementation - `DefaultBookmarkController` are the
result of extracting the container Fragment's business logic from
`BookmarkFragmentInteractor` in it's own standalone component.
* For #4396 - Refactored Bookmark related tests
Added a new `BookmarkControllerTest` tests class which complements the new
`BookmarkController` to ensure that it properly operates on `BookmarkFragment`
Also refactored the existing `BookmarkFragmentInteractorTest` to accommodate
`BookmarkFragmentInteractor`'s now more specialized behavior.
With this we can remove the `whenStarted` workaround. This new API
internally creates (and uses) a scope bound to the fragment's view and
will not invoked the lambda (send state updates) when the view is
detached or the fragment's lifecylce is stopped or destroyed.
This patch fixes a few issues:
- it was an extension on a CoroutineScope, but that was quite misleading
since the Main dispatcher would be always used regardless of what dispatcher
the owning CoroutineScope was configured with.
- timing was reliant on exact value of the undocumented Snackbar.LENGTH_LONG duration
- coroutine cancellation relied on cooperation of the 'operation' suspend function,
which we can't depend on
New 'allowUndo' fully controls its timing, doesn't imply a dispatcher to its consumers,
and doesn't rely on cooperation of passed-in suspend blocks for cancellation to work.
This is mostly necessary when we're running stuff in a coroutine, so the patch likely goes
overboard a bit with the nullability checks... but it shouldn't hurt.