Refactor visibility detection (#14)

* Refactor visibility detection

* Remove unneeded verification

* Update functional tests

* Tune hitpoint calculation for simple tap action

WebDriverAgentLib/Categories/XCUIElement+FBIsVisible.h

@@ -17,6 +17,11 @@ NS_ASSUME_NONNULL_BEGIN
 /*! Whether or not the element is visible */
 @property (atomic, readonly) BOOL fb_isVisible;
 
+/*! Visible rectange of the element relatively to its ancestors in container window hierarchy.
+ Element frame is returned instead if no parent window is detected.
+ The result may be equal to CGRectZero if the element is hidden */
+@property (readonly, nonatomic) CGRect fb_frameInWindow;
+
 @end
 
 
@@ -25,6 +30,11 @@ NS_ASSUME_NONNULL_BEGIN
 /*! Whether or not the element is visible */
 @property (atomic, readonly) BOOL fb_isVisible;
 
+/*! Visible rectange of the element relatively to its ancestors in container window hierarchy.
+ Element frame is returned instead if no parent window is detected.
+ The result may be equal to CGRectZero if the element is hidden */
+@property (readonly, nonatomic) CGRect fb_frameInWindow;
+
 @end
 
 NS_ASSUME_NONNULL_END

WebDriverAgentLib/Categories/XCUIElement+FBIsVisible.m

@@ -23,10 +23,35 @@ - (BOOL)fb_isVisible
   return self.fb_lastSnapshot.fb_isVisible;
 }
 
+- (CGRect)fb_frameInWindow
+{
+  return self.fb_lastSnapshot.fb_frameInWindow;
+}
+
 @end
 
 @implementation XCElementSnapshot (FBIsVisible)
 
+- (CGRect)fb_frameInContainer:(XCElementSnapshot *)container hierarchyIntersection:(nullable NSValue *)intersectionRectange
+{
+  CGRect currentRectangle = nil == intersectionRectange ? self.frame : [intersectionRectange CGRectValue];
+  XCElementSnapshot *parent = self.parent;
+  CGRect intersectionWithParent = CGRectIntersection(currentRectangle, parent.frame);
+  if (CGRectIsEmpty(intersectionWithParent) || parent == container) {
+    return intersectionWithParent;
+  }
+  return [parent fb_frameInContainer:container hierarchyIntersection:[NSValue valueWithCGRect:intersectionWithParent]];
+}
+
+- (CGRect)fb_frameInWindow
+{
+  XCElementSnapshot *parentWindow = [self fb_parentMatchingType:XCUIElementTypeWindow];
+  if (nil != parentWindow) {
+    return [self fb_frameInContainer:parentWindow hierarchyIntersection:nil];
+  }
+  return self.frame;
+}
+
 - (BOOL)fb_isVisible
 {
   CGRect frame = self.frame;
@@ -38,11 +63,8 @@ - (BOOL)fb_isVisible
     return [(NSNumber *)[self fb_attributeValue:FB_XCAXAIsVisibleAttribute] boolValue];
   }
   XCElementSnapshot *parentWindow = [self fb_parentMatchingType:XCUIElementTypeWindow];
