disable recycling on specific button (#4527)

* disable recycling on specific button
add more thorough test for view recycling
fix memory leak with android ActionBar
improve padding reset when view is recycled
improve reset of several controls

* stopping local animations when view is recycled
fix tns-ios version in tests/package.json

* Fix isClickable on android when reusing nativeView

apps/app/ui-tests-app/issues/issue-2942.xml

@@ -1,7 +1,8 @@
 <Page xmlns="http://schemas.nativescript.org/tns.xsd">
 <StackLayout>
   <Button height="50" width="100" text="hide keyboard" onTap="hideKeyboard" style.fontSize="8"/>
-  <Button height="50" width="100" loaded="onButtonLoaded" text="Click me 3rd (Android)" style.fontSize="8"/>
+  <Button height="50" width="100" loaded="onButtonLoaded" text="Click me 3rd (Android)" style.fontSize="8"
+    recycleNativeView="false"/>
   <ListView loaded="onListViewLoaded">
       <ListView.itemTemplate>
         <StackLayout>

tests/app/ui/helper.ts

@@ -279,13 +279,65 @@ export function nativeView_recycling_test(createNew: () => View, createLayout?:
     layout.addChild(newer);
     layout.addChild(first);
 
+    if (first.typeName !== "SearchBar") {
+        // There are way too many differences in native methods for search-bar.
+        compareUsingReflection(newer, first);
+    }
+
     TKUnit.assertEqual(newer.nativeView, nativeView, "nativeView not reused.");
     checkDefaults(newer, first, props, nativeGetters || defaultNativeGetters);
     checkDefaults(newer, first, styleProps, nativeGetters || defaultNativeGetters);
+
     layout.removeChild(newer);
     layout.removeChild(first);
 }
 
+function compareUsingReflection(recycledNativeView: View, newNativeView: View): void {
+    const recycled: android.view.View = recycledNativeView.nativeView;
+    const newer: android.view.View = newNativeView.nativeView;
+    TKUnit.assertNotEqual(recycled, newer);
+    const methods = newer.getClass().getMethods();
+    for (let i = 0, length = methods.length; i < length; i++) {
+        const method = methods[i];
+        const returnType = method.getReturnType();
+        const name = method.getName();
+        const skip = name.includes('ViewId')
+            || name.includes('Accessibility')
+            || name.includes('hashCode')
+            || name === 'getId'
+            || name === 'hasFocus'
+            || name === 'isDirty'
+            || name === 'toString';
+        if (skip || method.getParameterTypes().length > 0) {
+            continue;
+        }
+
+        if ((<any>java).lang.Comparable.class.isAssignableFrom(returnType)) {
+            const defValue = method.invoke(newer, null);
+            const currValue = method.invoke(recycled, null);
+            if (defValue !== currValue && defValue.compareTo(currValue) !== 0) {
+                throw new Error(`Actual: ${currValue}, Expected: ${defValue}, for method: ${method.getName()}`);
+            }
+        } else if (java.lang.String.class === returnType ||
+            java.lang.Character.class === returnType ||
+            (<any>java).lang.CharSequence.class === returnType ||
+            returnType === java.lang.Byte.TYPE ||
+            returnType === java.lang.Double.TYPE ||
+            returnType === java.lang.Float.TYPE ||
+            returnType === java.lang.Integer.TYPE ||
+            returnType === java.lang.Long.TYPE ||
+            returnType === java.lang.Short.TYPE ||
+            returnType === java.lang.Boolean.TYPE) {
+
+            const defValue = method.invoke(newer, null);
+            const currValue = method.invoke(recycled, null);
+            if ((currValue + '') !== (defValue + '')) {
+                throw new Error(`Actual: ${currValue}, Expected: ${defValue}, for method: ${method.getName()}`);
+            }
+        }
+    }
+}
+
 function checkDefaults(newer: View, first: View, props: Array<any>, nativeGetters: Map<string, (view) => any>): void {
     props.forEach(prop => {
         const name = (<any>prop).name;

tests/package.json

@@ -6,7 +6,7 @@
   "nativescript": {
     "id": "org.nativescript.UnitTestApp",
     "tns-ios": {
-      "version": "3.1.1"
+      "version": "3.1.0"
     },
     "tns-android": {
       "version": "3.1.1"

tns-core-modules/ui/action-bar/action-bar.android.ts

@@ -313,9 +313,13 @@ export class ActionBar extends ActionBarBase {
     }
 
     private static _setOnClickListener(item: ActionItem): void {
+        const weakRef = new WeakRef(item);
         item.actionView.android.setOnClickListener(new android.view.View.OnClickListener({
             onClick: function (v: android.view.View) {
-                item._raiseTap();
+                const owner = weakRef.get();
+                if (owner) {
+                    owner._raiseTap();
+                }
             }
         }));
     }

tns-core-modules/ui/animation/animation-common.ts

@@ -252,4 +252,4 @@ export abstract class AnimationBase implements AnimationBaseDefinition {
             curve: animation.curve
         });
     }
-}
+}

tns-core-modules/ui/animation/animation.android.ts

@@ -1,4 +1,7 @@
-import { AnimationDefinition } from ".";
+// Definitions.
+import { AnimationDefinition } from ".";
+import { View } from "../core/view";
+
 import { AnimationBase, Properties, PropertyAnimation, CubicBezierAnimationCurve, AnimationPromise, Color, traceWrite, traceEnabled, traceCategories } from "./animation-common";
 import {
     opacityProperty, backgroundColorProperty, rotateProperty,
@@ -88,6 +91,7 @@ export class Animation extends AnimationBase {
     private _propertyUpdateCallbacks: Array<Function>;
     private _propertyResetCallbacks: Array<Function>;
     private _valueSource: "animation" | "keyframe";
+    private _target: View;
 
     constructor(animationDefinitions: Array<AnimationDefinitionInternal>, playSequentially?: boolean) {
         super(animationDefinitions, playSequentially);
@@ -185,23 +189,17 @@ export class Animation extends AnimationBase {
             return;
         }
 
-        let i = 0;
+        this._propertyUpdateCallbacks.forEach(v => v());
-        let length = this._propertyUpdateCallbacks.length;
-        for (; i < length; i++) {
-            this._propertyUpdateCallbacks[i]();
-        }
         this._disableHardwareAcceleration();
         this._resolveAnimationFinishedPromise();
+        this._target._removeAnimation(this);
     }
 
     private _onAndroidAnimationCancel() { // tslint:disable-line 
-        let i = 0;
+        this._propertyResetCallbacks.forEach(v => v());
-        let length = this._propertyResetCallbacks.length;
-        for (; i < length; i++) {
-            this._propertyResetCallbacks[i]();
-        }
         this._disableHardwareAcceleration();
         this._rejectAnimationFinishedPromise();
+        this._target._removeAnimation(this);
     }
 
     private _createAnimators(propertyAnimation: PropertyAnimation): void {
@@ -225,14 +223,16 @@ export class Animation extends AnimationBase {
             throw new Error(`Animation value cannot be null or undefined; target: ${propertyAnimation.target}; property: ${propertyAnimation.property};`);
         }
 
+        this._target = propertyAnimation.target;
+
         let nativeArray;
-        let nativeView = <android.view.View>propertyAnimation.target.nativeView;
+        const nativeView = <android.view.View>propertyAnimation.target.nativeView;
-        let animators = new Array<android.animation.Animator>();
+        const animators = new Array<android.animation.Animator>();
-        let propertyUpdateCallbacks = new Array<Function>();
+        const propertyUpdateCallbacks = new Array<Function>();
-        let propertyResetCallbacks = new Array<Function>();
+        const propertyResetCallbacks = new Array<Function>();
         let originalValue1;
         let originalValue2;
-        let density = layout.getDisplayDensity();
+        const density = layout.getDisplayDensity();
         let xyObjectAnimators: any;
         let animatorSet: android.animation.AnimatorSet;
 
@@ -298,9 +298,10 @@ export class Animation extends AnimationBase {
                         propertyAnimation.target.style[backgroundColorProperty.name] = originalValue1;
                     } else {
                         propertyAnimation.target.style[backgroundColorProperty.keyframe] = originalValue1;
-                        if (propertyAnimation.target.nativeView && propertyAnimation.target[backgroundColorProperty.setNative]) {
+                    }
-                            propertyAnimation.target[backgroundColorProperty.setNative](propertyAnimation.target.style.backgroundColor);
+                    
-                        }
+                    if (propertyAnimation.target.nativeView && propertyAnimation.target[backgroundColorProperty.setNative]) {
+                        propertyAnimation.target[backgroundColorProperty.setNative](propertyAnimation.target.style.backgroundColor);
                     }
                 }));
                 animators.push(animator);
@@ -337,10 +338,11 @@ export class Animation extends AnimationBase {
                     } else {
                         propertyAnimation.target.style[translateXProperty.keyframe] = originalValue1;
                         propertyAnimation.target.style[translateYProperty.keyframe] = originalValue2;
-                        if (propertyAnimation.target.nativeView) {
+                    }
-                            propertyAnimation.target[translateXProperty.setNative](propertyAnimation.target.style.translateX);
+
-                            propertyAnimation.target[translateYProperty.setNative](propertyAnimation.target.style.translateY);
+                    if (propertyAnimation.target.nativeView) {
-                        }
+                        propertyAnimation.target[translateXProperty.setNative](propertyAnimation.target.style.translateX);
+                        propertyAnimation.target[translateYProperty.setNative](propertyAnimation.target.style.translateY);
                     }
                 }));
 
