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

[WinUI] Add workaround for Connectivity check on Win10 #19261

Merged
merged 13 commits into from
Jan 23, 2024

Conversation

Foda
Copy link
Member

@Foda Foda commented Dec 6, 2023

Description of Change

This PR adds a work-around for an issue only present on Windows 10 (21H2/22H2) where calling and accessing data from Windows.Networking.Connectivity.NetworkInformation is only permitted on the first thread that accesses it. See: microsoft/WindowsAppSDK#2965

The work-around uses the same underlying APIs (Win32 NLM) as the ones used by WinRT combined with the native .NET Networking APIs.

Issues Fixed

Mike Corsaro added 2 commits December 5, 2023 14:54
Reimplement `ConnectivityImplementation.ConnectionProfiles` to use native .net core APIs
@Foda Foda added platform/windows 🪟 area-essentials Essentials: Device, Display, Connectivity, Secure Storage, Sensors, App Info legacy-area-desktop Windows / WinUI / Project Reunion & Mac Catalyst / macOS specifics (Menus & other Controls)) partner/cat 😻 this is an issue that impacts one of our partners or a customer our advisory team is engaged with labels Dec 6, 2023
@Foda Foda requested review from mattleibow and PureWeen December 6, 2023 18:13
@Foda Foda requested a review from a team as a code owner December 6, 2023 18:13
@Foda
Copy link
Member Author

Foda commented Jan 8, 2024

/rebase

@github-actions github-actions bot force-pushed the foda/Win10NetworkIssue branch from c537c5a to 7f31319 Compare January 8, 2024 17:59
@PureWeen
Copy link
Member

/rebase

@github-actions github-actions bot force-pushed the foda/Win10NetworkIssue branch from 7f31319 to b3df247 Compare January 10, 2024 18:56
@rmarinho
Copy link
Member

Should we add a manual test to our plan to test the connectivity in all platforms ?

Copy link
Member

@PureWeen PureWeen left a comment

Choose a reason for hiding this comment

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

The state of the connectivity variable looks to be backwards inside the event.

If I enable my network connection, then the state is "None"
If I disable my network the connection, goes to "Internet"

It does look like this code fixes the threading crash issue.

image

@mattleibow
Copy link
Member

/rebase

@github-actions github-actions bot force-pushed the foda/Win10NetworkIssue branch from b3df247 to 58ed3f9 Compare January 18, 2024 17:01
@mattleibow
Copy link
Member

@Foda I see you reverted the events, are you doing something more or is this ready to review/test again?

@mattleibow
Copy link
Member

This PR seems to work on 22H2 in a VM, I am testing now on 21H2

@Foda
Copy link
Member Author

Foda commented Jan 19, 2024 via email

@mattleibow
Copy link
Member

Seems to work here too. Looking good!

mattleibow
mattleibow previously approved these changes Jan 19, 2024
@mattleibow mattleibow requested a review from PureWeen January 19, 2024 07:23
@mattleibow mattleibow enabled auto-merge (squash) January 22, 2024 15:52
@PureWeen PureWeen disabled auto-merge January 23, 2024 21:07
@PureWeen PureWeen merged commit 8c04a9d into main Jan 23, 2024
45 of 47 checks passed
@PureWeen PureWeen deleted the foda/Win10NetworkIssue branch January 23, 2024 21:07
@PureWeen
Copy link
Member

Failing test on Android is unrelated to this PR. This PR doesn't touch anything android related.

