Skip to content
This repository has been archived by the owner on Feb 20, 2023. It is now read-only.

[Bug]: Wallpaper loading on launch is delayed #26511

Closed
delia-pop opened this issue Aug 17, 2022 · 5 comments · Fixed by #26515, #26794, fork-house/fenix#13, #26949 or nathanmkaya/fenix#108
Closed

[Bug]: Wallpaper loading on launch is delayed #26511

delia-pop opened this issue Aug 17, 2022 · 5 comments · Fixed by #26515, #26794, fork-house/fenix#13, #26949 or nathanmkaya/fenix#108
Labels
🐞 bug Crashes, Something isn't working, .. eng:qa:verified QA Verified Feature:Wallpapers needs:triage Issue needs triage S3 Blocks non-critical functionality and a work around exists
Milestone

Comments

@delia-pop
Copy link

delia-pop commented Aug 17, 2022

Steps to reproduce

  1. Install the latest Nightly 105.0a1 from 08/17 or Update the app to the latest version and perform a clear data.
  2. From Settings > Homepage > Opening screen select "Homepage".
  3. From Settings > Homepage > Wallpapers select any wallpaper.
  4. Close and Re-open Fenix.
  5. On start-up, observe the wallpaper on the homepage.

Expected behaviour

The wallpaper loads without visual issues or significant delay, given a good and stable network connection.

Actual behaviour

The wallpaper loading on homepage is delayed with 1-3 seconds when launching the app, even with a good and stable network connection.
The issue occurs after every restart, not only the first launch.

Device name

Xiaomi 12 Pro

Android version

Android 12

Firefox release type

Firefox Nightly

Firefox version

latest Nightly 105.0a1 from 08/17

Device logs

No response

Additional information

Also reproduced with: Samsung Galaxy A32 (Android 11), Samsung Galaxy S22 Ultra (Android 12), Lenovo Yoga Tab 11 (Android 11).

Screenrecorder-2022-08-17-17-30-36-932_Trim.mp4

┆Issue is synchronized with this Jira Task

@delia-pop delia-pop added 🐞 bug Crashes, Something isn't working, .. S3 Blocks non-critical functionality and a work around exists needs:triage Issue needs triage Feature:Wallpapers labels Aug 17, 2022
MatthewTighe added a commit to MatthewTighe/fenix that referenced this issue Aug 17, 2022
@github-actions github-actions bot added the eng:reopen-for-qa Reopens and tags the issue for QA needed when the issue is merged label Aug 17, 2022
MatthewTighe added a commit to MatthewTighe/fenix that referenced this issue Aug 19, 2022
MatthewTighe added a commit to MatthewTighe/fenix that referenced this issue Aug 22, 2022
MatthewTighe added a commit to MatthewTighe/fenix that referenced this issue Aug 23, 2022
MatthewTighe added a commit to MatthewTighe/fenix that referenced this issue Aug 23, 2022
Mugurell pushed a commit to Mugurell/fenix that referenced this issue Aug 25, 2022
MozillaNoah pushed a commit to MatthewTighe/fenix that referenced this issue Aug 25, 2022
@mergify mergify bot closed this as completed in #26515 Aug 29, 2022
@github-actions github-actions bot reopened this Aug 29, 2022
@github-actions github-actions bot added eng:qa:needed QA Needed and removed eng:reopen-for-qa Reopens and tags the issue for QA needed when the issue is merged labels Aug 29, 2022
@github-actions github-actions bot added this to the 106 milestone Aug 29, 2022
@SoftVision-LorandJanos
Copy link

I've tried to verify this issue, and I was blocked by a crash: #26723.
Tested on the latest Nightly 106.0a1 (2022-08-31).
Device used: Oppo Find X5 (Android 12).
Removing the QA:needed label until the crash will be fixed.

@delia-pop
Copy link
Author

delia-pop commented Sep 1, 2022

#26723 is verified as fixed on the latest Nightly 106 updated from main locally. The app no longer crashes on startup when a wallpaper is selected.

However, in the locally updated Nightly build, this issue is still reproducible: after launching the app, the wallpaper is still displayed with a short delay. Talked to @Mugurell and we will wait for the official Nightly build containing the fix for the #26723 crash to become available on PlayStore of task index, and re-test this.
Tested with Google Pixel 6 (Android 13).

screen-20220901-121827_Trim.mp4

@Mugurell
Copy link
Contributor

Mugurell commented Sep 1, 2022

I indeed can reproduce again the slow wallpapers after the fix for #26723 which meant moving the operations to the Main thread.
With some logs I'm seeing that observing and showing the wallpaper do indeed start well before onCreateView finishes but then loading the wallpaper bitmap and applying it to the imageView pushes us significantly past the screen being shown.