@@ -353,7 +355,7 @@ export class Animation extends AnimationBase {
             case Properties.scale:
                 scaleXProperty._initDefaultNativeValue(style);
                 scaleYProperty._initDefaultNativeValue(style);
-                
+
                 xyObjectAnimators = Array.create(android.animation.Animator, 2);
 
                 nativeArray = Array.create("float", 1);
@@ -381,10 +383,11 @@ export class Animation extends AnimationBase {
                     } else {
                         propertyAnimation.target.style[scaleXProperty.keyframe] = originalValue1;
                         propertyAnimation.target.style[scaleYProperty.keyframe] = originalValue2;
-                        if (propertyAnimation.target.nativeView) {
+                    }
-                            propertyAnimation.target[scaleXProperty.setNative](propertyAnimation.target.style.scaleX);
+
-                            propertyAnimation.target[scaleYProperty.setNative](propertyAnimation.target.style.scaleY);
+                    if (propertyAnimation.target.nativeView) {
-                        }
+                        propertyAnimation.target[scaleXProperty.setNative](propertyAnimation.target.style.scaleX);
+                        propertyAnimation.target[scaleYProperty.setNative](propertyAnimation.target.style.scaleY);
                     }
                 }));
 
@@ -408,9 +411,10 @@ export class Animation extends AnimationBase {
                         propertyAnimation.target.style[rotateProperty.name] = originalValue1;
                     } else {
                         propertyAnimation.target.style[rotateProperty.keyframe] = originalValue1;
-                        if (propertyAnimation.target.nativeView) {
+                    }
-                            propertyAnimation.target[rotateProperty.setNative](propertyAnimation.target.style.rotate);
+                    
-                        }
+                    if (propertyAnimation.target.nativeView) {
+                        propertyAnimation.target[rotateProperty.setNative](propertyAnimation.target.style.rotate);
                     }
                 }));
                 animators.push(android.animation.ObjectAnimator.ofFloat(nativeView, "rotation", nativeArray));

tns-core-modules/ui/core/view-base/view-base.ts

@@ -133,8 +133,7 @@ export abstract class ViewBase extends Observable implements ViewBaseDefinition
     public static loadedEvent = "loaded";
     public static unloadedEvent = "unloaded";
 
-    public recycleNativeView: boolean;
+    private _recycleNativeView: boolean;
-
     private _iosView: Object;
     private _androidView: Object;
     private _style: Style;
@@ -204,6 +203,7 @@ export abstract class ViewBase extends Observable implements ViewBaseDefinition
     public _defaultPaddingRight: number;
     public _defaultPaddingBottom: number;
     public _defaultPaddingLeft: number;
+    private _isPaddingRelative: boolean;
 
     constructor() {
         super();
@@ -216,6 +216,14 @@ export abstract class ViewBase extends Observable implements ViewBaseDefinition
         return types.getClass(this);
     }
 
+    get recycleNativeView(): boolean {
+        return this._recycleNativeView;
+    }
+
+    set recycleNativeView(value: boolean) {
+        this._recycleNativeView = typeof value === "boolean" ? value : booleanConverter(value);
+    }
+
     get style(): Style {
         return this._style;
     }
@@ -630,6 +638,7 @@ export abstract class ViewBase extends Observable implements ViewBaseDefinition
         if (this._styleScope === view._styleScope) {
             view._setStyleScope(null);
         }
