[ios] Add tests / refactor validateDisplayLink (with explicit calls)

5 files changed, 273 insertions, 61 deletions

diff --git a/platform/ios/Integration Tests/MGLBackgroundIntegrationTest.m b/platform/ios/Integration Tests/MGLBackgroundIntegrationTest.m
index a6932dd60c..39c8923770 100644
--- a/platform/ios/Integration Tests/MGLBackgroundIntegrationTest.m
+++ b/platform/ios/Integration Tests/MGLBackgroundIntegrationTest.m
@@ -2,7 +2,7 @@
 #import "MGLMockApplication.h"
 
 @interface MGLMapView (BackgroundTests)
-@property (nonatomic) id<MGLApplication> application;
+@property (nonatomic, readonly) id<MGLApplication> application;
 @property (nonatomic, getter=isDormant) BOOL dormant;
 @property (nonatomic) CADisplayLink *displayLink;
 - (void)updateFromDisplayLink:(CADisplayLink *)displayLink;
@@ -30,8 +30,7 @@ typedef void (^MGLNotificationBlock)(NSNotification*);
 
 #pragma mark - MGLBackgroundIntegrationTest
 
-@interface MGLBackgroundIntegrationTest : MGLMapViewIntegrationTest
-@property (nonatomic) id<MGLApplication> oldApplication;
+@interface MGLBackgroundIntegrationTest : MGLMapViewIntegrationTest <MGLApplicationProvider>
 @property (nonatomic) MGLMockApplication *mockApplication;
 @property (nonatomic, copy) MGLNotificationBlock willEnterForeground;
 @property (nonatomic, copy) MGLNotificationBlock didEnterBackground;
@@ -49,22 +48,11 @@ typedef void (^MGLNotificationBlock)(NSNotification*);
     [[NSNotificationCenter defaultCenter] addObserver:self selector:@selector(didEnterBackground:) name:UIApplicationDidEnterBackgroundNotification object:self.mockApplication];
     
     [super setUp];
-    
-    // Setup MGLMapView to use our new mocked application
-    // Change notification handling here.
-    self.oldApplication = self.mapView.application;
-    self.mockApplication.delegate = self.oldApplication.delegate;
-    self.mapView.application = self.mockApplication;
 }
 
 - (void)tearDown {
-
     [[NSNotificationCenter defaultCenter] removeObserver:self];
     
-    // Swap back
-    self.mapView.application = self.oldApplication;
-    self.oldApplication = nil;
-    self.mockApplication.delegate = nil;
     self.mockApplication = nil;
     
     [super tearDown];
@@ -98,10 +86,16 @@ typedef void (^MGLNotificationBlock)(NSNotification*);
     return mapView;
 }
 
+#pragma mark - MGLApplicationProvider
+
+- (id<MGLApplication>)applicationForSender:(id)mapView
+{
+    return self.mockApplication;
+}
+
 #pragma mark - Tests
 
 - (void)testRendererWhenGoingIntoBackground {
-    
     XCTAssertFalse(self.mapView.isDormant);
     XCTAssertFalse(self.mapView.displayLink.isPaused);
     XCTAssert(self.mapView.application.applicationState == UIApplicationStateActive);
@@ -111,7 +105,6 @@ typedef void (^MGLNotificationBlock)(NSNotification*);
     //
     // Enter background
     //
-    
     XCTestExpectation *didEnterBackgroundExpectation = [self expectationWithDescription:@"didEnterBackground"];
     didEnterBackgroundExpectation.expectedFulfillmentCount = 1;
     didEnterBackgroundExpectation.assertForOverFulfill = YES;
@@ -119,7 +112,7 @@ typedef void (^MGLNotificationBlock)(NSNotification*);
     self.didEnterBackground = ^(__unused NSNotification *notification){
         typeof(self) strongSelf = weakSelf;
         MGLMapView *mapView = strongSelf.mapView;
-        
+
         // In general, because order of notifications is not guaranteed
         // the following asserts are somewhat meaningless (don't do this in
         // production) - however, because we're mocking their delivery (and
@@ -258,7 +251,7 @@ typedef void (^MGLNotificationBlock)(NSNotification*);
     };
     
     [self.mockApplication enterForeground];
-    XCTAssert(displayLinkCount == 0, @"updateDisplayLink was called %ld times", displayLinkCount);
+    XCTAssert(displayLinkCount == 1, @"updateDisplayLink was called %ld times", displayLinkCount);
     [self waitForExpectations:@[willEnterForegroundExpectation] timeout:1.0];
     
     XCTAssertFalse(self.mapView.isDormant);
@@ -358,4 +351,126 @@ typedef void (^MGLNotificationBlock)(NSNotification*);
     XCTAssert(!self.mapView.displayLink || self.mapView.displayLink.isPaused);
 }
 