Tried to take a profile of this but I don't find the interesting details in it - https://share.firefox.dev/3TwPa4x

My thoughts on why this might happen - Dispatchers.Main (current) vs Dispatchers.IO (previously used which showed significantly faster execution) revolves more around context switching (loading the bitmap happens on an IO thread) causing this delay with the Main thread probably having a long queue to execute at startup.

Will try some refactoring.

@Mugurell
Copy link
Contributor

Mugurell commented Sep 1, 2022

Running on an IO thread and then switching to Main with a simple

Handler(Looper.getMainLooper()).postAtFrontOfQueue

here does show a 50-100ms delay for just starting the runnable execution while if remaining with IO it's basically instant. But of course we should not update Views from other threads (curious that this works).

Mugurell added a commit to Mugurell/fenix that referenced this issue Sep 2, 2022
Split loading the bitmap from storage and actually setting it in two operations
with one that can run in parallel with onCreateView for HomeFragment and one
that can be used serially on the main thread to actually set the wallpaper.

This seems the best compromise to ensure that everytime the homescreen is shown
it will have the wallpaper set but does affect the performance - there is a
delay in showing HomeFrament to account for waiting for the wallpaper to be set.
@github-actions github-actions bot added the eng:reopen-for-qa Reopens and tags the issue for QA needed when the issue is merged label Sep 2, 2022
Mugurell added a commit to Mugurell/fenix that referenced this issue Sep 12, 2022
Split loading the bitmap from storage and actually setting it in two operations
with one that can run in parallel with onCreateView for HomeFragment and one
that can be used serially on the main thread to actually set the wallpaper.

This seems like the best compromise to ensure that everytime the homescreen is
shown it will have the wallpaper set but does affect the performance - there is
a delay in showing HomeFragment to account for waiting for the wallpaper to be
set.
In testing the new delay seems close to the one from the initial wallpapers
implementation. See more in mozilla-mobile#26794.
Mugurell added a commit to Mugurell/fenix that referenced this issue Sep 12, 2022
This should squeeze the most performance for users who haven't set a wallpaper.
Mugurell added a commit to Mugurell/fenix that referenced this issue Sep 12, 2022
Previously we were waiting until the AppState is updated with the wallpaper
previously chosen by the user (option persisted in SharedPreferences).
It is possible though that the previously chosen wallpaper has expired (or for
other reasons it is unavailable) in which case the AppState will be updated
with the default wallpaper while in SharedPreferences we still keep the name of
unavailable wallpapers.
Mugurell added a commit to Mugurell/fenix that referenced this issue Sep 12, 2022
…listeners

In the scenario of setting the wallpaper from both the observer and the
fragment there would be two layout listeners set for scaling the bitmap with
only the first being used immediately and the second being left to observe
future changes as when the keyboard is shown moment at which it would trigger
an unexpected wallpaper scaling.
By avoiding the apply the wallpaper from the observer and only applying it from
HomeFragment there will be only one layout change listener and so the above
issue will also be avoided.
MatthewTighe added a commit to MatthewTighe/fenix that referenced this issue Sep 12, 2022
MatthewTighe added a commit to MatthewTighe/fenix that referenced this issue Sep 12, 2022
MatthewTighe added a commit to MatthewTighe/fenix that referenced this issue Sep 12, 2022
MatthewTighe added a commit to MatthewTighe/fenix that referenced this issue Sep 13, 2022
MatthewTighe added a commit to MatthewTighe/fenix that referenced this issue Sep 13, 2022
@gabrielluong gabrielluong removed the eng:reopen-for-qa Reopens and tags the issue for QA needed when the issue is merged label Sep 13, 2022
@github-actions github-actions bot added the eng:reopen-for-qa Reopens and tags the issue for QA needed when the issue is merged label Sep 13, 2022
@mergify mergify bot closed this as completed in #26949 Sep 13, 2022
@github-actions github-actions bot reopened this Sep 13, 2022
@github-actions github-actions bot removed the eng:reopen-for-qa Reopens and tags the issue for QA needed when the issue is merged label Sep 13, 2022
@MatthewTighe MatthewTighe modified the milestones: 106, 107 Sep 19, 2022
@gabrielluong gabrielluong modified the milestones: 107, 106 Sep 20, 2022
@delia-pop
Copy link
Author

Verified as fixed on Nightly 107.0a1 from 09/20 with Google Pixel 6 (Android 13). The wallpaper set is displayed on homepage as soon as the app is opened, and the user does not notice a delay any more.

26511.mp4

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.