Fix lock screen dismiss animation not working on certain older systems #1550

joeljfischer · 2020-02-07T16:59:34Z

This PR is ready for review.

Risk

This PR makes no API changes.

Testing Plan

I have verified that I have not introduced new warnings in this PR (or explain why below)
I have run the unit tests with this PR
I have tested this PR against Core and verified behavior (if applicable, if not applicable, explain why below).

Unit Tests

Unit tests currently do not exist for the SDLLockScreenPresenter class, so no new unit tests were added in this bugfix PR. They were run and pass.

Core Tests

Connected while DD_OFF.
Repeatedly enable / disable DD as fast as possible.
Connected, backgrounded app, disconnected, reconnected, foregrounded app

Core version / branch / commit hash / module tested against: Sync Gen 3.0 (19205_DEVTEST), Sync Gen 3.0 (17276_DEVTEST)
HMI name / version / branch / commit hash / module tested against: Same as above

Summary

This PR tracks dismissals to prevent "double" dismissals causing the dismiss animation to break.
Changes the synchronization of the lock manager to not run a bunch of code async.
Changes the startup sequence to account for cases where the app is backgrounded when starting.

Changelog

Bug Fixes

The lock screen dismiss animation will no longer break on older SDL systems.
Disconnecting and reconnecting an app in the background will no longer prevent the lock screen from being dismissed in certain cases.

CLA

I have signed the CLA

NicoleYarroch

Found an issue.

SmartDeviceLink/SDLLockScreenPresenter.m

* Check that the lock screen is presented before attempting the dismissal

* Fix `isPresentedOrPresenting` needs to be called on the main thread

* Change `dispatch_async`s in lock screen classes to run sync * Check whether we actually dismissed a VC to ensure that when we stop the presenter, it doesn't clear the presented window or view controller * When starting the lock manager, check if we're in the background and abort if we are. If we aren't, check if we didn't erase the lock view controller (and by extension, the window), which means that we may still need to dismiss it, then clear the presenter and re-setup

NicoleYarroch · 2020-02-25T16:43:04Z

I got a crash when testing LMP with this branch installed to SYNC 3.0:

Open LMP on the device and keep in the foreground.
Set the SYNC 3.0 module to "parked".
Connect the device to SYNC 3.0 using a USB cord. Launch the SDL app. The lock screen shows and dismisses.
Trigger a DD_ON notification by setting the module to "moving".
Put app on device in background.
Pull USB cord. Device app should still be in background.
Bring app on device to foreground

Expected Result

Lock screen dismisses

Actual Result

Lock screen dismisses and app crashes.

2020-02-25 11:23:52.832912-0500 Livio Music[500:63894] [Lockscreen] Dismissing the lockscreen window
2020-02-25 11:23:52.835041-0500 Livio Music[500:63894] [Lockscreen] Presenting the lockscreen window
2020-02-25 11:23:53.394217-0500 Livio Music[500:63894] [Lockscreen] Lockscreen window dismissed
2020-02-25 11:23:53.396489-0500 Livio Music[500:63894] [Lockscreen] Presenting the lockscreen window
2020-02-25 11:23:53.402151-0500 Livio Music[500:63333] *** Terminating app due to uncaught exception 'NSInvalidArgumentException', reason: 'Application tried to present modally an active controller <SDLLockScreenRootViewController: 0x103c15340>.'
*** First throw call stack:
(0x1884ed54c 0x1882080c8 0x18bf39e08 0x18bf3c148 0x18bf3c6f4 0x18bf556bc 0x18bf51c98 0x18bf519b0 0x18c9b4108 0x18c9b3dac 0x18c9eae0c 0x18c9be088 0x18c9be5b0 0x18c9be708 0x18efb2038 0x1037ff32c 0x10380d3e0 0x188468700 0x18846345c 0x188462978 0x192592534 0x18c554f0c 0x10053d184 0x1882e2f04)
libc++abi.dylib: terminating with uncaught exception of type NSException

NicoleYarroch

Left a comment about the library crashing.

* updateLockScreenToShow now has a completion handler to allow the startup to ensure that it's finished dismissing or presenting before re-setting up the lock screen

