From fd9ff189bab62404f09ad912356d6ec76e99a644 Mon Sep 17 00:00:00 2001 From: vakrilov Date: Mon, 11 May 2015 15:31:04 +0300 Subject: [PATCH 1/9] List view attaches with weakEventHandler --- ui/core/weak-event-listener.d.ts | 2 +- ui/list-view/list-view-common.ts | 42 +++++++++++++++++++++----------- ui/list-view/list-view.ios.ts | 15 +++++++++--- 3 files changed, 41 insertions(+), 18 deletions(-) diff --git a/ui/core/weak-event-listener.d.ts b/ui/core/weak-event-listener.d.ts index 09e1792bc..1637a062b 100644 --- a/ui/core/weak-event-listener.d.ts +++ b/ui/core/weak-event-listener.d.ts @@ -33,7 +33,7 @@ declare module "ui/core/weak-event-listener" { /** * A string to use as key for key value pair instance. */ - key?: string; + key: string; } /** diff --git a/ui/list-view/list-view-common.ts b/ui/list-view/list-view-common.ts index 7700f03e9..5d36a53b7 100644 --- a/ui/list-view/list-view-common.ts +++ b/ui/list-view/list-view-common.ts @@ -1,4 +1,5 @@ import observable = require("data/observable"); +import observableArray = require("data/observable-array"); import view = require("ui/core/view"); import proxy = require("ui/core/proxy"); import definition = require("ui/list-view"); @@ -6,32 +7,22 @@ import dependencyObservable = require("ui/core/dependency-observable"); import builder = require("ui/builder"); import label = require("ui/label"); import color = require("color"); +import weakEventListener = require("ui/core/weak-event-listener"); var ITEMS = "items"; var ITEMTEMPLATE = "itemTemplate"; var ISSCROLLING = "isScrolling"; var LISTVIEW = "ListView"; -var ITEMSCHANGED = "_itemsChanged"; -var CHANGE = "change"; var SEPARATORCOLOR = "separatorColor"; +var WEAKEVENTKEY = "_observableArrayChanged"; export module knownTemplates { export var itemTemplate = "itemTemplate"; } function onItemsPropertyChanged(data: dependencyObservable.PropertyChangeData) { - var listView = data.object; - var itemsChanged = listView[ITEMSCHANGED]; - - if (data.oldValue instanceof observable.Observable) { - (data.oldValue).off(CHANGE, itemsChanged); - } - - if (data.newValue instanceof observable.Observable) { - (data.newValue).on(CHANGE, itemsChanged); - } - - listView.refresh(); + var listView = data.object; + listView._onItemsPropertyChanged(data); } function onItemTemplatePropertyChanged(data: dependencyObservable.PropertyChangeData) { @@ -79,6 +70,7 @@ export class ListView extends view.View implements definition.ListView { ); private _itemsChanged: (args: observable.EventData) => void; + private _weakEventListenerOptions: weakEventListener.WeakEventListenerOptions; constructor() { super(); @@ -143,6 +135,28 @@ export class ListView extends view.View implements definition.ListView { lbl.text = this._getDataItem(index) + ""; return lbl; } + + + public _onItemsPropertyChanged(data: dependencyObservable.PropertyChangeData) { + if (data.oldValue instanceof observable.Observable && this._weakEventListenerOptions) { + weakEventListener.WeakEventListener.removeWeakEventListener(this._weakEventListenerOptions); + this._weakEventListenerOptions = null; + } + + if (data.newValue instanceof observable.Observable) { + this._weakEventListenerOptions = { + targetWeakRef: new WeakRef(this), + sourceWeakRef: new WeakRef(data.newValue), + eventName: observableArray.ObservableArray.changeEvent, + handler: this._itemsChanged, + handlerContext: this, + key: WEAKEVENTKEY + }; + weakEventListener.WeakEventListener.addWeakEventListener(this._weakEventListenerOptions); + } + + this.refresh(); + } } function getExports(instance: view.View): any { diff --git a/ui/list-view/list-view.ios.ts b/ui/list-view/list-view.ios.ts index ec73bd716..dd5799778 100644 --- a/ui/list-view/list-view.ios.ts +++ b/ui/list-view/list-view.ios.ts @@ -151,6 +151,7 @@ export class ListView extends common.ListView { private _delegate; private _heights: Array; private _preparingCell: boolean = false; + private _isDataDirty: boolean = false; constructor() { super(); @@ -172,6 +173,9 @@ export class ListView extends common.ListView { public onLoaded() { super.onLoaded(); + if (this._isDataDirty) { + this.refresh(); + } this._ios.delegate = this._delegate; } @@ -185,8 +189,13 @@ export class ListView extends common.ListView { } public refresh() { - this._ios.reloadData(); - this.requestLayout(); + if (this.isLoaded) { + this._ios.reloadData(); + this.requestLayout(); + this._isDataDirty = false; + } else { + this._isDataDirty = true; + } } public getHeight(index: number): number { @@ -242,7 +251,7 @@ export class ListView extends common.ListView { cell.contentView.addSubview(view.ios); this._addView(view); } - + this._prepareItem(view, indexPath.row); cellHeight = this._layoutCell(view, indexPath); } From 0e8ea034c7b57bbc9e52245738d08d948f22035c Mon Sep 17 00:00:00 2001 From: vakrilov Date: Mon, 11 May 2015 18:57:42 +0300 Subject: [PATCH 2/9] ListView memory leak tests --- apps/tests/ui/helper.ts | 7 ++- apps/tests/ui/list-view/list-view-tests.ts | 73 +++++++++++++++------- 2 files changed, 54 insertions(+), 26 deletions(-) diff --git a/apps/tests/ui/helper.ts b/apps/tests/ui/helper.ts index 04cff1d1b..bfda9c74e 100644 --- a/apps/tests/ui/helper.ts +++ b/apps/tests/ui/helper.ts @@ -11,6 +11,7 @@ import styling = require("ui/styling"); var DELTA = 0.1; export var ASYNC = 0.2; +export var MEMORY_ASYNC = 2; export function do_PageTest(test: (views: Array) => void, content: view.View, secondView: view.View, thirdView: view.View) { var newPage: page.Page; @@ -33,7 +34,7 @@ export function do_PageTest(test: (views: Array) => void, content: vi export function do_PageTest_WithButton(test: (views: Array) => void) { var newPage: page.Page; var btn: button.Button; - var pageFactory = function(): page.Page { + var pageFactory = function (): page.Page { newPage = new page.Page(); btn = new button.Button(); newPage.content = btn; @@ -76,7 +77,7 @@ export function do_PageTest_WithStackLayout_AndButton(test: (views: Array) => void, assert: (views: Array) => void) { - + var newPage: page.Page; var stackLayout; var btn; @@ -188,7 +189,7 @@ export function buildUIWithWeakRefAndInteract(createFunc: ( try { navigate(pageFactory); - TKUnit.waitUntilReady(() => { return testFinished; }); + TKUnit.waitUntilReady(() => { return testFinished; }, MEMORY_ASYNC); } finally { goBack(); diff --git a/apps/tests/ui/list-view/list-view-tests.ts b/apps/tests/ui/list-view/list-view-tests.ts index 5522104d9..c8562b228 100644 --- a/apps/tests/ui/list-view/list-view-tests.ts +++ b/apps/tests/ui/list-view/list-view-tests.ts @@ -430,28 +430,28 @@ export function test_loadMoreItems_not_raised_when_showing_many_items() { } export function test_usingAppLevelConvertersInListViewItems() { - var listView = new listViewModule.ListView(); + var listView = new listViewModule.ListView(); - var dateConverter = function (value, format) { - var result = format; - var day = value.getDate(); - result = result.replace("DD", month < 10 ? "0" + day : day); - var month = value.getMonth() + 1; - result = result.replace("MM", month < 10 ? "0" + month : month); - result = result.replace("YYYY", value.getFullYear()); - return result; - }; + var dateConverter = function (value, format) { + var result = format; + var day = value.getDate(); + result = result.replace("DD", month < 10 ? "0" + day : day); + var month = value.getMonth() + 1; + result = result.replace("MM", month < 10 ? "0" + month : month); + result = result.replace("YYYY", value.getFullYear()); + return result; + }; - app.resources["dateConverter"] = dateConverter; + app.resources["dateConverter"] = dateConverter; var data = new observableArray.ObservableArray(); - data.push({date: new Date()}); + data.push({ date: new Date() }); function testAction(views: Array) { - listView.itemTemplate = "