+
+- (void)testMovingMapViewToNewWindow {
+    XCTAssertNotNil(self.mapView.window);
+    XCTAssertFalse(self.mapView.isDormant);
+    XCTAssertFalse(self.mapView.displayLink.isPaused);
+    XCTAssert(self.mapView.application.applicationState == UIApplicationStateActive);
+    
+    __block NSInteger displayLinkCount = 0;
+    
+    self.displayLinkDidUpdate = ^{
+        displayLinkCount++;
+    };
+
+    UIWindow *window = [[UIWindow alloc] initWithFrame:self.mapView.bounds];
+    [window addSubview:self.mapView];
+    
+    XCTAssertEqualObjects(self.mapView.window, window);
+    XCTAssertFalse(self.mapView.isDormant);
+    XCTAssertFalse(self.mapView.displayLink.isPaused);
+    XCTAssert(displayLinkCount == 1);
+}
+
+// This test requires us to KVO the map view's window.screen, and tear down/setup
+// the display link accordingly
+- (void)testDisplayLinkWhenMovingMapViewToAnotherScreenPENDING {
+    XCTAssertNotNil(self.mapView.window);
+    XCTAssertFalse(self.mapView.isDormant);
+    XCTAssertFalse(self.mapView.displayLink.isPaused);
+    XCTAssert(self.mapView.application.applicationState == UIApplicationStateActive);
+
+    UIScreen *thisScreen = self.mapView.window.screen;
+    UIScreen * _Nonnull otherScreen = nil;
+    
+    for (UIScreen *screen in [UIScreen screens]) {
+        if (screen != thisScreen) {
+            otherScreen = screen;
+            break;
+        }
+    }
+    
+    if (!otherScreen) {
+        printf("warning: no secondary screen detected - attempting nil screen\n");
+    }
+    
+    __block NSInteger displayLinkCount = 0;
+    
+    self.displayLinkDidUpdate = ^{
+        displayLinkCount++;
+    };
+
+    self.mapView.window.screen = otherScreen;
+    
+    XCTAssert(self.mapView.isDormant || otherScreen);
+    XCTAssert(self.mapView.displayLink.isPaused || otherScreen);
+    XCTAssert(displayLinkCount == 0);
+
+    displayLinkCount = 0;
+    
+    self.mapView.window.screen = thisScreen;
+
+    XCTAssertFalse(self.mapView.isDormant);
+    XCTAssertFalse(self.mapView.displayLink.isPaused);
+    XCTAssert(displayLinkCount == 0);
+}
+
+// We don't currently include view hierarchy visibility in our notion of "visible"
+// so this test will fail at the moment.
+- (void)testDisplayLinkWhenHidingMapViewsParentViewPENDING {
+
+    // Move views around for test
+    UIView *mapView = self.mapView;
+    UIView *parentView = [[UIView alloc] initWithFrame:mapView.frame];
+    UIView *grandParentView = mapView.superview;
+    [grandParentView addSubview:parentView];
+    [parentView addSubview:mapView];
+    
+    [mapView.topAnchor constraintEqualToAnchor:parentView.topAnchor].active = YES;
+    [mapView.leftAnchor constraintEqualToAnchor:parentView.leftAnchor].active = YES;
+    [mapView.rightAnchor constraintEqualToAnchor:parentView.rightAnchor].active = YES;
+    [mapView.bottomAnchor constraintEqualToAnchor:parentView.bottomAnchor].active = YES;
+
+    [grandParentView.topAnchor constraintEqualToAnchor:parentView.topAnchor].active = YES;
+    [grandParentView.leftAnchor constraintEqualToAnchor:parentView.leftAnchor].active = YES;
+    [grandParentView.rightAnchor constraintEqualToAnchor:parentView.rightAnchor].active = YES;
+    [grandParentView.bottomAnchor constraintEqualToAnchor:parentView.bottomAnchor].active = YES;
+
+    
+    XCTAssertNotNil(self.mapView.window);
+    XCTAssertFalse(self.mapView.isDormant);
+    XCTAssertFalse(self.mapView.displayLink.isPaused);
+
+    // Hide the parent view
+    parentView.hidden = YES;
+
+    XCTAssertFalse(self.mapView.isDormant);
+    XCTAssert(self.mapView.displayLink.isPaused);
+
+    // Show the parent view
+    parentView.hidden = NO;
+    XCTAssertFalse(self.mapView.isDormant);
+    XCTAssertFalse(self.mapView.displayLink.isPaused);
+}
+
+// We don't currently include view hierarchy visibility in our notion of "visible"
+// so this test will fail at the moment.
+- (void)testDisplayLinkWhenHidingMapViewsWindowPENDING {
+    
+    XCTAssertNotNil(self.mapView.window);
+    XCTAssertFalse(self.mapView.isDormant);
+    XCTAssertFalse(self.mapView.displayLink.isPaused);
+    
+    // Hide the window
+    self.mapView.window.hidden = YES;
+    
+    XCTAssertFalse(self.mapView.isDormant);
+    XCTAssert(self.mapView.displayLink.isPaused);
+    
+    // Show the window
+    self.mapView.window.hidden = NO;
+    XCTAssertFalse(self.mapView.isDormant);
+    XCTAssertFalse(self.mapView.displayLink.isPaused);
+}
 @end
