* 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 test seems to be hacking at the binding between Fenix and the
BrowserToolbar to simulate toolbar events passing to the Fenix
interactor.
This is rather clumsy test that relies on the magic working of mockk
instead of following a general unit testing strategy that would commonly
require the class to be re-written to allow for better testing instead.
It is far safer to remove this test since we are not guaranteeing
anything in it and instead we see intermittent failures that make us
lose more time.
So therefore.. 🔥
The expectation is that replacing `return` with `answers` will compute
the return value for the extension function again in order to avoid the
error, "no answer found for: Settings".
Theoretically, this should marginally decrease the duration of our unit
test suite. In my testing, for 1 iteration each (i.e. noise is very
possible), the duration changed from 9m 32s to 8m 21s – a 71s
improvement.
---
To identify tests that were running with robolectric that didn't need to
be, I removed the @RunWith(FenixRobo... from all relevant files:
sed -i '' "/@RunWith(FenixRobolectric/d" app/src/test/**/*.kt
I ran the tests and discovered which ones failed from the Classes tab of
the index.html test result file. Something like:
tests = document.querySelectorAll('table')[3].querySelectorAll('tr');
failureElements = tests.querySelectorAll('.failures');
// TODO: extract the test names
Then I copied these results to a text file and compared them to all the
files that had robolectric test runners to figure out which ones still
pass:
comm -1 -2 failures.txt changed_files.txt > robolectric_not_needed.txt
And undid the changes to the failing files:
for i in $(cat robolectric_not_needed.txt); do git checkout $i; done
Then I removed the import statements on those files:
for i in $(cut changed_files.txt); do sed -i '' "/import.*RunWith/d" $i; done
for i in $(cat changed_files.txt); do sed -i '' "/import.*RobolectricTestRunner/d" $i; done
* Navigate to home on toolbar click. Handle back press from search dialog
Update tests to show home behind search dialog. Remove unused test.
Jump back in show all button is clickable behind search dialog
Recently saved bookmarks show all button is clickable behind search dialog
* Add feature flag
* Past explorations show all button is clickable behind search dialog
Handle keyboard in controllers instead of viewholders. Update tests.
Allow collections to be visible behind search dialog
Dismiss keyboard and search dialog with navigateUp instead of just dismissing the keyboard
Verify navigateUp in tests
Adding ignore for flaky UI test
Only resize home behind search dialog
Add ignore for collection intermittent test
Cleanup
We use make the inactive tabs section of the tabstray collapsible in
this change, with a technical quirk: we want to make the "isExpanded"
state of the tabs stay for the lifetime of the app and not the tabs
tray, but this functionality does not exist.
In this patch, we're storing the UI state in a singleton class that
exists for the lifetime of the app, but a more concrete solution is to
use an AppStore that holds content like this, which we can land in a
future patch.
The errors that caused this to be @Ignored were addressed by a recent PR
landing on master (i.e. the one that renewed the probes this test is
testing).
* Remove references to preferences.open_links_in_private and preferences.private_search_suggestions in tests. These metrics have been expired and may be removed.
* Add ignores for performance metrics that have expired.
* Remove tabs_tray.cfr.dismiss and tabs_tray.cfr.go_to_settings telemetry probes.
* Remove metrics controller from signature and remove in tests
To remove the flash on refresh of the topsites list we have to use submitList, however using this too high up in the hierarchy of our listAdapters within listAdapters will cause children to refresh at once. The solution to this is to use submitList lower. Using it in TopSitesPagerAdapter.kt to update the TopSitesAdapter is the way to go. I've also had to use a dummy item for the "removed" Topsite ( with id = -1) so I can manually diff that before using submitList.
The reducer in this middleware assumes the SessionState is always a
TabSessionState which holds the lastMediaAccess. This is true for the
lastAccess long which is a persistent state.
The list of MediaSessionActions however can also come from Custom Tabs
which relies on a CustomTabSessionState.
For now, the temporary fix is to disable this feature by removing the
middleware and no longer adding the last accessed media to the
recent tabs list ("Jump back in") to avoid crashing users while we think
of a real fix.
* Title and button for home screen recently saved bookmarks section
Create bookmark item view with favicon and title
* View holders and interactors for recently saved bookmarks
Recent bookmark item view holder binding
Create adapter for recent bookmarks. Implement controller methods. Implement view holder bindings for items
Top level adapter for recent bookmarks section
Retrieve list of recent bookmarks on home
View holders and interactors for recently saved bookmarks
Recent bookmark item view holder binding
Create adapter for recent bookmarks. Implement controller methods. Implement view holder bindings for items
Top level adapter for recent bookmarks section
Retrieve list of recent bookmarks on home
Update list on app start and when bookmarks are added
View holders and interactors for recently saved bookmarks
Recent bookmark item view holder binding
Create adapter for recent bookmarks. Implement controller methods. Implement view holder bindings for items
Top level adapter for recent bookmarks section
Retrieve list of recent bookmarks on home
Update list on app start and when bookmarks are added
Make a use case for retrieving and updating the list of recently saved bookmarks
Add adapter items and define header viewholder binding
Use session interactor for header button clicks. Bind in the adapter
* Retrieve list of bookmarks asynchronously on home
Interactor and controller tests
Address review comments
Split up tests for recent bookmarks
Update to new interactors
Dark mode and light mode styles
Refactor bookmarks home stuff
* Add RecentBookmarksFeature to home
Move interactor to SessionControlInteractor
Clean up lint, styles, and dimens.
* Bookmarks use case tests for retrieving recently saved bookmarks. Linting.
* View holder tests
* Match ux to designs for colors, margins, and scrolling
* Clean up clean up
* Tests for the view bound feature
* Controller test
* Clean up: check state of store in feature tests; ellipsize textviews for bookmark item; remove unused attr; format
Co-authored-by: Jonathan Almeida <jalmeida@mozilla.com>
This new method allows a single source of truth for getting the up-to-date
account status without clients interesting in this needing to know what to
check for.
As in the previous version of tabs tray when tray is expanded while in portrait
it should have a 40dp offset and when the tray is expanded while in landscape
it should have a 0dp offset - effectively making it fullscreen.
These values were set in code, always overriding the xml set value so I
removed that xml declaration.
Setup a new TabSheetBehaviorManager with all the dependencies it needs to
set the initial tray's behavior.
This same manager will later be called to update behavior's properties.
- Renames `BrowserInteractor` to `DefaultBrowserTolbarInteractor`
- Renames `BrowserTooolbarViewInteractor` to `BrowserToolbarInteractor`
- Refactors `BrowserToolbarViewInteractor` interface from `BrowserToolbarView.kt` to `BrowserToolbarInteractor` as `BrowserToolbarInteractor`
We were crashing because we were specifying the wrong `CreditCardNumber` type when constructing the `UpdatableCreditCardFields`.
We only had the plaintext card number from the user input, but was placing it off as an encrypted card number, which resulted in the crash.
In our current MVI implementation the View Interactors are first called in
response to a direct user action and contain methods following the
"onXXHappened" naming scheme and then delegate other Interactors / Controllers
for specific actions.
Controllers contain the business logic for actually updating the app's state
and offer methods following the "handleXXAction" naming scheme.
We now have a BrowserTrayList.
- Renamed from `BaseBrowserTrayList`.
- Removed the abstract `configuration` and used a `lateinit var`
instead.
- Removed `NormalBrowserTrayList` and `PrivateBrowserTrayList`.
The main cause for this is that the `LifecycleProvider` needs to be set
to `State.RESUMED` to avoid the account manager's internal
`ObserverRegistry` from putting the UI observers into the paused state.
The rest of the changes is to rely the internal (safe) logic to
correctly sync and then update the tabs list.
The test failed with the rewrite of the code because it violates
one of our assumptions that only one Activity will be started. However,
since it doesn't rely on observed behavior and we made up the events,
it's value is questionable so it seems okay to remove, especially for
the gain of conciseness in the code.
Previously, to fix a memory leak, we were removing the adapter reference
entirely in order to have the `onDetachedFromRecyclerView` callback
invoked. This causes a side-effect where we can no longer reference the
adapter any more when we re-attach.
The simpler solution is to just invoke the needed callback directly
instead.
* For #16900: implement async navgraph inflation
For #16900: removed nav graph from xml
For #16900: inflate navGraph programatically
For #16900: Made NavGraph inflation asynchronous
For #16900: Changed to block with runBlocking
For #16900: Refactored blocking call into a function
For 16900: NavGraph inflation is now async
We now attach the nav graph (or check if its attached) on every nav call ( an extension function for NavController).
This is done by checking the value of the job stored in PerfNavController.map which keeps track of the job with the NavController as a Key.
If the job hasn't been completed, it will block the main thread until the job is done. The job itself is responsible for attaching the navgraph
to the navcontroller (and the inflation of the latter too)
For 16900: rebased upstream master
For 16900: Rebase on master
For #16900: Fixed Async Navgraph navigation per review comments.
1)The Asynchronous method is now found in NavGraphProvider.kt. It creates a job on the IO dispatcher
2)The Job is tracked through a WeakHashMap from Controller --> NavGraph
3)The Coroutine scope doesn't use MainScope() anymore
4)The Coroutine is cancelled if the Activity is destroyed
5)The tests mockk the blockForNavGraphInflation method through the FenixReoboelectricTestApplication instead of calling the mock every setup()
For #16900: inflateNavGraphAsync now takes navController
For #16900: Pass lifecycleScope to NavGraphProvider
For #16900: removed unused mock
For #16900: Added linter rules for navigate calls
We need linting rules to make sure no one calls the NavController.navigate() methods
For #16900: Added TestRule to help abstract the mocks in the code
For 16900: Fix linting problems
For #16900: Cleaned duplicated code in tests
For #16900: cleaned up NavGraphTestRule for finished test
For #16900: had to revert an accidentally edited file
For #16900: rebased master
* For #16900: Review nits for async navgraph
This is composed of squash commits, the original messages can be found below:
-> DisableNavGraphProviderAssertionRule + kdoc.
Use test rule in RobolectricApplication.
Fix failing CrashReporterControllerTest
Fix blame by -> navigate in tests.
This commit was generated by the following commands only:
```
find app/src/test -type f -exec sed -i '' "/import org.mozilla.fenix.ext.navigateBlockingForAsyncNavGraph/d" {} \;
find app/src/test -type f -exec sed -i "" "s/navigateBlockingForAsyncNavGraph/navigate/g" {} \;
git checkout app/src/test/resources/mockito-extensions/org.mockito.plugins.MockMaker
```
Fix various blame
This is expected to be squashed into the first commit so, if so, it'd
fix the blame.
Move test rule to helpers pkg.
add missing license header
Add import change I missed
fix unused imports
Replace robolectricTestrunner with test rule.
Improve navGraphProvider docs
Remove unnecessary rule as defined by robolectric.
add clarifying comment to robolectric
remove unnecessary space
* For #16900: nit fixes for MozillaNavigateCheck and lint fixes
3 squash commits:
*Changed violation message and fixed the lint rule for MozillaNavigateCheck
*Added suppression to NavController.kt
*Fixed detekt violations
* For 16900: Fixed failing tests
Co-authored-by: Michael Comella <michael.l.comella@gmail.com>
We moved the collection dialog code out from the old fragment, because it
had nothing to do with tabs tray, and into the collections package to be
re-usable in other parts of the app.
In addition, we also make use of it in the new tabs tray's
NavigationInteractor.
* Issue #18862: Add new addBookmark BookmarksUseCase
* Issue #18862: Add class for state binding features
* Issue #18862: Add delete multiple tabs to tray interactor
* Issue #18862: Add new actions to navigation interactor
* Issue #18862: Enable select mode from main tray menu
* Issue #18862: Add menu when in select mode
* Close#18862: Add multi-select banner to tabs tray
* Close#18862: Add select support for handle UI
We apply various layout changes to the "handle" UI in the tabs tray when
switching modes. It isn't quite clear to my, why we do this, if it's
really needed to meet the end result, and if there is a better way.
For now, we're simplying moving over that logic that we can re-evaluate
at a later time.
FindInPageIntegration which already updated the toolbar to make room for the
find in page bar now receives more data based on which it will be able to
better update the layout of BrowserFragment to to support showing the find in
page bar.
Copied the TabsTouchHelper from the `tabtray` package here so we don't
need to re-write our own because there's nothing more to add.
We can hook this up with our tabs tray here by putting it in the
`BaseBrowserTrayList` for our normal and private tabs list.
Removes the recyclerview-selection library and replaces it with the
SelectionHolder/SelectionInteractor with a Store.
This is an implementation that's similar to what we have in other UI
lists (library).
* Create new menu order for new tab
* Add new tab menu navigation. Dynamically update menu when sync auth is needed. Make new tab menu and browser menu consistent.
* Lint
Lint and refactoring tests
* Tests for default toolbar menu
* Feature flag for request desktop site
Add todos for UI test issue 17979
Add todos for UI tests
Lower Android versions don't offer the possibility of opening system settings
at a specific preference. In this cases we already shown a sumo article
detailing the manual steps each user is expected to perform to change the
system set default browser.
* Add intent processor for locale changes
* Recreate notification and notify in the service
* Use locale use cases to update notification
* Use notification id instead of tag
* Add locale use cases and restore locale in application
* Send locale to service instead of string
* Controller tests for locale
* Update Android Components version to 74.0.20210323143308
Co-authored-by: Arturo Mejia <arturomejiamarmol@gmail.com>
This refactors SyncLoginsPreferenceView to SyncPreferenceView so that it be can be used in the
Credit cards preference for the equivalent "Sync cards across devices" preference.
While we could easily move this into the metrics ping, it's better to
leave it in the other ping because it's less work and because (I think)
we'll be better able to match `framework_secondary` values to the clock
ticks if we combine them in the same ping.
We do this in order to make it easier to analyze in GLAM: see the metric
descriptions for more details.
Additionally, we change the time unit to milliseconds to make it easier
to analyze in GLAM.
Elise Richards
Gabriel Luong
Christian Sadilek
Mugurell
codrut.topliceanu
mcarare
Rohan Maity
Jonathan Almeida
Arturo Mejia
Roger Yang
Vitaly V. Pinchuk
Michael Comella
Grisha Kruglov
Sebastian Kaspari
Marc Leclair
Codrut Topliceanu
Mihai Adrian Carare
rxumoz
ZianeA
Ionut Cristian Bedregeanu
Kate Glazko
Kershan