Compared with my local runs, CI sees +1 runBlockingIncrement calls so seems to
take other code paths. As such, we search for a range of rather than a single
value. The main downside I can think of is this could make the test trickier to
debug but we can wait and see if that's a problem in practice before taking
action on it.
This test wasn't running in CI
https://github.com/mozilla-mobile/fenix/issues/20386 so we changed the
CI configuration so that it will. However, the test was then failing so
this is the revision that unignores the test.
I wonder if componentInit count is useful - it seems like it'd cause
more false positives than not - but I figure we can leave it in and see
how it goes.
* Update Android Components version to 93.0.20210901143120.
* For https://github.com/mozilla-mobile/fenix/issues/21043 - Integrate AC changes
* Fix breaking API changes of RestoreAction
Co-authored-by: Mugurell <Mugurell@users.noreply.github.com>
Co-authored-by: Christian Sadilek <christian.sadilek@gmail.com>
This is to investigate the intermittent mockk class
generation/loading issues. Since we can not reproduce
locally and the failures are intermittent they could
be caused by us running unit tests in parallel.
* Add telemetry probes for recent bookmarks on home screen. Tests for controller.
* Make the events into counters in the metrics ping
Update tests to reflect new metrics
Add data review link for new metrics
Mock new settings for startup metrics tests
Update metrics
Add test for recent bookmark glean events
* Recent bookmarks controller tests
Two new events are added:
- "inactive_tabs_expanded" for when the inactive tabs section is expanded
- "inactive_tabs_collapsed" for when the inactive tabs section is collapsed
For tracking when an inactive tab is opened / closed I've repurposed the
existing tabs tray telemetry (since the functionality uses the same code)
- tabs_tray.opened_existing_tab
- tabs_tray.closed_existing_tab
to support an extra "source" key indicating the feature from which a tab was
opened or closed. The current values for this new key are:
- "Tabs tray" for when a tab was opened/closed from tabs tray
- "Inactive tabs" for when a tab was openes/closed from the Inactive tabs
section of the tabs tray.
This means no code will be generated by Kotlin Android Extensions for caching
views and also for @Parcelize annotated classes.
As recommended in the official documentation
https://developer.android.com/topic/libraries/view-binding/migration#gradle
we need to switch on using `kotlinx.parcelize.Parcelize` instead of
`import kotlinx.android.parcel.Parcelize`
For https://github.com/mozilla-mobile/fenix/issues/17917 - Remove the `kotlin-android-extensions` plugin
This means no code will be generated by Kotlin Android Extensions for caching
views and also for @Parcelize annotated classes.
As recommended in the official documentation
https://developer.android.com/topic/libraries/view-binding/migration#gradle
we need to switch on using `kotlinx.parcelize.Parcelize` instead of
`import kotlinx.android.parcel.Parcelize`
This ensures that we do not attempt places initialization before
Megazord.init() finishes, and that touching BrowserStore does not
kick-off places initialization (due to the `cleanup` call being a
side-effect of creating metadata middleware, used as part of the
BrowserStore).
Glean initialization happens before initialize megazord, and it touches
core.store BrowserStore instance, kicking-off places initialization on
an IO thread (due to the cleanup call), which raced megazord
initialization on the main thread.
App init sequence is a bit of a mine-field, so this patch takes the easy
way out and doesn't attempt to re-order initialization sequence. Also,
initializing places as a side-effect of touching BrowserStore was also,
clearly, a bug.
This allows recording part of history metadata to ride the trains.
The UI features are still guarded by the secret settings flag (or,
enabled on debug builds).
The bug here was that we'd try to record `now - 0` as a viewTime delta.
This isn't just an obviously wrong value to record, but it will also
overflow our storage - we'll end up with a value on disk that doesn't
fit into an i32, but HistoryMetadata.total_view_time is i32 in our Rust
struct. Once that happens, reads that touch this bad row will result in
an overflow and a crash.
For details on the root cause, see the commit. We replaced the similar
proguard rules because:
- the key line was returning false instead of true
- the other line had the same outcome as the written code. I believe it
was a micro-optimization. Since perf seems fine without it, let's
remove it
I benchmarked this change on COLD MAIN first frame. We see an
improvement of 89ms:
- before: 1346ms
- after: 1257ms