diff --git a/platform/ios/Integration Tests/MGLMapViewIntegrationTest.m b/platform/ios/Integration Tests/MGLMapViewIntegrationTest.m
index 2d329f3a09..4779762583 100644
--- a/platform/ios/Integration Tests/MGLMapViewIntegrationTest.m
+++ b/platform/ios/Integration Tests/MGLMapViewIntegrationTest.m
@@ -1,6 +1,8 @@
 #import "MGLMapViewIntegrationTest.h"
 
 @interface MGLMapView (MGLMapViewIntegrationTest)
+@property (nonatomic) CADisplayLink *displayLink;
+- (void)setNeedsGLDisplay;
 - (void)updateFromDisplayLink:(CADisplayLink *)displayLink;
 @end
 
@@ -18,7 +20,9 @@
         }
     }
 
-    [super invokeTest];
+    @autoreleasepool {
+        [super invokeTest];
+    }
 }
 
 - (NSString*)validAccessToken {
@@ -131,13 +135,17 @@
 
 - (void)waitForMapViewToFinishLoadingStyleWithTimeout:(NSTimeInterval)timeout {
     XCTAssertNil(self.styleLoadingExpectation);
+    XCTAssertNotNil(self.mapView.displayLink);
+    XCTAssert(!self.mapView.displayLink.paused);
+    
+    [self.mapView setNeedsGLDisplay];
     self.styleLoadingExpectation = [self expectationWithDescription:@"Map view should finish loading style."];
     [self waitForExpectations:@[self.styleLoadingExpectation] timeout:timeout];
 }
 
 - (void)waitForMapViewToBeRenderedWithTimeout:(NSTimeInterval)timeout {
     XCTAssertNil(self.renderFinishedExpectation);
-    [self.mapView setNeedsDisplay];
+    [self.mapView setNeedsGLDisplay];
     self.renderFinishedExpectation = [self expectationWithDescription:@"Map view should be rendered"];
     [self waitForExpectations:@[self.renderFinishedExpectation] timeout:timeout];
 }
diff --git a/platform/ios/Integration Tests/MGLMockApplication.m b/platform/ios/Integration Tests/MGLMockApplication.m
index 06a8e173f1..113f33f442 100644
--- a/platform/ios/Integration Tests/MGLMockApplication.m
+++ b/platform/ios/Integration Tests/MGLMockApplication.m
@@ -9,6 +9,11 @@
 @implementation MGLMockApplication
 
 - (void)dealloc {
+
+    if (_applicationState != UIApplicationStateActive) {
+        [self enterForeground];
+    }
+    
     if (_delegate) {
         CFRelease((CFTypeRef)_delegate);
     }
diff --git a/platform/ios/src/MGLApplication_Private.h b/platform/ios/src/MGLApplication_Private.h
index a321224c2a..b41233e0cf 100644
--- a/platform/ios/src/MGLApplication_Private.h
+++ b/platform/ios/src/MGLApplication_Private.h
@@ -12,5 +12,13 @@ NS_ASSUME_NONNULL_BEGIN
 - (BOOL)openURL:(NSURL*)url NS_DEPRECATED_IOS(2_0, 10_0, "Please use openURL:options:completionHandler: instead") NS_EXTENSION_UNAVAILABLE_IOS("");
 @end
 
+// Conform UIApplication
+@interface UIApplication (MGLApplicationConformation) <MGLApplication>
+@end
+
+@protocol MGLApplicationProvider <NSObject>
+- (id<MGLApplication>)applicationForSender:(id)sender;
+@end
+
 NS_ASSUME_NONNULL_END
 
diff --git a/platform/ios/src/MGLMapView.mm b/platform/ios/src/MGLMapView.mm
index 6f053f0ac2..6fc859511a 100644
--- a/platform/ios/src/MGLMapView.mm
+++ b/platform/ios/src/MGLMapView.mm
@@ -192,10 +192,6 @@ public:
     NSString *viewReuseIdentifier;
 };
 
-@interface UIApplication (MGLApplicationConformation) <MGLApplication>
-@end
-
-
 #pragma mark - Private -
 
 @interface MGLMapView () <UIGestureRecognizerDelegate,
@@ -277,6 +273,7 @@ public:
 // Application properties
 @property (nonatomic) id<MGLApplication> application;
 @property (nonatomic) CADisplayLink *displayLink;
+@property (nonatomic) UIScreen *displayLinkScreen;
 @property (nonatomic) UIImage *lastSnapshotImage;
 
 
@@ -485,6 +482,8 @@ public:
     _opaque = NO;
     _atLeastiOS_12_2_0 = [NSProcessInfo.processInfo isOperatingSystemAtLeastVersion:(NSOperatingSystemVersion){12,2,0}];
 
+    // TODO: For testing, the mocked application is set after this point. Consider
+    // postponing until the correct application exists.
     BOOL background = _application.applicationState == UIApplicationStateBackground;
     if (!background)
     {
@@ -535,7 +534,7 @@ public:
     NSAssert(!_mbglMap, @"_mbglMap should be NULL");
     _mbglMap = new mbgl::Map(*_rendererFrontend, *_mbglView, *_mbglThreadPool, mapOptions, resourceOptions);
 
-    // start paused if in IB
+    // start paused if launched into the background
     if (background) {
         self.dormant = YES;
     }
@@ -691,7 +690,7 @@ public:
     _pendingLongitude = NAN;
     _targetCoordinate = kCLLocationCoordinate2DInvalid;
 
-    if ([UIApplication sharedApplication].applicationState != UIApplicationStateBackground) {
+    if (self.application.applicationState != UIApplicationStateBackground) {
         [MGLMapboxEvents pushTurnstileEvent];
         [MGLMapboxEvents pushEvent:MMEEventTypeMapLoad withAttributes:@{}];
     }
@@ -816,7 +815,7 @@ public:
         [self removeAnnotations:annotations];
     }
 
-    [self validateDisplayLink];
+    [self destroyDisplayLink];
 
     [self destroyCoreObjects];
 
@@ -849,6 +848,15 @@ public:
 
     _delegate = delegate;
 
+    if ([delegate conformsToProtocol:@protocol(MGLApplicationProvider)])
+    {
+        self.application = [(id<MGLApplicationProvider>)delegate applicationForSender:self];
+    }
+    else
+    {
+        self.application = [UIApplication sharedApplication];
+    }
+    
     _delegateHasAlphasForShapeAnnotations = [_delegate respondsToSelector:@selector(mapView:alphaForShapeAnnotation:)];
     _delegateHasStrokeColorsForShapeAnnotations = [_delegate respondsToSelector:@selector(mapView:strokeColorForShapeAnnotation:)];
     _delegateHasFillColorsForShapeAnnotations = [_delegate respondsToSelector:@selector(mapView:fillColorForPolygonAnnotation:)];
@@ -1156,7 +1164,7 @@ public:
     if (displayLink && displayLink != _displayLink) {
         return;
     }
-    
+
     if (_needsDisplayRefresh)
     {
         _needsDisplayRefresh = NO;
@@ -1224,44 +1232,103 @@ public:
 
     if ( ! self.dormant)
     {
-        [self validateDisplayLink];
+        [self.displayLink invalidate];
+        self.displayLink = nil;
         self.dormant = YES;
+        
+        if (_rendererFrontend)
+        {
+            _rendererFrontend->reduceMemoryUse();
+        }
+
         [self.glView deleteDrawable];
     }
 
     [self destroyCoreObjects];
 }
 
+- (BOOL)displayLinkShouldExist
+{
+    BOOL active = (self.application.applicationState == UIApplicationStateActive) ||
+                    [self supportsBackgroundRendering];
+    return active;
+}
+
+- (BOOL)mapViewIsVisible
+{
+    // "Visible" is not strictly true here - for example, the view hiearchy is not
+    // currently observed (e.g. looking at a parent's or the window's hidden
+    // status.
+    BOOL isVisible = self.superview && self.window && !self.isHidden;
+    return isVisible;
+}
+
+- (void)createDisplayLink
+{
+    MGLAssert(!self.displayLinkScreen, @"");
+    MGLAssert(!self.displayLink, @"");
+    MGLAssert(self.window.screen, @"");
+    
+    self.displayLinkScreen  = self.window.screen;
+    self.displayLink        = [self.window.screen displayLinkWithTarget :self selector :@selector(updateFromDisplayLink :)];
+    self.displayLink.paused = YES;
+
+    [self updateDisplayLinkPreferredFramesPerSecond];
+
+    [self.displayLink addToRunLoop:[NSRunLoop currentRunLoop] forMode:NSRunLoopCommonModes];
+}
+
+- (void)destroyDisplayLink
+{
+    [self.displayLink invalidate];
+    self.displayLink = nil;
+    self.displayLinkScreen = nil;
+}
+
+- (void)startDisplayLink
+{
+    MGLAssert(self.displayLink, @"");
+    MGLAssert([self mapViewIsVisible], @"Display link should only be started when allowed");
+    
+    self.displayLink.paused = NO;
+    [self setNeedsGLDisplay];
+    [self updateFromDisplayLink:self.displayLink];
+}
+
+- (void)stopDisplayLink
+{
+    self.displayLink.paused = YES;
+}
+
 - (void)validateDisplayLink
 {
-    BOOL isVisible = self.superview && self.window;
+    MGLAssert(!self.displayLink, @"");
     
-    if (isVisible && ! _displayLink)
+    if ([self displayLinkShouldExist])
     {
-        BOOL active = (self.application.applicationState == UIApplicationStateActive);
-
-        if (_mbglMap && self.mbglMap.getMapOptions().constrainMode() == mbgl::ConstrainMode::None)
+        if (!self.displayLink)
         {
-            self.mbglMap.setConstrainMode(mbgl::ConstrainMode::HeightOnly);
-        }
-        
-        _displayLink = [self.window.screen displayLinkWithTarget:self selector:@selector(updateFromDisplayLink:)];
-        
-        _displayLink.paused = !active;
-        
-        [self updateDisplayLinkPreferredFramesPerSecond];
-        [_displayLink addToRunLoop:[NSRunLoop currentRunLoop] forMode:NSRunLoopCommonModes];
-        
-        if (active)
-        {
-            _needsDisplayRefresh = YES;
-            [self updateFromDisplayLink:_displayLink];
+            // TODO: Move this logic, it doen't belong here.
+            if (_mbglMap && self.mbglMap.getMapOptions().constrainMode() == mbgl::ConstrainMode::None)
+            {
+                self.mbglMap.setConstrainMode(mbgl::ConstrainMode::HeightOnly);
+            }
+
+            [self createDisplayLink];
+            
+            // Now should it be running?
+            if ([self mapViewIsVisible])
+            {
+                [self startDisplayLink];
+            }
         }
     }
-    else if ( ! isVisible && _displayLink)
+    else
     {
-        [_displayLink invalidate];
-        _displayLink = nil;
+        if (self.displayLink)
+        {
+            [self destroyDisplayLink];
+        }
     }
 }
 
@@ -1383,10 +1450,11 @@ public:
         // appears to lessen the effects.
         CAEAGLLayer *eaglLayer = MGL_OBJC_DYNAMIC_CAST(_glView.layer, CAEAGLLayer);
         eaglLayer.presentsWithTransaction = NO;
-        
-        // Moved from didMoveToWindow
-        [self validateDisplayLink];
     }
+    
+    // Changing windows regardless of whether it's a new one, or the map is being
+    // removed from the hierarchy
+    [self destroyDisplayLink];
 }
 
 - (void)didMoveToWindow