-  // appFrame is always returned like the app is in portrait mode
-  // and all the further tests internally assume the app is in portrait mode even
-  // if it is in landscape. That is why we must get the parent's window frame in order
-  // to check if it intersects with the corresponding element's frame
-  if (nil != parentWindow && !CGRectIntersectsRect(frame, parentWindow.frame)) {
+  if (nil != parentWindow &&
+      CGRectIsEmpty([self fb_frameInContainer:parentWindow hierarchyIntersection:nil])) {
     return NO;
   }
   CGPoint midPoint = [self.suggestedHitpoints.lastObject CGPointValue];

WebDriverAgentLib/Utilities/FBBaseActionsSynthesizer.h

@@ -52,16 +52,6 @@ NS_ASSUME_NONNULL_BEGIN
  */
 - (BOOL)increaseDuration:(double)value;
 
-/**
- Recursively calculates the visible frame of the current element inside its container window.
- 
- @param selfSnapshot The snapshot of the current element
- @param frame The intersection between the current element's frame and the parent's one. Set to to nil for the initial call
- @param window The parent window of the current element's snapshot
- @return The coordinates of the visible element's rectange. If this rectange has zero width or height then this element is not visible
- */
-+ (CGRect)visibleFrameWithSnapshot:(XCElementSnapshot *)selfSnapshot currentIntersection:(nullable NSValue *)frame containerWindow:(XCElementSnapshot *)window;
-
 /**
  Calculate absolute gesture position on the screen based on provided element and positionOffset values.
  

WebDriverAgentLib/Utilities/FBBaseActionsSynthesizer.m

@@ -10,11 +10,12 @@
 #import "FBBaseActionsSynthesizer.h"
 
 #import "FBErrorBuilder.h"
+#import "FBLogger.h"
 #import "FBMacros.h"
 #import "FBMathUtils.h"
+#import "XCUIElement+FBIsVisible.h"
 #import "XCElementSnapshot.h"
 #import "XCElementSnapshot+FBHelpers.h"
-#import "XCElementSnapshot+FBHitPoint.h"
 #import "XCPointerEventPath.h"
 #import "XCSynthesizedEventRecord.h"
 #import "XCUIElement+FBUtilities.h"
@@ -40,51 +41,36 @@ - (BOOL)increaseDuration:(double)value
   return YES;
 }
 
-+ (CGRect)visibleFrameWithSnapshot:(XCElementSnapshot *)selfSnapshot currentIntersection:(nullable NSValue *)frame containerWindow:(XCElementSnapshot *)window
-{
-  XCElementSnapshot *parent = selfSnapshot.parent;
-  CGRect intersectionRect = frame == nil ?
-    CGRectIntersection(selfSnapshot.frame, parent.frame) :
-    CGRectIntersection([frame CGRectValue], parent.frame);
-  if (CGRectIsEmpty(intersectionRect) || parent == window) {
-    return intersectionRect;
-  }
-  return [self.class visibleFrameWithSnapshot:parent currentIntersection:[NSValue valueWithCGRect:intersectionRect] containerWindow:window];
-}
-
 - (nullable NSValue *)hitpointWithElement:(nullable XCUIElement *)element positionOffset:(nullable NSValue *)positionOffset error:(NSError **)error
 {
   CGPoint hitPoint;
   if (nil == element) {
     // Only absolute offset is defined
     hitPoint = [positionOffset CGPointValue];
   } else {
-    // The offset relative to an element is defined
+    // The offset relative to the element is defined
     XCElementSnapshot *snapshot = element.fb_lastSnapshot;
-    if (nil == positionOffset) {
-      hitPoint = snapshot.fb_hitPoint;
-      if (!CGPointEqualToPoint(hitPoint, CGPointMake(-1, -1))) {
-        return [NSValue valueWithCGPoint:hitPoint];
-      }
-    }
-    XCElementSnapshot *containerWindow = [snapshot fb_parentMatchingType:XCUIElementTypeWindow];
-    CGRect visibleFrame;
-    if (nil == containerWindow) {
-      visibleFrame = snapshot.frame;
-    } else {
-      visibleFrame = [self.class visibleFrameWithSnapshot:snapshot currentIntersection:nil containerWindow:containerWindow];
-    }
-    if (CGRectIsEmpty(visibleFrame)) {
-      NSString *description = [NSString stringWithFormat:@"The element '%@' is not visible on the screen", element];
+    CGRect frameInWindow = snapshot.fb_frameInWindow;
+    if (CGRectIsEmpty(frameInWindow)) {
+      NSString *description = [NSString stringWithFormat:@"The element '%@' is not visible on the screen", element.debugDescription];
       if (error) {
         *error = [[FBErrorBuilder.builder withDescription:description] build];
       }
       return nil;
     }
-    hitPoint = CGPointMake(visibleFrame.origin.x, visibleFrame.origin.y);
-    if (nil != positionOffset) {
+    if (nil == positionOffset) {
+      @try {
+        return [NSValue valueWithCGPoint:[snapshot hitPoint]];
+      } @catch (NSException *e) {
+        [FBLogger logFmt:@"Failed to fetch hit point for %@ - %@. Will use element frame in window for hit point calculation instead", element.debugDescription, e.reason];
+      }
+      hitPoint = CGPointMake(frameInWindow.origin.x + frameInWindow.size.width / 2, frameInWindow.origin.y + frameInWindow.size.height / 2);
+    } else {
+      CGPoint origin = snapshot.frame.origin;
+      hitPoint = CGPointMake(origin.x, origin.y);
       CGPoint offsetValue = [positionOffset CGPointValue];
       hitPoint = CGPointMake(hitPoint.x + offsetValue.x, hitPoint.y + offsetValue.y);
+      // TODO: Shall we throw an exception if hitPoint is out of the element frame?
     }
   }
   if (SYSTEM_VERSION_GREATER_THAN_OR_EQUAL_TO(@"10.0")) {

WebDriverAgentLib/Utilities/FBW3CActionsSynthesizer.m

@@ -16,6 +16,7 @@
 #import "FBMathUtils.h"
 #import "XCElementSnapshot+FBHitPoint.h"
 #import "XCElementSnapshot+FBHelpers.h"
+#import "XCUIElement+FBIsVisible.h"
 #import "XCUIElement+FBUtilities.h"
 #import "XCUIElement.h"
 #import "XCSynthesizedEventRecord.h"
@@ -127,26 +128,21 @@ - (nullable NSValue *)hitpointWithElement:(nullable XCUIElement *)element positi
     // Only absolute offset is defined
     hitPoint = [positionOffset CGPointValue];
   } else {
-    // An offset relative to an element is defined
+    // An offset relative to the element is defined
     XCElementSnapshot *snapshot = element.fb_lastSnapshot;
-    XCElementSnapshot *containerWindow = [snapshot fb_parentMatchingType:XCUIElementTypeWindow];
-    CGRect visibleFrame;
-    if (nil == containerWindow) {
-      visibleFrame = snapshot.frame;
-    } else {
-      visibleFrame = [self.class visibleFrameWithSnapshot:snapshot currentIntersection:nil containerWindow:containerWindow];
-    }
-    if (CGRectIsEmpty(visibleFrame)) {
-      NSString *description = [NSString stringWithFormat:@"The element '%@' is not visible on the screen", element];
+    if (CGRectIsEmpty(snapshot.fb_frameInWindow)) {
+      NSString *description = [NSString stringWithFormat:@"The element '%@' is not visible on the screen", element.debugDescription];
       if (error) {
         *error = [[FBErrorBuilder.builder withDescription:description] build];
       }
       return nil;
     }
-    hitPoint = CGPointMake(visibleFrame.origin.x + visibleFrame.size.width / 2, visibleFrame.origin.y + visibleFrame.size.height / 2);
+    CGRect frame = snapshot.frame;
+    hitPoint = CGPointMake(frame.origin.x + frame.size.width / 2, frame.origin.y + frame.size.height / 2);
     if (nil != positionOffset) {
       CGPoint offsetValue = [positionOffset CGPointValue];
       hitPoint = CGPointMake(hitPoint.x + offsetValue.x, hitPoint.y + offsetValue.y);
+      // TODO: Shall we throw an exception if hitPoint is out of the element frame?
     }
   }
   if (SYSTEM_VERSION_GREATER_THAN_OR_EQUAL_TO(@"10.0")) {

WebDriverAgentTests/IntegrationTests/FBAppiumTouchActionsIntegrationTests.m

@@ -344,7 +344,7 @@ - (void)testSwipePickerWheelWithRelativeCoordinates
     @{
       @"action": @"moveTo",
       @"options": @{
-          @"x": @(pickerFrame.origin.x),
+          @"x": @(pickerFrame.origin.x / 2),
           @"y": @(pickerFrame.origin.y),
           }
       },
@@ -369,7 +369,7 @@ - (void)testSwipePickerWheelWithAbsoluteCoordinates
     @{
       @"action": @"moveTo",
       @"options": @{
-          @"x": @(pickerFrame.origin.x),
+          @"x": @(pickerFrame.origin.x + pickerFrame.size.width / 2),
           @"y": @(pickerFrame.origin.y + pickerFrame.size.height),
           }
       },