Add an hourly device state read to update the header bar view

[buggmagnet]

This PR introduces a new method in the TunnelObserver protocol.
func tunnelManager(_ manager: TunnelManager, didPeriodicallyRead deviceState: DeviceState)

As the name implies, periodic reads from the DeviceState are now scheduled when the app becomes active via
func didBecomeActive(_ notification: Notification)

Right now, the timer will fire once, and then every hour. It is only active when the app is running, otherwise the timer is cancelled. This is to avoid impact on the battery.

Whenever users now resume the app, the HeaderBarView should display the most recent information it has available.

---

This change is 

[linear]

IOS-233 Time left in header bar is not updated

Users report that time left label in the header isn't being updated as time passes. We should add a timer that bumps the time left label so that it decrements accurately with time passing.

#4909

[mojganii]

Reviewable status: 0 of 5 files reviewed, 4 unresolved discussions (waiting on @buggmagnet)

---

ios/MullvadVPN/TunnelManager/TunnelManager.swift line 68 at r1 (raw file):

    private var deviceStateRefreshTimer: DispatchSourceTimer?
    private var isRunningPeriodicStateRefresh = false
    private let deviceStateRefreshPeriod: TimeInterval = 60 * 60

Did you get confirmation about the interval? it seems to me it's a little bit long and the result of that doesn't effects on having better user experience

---

ios/MullvadVPN/TunnelManager/TunnelManager.swift line 136 at r1 (raw file):

    }

    func startPeriodicStateRefresh() {

nit: StartPeriodicDeviceStateRefresh(). when it comes to state it's not well defined what it's talked about.

---

ios/MullvadVPN/TunnelManager/TunnelManager.swift line 154 at r1 (raw file):

    }

    func stopPeriodicStateRefresh() {

nit: StopPeriodicDeviceStateRefresh()

---

ios/MullvadVPN/TunnelManager/TunnelManager.swift line 941 at r1 (raw file):

            guard let deviceState = try? SettingsManager.readDeviceState(),
                  let self = self else { return }
            nslock.withLock {

do we need to do this part of the code with locking while this method is not accessible from others ?

[buggmagnet]

Reviewable status: 0 of 5 files reviewed, 4 unresolved discussions (waiting on @mojganii)

---

ios/MullvadVPN/TunnelManager/TunnelManager.swift line 68 at r1 (raw file):

Previously, mojganii wrote…

Did you get confirmation about the interval? it seems to me it's a little bit long and the result of that doesn't effects on having better user experience

It's by design. The timer will fire immediately when the application re enters foreground which is the desired effect -- refreshing the header bar view to display accurate information.

I don't think people spend hours showing the UI of the application, so this is high enough as to not impact battery life by firing too often.

---

ios/MullvadVPN/TunnelManager/TunnelManager.swift line 136 at r1 (raw file):

Previously, mojganii wrote…

nit: StartPeriodicDeviceStateRefresh(). when it comes to state it's not well defined what it's talked about.

Fair enough, I will also remove the refresh work as we already use it for a different purposes.
Since we only read the device state, I will rename this "Readings"

---

ios/MullvadVPN/TunnelManager/TunnelManager.swift line 941 at r1 (raw file):

Previously, mojganii wrote…

do we need to do this part of the code with locking while this method is not accessible from others ?

You're right, the lock is not needed here, thanks for catching that I will remove it.

[mojganii]

Reviewable status: 0 of 5 files reviewed, 3 unresolved discussions (waiting on @buggmagnet)

---

ios/MullvadVPN/TunnelManager/TunnelManager.swift line 68 at r1 (raw file):

Previously, buggmagnet wrote…

It's by design. The timer will fire immediately when the application re enters foreground which is the desired effect -- refreshing the header bar view to display accurate information.

I don't think people spend hours showing the UI of the application, so this is high enough as to not impact battery life by firing too often.

I am wondering if user buys more credit via other platforms while the app is active,our app will be notified quickly or user has to be wait for fire time?

[buggmagnet]

Reviewable status: 0 of 5 files reviewed, 1 unresolved discussion (waiting on @mojganii)

---

ios/MullvadVPN/TunnelManager/TunnelManager.swift line 68 at r1 (raw file):

Previously, mojganii wrote…

I am wondering if user buys more credit via other platforms while the app is active,our app will be notified quickly or user has to be wait for fire time?

The original mechanism in place hasn't changed for that, so whatever was in place should still work.
Also this fix is only for making sure the HeaderBarView is up to date when the app goes to the foreground after dormancy

[mojganii]

Reviewable status: 0 of 5 files reviewed, all discussions resolved

[pronebird]

Reviewed 3 of 5 files at r1, 2 of 2 files at r2.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @buggmagnet)

a discussion (no related file):
I think we already handle the device state refresh. This is the logic that's currently in place:

• When the app becomes active we call TunnelManager.refreshDeviceState() in tunnel manager, that makes sure to read new state from settings and propagate it to observers. (TunnelManager.setDeviceState() does the caching, persistence and propagation part).
• We poll tunnel status periodically to obtain the connection related info, but also last device check (see TunnelManager.setTunnelStatus()). Based on the last and new device check received from packet tunnel, tunnel manager infers whether it needs to refresh device state from settings or switch device to revoked state or any other.

Since we already implement the refresh of device state, I think the only missing piece here is to implement a timer in HeaderBarView that would re-format the label every once in a while. The date assigned to HeaderBar.timeLeft is probably up to date, but the timeLeftLabel.text is only assigned on didSet.

---

[pronebird]

Reviewable status: 0 of 6 files reviewed, 2 unresolved discussions (waiting on @buggmagnet)

---

ios/MullvadVPN/Containers/Root/HeaderBarView.swift line 250 at r3 (raw file):

        timeLeftTimer = Timer.scheduledTimer(withTimeInterval: timeLeftUpdateInterval, repeats: true) { _ in
            DispatchQueue.main.async { [weak self] in
                self?.timeLeft = self?.timeLeft

Couple of things:

1. I know it works but it would be cleaner to extract the didSet from var timeLeft: Date? into a separate function and call it from here and from there instead to keep it straightforward what we're trying to achieve.
2. Block timer will retain self strongly before retaining it weak before passing it into DispatchQueue.main.async {}. That's because strong is default I think.
3. Repeating Timer still needs to be invalidated from deinit because RunLoop holds a strong reference to it once the timer is scheduled.
4. Perhaps you don't need to wrap the timer handler into DispatchQueue.main because the timer is scheduled on current runloop which is RunLoop.main so the timer handler block should execute on main thread.

[pronebird]

Reviewed 6 of 6 files at r3, 1 of 1 files at r6.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @buggmagnet)

[mojganii]

Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @buggmagnet)