App crashes when rearranging tabs in TabView where TabItemsSource is used

dpaulino commented 4 years ago

Describe the bug If you try to rearrange tabs in the TabView which uses describes how to use TabItemsSource, the app crashes.

Steps to reproduce the bug

Steps to reproduce the behavior:

Open the app, version 1.2.15.0
navigate to tabview samples page
scroll down to the section titled A TabView bound to a collection of MyData objects
Try to rearrange the tabs in that section by dragging an dropping.
app crashes

Expected behavior

app does not crash

Actual behavior

app crashes after dropping the tab

Screenshots

Version Info

Gallery version (found on Settings page):

1.2.15.0

Windows 10 version:

[x] Insider Build (19041.264)
[ ] November 2019 Update (18363)
[ ] May 2019 Update (18362)
[ ] October 2018 Update (17763)
[ ] April 2018 Update (17134)
[ ] Fall Creators Update (16299)
[ ] Creators Update (15063)
[ ] Anniversary Update (14393)

Device form factor:

[x] Desktop
[ ] Mobile
[ ] Xbox
[ ] Surface Hub
[ ] IoT

Additional context

yoshiask commented 4 years ago

This is also reproducible on build 18363 (v1909).

stmoy commented 4 years ago

Thank you for reporting this issue! This looks to be a regression in the TabView control, so I'm going to move this over to the WinUI repo.

marcelwgn commented 4 years ago

I was able to get this under the debugger finally, and it seems that we a TabItemTemplate which binds to the content is an issues. If we remove the binding of content, the app does not crash. The error message:

Element is already the child of another element.
   at Windows.UI.Xaml.Controls.ContentControl.put_Content(Object value)
   at AppUIBasics.ControlPages.TabViewPage.XamlBindingSetters.Set_Windows_UI_Xaml_Controls_ContentControl_Content(ContentControl obj, Object value, String targetNullValue)
   at AppUIBasics.ControlPages.TabViewPage.TabViewPage_obj25_Bindings.Update_DataContent(Object obj, Int32 phase)
   at AppUIBas'.
Exception thrown at 0x00007FF8DDCEA599 (KernelBase.dll) in AppUIBasics.exe: WinRT originate error - 0x800F1000 : 'Element is already the child of another element.'.
Exception thrown at 0x00007FF8DDCEA599 (KernelBase.dll) in AppUIBasics.exe: WinRT originate error - 0x800F1000 : 'System.Runtime.InteropServices.COMException: No installed components were detected.

Element is already the child of another element.
   at Windows.UI.Xaml.Controls.ContentControl.put_Content(Object value)
   at AppUIBasics.ControlPages.TabViewPage.XamlBindingSetters.Set_Windows_UI_Xaml_Controls_ContentControl_Content(ContentControl obj, Object value, String targetNullValue)
   at AppUIBasics.ControlPages.TabViewPage.TabViewPage_obj25_Bindings.Update_DataContent(Object obj, Int32 phase)
   at AppUIBas'.
Exception thrown at 0x00007FF8DDCEA599 (KernelBase.dll) in AppUIBasics.exe: WinRT originate error - 0x800F1000 : 'Element is already the child of another element.'.
Exception thrown: 'System.Runtime.InteropServices.COMException' in AppUIBasics.exe
Exception thrown at 0x00007FF8DDCEA599 (KernelBase.dll) in AppUIBasics.exe: WinRT originate error - 0x800F1000 : 'System.Runtime.InteropServices.COMException: No installed components were detected.

Element is already the child of another element.
   at Windows.UI.Xaml.Controls.ContentControl.put_Content(Object value)
   at AppUIBasics.ControlPages.TabViewPage.XamlBindingSetters.Set_Windows_UI_Xaml_Controls_ContentControl_Content(ContentControl obj, Object value, String targetNullValue)
   at AppUIBasics.ControlPages.TabViewPage.TabViewPage_obj25_Bindings.Update_DataContent(Object obj, Int32 phase)
   at AppUIBas'.
Exception thrown at 0x00007FF8DDCEA599 in AppUIBasics.exe: Microsoft C++ exception: winrt::hresult_error at memory location 0x0000002C7D9FD3F0.

