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

Fixing more UIKit memory leaks #1863

Merged
merged 6 commits into from
Jan 30, 2017
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 3 additions & 3 deletions Frameworks/UIKit.Xaml/Button.xaml.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -189,9 +189,9 @@ void Button::OnApplyTemplate() {
////////////////////////////////////////////////////////////////////////////////////
// ObjectiveC Interop
////////////////////////////////////////////////////////////////////////////////////
UIKIT_XAML_EXPORT IInspectable* XamlCreateButton() {
auto button = ref new UIKit::Xaml::Button();
return InspectableFromObject(button).Detach();
UIKIT_XAML_EXPORT void XamlCreateButton(IInspectable** created) {
ComPtr<IInspectable> inspectable = InspectableFromObject(ref new UIKit::Xaml::Button());
*created = inspectable.Detach();
}

UIKIT_XAML_EXPORT void XamlRemovePointerEvents(const ComPtr<IInspectable>& inspectableButton) {
Expand Down
5 changes: 3 additions & 2 deletions Frameworks/UIKit.Xaml/ContentDialog.xaml.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -60,8 +60,9 @@ void ContentDialog::OnSelectionChanged(Platform::Object ^sender, SelectionChange
// ObjectiveC Interop
////////////////////////////////////////////////////////////////////////////////////

UIKIT_XAML_EXPORT IInspectable* XamlCreateContentDialog() {
return InspectableFromObject(ref new UIKit::Xaml::ContentDialog()).Detach();
UIKIT_XAML_EXPORT void XamlCreateContentDialog(IInspectable** created) {
ComPtr<IInspectable> inspectable = InspectableFromObject(ref new UIKit::Xaml::ContentDialog());
*created = inspectable.Detach();
}

UIKIT_XAML_EXPORT int XamlContentDialogPressedIndex(const ComPtr<IInspectable>& inspectableContentDialog) {
Expand Down
5 changes: 3 additions & 2 deletions Frameworks/UIKit.Xaml/Label.xaml.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -83,8 +83,9 @@ TextBlock^ Label::TextBlock::get() {
////////////////////////////////////////////////////////////////////////////////////

// Returns a UIKit::Label as an IInspectable
UIKIT_XAML_EXPORT IInspectable* XamlCreateLabel() {
return InspectableFromObject(ref new UIKit::Xaml::Label()).Detach();
UIKIT_XAML_EXPORT void XamlCreateLabel(IInspectable** created) {
ComPtr<IInspectable> inspectable = InspectableFromObject(ref new UIKit::Xaml::Label());
*created = inspectable.Detach();
}

// Retrieves the UIKit::Label's backing TextBlock as an IInspectable
Expand Down
14 changes: 7 additions & 7 deletions Frameworks/UIKit.Xaml/ObjCXamlControls.h
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ enum ControlStates { ControlStateNormal = 0, ControlStateHighlighted = 1 << 0, C
////////////////////////////////////////////////////////////////////////////////////

// Returns a UIKit::Button as an IInspectable
UIKIT_XAML_EXPORT IInspectable* XamlCreateButton();
UIKIT_XAML_EXPORT void XamlCreateButton(IInspectable** created);

UIKIT_XAML_EXPORT void XamlButtonApplyVisuals(const Microsoft::WRL::ComPtr<IInspectable>& inspectableButton,
const Microsoft::WRL::ComPtr<IInspectable>& inspectableText,
Expand Down Expand Up @@ -57,7 +57,7 @@ UIKIT_XAML_EXPORT void XamlRemoveLayoutEvent(const Microsoft::WRL::ComPtr<IInspe
////////////////////////////////////////////////////////////////////////////////////

// Returns a UIKit::Label as an IInspectable
UIKIT_XAML_EXPORT IInspectable* XamlCreateLabel();
UIKIT_XAML_EXPORT void XamlCreateLabel(IInspectable** created);

// Retrieves the UIKit::Label's backing TextBlock as an IInspectable
UIKIT_XAML_EXPORT IInspectable* XamlGetLabelTextBox(const Microsoft::WRL::ComPtr<IInspectable>& label);
Expand All @@ -82,35 +82,35 @@ UIKIT_XAML_EXPORT IInspectable* XamlGetFrameworkElementSublayerCanvasProperty(co
////////////////////////////////////////////////////////////////////////////////////

// Returns a UIKit::ProgressRing as an IInspectable
UIKIT_XAML_EXPORT IInspectable* XamlCreateProgressRing();
UIKIT_XAML_EXPORT void XamlCreateProgressRing(IInspectable** created);

////////////////////////////////////////////////////////////////////////////////////
// ScrollViewer.xaml.cpp
////////////////////////////////////////////////////////////////////////////////////

// Returns a UIKit::ScrollViewer as an IInspectable
UIKIT_XAML_EXPORT IInspectable* XamlCreateScrollViewer();
UIKIT_XAML_EXPORT void XamlCreateScrollViewer(IInspectable** created);

////////////////////////////////////////////////////////////////////////////////////
// Slider.xaml.cpp
////////////////////////////////////////////////////////////////////////////////////

// Returns a UIKit::Slider as an IInspectable
UIKIT_XAML_EXPORT IInspectable* XamlCreateSlider();
UIKIT_XAML_EXPORT void XamlCreateSlider(IInspectable** created);

////////////////////////////////////////////////////////////////////////////////////
// TextBox.xaml.cpp
////////////////////////////////////////////////////////////////////////////////////

// Returns a UIKit::TextBox as an IInspectable
UIKIT_XAML_EXPORT IInspectable* XamlCreateTextBox();
UIKIT_XAML_EXPORT void XamlCreateTextBox(IInspectable** created);

////////////////////////////////////////////////////////////////////////////////////
// ContentDialog.xaml.cpp
////////////////////////////////////////////////////////////////////////////////////

// Returns a UIKit::ContentDialog as an IInspectable
UIKIT_XAML_EXPORT IInspectable* XamlCreateContentDialog();
UIKIT_XAML_EXPORT void XamlCreateContentDialog(IInspectable** created);

// Get the index of the button pressed
UIKIT_XAML_EXPORT int XamlContentDialogPressedIndex(const Microsoft::WRL::ComPtr<IInspectable>& inspectableContentDialog);
Expand Down
5 changes: 3 additions & 2 deletions Frameworks/UIKit.Xaml/ProgressRing.xaml.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -31,8 +31,9 @@ ProgressRing::ProgressRing() {
////////////////////////////////////////////////////////////////////////////////////
// ObjectiveC Interop
////////////////////////////////////////////////////////////////////////////////////
UIKIT_XAML_EXPORT IInspectable* XamlCreateProgressRing() {
return InspectableFromObject(ref new UIKit::Xaml::ProgressRing()).Detach();
UIKIT_XAML_EXPORT void XamlCreateProgressRing(IInspectable** created) {
Microsoft::WRL::ComPtr<IInspectable> inspectable = InspectableFromObject(ref new UIKit::Xaml::ProgressRing());
*created = inspectable.Detach();
}

// clang-format on
5 changes: 3 additions & 2 deletions Frameworks/UIKit.Xaml/ScrollViewer.xaml.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -31,8 +31,9 @@ ScrollViewer::ScrollViewer() {
////////////////////////////////////////////////////////////////////////////////////
// ObjectiveC Interop
////////////////////////////////////////////////////////////////////////////////////
UIKIT_XAML_EXPORT IInspectable* XamlCreateScrollViewer() {
return InspectableFromObject(ref new UIKit::Xaml::ScrollViewer()).Detach();
UIKIT_XAML_EXPORT void XamlCreateScrollViewer(IInspectable** created) {
Microsoft::WRL::ComPtr<IInspectable> inspectable = InspectableFromObject(ref new UIKit::Xaml::ScrollViewer());
*created = inspectable.Detach();
}

// clang-format on
5 changes: 3 additions & 2 deletions Frameworks/UIKit.Xaml/Slider.xaml.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -36,8 +36,9 @@ void Slider::OnApplyTemplate() {
////////////////////////////////////////////////////////////////////////////////////
// ObjectiveC Interop
////////////////////////////////////////////////////////////////////////////////////
UIKIT_XAML_EXPORT IInspectable* XamlCreateSlider() {
return InspectableFromObject(ref new UIKit::Xaml::Slider()).Detach();
UIKIT_XAML_EXPORT void XamlCreateSlider(IInspectable** created) {
Microsoft::WRL::ComPtr<IInspectable> inspectable = InspectableFromObject(ref new UIKit::Xaml::Slider());
*created = inspectable.Detach();
}

// clang-format on
5 changes: 3 additions & 2 deletions Frameworks/UIKit.Xaml/TextBox.xaml.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -36,8 +36,9 @@ void TextBox::OnApplyTemplate() {
////////////////////////////////////////////////////////////////////////////////////
// ObjectiveC Interop
////////////////////////////////////////////////////////////////////////////////////
UIKIT_XAML_EXPORT IInspectable* XamlCreateTextBox() {
return InspectableFromObject(ref new UIKit::Xaml::TextBox()).Detach();
UIKIT_XAML_EXPORT void XamlCreateTextBox(IInspectable** created) {
Microsoft::WRL::ComPtr<IInspectable> inspectable = InspectableFromObject(ref new UIKit::Xaml::TextBox());
*created = inspectable.Detach();
}

// clang-format on
2 changes: 1 addition & 1 deletion Frameworks/UIKit/UIButton.mm
Original file line number Diff line number Diff line change
Expand Up @@ -184,7 +184,7 @@ - (void)_initUIButton {
_contentVerticalAlignment = UIControlContentVerticalAlignmentCenter;
_contentHorizontalAlignment = UIControlContentHorizontalAlignmentCenter;

__block UIButton* weakSelf = self;
__weak UIButton* weakSelf = self;
XamlControls::HookButtonPointerEvents(_xamlButton,
^(RTObject* sender, WUXIPointerRoutedEventArgs* e) {
// We mark the event as handled here. The method _processPointerPressedCallback
Expand Down
9 changes: 6 additions & 3 deletions Frameworks/UIKit/XamlControls.mm
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,8 @@
// Button
////////////////////////////////////////////////////////////////////////////////////
WXCButton* CreateButton() {
ComPtr<IInspectable> inspectable(XamlCreateButton());
ComPtr<IInspectable> inspectable;
XamlCreateButton(&inspectable);
return _createRtProxy([WXCButton class], inspectable.Get());
}

Expand All @@ -53,7 +54,8 @@ void HookLayoutEvent(WXCButton* button, WUXIPointerEventHandler layoutHook) {
// ContentDialog
////////////////////////////////////////////////////////////////////////////////////
WXCContentDialog* CreateContentDialog() {
ComPtr<IInspectable> inspectable(XamlCreateContentDialog());
ComPtr<IInspectable> inspectable;
XamlCreateContentDialog(&inspectable);
return _createRtProxy([WXCContentDialog class], inspectable.Get());
}

Expand Down Expand Up @@ -95,7 +97,8 @@ void XamlContentDialogSetDestructiveButtonIndex(WXCContentDialog* contentDialog,
// Label
////////////////////////////////////////////////////////////////////////////////////
WXCGrid* CreateLabel() {
Microsoft::WRL::ComPtr<IInspectable> inspectable(XamlCreateLabel());
Microsoft::WRL::ComPtr<IInspectable> inspectable;
XamlCreateLabel(&inspectable);
return _createRtProxy([WXCGrid class], inspectable.Get());
}

Expand Down
5 changes: 5 additions & 0 deletions tests/functionaltests/FunctionalTests.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -476,6 +476,7 @@ extern void UIActivityIndicatorViewGetXamlElement();

extern void UIButtonCreateXamlElement();
extern void UIButtonGetXamlElement();
extern void UIButtonCheckForLeaks();
extern void UIButtonTitleColorChanged();
extern void UIButtonTextChanged();

Expand Down Expand Up @@ -606,6 +607,10 @@ class UIKitTests {
FrameworkHelper::RunOnUIThread(&UIButtonGetXamlElement);
}

TEST_METHOD(UIButton_CheckForLeaks) {
UIButtonCheckForLeaks();
}

TEST_METHOD(UIButton_TitleColorChanged) {
UIButtonTitleColorChanged();
}
Expand Down
3 changes: 2 additions & 1 deletion tests/functionaltests/Tests/UIKitTests/UIActionSheetTests.mm
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,8 @@

TEST(UIActionSheet, CreateXamlElement) {
// TODO: Switch to UIKit.Xaml projections when they're available.
Microsoft::WRL::ComPtr<IInspectable> xamlElement(XamlCreateContentDialog());
Microsoft::WRL::ComPtr<IInspectable> xamlElement;
XamlCreateContentDialog(&xamlElement);
ASSERT_TRUE(xamlElement);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,8 @@

TEST(UIActivityIndicatorView, CreateXamlElement) {
// TODO: Switch to UIKit.Xaml projections when they're available.
Microsoft::WRL::ComPtr<IInspectable> xamlElement(XamlCreateProgressRing());
Microsoft::WRL::ComPtr<IInspectable> xamlElement;
XamlCreateProgressRing(&xamlElement);
ASSERT_TRUE(xamlElement);
}

Expand Down
64 changes: 63 additions & 1 deletion tests/functionaltests/Tests/UIKitTests/UIButtonTests.mm
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,8 @@

TEST(UIButton, CreateXamlElement) {
// TODO: Switch to UIKit.Xaml projections when they're available.
Microsoft::WRL::ComPtr<IInspectable> xamlElement(XamlCreateButton());
Microsoft::WRL::ComPtr<IInspectable> xamlElement;
XamlCreateButton(&xamlElement);
ASSERT_TRUE(xamlElement);
}

Expand All @@ -38,6 +39,67 @@
ASSERT_TRUE([backingElement isKindOfClass:[WXFrameworkElement class]]);
}

@interface UIDeallocTestButton : UIButton {
std::shared_ptr<UXEvent> _event;
}
@end

@implementation UIDeallocTestButton

- (void)setDeallocEvent:(std::shared_ptr<UXEvent>)event {
_event = event;
}

-(void)dealloc {
_event->Set();
[super dealloc];
}
@end

TEST(UIButton, CheckForLeaks) {
Microsoft::WRL::WeakRef weakXamlElement;
{
StrongId<UIButtonWithControlsViewController> buttonVC;
buttonVC.attach([[UIButtonWithControlsViewController alloc] init]);
UXTestAPI::ViewControllerPresenter testHelper(buttonVC);

__block UIDeallocTestButton* testButton = nil;
__block auto event = UXEvent::CreateAuto();

// Create and render the button
dispatch_sync(dispatch_get_main_queue(), ^{
testButton = [[UIDeallocTestButton alloc] initWithFrame:CGRectMake(100, 100, 100, 100)];
testButton.backgroundColor = [UIColor redColor];
[testButton setDeallocEvent:event];
[[buttonVC view] addSubview:testButton];
});

// Grab a weak reference to the backing Xaml element
Microsoft::WRL::ComPtr<IInspectable> xamlElement([[testButton xamlElement] comObj]);
ASSERT_TRUE_MSG(SUCCEEDED(xamlElement.AsWeak(&weakXamlElement)), "Failed to acquire a weak reference to the backing Xaml element.");
xamlElement = nullptr;

// Free the button
dispatch_sync(dispatch_get_main_queue(), ^{
// Nil out testButton and remove it from its superview
[testButton removeFromSuperview];
[testButton release];
testButton = nil;
});

// Validate that dealloc was called
ASSERT_TRUE_MSG(event->Wait(c_testTimeoutInSec), "FAILED: Waiting for dealloc call failed!");
}

// Unfortunately we have to wait a bit for the button to actually finish deallocation
[NSThread sleepForTimeInterval:.25];

// Validate that we can no longer acquire a strong reference to the UIButton's backing Xaml element
Microsoft::WRL::ComPtr<IInspectable> xamlElement;
weakXamlElement.As(&xamlElement);
ASSERT_EQ_MSG(xamlElement.Get(), nullptr, "Unexpectedly able to reacquire a strong reference to the backing Xaml element.");
}

TEST(UIButton, TitleColorChanged) {
__block auto uxEvent = UXEvent::CreateManual();
__block auto xamlSubscriber = std::make_shared<XamlEventSubscription>();
Expand Down
3 changes: 2 additions & 1 deletion tests/functionaltests/Tests/UIKitTests/UIScrollViewTests.mm
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,8 @@

TEST(UIScrollView, CreateXamlElement) {
// TODO: Switch to UIKit.Xaml projections when they're available.
Microsoft::WRL::ComPtr<IInspectable> xamlElement(XamlCreateScrollViewer());
Microsoft::WRL::ComPtr<IInspectable> xamlElement;
XamlCreateScrollViewer(&xamlElement);
ASSERT_TRUE(xamlElement);
}

Expand Down
3 changes: 2 additions & 1 deletion tests/functionaltests/Tests/UIKitTests/UISliderTests.mm
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,8 @@

TEST(UISlider, CreateXamlElement) {
// TODO: Switch to UIKit.Xaml projections when they're available.
Microsoft::WRL::ComPtr<IInspectable> xamlElement(XamlCreateSlider());
Microsoft::WRL::ComPtr<IInspectable> xamlElement;
XamlCreateSlider(&xamlElement);
ASSERT_TRUE(xamlElement);
}

Expand Down
3 changes: 2 additions & 1 deletion tests/functionaltests/Tests/UIKitTests/UITextFieldTests.mm
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,8 @@

TEST(UITextField, CreateXamlElement) {
// TODO: Switch to UIKit.Xaml projections when they're available.
Microsoft::WRL::ComPtr<IInspectable> xamlElement(XamlCreateTextBox());
Microsoft::WRL::ComPtr<IInspectable> xamlElement;
XamlCreateTextBox(&xamlElement);
ASSERT_TRUE(xamlElement);
}

Expand Down