Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix MasterDetailPage/NavigationPage leaks on iPad #426

Merged
merged 2 commits into from Oct 12, 2016

Conversation

@hartez
Copy link
Member

commented Oct 6, 2016

Description of Change

Fixes leaks preventing MasterDetailPage and its child pages from being garbage collected on iPad.

Bugs Fixed

API Changes

None

Behavioral Changes

None

PR Checklist

  • Has tests (if omitted, state reason in description)
  • Rebased on top of master at time of PR
  • Changes adhere to coding standard
  • Consolidate commits as makes sense
hartez added 2 commits Oct 5, 2016
@@ -121,7 +121,7 @@ public _44166MDP()

Master = new _44166Master();
Detail = new _44166Detail();
}
}

This comment has been minimized.

Copy link
@samhouts

samhouts Oct 7, 2016

Member

um...spaces?

This comment has been minimized.

Copy link
@hartez

hartez Oct 7, 2016

Author Member

Yeah, had to reinstall VS yesterday, and spaces is the default in VS. Thought I'd caught all of them, but I guess not.

This comment has been minimized.

Copy link
@StephaneDelcroix

StephaneDelcroix Oct 9, 2016

Member

from now on, I'll refer to @samhouts as Style Cop 👮

ClearControllers();
}

base.Dispose(disposing);

This comment has been minimized.

Copy link
@StephaneDelcroix

StephaneDelcroix Oct 9, 2016

Member

this is reshuffling to reduce nesting, right? or is there any change in it ?

This comment has been minimized.

Copy link
@hartez

hartez Oct 10, 2016

Author Member

It's a fix to the Dispose method not marking the item as _disposed = true when running from the finalizer, plus it adds the ClearControllers() call that was missing from the tablet version.

@rmarinho rmarinho merged commit 50ac8e0 into master Oct 12, 2016

3 of 5 checks passed

iOS10-UITests Finished TeamCity Build Xamarin.Forms :: Debug :: Cycle8 :: UI Tests :: OSX Test Cloud Package - Run iOS Unified iOS10 : Canceled (Exit co…
Details
iOS9-UITests Finished TeamCity Build Xamarin.Forms :: Debug :: Cycle8 :: UI Tests :: OSX Test Cloud Package - Run iOS Unified iOS9 : Tests failed: 1, p…
Details
Android-UITests Finished TeamCity Build Xamarin.Forms :: Debug :: Cycle8 :: UI Tests :: OSX Test Cloud Package - Run Android 6.0.1 : Tests passed: 348, ig…
Details
OSX-Debug-C8 Finished TeamCity Build Xamarin.Forms :: Debug :: Cycle8 :: OSX Debug : Running
Details
Windows-Debug-C8 Finished TeamCity Build Xamarin.Forms :: Debug :: Cycle8 :: Windows Debug : Tests passed: 3456, ignored: 8
Details
jassmith pushed a commit that referenced this pull request Oct 12, 2016
Fix MasterDetailPage/NavigationPage leaks on iPad (#426)
* Remove Master page property changed handler to eliminate leak

* Fix memory leaks with MasterDetailPage and NavigationPage on iOS

@hartez hartez deleted the fix-bugzilla44166_ios branch Nov 14, 2016

@samhouts samhouts modified the milestones: 2.3.4, 2.3.3 Jun 27, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
6 participants
You can’t perform that action at this time.