marcelwgn commented 4 years ago

@stmoy @ranjeshj Mind if I look into a fix for this issue?

ranjeshj commented 4 years ago

Thanks @chingucoding. You can go ahead with investigating a fix for this.

marcelwgn commented 4 years ago

This seems to be an issue with binding (and probably listview in that regard). Using Binding instead of x:Bind fixes the crashes, but results in "detached" bindings:

gif

ranjeshj commented 4 years ago

Can you track back and see when it stopped working by trying each of the pre-releases builds ? I assume this is not an issue in the platform's listview since we have not made any recent changes there.

marcelwgn commented 4 years ago

I've tested the XAML Controls Gallery sample with the commit it was added with, and it still crashed. The sample was added 7 months ago and used WinUI 2.2, I am trying to find out which commit exactly could have caused this (the first versions of drag and drop did in fact not crash), but building the solution in those old states is difficult at times.

ranjeshj commented 4 years ago

Might be easier to do in a blank app than controls gallery. Thanks!

marcelwgn commented 4 years ago

The earliest version where the crash occurs is 2.2.190822001-prerelease. I will look into what changed between the release before that and that prerelease.

marcelwgn commented 4 years ago

So I've finally found out what causes the crash. Turns out using ItemsStackPanel as the ListView's panel, is causing the crash. Unfortunately removing the ItemsStackPanel (e.g. switching to StackPanel or removing the setter for the ItemsPanel property) results in the TabViewItems not rendering correctly.

What would be the best direction moving forward with this? @ranjeshj @teaP

Edit: #1158 introduced the use of ItemsStackPanel.

ranjeshj commented 4 years ago

@RBrid can you please help investigate this crash ?

RBrid commented 4 years ago

@dpaulino, @chingucoding, do you know how easily I should be running into this crash? I have been trying ~100 times with the latest master bits and have not hit the problem. Thanks!

dpaulino commented 4 years ago

@RBrid it crashes instantly for me in the xaml controls gallery app. Makes sure you scroll down to the demo for TabViewItemsSource. I just tried it now and still crashes instantly when I attempt a tab rearrange

dpaulino commented 4 years ago

The repro steps I noted at the top of the issue reliably reproduces the crash. Note that I'm using the app that is deployed to the store. And I'm not sure if master branch is ahead of the store build.

stmoy commented 4 years ago

@rbrid - I am able to repo 100% as well using the debug version of the Gallery from GitHub. It sounds like this also crashes on the store version. Perhaps it's OS related? I'm on 19635.mn_release.200522.

dpaulino commented 4 years ago

In case it helps, I'm on Windows 10 2004 Build 19041.264

RBrid commented 4 years ago

I could not repro whether I used the GitHub master version or the Microsoft Store version, on my machine which runs Windows 10.0.18362.836 (files from 05/12/2020).

I then installed the Store version on another machine that runs Windows 10.0.18362.1 (files from 03/18/2019). Bam! Instant and consistent crash.

So, it looks like, somehow we fixed it in the OS between the two versions above.

Although 19635.mn_release.200522 came after 10.0.18362.836, right? This is puzzling.

dpaulino commented 4 years ago

@RBrid can you record a video of your repro attempt and share it here? I'm curious if there's anything different about what you do and what I have done

RBrid commented 4 years ago

FYI I can also repro on a third machine with the Store version. It runs OS 10.0.18362.329 (from 01/10/2020).

What is special about the machine which runs Windows 10.0.18362.836 on which I cannot repro? Maybe it is that I use a 'Remote Desktop Connection' to try to repro on that machine since it is far far away from me...

RBrid commented 4 years ago

I think my assumption above was right. If you turn off system animations on your machine, the bug will no longer repro. I think Remote Desktop Connection turns off animations by default. (See 'Show animations in Windows' in the Settings app).

I can now repro on my Windows 10.0.18362.836 machine after turning on system animations on it.

dpaulino commented 4 years ago

That's fascinating. I'll try to test that as well when I get back to my desktop.

RBrid commented 4 years ago

I created PR https://github.com/microsoft/Xaml-Controls-Gallery/pull/457 for the Xaml Controls Gallery repo.

A few details: The Content of the dropped TabViewItem is being reparented while it is still parented by a CListViewItem : CContentControl.

Some of the stack trace:

07 0767d4b4 19099e7c Windows_UI_Xaml!DirectUI::ContentControlGenerated::put_Content+0x1b [onecoreuap\windows\dxaml\xcp\dxaml\lib\winrtgeneratedclasses\contentcontrol.g.cpp @ 69] 
08 0767d524 1b1fa835 Windows!DomainBoundILStubClass.IL_STUB_CLRtoWinRT(System.Object)+0x94
09 0767d548 1b1fa755 MUXControlsTestApp_4ea0000!MUXControlsTestApp.TabViewTabItemsSourcePage+XamlBindingSetters.Set_Windows_UI_Xaml_Controls_ContentControl_Content(Windows.UI.Xaml.Controls.ContentControl, System.Object, System.String)+0x95 [D:\microsoft-ui-xaml1\BuildOutput\Intermediates\MUXControlsTestApp.TAEF\obj\TabViewTabItemsSourcePage.g.cs @ 36] 
0a 0767d574 1b1fa627 MUXControlsTestApp_4ea0000!MUXControlsTestApp.TabViewTabItemsSourcePage+TabViewTabItemsSourcePage_obj4_Bindings.Update_DataContent(System.Object, Int32)+0xdd [D:\microsoft-ui-xaml1\BuildOutput\Intermediates\MUXControlsTestApp.TAEF\obj\TabViewTabItemsSourcePage.g.cs @ 194] 
0b 0767d598 1b1fa9a6 MUXControlsTestApp_4ea0000!MUXControlsTestApp.TabViewTabItemsSourcePage+TabViewTabItemsSourcePage_obj4_Bindings.Update_(MUXControlsTestApp.MyData, Int32)+0x7f [D:\microsoft-ui-xaml1\BuildOutput\Intermediates\MUXControlsTestApp.TAEF\obj\TabViewTabItemsSourcePage.g.cs @ 181] 
0c 0767d5d8 1b1f4b23 MUXControlsTestApp_4ea0000!MUXControlsTestApp.TabViewTabItemsSourcePage+TabViewTabItemsSourcePage_obj4_Bindings.ProcessBindings(System.Object, Int32, Int32, Int32 ByRef)+0x156 [D:\microsoft-ui-xaml1\BuildOutput\Intermediates\MUXControlsTestApp.TAEF\obj\TabViewTabItemsSourcePage.g.cs @ 132] 
0d 0767d60c 7b6615bf Windows!DomainBoundILStubClass.IL_STUB_WinRTtoCLR(IntPtr, Int32, Int32, IntPtr)+0x33
WARNING: Stack unwind information not available. Following frames may be wrong.
0e 0767d638 7b498e4f CoreCLR!GetCLRRuntimeHost+0x29e9d7
0f 0767d688 013dd060 CoreCLR!GetCLRRuntimeHost+0xd6267
10 0767d6b4 7a6891bc CLRStub[StubLinkStub]@1ac45e30013dd060
11 0767d720 7a597639 Windows_UI_Xaml!DirectUI::ListViewBase::SetupContainerContentChangingAfterPrepare+0x55c [onecoreuap\windows\dxaml\xcp\dxaml\lib\listviewbase_partial_containerphase.cpp @ 273] 
12 0767d758 7a5938b3 Windows_UI_Xaml!DirectUI::ModernCollectionBasePanel::PrepareContainerViaItemsHost+0xdf [onecoreuap\windows\dxaml\xcp\dxaml\lib\moderncollectionbasepanel_iicg2_partial.cpp @ 444] 
13 0767d8c4 7a5923cb Windows_UI_Xaml!DirectUI::ModernCollectionBasePanel::Generate+0x51a [onecoreuap\windows\dxaml\xcp\dxaml\lib\moderncollectionbasepanel_partial.cpp @ 1376] 
14 0767d960 7a596579 Windows_UI_Xaml!DirectUI::ModernCollectionBasePanel::RunGenerate+0xb5 [onecoreuap\windows\dxaml\xcp\dxaml\lib\moderncollectionbasepanel_partial.cpp @ 902] 
15 0767d9a0 7a5961f0 Windows_UI_Xaml!DirectUI::ModernCollectionBasePanel::RunVirtualization+0x151 [onecoreuap\windows\dxaml\xcp\dxaml\lib\moderncollectionbasepanel_partial.cpp @ 628] 
16 0767d9c0 7a6addf7 Windows_UI_Xaml!DirectUI::ModernCollectionBasePanel::MeasureOverride+0x90 [onecoreuap\windows\dxaml\xcp\dxaml\lib\moderncollectionbasepanel_partial.cpp @ 521] 
17 0767d9ec 7a6ad742 Windows_UI_Xaml!DirectUI::FrameworkElementGenerated::MeasureOverrideProtected+0x117 [onecoreuap\windows\dxaml\xcp\dxaml\lib\winrtgeneratedclasses\frameworkelement.g.cpp @ 1056] 
18 0767da68 7ab271de Windows_UI_Xaml!DirectUI::FrameworkElement::MeasureOverrideFromCore+0xb2 [onecoreuap\windows\dxaml\xcp\dxaml\lib\frameworkelement_partial.cpp @ 255] 
19 0767da7c 7a6ea526 Windows_UI_Xaml!CFxCallbacks::FrameworkElement_MeasureOverride+0x22 [onecoreuap\windows\dxaml\xcp\dxaml\lib\fxcallbacks.cpp @ 865] 
1a 0767dae4 7a6ea8f6 Windows_UI_Xaml!CFrameworkElement::MeasureCore+0x4c6 [onecoreuap\windows\dxaml\xcp\core\core\elements\framework.cpp @ 1597] 
1b 0767db48 7a6e482c Windows_UI_Xaml!CUIElement::MeasureInternal+0x176 [onecoreuap\windows\dxaml\xcp\core\core\elements\uielement.cpp @ 4106] 
1c 0767db88 7a6e48d4 Windows_UI_Xaml!CUIElement::Measure+0x44c [onecoreuap\windows\dxaml\xcp\core\core\elements\uielement.cpp @ 3966] 
...
31 0767e0c8 7a6d85e0 Windows_UI_Xaml!CUIElement::Measure+0x4f4 [onecoreuap\windows\dxaml\xcp\core\core\elements\uielement.cpp @ 4016] 
32 0767e108 7a5919bb Windows_UI_Xaml!CLayoutManager::UpdateLayout+0x270 [onecoreuap\windows\dxaml\xcp\core\layout\layoutmanager.cpp @ 252] 
33 (Inline) -------- Windows_UI_Xaml!CLayoutManager::UpdateLayout+0xd [onecoreuap\windows\dxaml\xcp\core\layout\layoutmanager.cpp @ 199] 
34 0767e128 7a591954 Windows_UI_Xaml!CUIElement::UpdateLayout+0x35 [onecoreuap\windows\dxaml\xcp\core\core\elements\uielement.cpp @ 3714] 
35 0767e13c 7af5434a Windows_UI_Xaml!DirectUI::UIElementGenerated::UpdateLayout+0x34 [onecoreuap\windows\dxaml\xcp\dxaml\lib\winrtgeneratedclasses\uielement.g.cpp @ 4786] 
36 0767e194 7af5357d Windows_UI_Xaml!DirectUI::ListViewBase::OnReorderDrop+0x21f [onecoreuap\windows\dxaml\xcp\dxaml\lib\listviewbase_partial_reorder.cpp @ 1588] 
37 0767e1c8 0a406b90 Windows_UI_Xaml!DirectUI::ListViewBase::OnDrop+0x12d [onecoreuap\windows\dxaml\xcp\dxaml\lib\listviewbase_partial_reorder.cpp @ 1386] 
38 0767e2c8 0bd63fa3 Microsoft_UI_Xaml!winrt::impl::consume_Windows_UI_Xaml_Controls_IControlOverrides<winrt::Windows::UI::Xaml::Controls::IControlOverrides>::OnDrop+0x60 [D:\microsoft-ui-xaml1\BuildOutput\Intermediates\Microsoft.UI.Xaml\obj\Generated Files\winrt\Windows.UI.Xaml.Controls.h @ 4831] 
39 0767e3d8 0bd6409d Microsoft_UI_Xaml!winrt::Windows::UI::Xaml::Controls::IControlOverridesT<TabViewListView>::OnDrop+0xa3 [D:\microsoft-ui-xaml1\BuildOutput\Intermediates\Microsoft.UI.Xaml\obj\Generated Files\winrt\Windows.UI.Xaml.Controls.h @ 64395] 
3a 0767e4fc 7ad859dc Microsoft_UI_Xaml!winrt::impl::produce<TabViewListView,winrt::Windows::UI::Xaml::Controls::IControlOverrides>::OnDrop+0x8d [D:\microsoft-ui-xaml1\BuildOutput\Intermediates\Microsoft.UI.Xaml\obj\Generated Files\winrt\Windows.UI.Xaml.Controls.h @ 30099] 
3b 0767e51c 7a8fcd3a Windows_UI_Xaml!DirectUI::ControlGenerated::OnDropProtected+0x43 [onecoreuap\windows\dxaml\xcp\dxaml\lib\winrtgeneratedclasses\control.g.cpp @ 868] 
3c 0767e544 7a736f03 Windows_UI_Xaml!DirectUI::Control::FireEvent+0x1c5b4b [onecoreuap\windows\dxaml\xcp\dxaml\lib\control_partial.cpp @ 461] 
3d 0767e574 7a736b14 Windows_UI_Xaml!DirectUI::DXamlCore::FireEvent+0xe3 [onecoreuap\windows\dxaml\xcp\dxaml\lib\dxamlcore.cpp @ 1969] 
3e (Inline) -------- Windows_UI_Xaml!AgCoreCallbacks::FireEvent+0x22 [onecoreuap\windows\dxaml\xcp\dxaml\lib\fxcallbacks.cpp @ 91] 
3f (Inline) -------- Windows_UI_Xaml!CFxCallbacks::JoltHelper_FireEvent+0x22 [onecoreuap\windows\dxaml\xcp\dxaml\lib\fxcallbacks.cpp @ 1035] 
40 0767e5bc 7a73676a Windows_UI_Xaml!CCoreServices::CLR_FireEvent+0x394 [onecoreuap\windows\dxaml\xcp\core\dll\xcpcore.cpp @ 3224] 
41 0767e5dc 7a6d50fe Windows_UI_Xaml!CommonBrowserHost::CLR_FireEvent+0x1a [onecoreuap\windows\dxaml\xcp\control\common\shared\commonbrowserhost.hpp @ 771] 
42 0767e620 7a6d4f22 Windows_UI_Xaml!CControlBase::ScriptCallback+0x11e [onecoreuap\windows\dxaml\xcp\control\common\shared\controlbase.cpp @ 267] 
43 0767e684 7a8fdccd Windows_UI_Xaml!CXcpDispatcher::OnScriptCallback+0x112 [onecoreuap\windows\dxaml\xcp\win\shared\xcpwindow.cpp @ 1338] 
44 0767e6a8 7a6d4ce6 Windows_UI_Xaml!CXcpDispatcher::OnReentrancyProtectedWindowMessage+0x1c3abd [onecoreuap\windows\dxaml\xcp\win\shared\xcpwindow.cpp @ 1022] 
45 0767e6d4 7a6d4b6d Windows_UI_Xaml!CXcpDispatcher::ProcessMessage+0xc6 [onecoreuap\windows\dxaml\xcp\win\shared\xcpwindow.cpp @ 890] 
46 0767e708 755c44bb Windows_UI_Xaml!CXcpDispatcher::WindowProc+0x4d [onecoreuap\windows\dxaml\xcp\win\shared\xcpwindow.cpp @ 839] 
47 0767e734 755a4ffc user32!_InternalCallWinProc+0x2b [internal\onecoreuapwindows\private\lib\i386\Core\ntuser\client\callproc.asm @ 116] 
48 (Inline) -------- user32!InternalCallWinProc+0x1a [internal\onecoreuapwindows\private\inc\core\ntuser\client\callproc.h @ 87] 
49 0767e818 755a4b9a user32!UserCallWinProcCheckWow+0x3ac [windows\core\ntuser\client\clmsg.cxx @ 279] 
4a 0767e87c 755ae1df user32!DispatchClientMessage+0xea [windows\core\ntuser\client\wow6432\objfre\i386\client.cxx @ 3442] 
4b 0767e8b8 775b428d user32!__fnDWORD+0x3f [internal\onecoreuapwindows\inc\ntuser\inc\ntcb.h @ 1250] 
4c 0767e958 76152c0c ntdll!KiUserCallbackDispatcher+0x4d [minkernel\ntos\rtl\i386\userdisp.asm @ 524] 
4d 0767e958 755a4979 win32u!NtUserMessageCall+0xc [windows\core\umode\wow6432\objfre\i386\usrstubs.asm @ 185] 
4e 0767e958 755a4473 user32!SendMessageWorker+0x3c9 [windows\core\ntuser\client\clmsg.cxx @ 890] 
4f 0767e988 7a5f647a user32!SendMessageW+0x123 [internal\onecoreuapwindows\private\inc\core\ntuser\client\cltxt.h @ 906] 
50 (Inline) -------- Windows_UI_Xaml!CXcpDispatcher::SendMessageW+0x10 [onecoreuap\windows\dxaml\xcp\win\shared\xcpwindow.cpp @ 809] 
51 0767e9d0 7a6b5ad7 Windows_UI_Xaml!CXcpBrowserHost::SyncScriptCallbackRequest+0xea [onecoreuap\windows\dxaml\xcp\host\win\browserdesktop\winbrowserhost.cpp @ 1048] 
52 0767ea10 7a6b559f Windows_UI_Xaml!CEventManager::RaiseControlEvents+0xc7 [onecoreuap\windows\dxaml\xcp\core\dll\eventmgr.cpp @ 1133] 
53 0767ea68 7a973f3a Windows_UI_Xaml!CEventManager::Raise+0x1ff [onecoreuap\windows\dxaml\xcp\core\dll\eventmgr.cpp @ 894] 
54 0767eaa8 7a973fd5 Windows_UI_Xaml!CDragDropState::RaiseDragOverOrDropEvents+0x81 [onecoreuap\windows\dxaml\xcp\core\input\inputservices.cpp @ 16006] 
55 0767eabc 7ad1c6e4 Windows_UI_Xaml!CDragDropState::RaiseEvents+0x20 [onecoreuap\windows\dxaml\xcp\core\input\inputservices.cpp @ 15823] 
56 0767eae4 7aad3ddb Windows_UI_Xaml!ContentRootInput::DragDropProcessor::ProcessWinRtDragDrop+0xf9 [onecoreuap\windows\dxaml\xcp\components\contentroot\dragdropprocessor.cpp @ 304] 
57 0767eb30 7ab6d0f3 Windows_UI_Xaml!DragDrop_RaiseEvent+0xf6 [onecoreuap\windows\dxaml\xcp\core\dll\pinvokes.cpp @ 4703] 
58 0767eb9c 7ab8646a Windows_UI_Xaml!DirectUI::DropOperationTarget::RaiseDragDropEventActionAsync+0x26f [onecoreuap\windows\dxaml\xcp\dxaml\lib\dropoperationtarget.cpp @ 240] 
59 0767ebe0 7ab53afb Windows_UI_Xaml!DirectUI::RaiseDragDropEventAsyncOperation::OnStart+0x6a [onecoreuap\windows\dxaml\xcp\dxaml\lib\raisedragdropeventasyncoperation.cpp @ 28] 
5a 0767ebf4 7ab6d248 Windows_UI_Xaml!Microsoft::WRL::AsyncBase<Windows::Foundation::IAsyncOperationCompletedHandler<enum Windows::ApplicationModel::DataTransfer::DataPackageOperation>,Microsoft::WRL::Details::Nil,1,Microsoft::WRL::AsyncOptions<-1,0,&GUID_CAUSALITY_WINDOWS_PLATFORM_ID,2> >::Start+0x2b [sdk\inc\wrl\async.h @ 632] 
5b (Inline) -------- Windows_UI_Xaml!DirectUI::DXamlAsyncBaseImpl<Windows::Foundation::IAsyncOperationCompletedHandler<enum Windows::ApplicationModel::DataTransfer::DataPackageOperation>,Windows::Foundation::IAsyncOperation<enum Windows::ApplicationModel::DataTransfer::DataPackageOperation>,Microsoft::WRL::AsyncOptions<-1,0,&GUID_CAUSALITY_WINDOWS_PLATFORM_ID,2>,DirectUI::IDragOperationDeferralTarget,1>::StartOperation+0x18 [onecoreuap\windows\dxaml\xcp\dxaml\lib\dxamlasyncbase.h @ 55] 
5c 0767ec10 7ab6c9fe Windows_UI_Xaml!DirectUI::DropOperationTarget::RaiseDragDropEventOperationAsync+0x53 [onecoreuap\windows\dxaml\xcp\dxaml\lib\dropoperationtarget.cpp @ 167] 
5d 0767ec2c 5c3863f0 Windows_UI_Xaml!DirectUI::DropOperationTarget::DropAsync+0x1e [onecoreuap\windows\dxaml\xcp\dxaml\lib\dropoperationtarget.cpp @ 140] 
5e 0767ed20 760cf4d4 DataExchange!DropTargetInternal::Drop+0xb0 [onecoreuap\shell\dataexchange\winrt\lib\droptargetinternal.cpp @ 74]

Without animations, the old listview item container gets put into the recycling pool early enough that it is picked up to contain the dropped content. As part of this stack:

00 0767d738 7a598638 Windows_UI_Xaml!DirectUI::ModernCollectionBasePanel::GenerateContainerAtIndexImpl+0x1c9 [onecoreuap\windows\dxaml\xcp\dxaml\lib\moderncollectionbasepanel_iicg2_partial.cpp @ 349] 
01 0767d75c 7a593864 Windows_UI_Xaml!DirectUI::ModernCollectionBasePanelGenerated::GenerateContainerAtIndex+0x48 [onecoreuap\windows\dxaml\xcp\dxaml\lib\winrtgeneratedclasses\moderncollectionbasepanel.g.cpp @ 567] 
02 0767d8c4 7a5923cb Windows_UI_Xaml!DirectUI::ModernCollectionBasePanel::Generate+0x4cb [onecoreuap\windows\dxaml\xcp\dxaml\lib\moderncollectionbasepanel_partial.cpp @ 1374] 
03 0767d960 7a596579 Windows_UI_Xaml!DirectUI::ModernCollectionBasePanel::RunGenerate+0xb5 [onecoreuap\windows\dxaml\xcp\dxaml\lib\moderncollectionbasepanel_partial.cpp @ 902] 
04 0767d9a0 7a5961f0 Windows_UI_Xaml!DirectUI::ModernCollectionBasePanel::RunVirtualization+0x151 [onecoreuap\windows\dxaml\xcp\dxaml\lib\moderncollectionbasepanel_partial.cpp @ 628] 
05 0767d9c0 7a6addf7 Windows_UI_Xaml!DirectUI::ModernCollectionBasePanel::MeasureOverride+0x90 [onecoreuap\windows\dxaml\xcp\dxaml\lib\moderncollectionbasepanel_partial.cpp @ 521] 
…

