diff --git a/WebDriverAgentLib/Categories/XCUIElement+FBUtilities.m b/WebDriverAgentLib/Categories/XCUIElement+FBUtilities.m index b627fc862f..023aa3ecc0 100644 --- a/WebDriverAgentLib/Categories/XCUIElement+FBUtilities.m +++ b/WebDriverAgentLib/Categories/XCUIElement+FBUtilities.m @@ -139,18 +139,21 @@ - (void)fb_waitUntilStableWithTimeout:(NSTimeInterval)timeout return; } - NSTimeInterval previousTimeout = FBConfiguration.waitForIdleTimeout; - BOOL previousQuiescence = self.application.fb_shouldWaitForQuiescence; - FBConfiguration.waitForIdleTimeout = timeout; - if (!previousQuiescence) { - self.application.fb_shouldWaitForQuiescence = YES; - } - [[[self.application applicationImpl] currentProcess] - fb_waitForQuiescenceIncludingAnimationsIdle:YES]; - if (previousQuiescence != self.application.fb_shouldWaitForQuiescence) { - self.application.fb_shouldWaitForQuiescence = previousQuiescence; + XCUIApplication *application = self.application; + // Recursive synchronization also covers nested stability checks on this thread. + @synchronized (FBConfiguration.class) { + NSTimeInterval previousTimeout = FBConfiguration.waitForIdleTimeout; + BOOL previousQuiescence = application.fb_shouldWaitForQuiescence; + @try { + FBConfiguration.waitForIdleTimeout = timeout; + application.fb_shouldWaitForQuiescence = YES; + [[[application applicationImpl] currentProcess] + fb_waitForQuiescenceIncludingAnimationsIdle:YES]; + } @finally { + application.fb_shouldWaitForQuiescence = previousQuiescence; + FBConfiguration.waitForIdleTimeout = previousTimeout; + } } - FBConfiguration.waitForIdleTimeout = previousTimeout; } - (void)fb_raiseStaleElementExceptionWithError:(NSError *)error __attribute__((noreturn)) diff --git a/WebDriverAgentLib/Commands/FBElementCommands.m b/WebDriverAgentLib/Commands/FBElementCommands.m index 9c2ea009cd..399d20ed9e 100644 --- a/WebDriverAgentLib/Commands/FBElementCommands.m +++ b/WebDriverAgentLib/Commands/FBElementCommands.m @@ -346,15 +346,15 @@ + (NSArray *)routes + (id)handlePressAndDragCoordinateWithVelocity:(FBRouteRequest *)request { - FBSession *session = request.session; + XCUIApplication *application = request.session.activeApplication; CGVector startOffset = CGVectorMake((CGFloat)[request.arguments[@"fromX"] doubleValue], (CGFloat)[request.arguments[@"fromY"] doubleValue]); XCUICoordinate *startCoordinate = [self.class gestureCoordinateWithOffset:startOffset - element:session.activeApplication]; + element:application]; CGVector endOffset = CGVectorMake((CGFloat)[request.arguments[@"toX"] doubleValue], (CGFloat)[request.arguments[@"toY"] doubleValue]); XCUICoordinate *endCoordinate = [self.class gestureCoordinateWithOffset:endOffset - element:session.activeApplication]; + element:application]; [startCoordinate pressForDuration:[request.arguments[@"pressDuration"] doubleValue] thenDragToCoordinate:endCoordinate withVelocity:[request.arguments[@"velocity"] doubleValue] diff --git a/WebDriverAgentLib/Utilities/FBImageProcessor.m b/WebDriverAgentLib/Utilities/FBImageProcessor.m index 7d2ada7593..e333940716 100644 --- a/WebDriverAgentLib/Utilities/FBImageProcessor.m +++ b/WebDriverAgentLib/Utilities/FBImageProcessor.m @@ -120,7 +120,9 @@ + (nullable NSData *)fixedImageDataWithImageData:(NSData *)imageData } UIGraphicsImageRendererFormat *format = [[UIGraphicsImageRendererFormat alloc] init]; - format.scale = scalingFactor; + // scaledSize already applies the requested factor in points. Preserve the + // image's pixels-per-point scale instead of applying that factor twice. + format.scale = image.scale; UIGraphicsImageRenderer *renderer = [[UIGraphicsImageRenderer alloc] initWithSize:scaledSize format:format]; UIImageOrientation desiredOrientation = orientation == nil diff --git a/WebDriverAgentLib/Utilities/FBXCAXClientProxy.m b/WebDriverAgentLib/Utilities/FBXCAXClientProxy.m index f84f803fb1..57d1660293 100644 --- a/WebDriverAgentLib/Utilities/FBXCAXClientProxy.m +++ b/WebDriverAgentLib/Utilities/FBXCAXClientProxy.m @@ -20,6 +20,7 @@ @interface FBXCAXClientProxy () @property (nonatomic) NSMutableDictionary *appsCache; +@property (nonatomic, nullable) id cachedSystemApplication; @end @@ -67,7 +68,16 @@ - (BOOL)setAXTimeout:(NSTimeInterval)timeout error:(NSError **)error - (id)systemApplication { - return [FBAXClient systemApplication]; + @synchronized (self) { + id systemApplication = self.cachedSystemApplication; + if (nil == systemApplication) { + // The system application's identity cannot change without it being killed, + // which takes WDA down with it, so it is safe to cache it forever. + systemApplication = [FBAXClient systemApplication]; + self.cachedSystemApplication = systemApplication; + } + return systemApplication; + } } - (NSDictionary *)defaultParameters diff --git a/WebDriverAgentLib/Utilities/FBXCodeCompatibility.m b/WebDriverAgentLib/Utilities/FBXCodeCompatibility.m index f2cf03a24a..db63f337ee 100644 --- a/WebDriverAgentLib/Utilities/FBXCodeCompatibility.m +++ b/WebDriverAgentLib/Utilities/FBXCodeCompatibility.m @@ -8,15 +8,20 @@ #import "FBXCodeCompatibility.h" +#import + #import "FBXCAXClientProxy.h" #import "FBConfiguration.h" #import "FBErrorBuilder.h" #import "FBLogger.h" +#import "FBRunLoopSpinner.h" #import "XCUIApplication+FBHelpers.h" #import "XCUIElementQuery.h" #import "FBXCTestDaemonsProxy.h" #import "XCTestManager_ManagerInterface-Protocol.h" +static const NSTimeInterval TESTMANAGERD_VERSION_TIMEOUT_SEC = 20.; + @implementation XCUIElementQuery (FBCompatibility) - (id)fb_uniqueSnapshotWithError:(NSError **)error @@ -85,12 +90,25 @@ NSInteger FBTestmanagerdVersion(void) dispatch_once(&getTestmanagerdVersion, ^{ id proxy = [FBXCTestDaemonsProxy testRunnerProxy]; if ([(NSObject *)proxy respondsToSelector:@selector(_XCT_exchangeProtocolVersion:reply:)]) { - [FBRunLoopSpinner spinUntilCompletion:^(void(^completion)(void)){ - [proxy _XCT_exchangeProtocolVersion:testmanagerdVersion reply:^(unsigned long long code) { - testmanagerdVersion = (NSInteger) code; - completion(); - }]; + __block atomic_bool didReply = false; + __block NSInteger receivedVersion = 0; + [proxy _XCT_exchangeProtocolVersion:testmanagerdVersion reply:^(unsigned long long code) { + receivedVersion = (NSInteger) code; + atomic_store(&didReply, true); + }]; + // Commands are served from the main thread, so a daemon that never replies + // must not block it forever. The run loop keeps spinning to deliver the reply + BOOL isReplied = [[[FBRunLoopSpinner new] + timeout:TESTMANAGERD_VERSION_TIMEOUT_SEC] + spinUntilTrue:^BOOL{ + return atomic_load(&didReply); }]; + if (isReplied) { + testmanagerdVersion = receivedVersion; + } else { + [FBLogger logFmt:@"Did not receive a testmanagerd protocol version reply within %.0f seconds; assuming the newest/full-featured protocol", TESTMANAGERD_VERSION_TIMEOUT_SEC]; + testmanagerdVersion = 0xFFFF; + } } else { testmanagerdVersion = 0xFFFF; } diff --git a/WebDriverAgentLib/Utilities/FBXPath.m b/WebDriverAgentLib/Utilities/FBXPath.m index 68db53ff25..b37a638d25 100644 --- a/WebDriverAgentLib/Utilities/FBXPath.m +++ b/WebDriverAgentLib/Utilities/FBXPath.m @@ -463,6 +463,19 @@ + (int)recordElementAttributes:(xmlTextWriterPtr)writer indexPath:(nullable NSString *)indexPath includedAttributes:(nullable NSSet *)includedAttributes { + return [self recordElementAttributes:writer forElement:element indexPath:indexPath + includedAttributes:includedAttributes siblingIndex:nil]; +} + ++ (int)recordElementAttributes:(xmlTextWriterPtr)writer + forElement:(id)element + indexPath:(nullable NSString *)indexPath + includedAttributes:(nullable NSSet *)includedAttributes + siblingIndex:(nullable NSNumber *)siblingIndex +{ + FBXCElementSnapshotWrapper *wrapped = [FBXCElementSnapshotWrapper ensureWrapped:element]; + // Calculated once for all dimension attributes + NSDictionary *rect = nil; for (Class attributeCls in FBElementAttribute.supportedAttributes) { // include all supported attributes by default unless enumerated explicitly if (includedAttributes && ![includedAttributes containsObject:attributeCls]) { @@ -478,8 +491,16 @@ + (int)recordElementAttributes:(xmlTextWriterPtr)writer !FBDoesElementSupportMinMaxValue(element.elementType)) { continue; } - int rc = [attributeCls recordWithWriter:writer - forElement:[FBXCElementSnapshotWrapper ensureWrapped:element]]; + int rc; + if (attributeCls == FBIndexAttribute.class && nil != siblingIndex) { + // The caller already knows the position, so do not scan the siblings to find it + rc = [attributeCls recordWithWriter:writer forValue:siblingIndex.stringValue]; + } else if ([attributeCls isSubclassOfClass:FBDimensionAttribute.class]) { + rect = rect ?: wrapped.wdRect; + rc = [attributeCls recordWithWriter:writer forValue:[rect[[attributeCls name]] description]]; + } else { + rc = [attributeCls recordWithWriter:writer forElement:wrapped]; + } if (rc < 0) { return rc; } @@ -518,6 +539,17 @@ + (int)writeXmlWithRootElement:(id)root elementStore:(nullable NSMutableDictionary *)elementStore includedAttributes:(nullable NSSet *)includedAttributes writer:(xmlTextWriterPtr)writer +{ + return [self writeXmlWithRootElement:root indexPath:indexPath elementStore:elementStore + includedAttributes:includedAttributes writer:writer siblingIndex:nil]; +} + ++ (int)writeXmlWithRootElement:(id)root + indexPath:(nullable NSString *)indexPath + elementStore:(nullable NSMutableDictionary *)elementStore + includedAttributes:(nullable NSSet *)includedAttributes + writer:(xmlTextWriterPtr)writer + siblingIndex:(nullable NSNumber *)siblingIndex { NSAssert((indexPath == nil && elementStore == nil) || (indexPath != nil && elementStore != nil), @"Either both or none of indexPath and elementStore arguments should be equal to nil", nil); @@ -537,7 +569,8 @@ + (int)writeXmlWithRootElement:(id)root rc = [self recordElementAttributes:writer forElement:root indexPath:indexPath - includedAttributes:includedAttributes]; + includedAttributes:includedAttributes + siblingIndex:siblingIndex]; if (rc < 0) { return rc; } @@ -553,7 +586,8 @@ + (int)writeXmlWithRootElement:(id)root indexPath:newIndexPath elementStore:elementStore includedAttributes:includedAttributes - writer:writer]; + writer:writer + siblingIndex:@(i)]; if (rc < 0) { return rc; } diff --git a/WebDriverAgentLib/Utilities/LRUCache/LRUCache.m b/WebDriverAgentLib/Utilities/LRUCache/LRUCache.m index 731d744046..683fbadf80 100644 --- a/WebDriverAgentLib/Utilities/LRUCache/LRUCache.m +++ b/WebDriverAgentLib/Utilities/LRUCache/LRUCache.m @@ -80,7 +80,7 @@ - (nullable LRUCacheNode *)moveNodeToHead:(nullable LRUCacheNode *)node } LRUCacheNode *nextNode = node.next; if (nil != nextNode) { - nextNode.prev = node.prev; + nextNode.prev = previousNode; } if (node == self.tailNode) { self.tailNode = previousNode; @@ -94,19 +94,22 @@ - (void)removeNode:(nullable LRUCacheNode *)node return; } + LRUCacheNode *previousNode = node.prev; if (nil != node.next) { - node.next.prev = node.prev; + node.next.prev = previousNode; } if (node == self.headNode) { self.headNode = node.next; } - if (nil != node.prev) { - node.prev.next = node.next; + if (nil != previousNode) { + previousNode.next = node.next; } if (node == self.tailNode) { - self.tailNode = node.prev; + self.tailNode = previousNode; } [self.store removeObjectForKey:(id)node.key]; + node.next = nil; + node.prev = nil; } - (nullable LRUCacheNode *)addNodeToHead:(nullable LRUCacheNode *)node diff --git a/WebDriverAgentLib/Utilities/LRUCache/LRUCacheNode.h b/WebDriverAgentLib/Utilities/LRUCache/LRUCacheNode.h index adde370195..e455391dac 100644 --- a/WebDriverAgentLib/Utilities/LRUCache/LRUCacheNode.h +++ b/WebDriverAgentLib/Utilities/LRUCache/LRUCacheNode.h @@ -26,8 +26,8 @@ NS_ASSUME_NONNULL_BEGIN @property (nonatomic, readonly) id key; /*! Pointer to the next node */ @property (nonatomic, nullable) LRUCacheNode *next; -/*! Pointer to the previous node */ -@property (nonatomic, nullable) LRUCacheNode *prev; +/*! Pointer to the previous node. Weak to avoid retain cycles between neighbour nodes */ +@property (nonatomic, weak, nullable) LRUCacheNode *prev; /** Factory method to create a new cache node with the given value and key diff --git a/WebDriverAgentTests/IntegrationTests/FBImageProcessorTests.m b/WebDriverAgentTests/IntegrationTests/FBImageProcessorTests.m index 4eddbc43e1..acc6b67af8 100644 --- a/WebDriverAgentTests/IntegrationTests/FBImageProcessorTests.m +++ b/WebDriverAgentTests/IntegrationTests/FBImageProcessorTests.m @@ -9,6 +9,7 @@ #import #import "FBImageProcessor.h" +@import UniformTypeIdentifiers; #import "FBIntegrationTestCase.h" @@ -80,3 +81,36 @@ + (CGSize)sizeFromSize:(CGSize)size scalingFactor:(CGFloat)scalingFactor { @end + +// Synthetic images keep these renderer regressions independent of the app's screen. +@interface FBImageRendererScalingTests : XCTestCase +@end + +@implementation FBImageRendererScalingTests + +- (void)testOrientationFixDoesNotApplyScalingTwice +{ + for (NSNumber *sourceScale in @[@1, @2, @3]) { + UIGraphicsImageRendererFormat *format = [UIGraphicsImageRendererFormat new]; + format.scale = sourceScale.doubleValue; + UIGraphicsImageRenderer *renderer = [[UIGraphicsImageRenderer alloc] + initWithSize:CGSizeMake(120, 80) format:format]; + NSData *data = [renderer PNGDataWithActions:^(UIGraphicsImageRendererContext *context) { + [UIColor.redColor setFill]; + [context fillRect:CGRectMake(0, 0, 120, 80)]; + }]; + FBImageProcessor *processor = [FBImageProcessor new]; + NSData *scaled = [processor scaledImageWithData:data + uti:UTTypePNG + scalingFactor:0.5 + compressionQuality:1.0 + error:nil]; + UIImage *image = [UIImage imageWithData:scaled]; + XCTAssertNotNil(image); + XCTAssertEqual(CGImageGetWidth(image.CGImage), (size_t)(60 * sourceScale.integerValue)); + XCTAssertEqual(CGImageGetHeight(image.CGImage), (size_t)(40 * sourceScale.integerValue)); + } +} + +@end + diff --git a/WebDriverAgentTests/UnitTests/FBElementUtilitiesTests.m b/WebDriverAgentTests/UnitTests/FBElementUtilitiesTests.m index 5dad1c79de..c8b462e663 100644 --- a/WebDriverAgentTests/UnitTests/FBElementUtilitiesTests.m +++ b/WebDriverAgentTests/UnitTests/FBElementUtilitiesTests.m @@ -12,12 +12,52 @@ #import "FBElement.h" #import "XCUIElementDouble.h" #import "FBElementUtils.h" +#import "FBConfiguration.h" +#import "XCUIElement+FBUtilities.h" +#import + +@interface FBQuiescenceApplicationDouble : NSObject +@property (nonatomic) BOOL fb_shouldWaitForQuiescence; +@property (nonatomic) NSTimeInterval observedTimeout; +@end +@implementation FBQuiescenceApplicationDouble +- (id)applicationImpl { return self; } +- (id)currentProcess { return self; } +- (void)fb_waitForQuiescenceIncludingAnimationsIdle:(BOOL)includingAnimations +{ + self.observedTimeout = FBConfiguration.waitForIdleTimeout; + @throw [NSException exceptionWithName:@"QuiescenceFailure" reason:@"test" userInfo:nil]; +} +@end @interface FBElementUtilitiesTests : XCTestCase @end @implementation FBElementUtilitiesTests +- (void)testStabilityWaitRestoresSettingsAfterAnException +{ + NSTimeInterval originalTimeout = FBConfiguration.waitForIdleTimeout; + FBQuiescenceApplicationDouble *application = [FBQuiescenceApplicationDouble new]; + XCUIElementDouble *element = [XCUIElementDouble new]; + element.application = (id)application; + SEL selector = @selector(fb_waitUntilStableWithTimeout:); +#pragma clang diagnostic push +#pragma clang diagnostic ignored "-Wcast-function-type-strict" + void (*wait)(id, SEL, NSTimeInterval) = (void (*)(id, SEL, NSTimeInterval)) + class_getMethodImplementation(XCUIElement.class, selector); +#pragma clang diagnostic pop + @try { + FBConfiguration.waitForIdleTimeout = 17; + XCTAssertThrowsSpecificNamed(wait(element, selector, 2), NSException, @"QuiescenceFailure"); + XCTAssertEqual(application.observedTimeout, 2); + XCTAssertFalse(application.fb_shouldWaitForQuiescence); + XCTAssertEqual(FBConfiguration.waitForIdleTimeout, 17); + } @finally { + FBConfiguration.waitForIdleTimeout = originalTimeout; + } +} + - (void)testTypesFiltering { NSMutableArray *elements = [NSMutableArray new]; XCUIElementDouble *el1 = [XCUIElementDouble new]; diff --git a/WebDriverAgentTests/UnitTests/FBLRUCacheTests.m b/WebDriverAgentTests/UnitTests/FBLRUCacheTests.m index e8c6528205..932e3bf95e 100644 --- a/WebDriverAgentTests/UnitTests/FBLRUCacheTests.m +++ b/WebDriverAgentTests/UnitTests/FBLRUCacheTests.m @@ -15,6 +15,40 @@ @interface FBLRUCacheTests : XCTestCase @implementation FBLRUCacheTests +- (void)testDestroyingCacheReleasesAllValues +{ + __weak NSObject *firstValue; + __weak NSObject *secondValue; + @autoreleasepool { + LRUCache *cache = [[LRUCache alloc] initWithCapacity:2]; + NSObject *first = [NSObject new]; + NSObject *second = [NSObject new]; + firstValue = first; + secondValue = second; + [cache setObject:first forKey:@"first"]; + [cache setObject:second forKey:@"second"]; + XCTAssertNotNil([cache objectForKey:@"first"]); + } + XCTAssertNil(firstValue); + XCTAssertNil(secondValue); +} + +- (void)testRemovingValueReleasesItWhileCacheRemainsAlive +{ + LRUCache *cache = [[LRUCache alloc] initWithCapacity:3]; + __weak NSObject *removedValue; + @autoreleasepool { + NSObject *value = [NSObject new]; + removedValue = value; + [cache setObject:@"oldest" forKey:@"oldest"]; + [cache setObject:value forKey:@"removed"]; + [cache setObject:@"newest" forKey:@"newest"]; + [cache removeObjectForKey:@"removed"]; + } + XCTAssertNil(removedValue); + XCTAssertEqualObjects(cache.allObjects, (@[@"newest", @"oldest"])); +} + - (void)assertArray:(NSArray *)array1 equalsTo:(NSArray *)array2 { XCTAssertEqualObjects(array1, array2);