* For https://github.com/mozilla-mobile/android-components/issues/10560: Fixed bug causing external links to open tabs irrespective of the session's private status
* Updated findTabByUrl function names and comments for better clarity
* Fixed import detekt issue
* Refactored selectors test for better clarity
Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com>
Not using a write scope means we could have concurrent writes, leading
to sqlite throwing "database locked" exceptions.
All other writers here use the writeScope.
We wanted to introduce this action for testing purposes in client apps
that want to "turn back time" for a certain tab.
The difficulty is in ensuring clients do not misuse these actions, and
we thought of a few strategies:
- Option 1: Make a separate grouping called `DebugAction` and document
that these are special actions.
- Option 2: Using a middleware, we only allow changes to the store
depending on some dynmamic logic in the client app. This is a bit
complicated and requries the user to restart the app to add this
special middleware.
- Option 3: Add an annotation that requires the client to opt into using
the action.
In this patch, I decided to go with a combination of options 1 and 3
which gave us the right about of warning and flexibility.
With option 2, we were still required to add new actions to the store
and the middleware would not have prevented misuse in those cases.
Mainly, does two things:
- refactors SessionState.Source into a richer form (splitting sources
into Internal and External, where External ones track information
about originating package)
- adds persistence of External sources into tab session state; we don't
want to persist Internal sources as that was explicitly removed before
for causing various issues (e.g. UI behaving incorrectly after
restoring tabs with various internal sources set)
These follow a few more simple patterns I was able to find:
- the testDispatcher is not used anywhere else: remove it
- the testDispatcher is used to set a main dispatcher: this is redundant
to the test rule so remove it both the dispatcher & the main dispatcher
- the testDispatcher is actually used elsewhere: for simplicity, change
the reference to point at the MainCoroutineRule.
This is redundant to the built-in TestCoroutineDispatcher() method.
TestCoroutineDispatcher is also preferred because it has enhanced
testing functionality including defining a context-specific
`runBlockingTest` function, the ability to control `delay` timings, etc.
This new property helps with separating the current responsibilities of
lastMediaAccess such that after this:
- lastMediaAccess is only updated when media starts playing allowing clients to
order media tabs and find the first / last tab with in progress media.
- lastHadMediaSessionActive indicates whether a MediaSession should be active
for this tab and serves as a backup for lastMediaUrl for the situations where a
website might allow media to continue playing even when the users accesses
another page (with another URL) in that same HTML document.
Renamed "BrowserState.updateTabState" to a more appropriate
"BrowserState.updateTabOrCustomTabState" signaling that it can be used to
update any tab or custom tab.
Created two new "BrowserState.updateTabState" and
"BrowserState.updateCustomTabState" extension methods to allow updating the
state of either tabs or custom tabs depending on the properties needing update.
By this we are forcing the clients to choose what type of SessionState they
want updated and in so limiting the situations in which the old API would try
to update any SessionState and could throw a ClassCastException if in the
"update" lambda parameter callers would try to update a property not existing
in one SessionState implementation but available in other.
LastMediaAccessState contains both
- lastMediaAccess - timestamp for the last time media started playing
(which is reset to 0 when GV deactivates the MediaSession)
- lastMediaUrl - tab url when media started playing
By combining this two properties we'll know that a tab has in progress media
even when:
- the user navigates to another page in the same document
but media continues to play, MediaSession exists, lastMediaAccess is not reset,
- media starts playing in another tab
but the previous media tab has the same url as lastMediaUrl.
After media starting to play in a tab only if
- user navigated to another website and
- MediaSession is deactivated (happens when navigating to another website or
when media starts playing in another tab)
will we consider that this tab doesn't anymore have in progress media.