@@ -1405,7 +1473,6 @@ public:
 
 - (void)didMoveToSuperview
 {
-    [self validateDisplayLink];
     [self installConstraints];
     [super didMoveToSuperview];
 }
@@ -1417,7 +1484,7 @@ public:
     // the app delegate's application:supportedInterfaceOrientationsForWindow:
     // method) and the device's supported orientations to determine whether to rotate.
     
-    UIApplication *application = [UIApplication sharedApplication];
+    id<MGLApplication> application = self.application;
     
     if (window && [application.delegate respondsToSelector:@selector(application:supportedInterfaceOrientationsForWindow:)]) {
         self.applicationSupportedInterfaceOrientations = [application.delegate application:application supportedInterfaceOrientationsForWindow:window];
@@ -1555,6 +1622,8 @@ public:
     {
         _rendererFrontend->reduceMemoryUse();
     }
+    
+    [self stopDisplayLink];
 }
 
 - (void)sleepGL:(__unused NSNotification *)notification
@@ -1577,6 +1646,8 @@ public:
         _rendererFrontend->reduceMemoryUse();
     }
     
+    [self destroyDisplayLink];
+    
     if ( ! self.dormant)
     {
         self.dormant = YES;
@@ -1627,7 +1698,7 @@ public:
 
         [self.glView bindDrawable];
 
-        [self validateDisplayLink];
+        [self createDisplayLink];
         
         [self validateLocationServices];
 
@@ -1636,12 +1707,19 @@ public:
     }
 }
 
+
 - (void)resumeGL
 {
     self.lastSnapshotImage = nil;
 
     // Only restart the display link if we're not hidden
-    self.displayLink.paused = self.hidden;
+//    self.displayLink.paused = ![self displayLinkShouldRun];
+    MGLAssert(self.displayLink, @"");
+    
+    if ([self mapViewIsVisible])
+    {
+        [self startDisplayLink];
+    }
 }
 
 
@@ -1649,9 +1727,7 @@ public:
 {
     super.hidden = hidden;
     
-    BOOL inactive = (self.application.applicationState != UIApplicationStateActive);
-    
-    self.displayLink.paused = hidden || inactive;
+    self.displayLink.paused = ![self mapViewIsVisible];
 }
 
 - (void)tintColorDidChange