+        
         if (view.isLoaded) {
             view.onUnloaded();
         }
@@ -662,9 +671,13 @@ export abstract class ViewBase extends Observable implements ViewBaseDefinition
 
     private resetNativeViewInternal(): void {
         const nativeView = this.nativeView;
-        if (nativeView && this.recycleNativeView && isAndroid) {
+        if (nativeView && this._recycleNativeView && isAndroid) {
             resetNativeView(this);
-            nativeView.setPadding(this._defaultPaddingLeft, this._defaultPaddingTop, this._defaultPaddingRight, this._defaultPaddingBottom);
+            if (this._isPaddingRelative) {
+                nativeView.setPaddingRelative(this._defaultPaddingLeft, this._defaultPaddingTop, this._defaultPaddingRight, this._defaultPaddingBottom);
+            } else {
+                nativeView.setPadding(this._defaultPaddingLeft, this._defaultPaddingTop, this._defaultPaddingRight, this._defaultPaddingBottom);
+            }
             this.resetNativeView();
         }
         if (this._cssState) {
@@ -689,7 +702,7 @@ export abstract class ViewBase extends Observable implements ViewBaseDefinition
 
         let nativeView;
         if (isAndroid) {
-            if (this.recycleNativeView) {
+            if (this._recycleNativeView) {
                 nativeView = <android.view.View>getNativeView(context, this.typeName);
             }
 
@@ -699,6 +712,10 @@ export abstract class ViewBase extends Observable implements ViewBaseDefinition
 
             this._androidView = nativeView;
             if (nativeView) {
+                if (this._isPaddingRelative === undefined) {
+                    this._isPaddingRelative = nativeView.isPaddingRelative();
+                }
+
                 let result: android.graphics.Rect = (<any>nativeView).defaultPaddings;
                 if (result === undefined) {
                     result = org.nativescript.widgets.ViewHelper.getPadding(nativeView);
@@ -789,7 +806,7 @@ export abstract class ViewBase extends Observable implements ViewBaseDefinition
         }
 
         const nativeView = this.nativeView;
-        if (nativeView && this.recycleNativeView && isAndroid) {
+        if (nativeView && this._recycleNativeView && isAndroid) {
             const nativeParent = isAndroid ? (<android.view.View>nativeView).getParent() : (<UIView>nativeView).superview;
             if (!nativeParent) {
                 putNativeView(this._context, this);
@@ -979,7 +996,7 @@ export const idProperty = new Property<ViewBase, string>({ name: "id", valueChan
 idProperty.register(ViewBase);
 
 export function booleanConverter(v: string): boolean {
-    let lowercase = (v + '').toLowerCase();
+    const lowercase = (v + '').toLowerCase();
     if (lowercase === "true") {
         return true;
     } else if (lowercase === "false") {

tns-core-modules/ui/core/view/view-common.ts

@@ -50,15 +50,17 @@ export abstract class ViewCommon extends ViewBase implements ViewDefinition {
     private _measuredWidth: number;
     private _measuredHeight: number;
 
+    private _isLayoutValid: boolean;
+    private _cssType: string;
+
+    private _localAnimations: Set<am.Animation>;
+
     _currentWidthMeasureSpec: number;
     _currentHeightMeasureSpec: number;
 
     _setMinWidthNative: (value: Length) => void;
     _setMinHeightNative: (value: Length) => void;
 
-    private _isLayoutValid: boolean;
-    private _cssType: string;
-
     public _gestureObservers = {};
 
     observe(type: GestureTypes, callback: (args: GestureEventData) => void, thisArg?: any): void {
@@ -714,8 +716,35 @@ export abstract class ViewCommon extends ViewBase implements ViewDefinition {
 
     public createAnimation(animation: any): am.Animation {
         ensureAnimationModule();
+        if (!this._localAnimations) {
+            this._localAnimations = new Set();
+        }
         animation.target = this;
-        return new animationModule.Animation([animation]);
+        const anim = new animationModule.Animation([animation]);
+        this._localAnimations.add(anim);
+        return anim;
+    }
+
+    public _removeAnimation(animation: am.Animation): boolean {
+        const localAnimations = this._localAnimations;
+        if (localAnimations && localAnimations.has(animation)) {
+            localAnimations.delete(animation);
+            if (animation.isPlaying) {
+                animation.cancel();
+            }
+
+            return true;
+        }
+
+        return false;
+    }
+
+    public resetNativeView(): void {
+        if (this._localAnimations) {
+            this._localAnimations.forEach(a => this._removeAnimation(a));
+        }
+
+        super.resetNativeView();
     }
 
     public toString(): string {

tns-core-modules/ui/core/view/view.android.ts

@@ -70,6 +70,7 @@ function initializeTouchListener(): void {
 }
 
 export class View extends ViewCommon {
+    private _isClickable: boolean;
     private touchListenerIsSet: boolean;
     private touchListener: android.view.View.OnTouchListener;
 
@@ -94,6 +95,7 @@ export class View extends ViewCommon {
         if (this.touchListenerIsSet) {
             this.nativeView.setOnTouchListener(null);
             this.touchListenerIsSet = false;
+            this.nativeView.setClickable(this._isClickable);
         }
 
         this._cancelAllAnimations();
@@ -104,6 +106,11 @@ export class View extends ViewCommon {
         return this._gestureObservers && Object.keys(this._gestureObservers).length > 0
     }
 
+    public initNativeView(): void {
+        super.initNativeView();
+        this._isClickable = this.nativeView.isClickable();
+    }
+
     private setOnTouchListener() {
         if (this.nativeView && this.hasGestureObservers()) {
             this.touchListenerIsSet = true;

tns-core-modules/ui/core/view/view.d.ts

@@ -558,6 +558,10 @@ export abstract class View extends ViewBase implements ApplyXmlAttributes {
      * @private
      */
     _redrawNativeBackground(value: any): void;
+    /**
+     * @private
+     */
+    _removeAnimation(animation: Animation): boolean
     //@endprivate
 
     /**

tns-core-modules/ui/html-view/html-view.android.ts

@@ -8,11 +8,21 @@ export class HtmlView extends HtmlViewBase {
     nativeView: android.widget.TextView;
 
     public createNativeView() {
-        const textView = new android.widget.TextView(this._context);
+        return new android.widget.TextView(this._context);
+    }
+
+    public initNativeView(): void {
+        super.initNativeView();
+        const nativeView = this.nativeView;
+
         // This makes the html <a href...> work
-        textView.setLinksClickable(true);
+        nativeView.setLinksClickable(true);
-        textView.setMovementMethod(android.text.method.LinkMovementMethod.getInstance());
+        nativeView.setMovementMethod(android.text.method.LinkMovementMethod.getInstance());
-        return textView;
+    }
+
+    public resetNativeView(): void {
+        super.resetNativeView();
+        this.nativeView.setAutoLinkMask(0);
     }
 
     [htmlProperty.getDefault](): string {

tns-core-modules/ui/search-bar/search-bar.android.ts

@@ -186,11 +186,14 @@ export class SearchBar extends SearchBarBase {
         this.nativeView.setQuery(text, false);
     }
     [hintProperty.getDefault](): string {
-        return "";
+        return null;
     }
     [hintProperty.setNative](value: string) {
-        const text = (value === null || value === undefined) ? '' : value.toString();
+        if (value === null || value === undefined) {
-        this.nativeView.setQueryHint(text);
+            this.nativeView.setQueryHint(null);
+        } else {
+            this.nativeView.setQueryHint(value.toString());
+        }
     }
     [textFieldBackgroundColorProperty.getDefault](): android.graphics.drawable.Drawable {
         const textView = this._getTextView();
@@ -202,7 +205,7 @@ export class SearchBar extends SearchBarBase {
             textView.setBackgroundColor(value.android);
         } else {
             org.nativescript.widgets.ViewHelper.setBackground(textView, value);
-        }        
+        }
     }
     [textFieldHintColorProperty.getDefault](): number {
         const textView = this._getTextView();
@@ -220,7 +223,7 @@ export class SearchBar extends SearchBarBase {
             const id = this.nativeView.getContext().getResources().getIdentifier("search_src_text", "id", pkgName);
             this._searchTextView = <android.widget.TextView>this.nativeView.findViewById(id);
         }
-        
+
         return this._searchTextView;
     }
 

tns-core-modules/ui/slider/slider.android.ts

@@ -69,6 +69,14 @@ export class Slider extends SliderBase {
         super.disposeNativeView();
     }
 
+    public resetNativeView(): void {
+        super.resetNativeView();
+        const nativeView = this.nativeView;
+        nativeView.setMax(100);
+        nativeView.setProgress(0);
+        nativeView.setKeyProgressIncrement(1);
+    }
+
     /**
      * There is no minValue in Android. We simulate this by subtracting the minValue from the native value and maxValue. 
      * We need this method to call native setMax and setProgress methods when minValue property is changed,
@@ -132,4 +140,4 @@ export class Slider extends SliderBase {
     [backgroundInternalProperty.setNative](value: Background) {
         //
     }
-}
+}

tns-core-modules/ui/text-base/text-base.android.ts

@@ -162,7 +162,7 @@ export class TextBase extends TextBaseCommon {
         switch (value) {
             case "initial":
             case "left":
-                this.nativeView.setGravity(android.view.Gravity.LEFT | verticalGravity);
+                this.nativeView.setGravity(android.view.Gravity.START | verticalGravity);
                 break;
 
             case "center":
@@ -170,7 +170,7 @@ export class TextBase extends TextBaseCommon {
                 break;
 
             case "right":
-                this.nativeView.setGravity(android.view.Gravity.RIGHT | verticalGravity);
+                this.nativeView.setGravity(android.view.Gravity.END | verticalGravity);
                 break;
         }
     }

tns-core-modules/ui/text-view/text-view.android.ts

@@ -7,7 +7,12 @@ export class TextView extends EditableTextBase implements TextViewDefinition {
 
     public _configureEditText(editText: android.widget.EditText) {
         editText.setInputType(android.text.InputType.TYPE_CLASS_TEXT | android.text.InputType.TYPE_TEXT_VARIATION_NORMAL | android.text.InputType.TYPE_TEXT_FLAG_CAP_SENTENCES | android.text.InputType.TYPE_TEXT_FLAG_MULTI_LINE | android.text.InputType.TYPE_TEXT_FLAG_NO_SUGGESTIONS);
-        editText.setGravity(android.view.Gravity.TOP | android.view.Gravity.LEFT);
+        editText.setGravity(android.view.Gravity.TOP | android.view.Gravity.START);
+    }
+
+    public resetNativeView(): void {
+        super.resetNativeView();
+        this.nativeView.setGravity(android.view.Gravity.TOP | android.view.Gravity.START);
     }
 }