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
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 disabled the allowaccessmodification proguard option because it broke
functionality or crashed the app (I can't rememeber). As far as we know,
the R8 bug was fixed in the R8 bundled with the Android Gradle Plugin
v4.1. We're now on AGP v7.0.0-rc1 so we should be able to revert this
now.
This commit reverts the following commits:
Revert "Proguard/r8: Do not allow access modification."
This reverts commit 98bf27fdd4.
Revert "Dump `proguard-android-optimize.txt` into local configuration for later modification"
This reverts commit 88fe3fbf82.
This differs from `tab_view_setting` which tells us what the user's tab
setting is at startup. It does not tell us if the user explicitly
changed it instead of just using the default (which was recently
changed in #19809).
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
* Renew product telemetry probes expiring in august 2021
* Add placeholder for data reviews
* Allow unneeded metrics to expire in August. To be re-evaluated later.
* Add link to data review
Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com>
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.
The change to the function makes it so when the Settings.kt class is initialized, the isDefaultBrowser, which calls the
BrowserCache, won't get called right away. `isDefaultBrowser()` is known to take quite a while on start up on the G5+ (approx
30-40ms).
Apparently, it had been using the wrong mapping this whole time. I can't
wrap my head around fenix's glean wrapper so I'm not sure if this
resulted in bugs or if my change changes the behavior. However, I don't
think anyone is using this probe so I'm requesting to remove it. If we
don't end up removing it, then we can check for bugs/changes in
behavior. This isn't ideal but I'd rather not spend the time on it if we
don't have to.
* 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>