rmarinho added a commit that referenced this pull request Jan 29, 2024
* Add Obsolete tag for old IndexOf (#20068)

* Split the InputTransparency tests (#19925)

* Bump Xamarin.UITest to 4.3.4 (#20067)

* Bump Xamarin.UITest to 4.3.4

* Update NUnit

* Restore dotnet tool (#20106)

* Do not use underscores in the ApplicationId (#19377)

* Do not use underscores in the ApplicationId

Underscores are not supported on Windows

* Update DotnetInternal.cs

* [WinUI] Add workaround for Connectivity check on Win10  (#19261)

* Implemented a Win10 work-around for connectivity thread issue

Reimplement `ConnectivityImplementation.ConnectionProfiles` to use native .net core APIs

* Remove prop bag ID

* Replace network availability changed event with native .net API

* Remove explicit file include

* Fix file naming

* Implemented a Win10 work-around for connectivity thread issue

Reimplement `ConnectivityImplementation.ConnectionProfiles` to use native .net core APIs

* Remove prop bag ID

* Replace network availability changed event with native .net API

* Remove explicit file include

* Fix file naming

* * Revert change to remove `Windows.Networking.Connectivity.NetworkInformation.NetworkStatusChanged`

* Update Connectivity.uwp.cs

---------

Co-authored-by: Mike Corsaro <mikecorsaro@microsoft.com>
Co-authored-by: Matthew Leibowitz <mattleibow@live.com>

* [Windows] Adjust recycle check in ItemContentControl (#20079)

* Adjust recycle check in `ItemContentControl` to use Content property

* Fix confusing line

* Rectify the scopes in the builder (#19932)

* Update System.Drawing.Common (#20122)

* Enable building WASDK Self-Contained packaged apps (#20019)

* Translucent and Transparent NavigationBar on iOS (#19204)

* Changes for enabling translucent navigation bar ios

* Add UITest for NavigationPage Safe Area Translucence

* remove UIKit

* Move UITest from gallery to Issues

* push a new page for UITests, consolidate code, and save shadowimage

* Changes for enabling translucent navigation bar ios

* Add UITest for NavigationPage Safe Area Translucence

* remove UIKit

* Move UITest from gallery to Issues

* make the extension method

* use the background color alpha for translucent

* use same alpha logic for pre ios 15

* accidently added testing comments

* add more UITests for the different NavigationPage and Flyout Page scenarios

* use additionalsafeareainsets for the secondary toolbar

* missing new line

* only run the secondary toolbar offset when we have a secondary toolbar

* use existing childViewControllers

* remove the shadow stuff and simplify the expression with null

* style and fixes from merge conflicts

* standardAppearance and scrolledgeappearance should be kept in

* changes after talking with Shane

* change shell behavior to be more like navrenderer with preiOS13

* move code if we are transparent pre13 shell

* remove new lines

* add screenshot tests

* be able to remove and add secondarybar additionalsafeareas

* reset the xaml on the Sandboc MainPage (#20082)

* [Android] Fix gif animation initial state (#14295)

* Fix gifs initial animation state on Android

* Device tests

* Auto-format source code

* Updated sample

* Updated device tests

* Refactor code

* Update src/Core/src/Handlers/Image/ImageHandler.Android.cs

Co-authored-by: Matthew Leibowitz <mattleibow@live.com>

* Remove unnecessary change

* Fix build errors

* Merge branch 'main' into fix-7019

* Fix merge mistake

---------

Co-authored-by: GitHub Actions Autoformatter <autoformat@example.com>
Co-authored-by: Javier Suárez <6755973+jsuarezruiz@users.noreply.github.com>
Co-authored-by: Matthew Leibowitz <mattleibow@live.com>
Co-authored-by: Gerald Versluis <gerald.versluis@microsoft.com>

* Enable Windows Image device tests (#20167)

* [X] allow x:Type extension for BPConverter (#18540)

- fixes #18324

* Source Generator Performance Improvements (#19990)

* Source Generator Performance Improvements:
- Reversed lookup order (Extension second)
- Introduced type cache

```
PERF PROGRESS:
SourceGen - Maui.Controls.Sample (262 XAML files)

1) Unchanged:
- 15640 GetTypeByMetadata calls
- 223s

2) Extensions lookup in XmlTypeXamlExtensions.GetTypeReference() second
- 6232 GetTypeByMetadata calls (~60% reduction)
- 90s                          (~60% reduction)

3) With type cache
- 203 GetTypeByMetadata calls (~97% reduction => ~99% total reduction)
- 6s                          (~93% reduction => ~97% total reduction => 37 times faster!)
```

* - Set uinitial lookupNames capacity to 2 since there won't be more than 2

* Added appium UITest for FlyoutNavigationBetweenItemsWithNavigationStacks (#19444) Fixes #16675

* Add better exception for missing Maps on Windows (#19046)

* Add better exception for missing Maps on Windows

* Update AppHostBuilderExtensions.cs

* [ci] Add missing demands (#20183)

* Add comments to internal methods for XAML Hot Reload usage (#20215)

* Add comments to internal methods for XAML Hot Reload usage

* spacing

* Adding Mobile tag to MAUI project templates (#20191)

Co-authored-by: Luke Westendorf <lukewest@microsoft.com>

---------

Co-authored-by: Tim Miller <drasticactions@users.noreply.github.com>
Co-authored-by: Matthew Leibowitz <mattleibow@live.com>
Co-authored-by: Gerald Versluis <gerald.versluis@microsoft.com>
Co-authored-by: Mike Corsaro <corsarom@gmail.com>
Co-authored-by: Mike Corsaro <mikecorsaro@microsoft.com>
Co-authored-by: TJ Lambert <50846373+tj-devel709@users.noreply.github.com>
Co-authored-by: Javier Suárez <javiersuarezruiz@hotmail.com>
Co-authored-by: GitHub Actions Autoformatter <autoformat@example.com>
Co-authored-by: Javier Suárez <6755973+jsuarezruiz@users.noreply.github.com>
Co-authored-by: Stephane Delcroix <stephane@delcroix.org>
Co-authored-by: Marco Goertz <mgoertz@microsoft.com>
Co-authored-by: MSLukeWest <42553283+MSLukeWest@users.noreply.github.com>
Co-authored-by: Luke Westendorf <lukewest@microsoft.com>
@github-actions github-actions bot locked and limited conversation to collaborators Feb 23, 2024
@Eilon Eilon added t/desktop The issue relates to desktop scenarios (MacOS/MacCatalyst/Windows/WinUI/WinAppSDK) and removed legacy-area-desktop Windows / WinUI / Project Reunion & Mac Catalyst / macOS specifics (Menus & other Controls)) labels May 10, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area-essentials Essentials: Device, Display, Connectivity, Secure Storage, Sensors, App Info fixed-in-8.0.7 fixed-in-9.0.100-preview.1.9973 partner/cat 😻 this is an issue that impacts one of our partners or a customer our advisory team is engaged with platform/windows 🪟 t/desktop The issue relates to desktop scenarios (MacOS/MacCatalyst/Windows/WinUI/WinAppSDK)
Projects
None yet
7 participants