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

Code Quality: WinAppSdk 1.6 #16156

Open
wants to merge 15 commits into
base: main
Choose a base branch
from
Open

Code Quality: WinAppSdk 1.6 #16156

wants to merge 15 commits into from

Conversation

yaira2
Copy link
Member

@yaira2 yaira2 commented Sep 8, 2024

  • Updated WinAppSdk to 1.6
  • Removed workaround for auto hide taskbar

@yaira2 yaira2 marked this pull request as draft September 8, 2024 16:24
@f1amy
Copy link

f1amy commented Sep 9, 2024

Will this utilize new native AOT compilation?

@0x5bfa
Copy link
Member

0x5bfa commented Sep 9, 2024

Not yet, we have still some blockers, such as runtime marshaling and reflection. I've been removing it.

@yaira2
Copy link
Member Author

yaira2 commented Sep 9, 2024

Will this utilize new native AOT compilation?

Aside from what @0x5bfa mentioned, the AOT support in WinAppSdk 1.6 is a bit rough and we might have to wait for future versions of WinAppSdk before we can enable it.

@yaira2
Copy link
Member Author

yaira2 commented Sep 11, 2024

The two issues we're currently having with this PR are

  1. The accessibility tests (fyi @marcelwgn)
  2. Shell previews cause Files to crash
    image

@hez2010
Copy link
Member

hez2010 commented Sep 12, 2024

  1. You need to add the latest CsWinRT reference to all projects using WinRT APIs, they have a source generator for this.
  2. You can use <WindowsSdkPackageVersion>10.0.22621.42</WindowsSdkPackageVersion>.
  3. Some classes need to be marked as partial after you update CsWinRT.

@yaira2
Copy link
Member Author

yaira2 commented Sep 12, 2024

You need to add the latest CsWinRT reference to all projects using WinRT APIs, they have a source generator for this.

The last time we tried updating it caused a bunch of crashes, but it could be that updating WinAppSdk will solve this issue.

Some classes need to be marked as partial after you update CsWinRT.

How do you know which ones?

@hez2010
Copy link
Member

hez2010 commented Sep 12, 2024

How do you know which ones?

The analyzer bundled with WinRT will tell you and yield a compilation error if there's anything missing.

@hez2010
Copy link
Member

hez2010 commented Sep 12, 2024

I will check later today.

@yaira2
Copy link
Member Author

yaira2 commented Sep 29, 2024

@hez2010 @marcelwgn any ideas regarding the testing project?

Copy link
Member

@0x5bfa 0x5bfa left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Btw PreviewHandler.cs looks great.
The CI log is expired, rerunning may help others look into.

@hez2010
Copy link
Member

hez2010 commented Sep 29, 2024

@hez2010 @marcelwgn any ideas regarding the testing project?

Resolved in #16276.

@yaira2 yaira2 added the ready to merge Pull requests that are approved and ready to merge label Sep 29, 2024
@yaira2
Copy link
Member Author

yaira2 commented Sep 30, 2024

This seems to have the same issues we encountered the last time we updated to .NET 8.0.8. I was hoping the latest version of WinAppSdk would resolve the issues, but unfortunately it hasn't.

@yaira2
Copy link
Member Author

yaira2 commented Sep 30, 2024

Here are some stacktraces

System.Runtime.InteropServices.COMException: null
  ?, in void AppLifecycleHelper.HandleAppUnhandledException(Exception ex, bool showToastNotification)
  ?, in int UnhandledExceptionEventHandler.Do_Abi_Invoke(IntPtr thisPtr, IntPtr sender, IntPtr e)
  ?, in void IFrameMethods.GoBack(IObjectReference _obj)
  ?, in void ModernShellPage.Back_Click()
  ?, in Task NavigateBackAction.ExecuteAsync(object parameter)
  ?, in async void ActionCommand.Execute(object parameter)
  ?, in void AsyncMethodBuilderCore.Start<TStateMachine>(ref TStateMachine stateMachine)
  ?, in void ActionCommand.Execute(object parameter)
  ?, in int Vftbl.Do_Abi_Execute_3(IntPtr thisPtr, IntPtr parameter)
  ?, in void IApplicationStaticsMethods.Start(IObjectReference _obj, ApplicationInitializationCallback callback)
  ?, in void Program.Main()
