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

BackgroundTaskBuilder Implementation #4831

Merged
merged 6 commits into from
Nov 4, 2024
Merged

Conversation

godlytalias
Copy link
Contributor

@godlytalias godlytalias commented Oct 25, 2024

API Spec : #4822
Issue : #3840

Added implementation to support the registration of Background Tasks with full trust COM Components. Current BackgroundTaskBuilder is designed for UWP applications and many of the Background Task triggers are not supported for full trust COM Components and are supported only to be registered with Windows Runtime components that is to be launched with backgroundtaskhost process. Due to this, WinAppSDK desktop applications won’t be able to directly register the full trust COM components to be launched with background task triggers and have to do workaround of including the Windows Runtime components in the project.
In this implementation, a BackgroundTaskBuilder API internally registers the entry point of background task as the UniversalBGTask runtime component which then forwards the call to the full trust COM component registered by the application.

Testing

  • Tested with locally generated nuget packages

Pending Action Items

  • In self-contained mode, manifest declaration for the UniversalBGTask class is to be added manually, this need to be fixed
  • Support for C# applications to be verified

Dependent Work Items

@godlytalias godlytalias added this to the 1.7 milestone Oct 25, 2024
@godlytalias
Copy link
Contributor Author

/azp run

Copy link

Azure Pipelines successfully started running 1 pipeline(s).

Signed-off-by: godlytalias <[email protected]>
@godlytalias godlytalias added the api-design Updates to Project Reunion API surfaces label Oct 29, 2024
@godlytalias
Copy link
Contributor Author

/azp run

Copy link

Azure Pipelines successfully started running 1 pipeline(s).

Signed-off-by: godlytalias <[email protected]>
@godlytalias
Copy link
Contributor Author

/azp run

Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@mominshaikhdevs
Copy link

  1. Is this API "MediumIL apps" only? (meaning supports only full trust applications?)
  2. will this API work in "HighIL apps"?
  3. will this API work in "AppContainer apps"? (#219 - AppContainer for Win32 Apps + Win32 App Isolation repo?)

@godlytalias
Copy link
Contributor Author

@mominshaikhdevs It will work in Medium IL & High IL apps. API would work in AppContainer apps as well, however the behavior of background task feature flow is to be tested.

@mominshaikhdevs
Copy link

It will be best to have a proper test for them. otherwise we will be getting weird cases, where some portion of these APIs work in fine in MediumIL and HighIL but not in AppContainer.

@godlytalias
Copy link
Contributor Author

/azp run

Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@godlytalias
Copy link
Contributor Author

As we are approaching to the Experimental 1 release snap date, merging the PR. Please review and I will incorporate the comments in upcoming PRs. Thanks

@godlytalias godlytalias merged commit 8e8ed8b into main Nov 4, 2024
26 of 27 checks passed
@godlytalias godlytalias deleted the user/godlyalias/bgtaskImpl branch November 4, 2024 18:12

winrt::Windows::ApplicationModel::Background::IBackgroundTask bgTask = nullptr;
winrt::hresult hr = CoCreateInstance(comClsId, nullptr, CLSCTX_LOCAL_SERVER, IID_IBackgroundTask, reinterpret_cast<void**>(&bgTask));
bgTask.Run(taskInstance);
Copy link
Contributor

Choose a reason for hiding this comment

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

This is gonna AV if CoCreateInstance failed. and bgTask is still nullptr.

Copy link
Contributor Author

@godlytalias godlytalias Mar 6, 2025

Choose a reason for hiding this comment

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

Thanks @jlaanstra for the review, This method gets called from Broker Infrastructure and not handle any error results, Isn't it fine to throw exception here in that case? It will kill the backgroundtaskhost.exe process that gets created by broker infrastructure in that case.
Downside of silently handling the error would be that, broker infrastructure thinks that the task executed successfully, but the call haven't reached the application side.

Copy link
Contributor

@jlaanstra jlaanstra Mar 6, 2025

Choose a reason for hiding this comment

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

This won't throw an exception. You'll try to invoke a function on a nullptr which will terminate. If you want to throw you should throw the hr.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, It may not make much difference from BI side as the BI will just get to know that the backgroundtaskhost process got disconnected as it runs down. However I see that it is better to throw the right exception which may help during debugging.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
api-design Updates to Project Reunion API surfaces needs-triage
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants