fix(ios): avoid leaking objects in hasProperty callback (7_5_X) (#10525)

* fix(ios): avoid leaking objects in hasProperty callback

* do it the objc way

* add unit tests

* add test for TIMOB-26452

* mark broken tests on android

iphone/Classes/KrollBridge.m

@@ -100,6 +100,24 @@ - (void)gc
 {
 }
 
+- (BOOL)hasProperty:(NSString *)propertyName
+{
+  if (dynprops != nil && dynprops[propertyName] != nil) {
+    return YES;
+  }
+
+  id module = modules[propertyName];
+  if (module != nil) {
+    return YES;
+  }
+  module = [host moduleNamed:propertyName context:pageContext];
+  if (module != nil) {
+    return YES;
+  }
+
+  return [super hasProperty:propertyName];
+}
+
 - (id)valueForKey:(NSString *)key
 {
   // allow dynprops to override built-in modules

iphone/Classes/KrollObject.m

@@ -182,12 +182,7 @@ bool KrollHasProperty(TiContextRef jsContext, TiObjectRef object, TiStringRef pr
 
   NSString *name = (NSString *)TiStringCopyCFString(kCFAllocatorDefault, propertyName);
   [name autorelease];
-  id result = [o valueForKey:name];
-  if (result != nil) {
-    return true;
-  }
-
-  return false;
+  return [o hasProperty:name];
 }
 
 //
@@ -833,6 +828,82 @@ - (id)_valueForKey:(NSString *)key
   return nil;
 }
 
+/**
+ Checks if a property with the given name exists on our target.
+
+ Contains all the magic of valueForKey withouth trying to retrieve any actual
+ value.
+
+ The checks for property existance are done in the following order:
+  * The Kroll object's own statics and properties cache
+  * Dynamic getter and setter in the form of getSomeProperty or setSomeProperty
+  * Property on the actual target
+  * "toString" and "valueOf" are always available on all objects
+  * "className" has a special handling with valueForUndefinedKey, return true
+    for the sake of simplicity
+  * Method with the same name on the target and single parameter
+  * Method with the same name on the target and no parameter
+  * Create factory method
+
+ As soon as one of the above checks passes this method returns true, meaning
+ the property exists. If none of the checks passed the property does not exists
+ and the method returns false.
+
+ @param propertyName The property name to check for.
+ */
+- (BOOL)hasProperty:(NSString *)propertyName
+{
+  if (statics != nil && statics[propertyName] != nil) {
+    return YES;
+  }
+
+  if (properties != nil && properties[propertyName] != nil) {
+    return YES;
+  }
+
+  if (([propertyName hasPrefix:@"get"] || [propertyName hasPrefix:@"set"]) && (propertyName.length >= 4) &&
+      [NSCharacterSet.uppercaseLetterCharacterSet characterIsMember:[propertyName characterAtIndex:3]]) {
+    return YES;
+  }
+
+  objc_property_t p = class_getProperty([target class], propertyName.UTF8String);
+  if (p != NULL) {
+    return YES;
+  }
+
+  if ([propertyName isEqualToString:@"toString"] || [propertyName isEqualToString:@"valueOf"]) {
+    return YES;
+  }
+
+  if ([propertyName isEqualToString:@"className"]) {
+    return YES;
+  }
+
+  SEL selector = NSSelectorFromString([NSString stringWithFormat:@"%@:", propertyName]);
+  if ([target respondsToSelector:selector]) {
+    return YES;
+  }
+
+  selector = NSSelectorFromString([NSString stringWithFormat:@"%@", propertyName]);
+  if ([target respondsToSelector:selector]) {
+    return YES;
+  }
+
+  id result = [target valueForKey:propertyName];
+  if (result != nil) {
+    return YES;
+  }
+
+  if ([propertyName hasPrefix:@"create"]) {
+    SEL selector = @selector(createProxy:forName:context:);
+    if ([target respondsToSelector:selector]) {
+      return YES;
+    }
+  }
+
+  return NO;
+}
+
 - (id)valueForKey:(NSString *)key
 {
   BOOL executionSet = NO;

tests/Resources/core.runtime.addontest.js

@@ -0,0 +1,122 @@
+/*
+ * Appcelerator Titanium Mobile
+ * Copyright (c) 2011-Present by Appcelerator, Inc. All Rights Reserved.
+ * Licensed under the terms of the Apache Public License
+ * Please see the LICENSE included with this distribution for details.
+ */
+/* eslint-env mocha */
+/* global Ti */
+/* eslint no-unused-expressions: "off" */
+
+'use strict';
+
+var should = require('./utilities/assertions');
+
+describe('Core', () => {
+	describe('Runtime', () => {
+		describe('hasProperty', () => {
+			describe('Top-Module', () => {
+				describe('Submodules', () => {
+					// @fixme the not.have.property check throws an error on android
+					it.androidBroken('should check for sub-module', () => {
+						Ti.should.have.property('UI');
+						Ti.should.not.have.property('Foo');
+					});
+				});
+
+				describe('Custom properties', () => {
+					// @fixme the not.have.property check throws an error on android
+					it.androidBroken('should check for custom properties', () => {
+						Ti.should.not.have.property('custom');
+						Ti.custom = {};
+						Ti.should.have.property('custom');
+					});
+				});
+			});
+
+			describe('Proxy', () => {
+				describe('Getter/Setter', () => {
+					it('should check for existing getter method', () => {
+						const tabGroup = Ti.UI.createTabGroup();
+						tabGroup.should.have.property('getTabs');
+					});
+
+					it.ios('should check for dynamic getter method', () => {
+						const view = Ti.UI.createView();
+						view.should.have.property('getFoo');
+					});
+
+					it('should check for existing setter method', () => {
+						const webView = Ti.UI.createWebView();
+						webView.should.have.property('setHtml');
+					});
+
+					it.ios('should check for dynamic setter method', () => {
+						const view = Ti.UI.createView();
+						view.should.have.property('setFoo');
+					});
+				});
+
+				describe('Properties', () => {
+					it('should check for properties on the object\'s target', () => {
+						const view = Ti.UI.createView({
+							backgroundColor: 'black'
+						});
+						view.should.not.have.property('bollocks');
+						view.should.have.property('backgroundColor');
+					});
+
+					it('should check for custom properties', () => {
+						const view = Ti.UI.createView();
+						view.should.not.have.property('bollocks');
+						view.should.not.have.property('foo');
+						view.foo = 'bar';
+						view.should.have.property('foo');
+						view.foo.should.be.equal('bar');
+					});
+
+					it('should properly handle properties with value of nil (TIMOB-26452)', () => {
+						should(Ti.Geolocation).have.property('lastGeolocation');
+						if (Ti.Platform.osname === 'android') {
+							should(Ti.Geolocation.lastGeolocation).be.equal('{}');
+						} else {
+							should.not.exist(Ti.Geolocation.lastGeolocation);
+						}
+					});
+				});
+
+				describe('toString/valueOf', () => {
+					it('should be available on any proxy', () => {
+						const view = Ti.UI.createView();
+						view.should.have.property('toString');
+						view.should.have.property('valueOf');
+					});
+				});
+
+				describe('className', () => {
+					it.ios('should be availably on any proxy', () => {
+						const view = Ti.UI.createView();
+						view.should.have.property('className');
+					});
+				});
+
+				describe('Methods', () => {
+					it('should check for methods on the object\'s target', () => {
+						const view = Ti.UI.createView();
+						view.should.not.have.property('addSomething');
+						view.should.have.property('add');
+						should(view.add).be.a.Function;
+					});
+				});
+
+				describe('Factory Methods', () => {
+					it('should check for dynamic create factory method', () => {
+						Ti.UI.should.have.property('createView');
+						const view = Ti.UI.createView();
+						view.should.not.have.property('createSomething');
+					});
+				});
+			});
+		});
+	});
+});