HomeFragment should observe wallpaper changes.

[MatthewTighe]

With the upcoming wallpaper selection tool defined in https://mozilla-hub.atlassian.net/browse/FNXV2-21134, we need a way to actively observe changes to the current wallpaper when they are updated from the tool. The following conditions should be met:

• there is no noticeable flashing from an already selected wallpaper that loads after the rest of the home screen during startup or returning to home from anywhere else in the app
• wallpapers behave correctly during orientation changes
• the current wallpaper is observed and updated

For context, this issue is a result of conversation that took place in this comment thread: #26515 (comment)

┆Issue is synchronized with this Jira Task

[Mugurell]

With the code from b22e500 applied I see cases in which the wallpaper is not applied until coming back to the hoomescreen.

WallpaperOrNot.mp4

[Mugurell]

Testing the implementation from b22e500 on a OnePlus 7T Pro I see that :

• onCreateView from which the wallpaper is set (synchronously) already takes ~140ms.
• setWallpaperToCurrent would add another ~100ms.
  (Looks like there is enough time to offload the most heavy processing to another thread while onCreateView is churning and we should have all data ready in onStart)
• requireComponents.useCases.wallpaperUseCases.loadBitmap(wallpaper) takes ~80ms
• bitmap.scaleToBottomOfView takes ~2
  (Adding this here just because I think that loading and scaling the bitmap can be done together on a separate thead).

At a quick glance it seems like we can set the wallpaper without affecting performance or at least significantly reducing the slowdown.

An important issue that remains is shown in the above video and seems like a newly introduced one: reading the current wallpaper from the appState may happen too soon and then we would not show the wallpaper.
Seems like previously we were waiting for state updates.

[Mugurell]

Looking at this more it seems like the previous idea of parallelizing setting the wallpaper with all the setup in HomeFragment up until onStart is blocked by wallpaperUseCases.initialize which updates the wallpaper in store and takes ~250ms-~300ms. We're in a race with that.
Trying to read the current wallpaper a bit higher in HomeFragment would get the default wallpaper so it looks like we're left with 2 scenarios:

• stick with the current implementation which observes store updates and automatically sets the wallpaper when available. Just move displayWallpaperIfEnabled higher and observe the store from at least the start of onCreatedView - switch off from the current store.flow call which will only start observing in onStart.
  In my opinion this is the best approach and would allow setting the wallpaper as soon as that is available, certainly faster than currently (just because we don't wait until onStart to start those 100ms needed for setting the wallpaper).
• use the approach proposed in For #26511: load homescreen wallpaper in blocking coroutine #26515 complete with the observeWallpaperChanges call which would improve things but might skip some first wallpaper setups because of the race with wallpapers initialization. Don't think we have a way of knowing now home many times we'd loose the race, I'd say it's highly dependent on the device and future app refactorings.

[MatthewTighe]

I don't have a strong opinion about the direction we take with this, but wanted to add some additional context.

wallpaperUseCases.initialize which updates the wallpaper in store and takes ~250ms-~300ms.

At a glance, these numbers look pretty concerning. However, we are now loading metadata about wallpapers from the remote server. Accounting for network time, this actually looks like it could be on the low-end, especially if there's ever problems with our service.

The current implementation waits until this networking is finished before setting the current wallpaper. This is in order to check whether the current wallpaper is part of an expired promotion or not. It also allows us to construct the fully qualified data type of the current wallpaper using the remote metadata.

Thinking about it more now, I don't think there's any good reason for for this because:

• we let users keep expired wallpapers that are currently selected
• we can cache the other metadata required for general use. this would be things like the textColor and cardColor properties, but we don't really need to care about the collection for the purposes of actually showing the wallpaper on the home screen.

I had planned to improve storage for wallpapers generally as part of #26033, but it slipped the time constraints of the MR. I think a more robust Room or DataStore implementation would be good now that the data is growing in complexity, but I'm also not sure it's worth doing until the shape of the data has stabilized. I'm not convinced that's happened yet.

For now, we could also cache the other properties I mentioned in settings and construct a useful Wallpaper instance during wallpapersUseCases.initialize rather than waiting for the network, if there are cases where it is not available to us when we need it. I'm also concerned that users with slower network connections may experience the problem more frequently.

[MatthewTighe]

Actually, since this is a bug that will affect beta and release I am going to hack together a caching mechanism to add to #26515 so that we can resolve the bug ASAP

[MatthewTighe]

I'm now realizing that the networking code had not been introduced yet on the patch in question. There is still quite a bit of IO however, including downloading wallpapers if any are missing. Any change required for it will also be required for the networking code as per my concerns above.

The networking code has landed in main now, however. #26515 probably should have been landed and uplifted before landing 299f887. We have two options:
1.

• revert 299f887
• update For #26511: load homescreen wallpaper in blocking coroutine #26515 with change
• uplift
• reintroduce 299f887
• add change to 299f887

• add change to both
• uplift code that includes 299f887

[MatthewTighe]

switch off from the current store.flow call which will only start observing in onStart.
In my opinion this is the best approach and would allow setting the wallpaper as soon as that is available, certainly faster than currently (just because we don't wait until onStart to start those 100ms needed for setting the wallpaper).

I think the issue here is actually that store.flow() is launched in viewLifecycleOwner.lifecycleScope, which will not run its coroutines until the it is in at least the CREATED lifecycle state, as I commented on in #26515 (comment). I think this means that the observation won't start until after the screen is rendered, causing the pop-in. I may be wrong about that and would be happy to be. I'm not sure if there's a way to observe the store before rendering if I'm right, though.

[Mugurell]

switch off from the current store.flow call which will only start observing in onStart.
In my opinion this is the best approach and would allow setting the wallpaper as soon as that is available, certainly faster than currently (just because we don't wait until onStart to start those 100ms needed for setting the wallpaper).

I think the issue here is actually that store.flow() is launched in viewLifecycleOwner.lifecycleScope, which will not run its coroutines until the it is in at least the CREATED lifecycle state, as I commented on in #26515 (comment). I think this means that the observation won't start until after the screen is rendered, causing the pop-in. I may be wrong about that and would be happy to be. I'm not sure if there's a way to observe the store before rendering if I'm right, though.

The issue with that call is actually the usage of flow which will only start observing the Store once the owner get's past onStart - once the screen is visible.
This is a much too later time. If only now we start observing the store and then actually setting the wallpaper takes ~100ms than on 60Hz we are loosing 6 frames. This will be noticeable no matter how fast downloading and persisting the wallpapers happen to make them available to the HomeFragment.

(Not sure why we chose to use onStart - it is a given that this will cause delays in what gets displayed. I propose using onCreate here instead. @csadilek)

For a Fragment the onCreate state actually refers to onCreateView so viewLifecycleOwner.lifecycleScope will help run a coroutine once the Fragment is at least in the onCreateView state which would be great since at this time we are not being displayed on the screen - we're only now creating the View to be displayed.

In regards to being able at this time to observe the store which is updated from FenixApplication with a new wallpaper before the Homescreen is displayed using observeManually seems to work great for me.
As per the above numbers, with onCreateView taking ~140ms, ~80ms of which are needed just to inflate the layout then we can have a headstart of ~60ms to set the wallpaper which is enough to set it around the time onStart completes (and the HomeFragment is visible).
(We could try to get an even bigger headstart and not wait for the inflation but if we'll try to set the wallpaper before the inflation is finished things would blow up. Adding support for this scenario would mean a greater complexity which seems to not be needed.)

[delia-pop]

Tested with the latest Nightly from 09/02 with Google Pixel 6 (Android 13). Issues #26511 and #26638 are still reproducible in the current version.
I will re-test when the issues are fixed and leave the QA needed label until then.

screen-20220902-175502_Trim.mp4

[Mugurell]

Issue fixed by Matt in #26949

[delia-pop]

Verified as fixed on Nightly 107.0a1 from 09/20 with Google Pixel 6 (Android 13). The wallpaper set is properly displayed on homepage when the app is opened, when navigating to and from homepage and when the orientation is changed.