From 0548aaf8da68f104d87d3e36ecc53b31e73028aa Mon Sep 17 00:00:00 2001 From: Douglas Machado Date: Tue, 2 Aug 2022 13:00:18 -0300 Subject: [PATCH] fix(android): FragmentClass memory leak (#9983) * chore: resilience to create/destroy flow around actionItems Co-authored-by: Nathan Walker --- packages/core/ui/action-bar/action-bar-common.ts | 12 ++++++++---- packages/core/ui/action-bar/index.android.ts | 6 +++--- packages/core/ui/action-bar/index.ios.ts | 8 ++++---- packages/core/ui/core/view/index.android.ts | 10 +++++++++- 4 files changed, 24 insertions(+), 12 deletions(-) diff --git a/packages/core/ui/action-bar/action-bar-common.ts b/packages/core/ui/action-bar/action-bar-common.ts index 5249eb18f..f5610e95a 100644 --- a/packages/core/ui/action-bar/action-bar-common.ts +++ b/packages/core/ui/action-bar/action-bar-common.ts @@ -25,6 +25,10 @@ export class ActionBarBase extends View implements ActionBarDefinition { get navigationButton(): NavigationButton { return this._navigationButton; } + disposeNativeView() { + this._actionItems = null; + super.disposeNativeView(); + } set navigationButton(value: NavigationButton) { if (this._navigationButton !== value) { if (this._navigationButton) { @@ -114,7 +118,7 @@ export class ActionBarBase extends View implements ActionBarDefinition { get _childrenCount(): number { let actionViewsCount = 0; - this._actionItems.getItems().forEach((actionItem) => { + this._actionItems?.getItems().forEach((actionItem) => { if (actionItem.actionView) { actionViewsCount++; } @@ -140,7 +144,7 @@ export class ActionBarBase extends View implements ActionBarDefinition { public _addArrayFromBuilder(name: string, value: Array) { if (name === 'actionItems') { - this.actionItems.setItems(value); + this.actionItems?.setItems(value); } } @@ -162,13 +166,13 @@ export class ActionBarBase extends View implements ActionBarDefinition { callback(navigationButton); } - this.actionItems.getItems().forEach((actionItem) => { + this.actionItems?.getItems().forEach((actionItem) => { callback(actionItem); }); } public _isEmpty(): boolean { - if (this.title || this.titleView || (this.android && this.android.icon) || this.navigationButton || this.actionItems.getItems().length > 0) { + if (this.title || this.titleView || (this.android && this.android.icon) || this.navigationButton || this.actionItems?.getItems().length > 0) { return false; } diff --git a/packages/core/ui/action-bar/index.android.ts b/packages/core/ui/action-bar/index.android.ts index 0a1bcdd15..1c2801ebe 100644 --- a/packages/core/ui/action-bar/index.android.ts +++ b/packages/core/ui/action-bar/index.android.ts @@ -164,7 +164,7 @@ export class ActionBar extends ActionBarBase { if (value instanceof NavigationButton) { this.navigationButton = value; } else if (value instanceof ActionItem) { - this.actionItems.addItem(value); + this.actionItems?.addItem(value); } else if (value instanceof View) { this.titleView = value; } @@ -257,7 +257,7 @@ export class ActionBar extends ActionBarBase { // Find item with the right ID; let menuItem: ActionItem = undefined; - const items = this.actionItems.getItems(); + const items = this.actionItems?.getItems() ?? []; for (let i = 0; i < items.length; i++) { if ((items[i])._getItemId() === itemId) { menuItem = items[i]; @@ -352,7 +352,7 @@ export class ActionBar extends ActionBarBase { public _addActionItems() { const menu = this.nativeViewProtected.getMenu(); - const items = this.actionItems.getVisibleItems(); + const items = this.actionItems?.getVisibleItems() ?? []; menu.clear(); for (let i = 0; i < items.length; i++) { diff --git a/packages/core/ui/action-bar/index.ios.ts b/packages/core/ui/action-bar/index.ios.ts index 9aed20224..0b624cec6 100644 --- a/packages/core/ui/action-bar/index.ios.ts +++ b/packages/core/ui/action-bar/index.ios.ts @@ -148,7 +148,7 @@ export class ActionBar extends ActionBarBase { if (value instanceof NavigationButton) { this.navigationButton = value; } else if (value instanceof ActionItem) { - this.actionItems.addItem(value); + this.actionItems?.addItem(value); } else if (value instanceof View) { this.titleView = value; } @@ -290,7 +290,7 @@ export class ActionBar extends ActionBarBase { } private populateMenuItems(navigationItem: UINavigationItem) { - const items = this.actionItems.getVisibleItems(); + const items = this.actionItems?.getVisibleItems() ?? []; const leftBarItems = NSMutableArray.new(); const rightBarItems = NSMutableArray.new(); for (let i = 0; i < items.length; i++) { @@ -449,7 +449,7 @@ export class ActionBar extends ActionBarBase { View.measureChild(this, this.titleView, UNSPECIFIED, UNSPECIFIED); } - this.actionItems.getItems().forEach((actionItem) => { + this.actionItems?.getItems().forEach((actionItem) => { const actionView = actionItem.actionView; if (actionView) { View.measureChild(this, actionView, UNSPECIFIED, UNSPECIFIED); @@ -473,7 +473,7 @@ export class ActionBar extends ActionBarBase { } } - this.actionItems.getItems().forEach((actionItem) => { + this.actionItems?.getItems().forEach((actionItem) => { const actionView = actionItem.actionView; if (actionView && actionView.ios) { const measuredWidth = actionView.getMeasuredWidth(); diff --git a/packages/core/ui/core/view/index.android.ts b/packages/core/ui/core/view/index.android.ts index 5528f4b5a..809482fe1 100644 --- a/packages/core/ui/core/view/index.android.ts +++ b/packages/core/ui/core/view/index.android.ts @@ -324,7 +324,15 @@ export class View extends ViewCommon { constructor() { super(); - this.on(View.loadedEvent, () => setupAccessibleView(this)); + const weakRef = new WeakRef(this); + const handler = () => { + const owner = weakRef.get(); + if (owner) { + setupAccessibleView(owner); + owner.off(View.loadedEvent, handler); + } + }; + this.on(View.loadedEvent, handler); } // TODO: Implement unobserve that detach the touchListener.