System.Runtime.InteropServices.COMException: null
  ?, in void AppLifecycleHelper.HandleAppUnhandledException(Exception ex, bool showToastNotification)
  ?, in int UnhandledExceptionEventHandler.Do_Abi_Invoke(IntPtr thisPtr, IntPtr sender, IntPtr e)
  ?, in bool IFrameMethods.Navigate(IObjectReference _obj, Type sourcePageType, object parameter, NavigationTransitionInfo infoOverride)
  ?, in void ModernShellPage.NavigateToPath(string navigationPath, Type sourcePageType, NavigationArguments navArgs)
  ?, in async Task NavigationHelpers.OpenPathAsync(bool forceOpenInNewTab, bool openFolderInNewTabSetting, string path, string text, IShellPage associatedInstance, IEnumerable<string> selectItems)
  ?, in void AsyncMethodBuilderCore.Start<TStateMachine>(ref TStateMachine stateMachine)
  ?, in Task NavigationHelpers.OpenPathAsync(bool forceOpenInNewTab, bool openFolderInNewTabSetting, string path, string text, IShellPage associatedInstance, IEnumerable<string> selectItems)
  ?, in async Task<FilesystemResult> NavigationHelpers.OpenDirectory(string path, IShellPage associatedInstance, IEnumerable<string> selectItems, ShellLinkItem shortcutInfo, bool forceOpenInNewTab)
  ?, in void AsyncStateMachineBox<TStateMachine>.ExecutionContextCallback(object s)
  ?, in void AsyncStateMachineBox<TStateMachine>.MoveNext(Thread threadPoolThread) x 2
  ?, in void AwaitTaskContinuation.RunCallback(ContextCallback callback, object state, ref Task currentTask)
  ?, in void Task.RunContinuations(object continuationObject)
  ?, in void AsyncTaskMethodBuilder<TResult>.SetExistingTaskResult(Task<TResult> task, TResult result)
  ?, in async Task<FilesystemResult> FilesystemTasks.OnSuccess<T>(Task<FilesystemResult<T>> wrapped, Action<T> func)
  ?, in void AsyncStateMachineBox<TStateMachine>.ExecutionContextCallback(object s)
  ?, in void AsyncStateMachineBox<TStateMachine>.MoveNext(Thread threadPoolThread) x 2
  ?, in void AwaitTaskContinuation.RunCallback(ContextCallback callback, object state, ref Task currentTask)
  ?, in void Task.RunContinuations(object continuationObject)
  ?, in void AsyncTaskMethodBuilder<TResult>.SetExistingTaskResult(Task<TResult> task, TResult result)
  ?, in async Task<FilesystemResult<StorageFolderWithPath>> ShellViewModel.GetFolderWithPathFromPathAsync(string value, CancellationToken cancellationToken)
  ?, in void AsyncStateMachineBox<TStateMachine>.ExecutionContextCallback(object s)
  ?, in void AsyncStateMachineBox<TStateMachine>.MoveNext(Thread threadPoolThread) x 2
  ?, in void AwaitTaskContinuation.RunCallback(ContextCallback callback, object state, ref Task currentTask)
  ?, in void Task.RunContinuations(object continuationObject)
  ?, in void AsyncTaskMethodBuilder<TResult>.SetResult(TResult result)
  ?, in async Task<FilesystemResult<T>> FilesystemTasks.Wrap<T>(Func<Task<T>> wrapped)
  ?, in void AsyncStateMachineBox<TStateMachine>.MoveNext(Thread threadPoolThread)
  ?, in void AwaitTaskContinuation.RunCallback(ContextCallback callback, object state, ref Task currentTask)
  ?, in void Task.RunContinuations(object continuationObject)
  ?, in async Task<StorageFolderWithPath> StorageFileExtensions.DangerousGetFolderWithPathFromPathAsync(string value, StorageFolderWithPath rootFolder, StorageFolderWithPath parentFolder)
  ?, in void AsyncStateMachineBox<TStateMachine>.MoveNext(Thread threadPoolThread)
  ?, in void AwaitTaskContinuation.RunCallback(ContextCallback callback, object state, ref Task currentTask)
  ?, in void Task.RunContinuations(object continuationObject)
  ?, in void AsyncInfoToTaskBridge<TResult, TProgress>.Complete(IAsyncInfo asyncInfo, Func<IAsyncInfo, TResult> getResultsFunction, AsyncStatus asyncStatus)
  ?, in void TaskToAsyncInfoAdapter<TCompletedHandler, TProgressHandler, TResult, TProgressInfo>.TaskCompleted()
  ?, in bool ThreadPoolTaskScheduler.TryExecuteTaskInline(Task task, bool taskWasPreviouslyQueued)
  ?, in void TaskContinuation.InlineIfPossibleOrElseQueue(Task task, bool needsProtection)
  ?, in void ContinueWithTaskContinuation.Run(Task completedTask, bool canInlineContinuationTask)
  ?, in void Task.RunContinuations(object continuationObject)
  ?, in async IAsyncOperation<BaseStorageFolder> BaseStorageFolder.GetFolderFromPathAsync(string path)+(?) => { }
  ?, in void AsyncStateMachineBox<TStateMachine>.MoveNext(Thread threadPoolThread)
  ?, in void AwaitTaskContinuation.RunCallback(ContextCallback callback, object state, ref Task currentTask)
  ?, in void Task.RunContinuations(object continuationObject)
  ?, in void AsyncInfoToTaskBridge<TResult, TProgress>.Complete(IAsyncInfo asyncInfo, Func<IAsyncInfo, TResult> getResultsFunction, AsyncStatus asyncStatus)
  ?, in void TaskToAsyncInfoAdapter<TCompletedHandler, TProgressHandler, TResult, TProgressInfo>.TaskCompleted()
  ?, in bool ThreadPoolTaskScheduler.TryExecuteTaskInline(Task task, bool taskWasPreviouslyQueued)
  ?, in void TaskContinuation.InlineIfPossibleOrElseQueue(Task task, bool needsProtection)
  ?, in void ContinueWithTaskContinuation.Run(Task completedTask, bool canInlineContinuationTask)
  ?, in void Task.RunContinuations(object continuationObject)
  ?, in async IAsyncOperation<BaseStorageFolder> SystemStorageFolder.FromPathAsync(string path)+(?) => { }
  ?, in void AsyncStateMachineBox<TStateMachine>.MoveNext(Thread threadPoolThread)
  ?, in void DispatcherQueueSynchronizationContext.Post(SendOrPostCallback d, object state)+() => { }
  ?, in int DispatcherQueueHandler.Do_Abi_Invoke(IntPtr thisPtr)
  ?, in void IApplicationStaticsMethods.Start(IObjectReference _obj, ApplicationInitializationCallback callback)
  ?, in void Program.Main()