joeljfischer · 2020-02-25T21:39:46Z

@NicoleYarroch I believe I have fixed the issue.

NicoleYarroch

Left some comments

SmartDeviceLink/SDLLockScreenPresenter.m

NicoleYarroch

Lock screen does not show if you follow these steps with iPhone 11 (iOS 13.3.1) and SYNC v3.0:

Set head unit moving status to "parked".
Connect device to head unit with a USB cord and launch SDL app by tapping on the SDL app icon on the head unit.
Open the app on the device.
Toggle the driver distraction status by setting the moving status to "speed".
Put app on the device in the background. App should not be killed.
Disconnect USB cord. App should not be killed.
Connect USB cord.
Open the app on the device.

Expected Result

Lock screen should show since the head unit moving status is still set to "speed".

Actual Result

Lock screen is dismissed.

SmartDeviceLink/SDLLockScreenPresenter.m

* Fix the lock screen not showing if the app is started, DD is on, the app is put into the background on the phone, then the app is disconnected, reconnected, then put into the foreground on the phone. * Stopping the LSM presenter now takes a completion handler * Starting the LSM now checks the presenter immediately to see if the lock screen should be presented

NicoleYarroch

Bug 1

The locks screen is still presented when these steps are followed:

App on device is closed.
Connect app to TDK that is set to "Parked". Launch SDL app on HMI if necessary.
Open app on device.
Toggle the TDK's moving status to "Speed".
Put app on device in background.
Pull USB cord.
Bring app on device back to foreground

Expected Result

Lock screen does not show because the device is not connected to the TDK

Actual Resul

Lock screen dismisses but then shows again.

Bug 2

The lock screen is not presented when these steps are followed:

Open app on device. Put app on device in background.
Connect app to TDK that is set to "Speed". Launch SDL app on HMI if necessary.
Bring app on device to foreground.

Expected Result

Lock screen shows because the TDK moving status is set to "speed"

Actual Resul

Lock screen does not show.

joeljfischer · 2020-03-11T15:11:37Z

@NicoleYarroch Bug 1 is fixed, Bug 2 I only saw once out of 8 tries and have not been able to replicate it since.

* Clear the last lock notification to ensure that the lock screen is not presented when not connected

NicoleYarroch

Found an issue where the lockscreen never presents.

SmartDeviceLink/SDLLockScreenManager.m

SmartDeviceLink/SDLLockScreenPresenter.m

NicoleYarroch · 2020-03-12T17:31:41Z

SmartDeviceLink/SDLLockScreenPresenter.m

    // Store the expected state of the lockscreen
    self.shouldShowLockScreen = show;

    if (show) {
        [self sdl_presentWithCompletionHandler:^{
-            if (self.shouldShowLockScreen) { return; }
+            if (self.shouldShowLockScreen) {


Use weakSelf/strongSelf in this block

NicoleYarroch · 2020-03-12T17:31:57Z

SmartDeviceLink/SDLLockScreenPresenter.m

-        self.lockWindow = nil;
+    [self sdl_dismissWithCompletionHandler:^(BOOL success) {
+        if (success) {
+            self.lockWindow = nil;


Use weakSelf

SmartDeviceLink/SDLLockScreenPresenter.m

* Make sure `stop` completion handler is always called * Use strong / weak in all blocks

NicoleYarroch

Smoke tested with both template app (iOS 13, 12, and 11) and with streaming video/audio app (iOS 13, 12, and 11)

Fix lock screen dismiss animation not working on certain older systems

6329acf

joeljfischer added bug A defect in the library manager-lockscreen Relating to the manager layer - lockscreen labels Feb 7, 2020

joeljfischer self-assigned this Feb 7, 2020

NicoleYarroch self-requested a review February 10, 2020 20:44

NicoleYarroch added this to In progress in v6.6.0 via automation Feb 10, 2020

NicoleYarroch suggested changes Feb 20, 2020

View reviewed changes

SmartDeviceLink/SDLLockScreenPresenter.m Show resolved Hide resolved

v6.6.0 automation moved this from In progress to Waiting for Review Feb 20, 2020

joeljfischer added 3 commits February 24, 2020 11:45

Fix an iOS 12 issue

ab79e8c

* Check that the lock screen is presented before attempting the dismissal

Fix main thread bug

0b29de3

* Fix `isPresentedOrPresenting` needs to be called on the main thread

NicoleYarroch self-requested a review February 25, 2020 16:06

NicoleYarroch suggested changes Feb 25, 2020

View reviewed changes

Fixing lock screen crash

09f8914

* updateLockScreenToShow now has a completion handler to allow the startup to ensure that it's finished dismissing or presenting before re-setting up the lock screen

NicoleYarroch suggested changes Feb 26, 2020

View reviewed changes

SmartDeviceLink/SDLLockScreenPresenter.m Show resolved Hide resolved

SmartDeviceLink/SDLLockScreenPresenter.m Outdated Show resolved Hide resolved

Fix tests

81e77e6

NicoleYarroch suggested changes Mar 4, 2020

View reviewed changes

SmartDeviceLink/SDLLockScreenPresenter.m Outdated Show resolved Hide resolved

joeljfischer added 2 commits March 9, 2020 14:37

Fix tests crashing

3807cb5

joeljfischer requested a review from NicoleYarroch March 9, 2020 18:48

NicoleYarroch suggested changes Mar 9, 2020

View reviewed changes

Fix lock screen bug

b3bff8f

* Clear the last lock notification to ensure that the lock screen is not presented when not connected

NicoleYarroch self-requested a review March 12, 2020 14:46

NicoleYarroch suggested changes Mar 12, 2020

View reviewed changes

joeljfischer added 2 commits March 13, 2020 14:38

weak / strong fixes

76f7c5d

Fix lock screen bug

ea82228

* Make sure `stop` completion handler is always called * Use strong / weak in all blocks

v6.6.0 automation moved this from Waiting for Review to Reviewer approved Mar 16, 2020

NicoleYarroch approved these changes Mar 16, 2020

View reviewed changes

joeljfischer merged commit 1699ef8 into develop Mar 16, 2020

v6.6.0 automation moved this from Reviewer approved to Done Mar 16, 2020

joeljfischer deleted the bugfix/issue-1504-lock-screen-dismiss-animation branch March 16, 2020 15:58

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix lock screen dismiss animation not working on certain older systems #1550

Fix lock screen dismiss animation not working on certain older systems #1550

joeljfischer commented Feb 7, 2020 •

edited

NicoleYarroch left a comment

NicoleYarroch commented Feb 25, 2020

NicoleYarroch left a comment

joeljfischer commented Feb 25, 2020

NicoleYarroch left a comment

NicoleYarroch left a comment

NicoleYarroch left a comment

joeljfischer commented Mar 11, 2020

NicoleYarroch left a comment

NicoleYarroch Mar 12, 2020

NicoleYarroch Mar 12, 2020

NicoleYarroch left a comment

Fix lock screen dismiss animation not working on certain older systems #1550

Fix lock screen dismiss animation not working on certain older systems #1550

Conversation

joeljfischer commented Feb 7, 2020 • edited

Risk

Testing Plan

Unit Tests

Core Tests

Summary

Changelog

Bug Fixes

CLA

NicoleYarroch left a comment

Choose a reason for hiding this comment

NicoleYarroch commented Feb 25, 2020

Expected Result

Actual Result

NicoleYarroch left a comment

Choose a reason for hiding this comment

joeljfischer commented Feb 25, 2020

NicoleYarroch left a comment

Choose a reason for hiding this comment

NicoleYarroch left a comment

Choose a reason for hiding this comment

Expected Result

Actual Result

NicoleYarroch left a comment

Choose a reason for hiding this comment

Bug 1

Expected Result

Actual Resul

Bug 2

Expected Result

Actual Resul

joeljfischer commented Mar 11, 2020

NicoleYarroch left a comment

Choose a reason for hiding this comment

NicoleYarroch Mar 12, 2020

Choose a reason for hiding this comment

NicoleYarroch Mar 12, 2020

Choose a reason for hiding this comment

NicoleYarroch left a comment

Choose a reason for hiding this comment

joeljfischer commented Feb 7, 2020 •

edited