When animations run, the old container has not been put into the recycling pool and a brand new container is being instantiated and populated through the DataContext. The UIElement content (that is still hosted in the old container) is being parented into the newly created container – this causes the “element already parented” error.

The fix is to either not use UIElement in the DataContext - here it's a Frame, or remove animations from the inner ListView's ItemContainerTransitions. See the PR sent out for the Xaml Controls Gallery app.

stmoy commented 4 years ago

Thanks @RBrid! A few questions:

I thought @chingucoding identified that this was a regression? It sounds like this is a fundamental issue with ListView though, so I wouldn't expect it to be a regression. Any ideas about this disconnect? Is it not actually a regression?
I'm worried about the app-developer experience here. Since the TabView has both the tab strip and the page content, using a UIElement like a frame is a natural thing to add to the content area. It sounds like there is some guidance that is needed here, but I don't think the app-code that is included in the PR is what we want to developers to do. Brainstorming guidance:
- Option 1 - If using TabItemsSource, do not use UIElement in your data item (ie. don't do this: link) - instead, manage your page content separately from the TabView
- Option 2 - If using TabItemsSource, disable tab rearrange
- Option 3 - If using TabItemsSource, disable theme animations

Honestly, none of these options seem great. Using TabItemsSource seems like a common scenario. @michael-hawker, how does the WCT support this case?

shaheedmalik commented 4 years ago

I tested this on my system as well running the Store Version of XAML and it crashes. OS Build 19631.1

RBrid commented 4 years ago

This does not appear to be a regression. The TabView started to use an ItemsStackPanel in its inner ListView thus the ModernCollectionBasePanel involvement in the stack above. We did not run into this old bug before the TabView used a ModernCollectionBasePanel virtualizing panel.

stmoy commented 4 years ago

Hmm, so it sounds like the change to use ModernCollectionBasePanel introduced the issue? What was it using before? Should we revert?

marcelwgn commented 4 years ago

The problem is that, if we don't use the ItemsStackPanel, the items don't render properly (at least it did when I tried).

RBrid commented 4 years ago

Before the TabView used a regular StackPanel. I have been experimenting with a ListView alone and as I said it repros there too. With a StackPanel, I am seeing this report in VS when re-ordering items data-bound to a UIElement:

"Exception thrown: 'System.Runtime.InteropServices.COMException' in MUXControlsTestApp.exe WinRT information: Element is already the child of another element."

But the app does not crash and the dropped item loses its content.

With an ItemsStackPanel, the same error causes a crash. So none of the two panels work properly.

RBrid commented 4 years ago

FYI, the animations

<Setter Property="ItemContainerTransitions">
  <Setter.Value>
    <TransitionCollection>
      <AddDeleteThemeTransition />
      <ContentThemeTransition />
      <ReorderThemeTransition />
      <EntranceThemeTransition IsStaggeringEnabled="False" />
    </TransitionCollection>
  </Setter.Value>
</Setter>

were added to the TabView's inner ListView by Tammy on 07/17/2019, in TabView.xaml.

michael-hawker commented 4 years ago

@stmoy the Toolkit version uses StackPanel and not ItemsStackPanel, so that's probably how we've avoided this issue.

stmoy commented 4 years ago

Gotcha, thanks all. @RBrid, it sounds like this scenario is broken with either a StackPanel or an ItemsStackPanel. No good!

Since neither panel works, it seems like we'll need to do two things:

Update the sample
Update the guidance

I'm still not sure how to do Step 2. What will we recommend people do? I see three options to update the guidance:

Either: Don't put UIElements in the TabView's content area when bound with a TabItemsSource
Or: Disable rearrange when using a TabItemsSource
Or: Disable the animations when using a TabItemsSource

Are there other options? Thoughts on the best recommendation? Disabling the animations is probably the best approach that maintains functionality, but I'm not sure if the implementation in the PR is the most straightforward/discoverable way to do it.

michael-hawker commented 4 years ago

@stmoy @rbrid, that's an interesting note about the item losing content. I seem to remember someone mentioning an issue with TabView in the Toolkit and datacontext disappearing. I don't seem to be able to find an issue for it though, and I don't remember if it as related to drag and drop, but if so that could have been the underlying issue there as well.

RBrid commented 4 years ago

I am currently looking into Issue #2455 - it may be related. The scenario is a bit different because there the TabItemsSource is an observable collection of TabViewItem, as opposed to an observable collection of data objects.

ranjeshj commented 4 years ago

Can you see if removing ReorderThemeTransition from the transition collection avoids the crash ? If that works, perhaps that is a more palatable workaround since the crash is in the OS XAML code and we cannot fix that until winui3 at the earliest.

RBrid commented 4 years ago

As you can see in my fix https://github.com/microsoft/Xaml-Controls-Gallery/pull/457, I am removing the AddDeleteThemeTransition/ContentThemeTransition animations. Removing ReorderThemeTransition was the first thing I had tried and it did not help unfortunately. Obviously replacing the tabViewItemsSourceSampleListView.ItemContainerTransitions collection with a new completely empty one also would work (instead of the fix's 'for' loop).

microsoft / microsoft-ui-xaml

App crashes when rearranging tabs in TabView where TabItemsSource is used #2457