Just to clarify, this issue happens in .NET 8.0.7 as well, but it's a lot more frequent in 8.0.8.

@yaira2
Copy link
Member Author

yaira2 commented Sep 30, 2024

This might be relevant dotnet/maui#22790

src/Files.App (Package)/Files.Package.wapproj Outdated Show resolved Hide resolved
src/Files.App.Controls/Files.App.Controls.csproj Outdated Show resolved Hide resolved
src/Files.App/Files.App.csproj Outdated Show resolved Hide resolved
src/Files.App/Files.App.csproj Outdated Show resolved Hide resolved
@yaira2
Copy link
Member Author

yaira2 commented Oct 6, 2024

We might have to wait for .NET 9 to see if this is resolved.

@hez2010
Copy link
Member

hez2010 commented Oct 16, 2024

The crashing issue is unrelated to .NET.
I just investigated this issue and it turns out to be an AV due to CsWinRT not calling AddRef on the COM object.

microsoft/CsWinRT#1834

@hez2010
Copy link
Member

hez2010 commented Oct 28, 2024

Let's wait for microsoft/CsWinRT#1848 and try again when a new version of CsWinRT including that fix comes out.

@yaira2
Copy link
Member Author

yaira2 commented Oct 28, 2024

Looking forward. Assuming this fixes the issue, we can also remove the global.json file.

@Jay-o-Way
Copy link
Contributor

Ping

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ready to merge Pull requests that are approved and ready to merge
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants