Windows: Fix fml::GetFullHandlePath for WINUWP target

[clarkezone]

Although the documentation claims that GetFinalPathNameByHandle is supported for UWP apps, turns out it does not work correctly hence the need to workaround by maintaining a map of file handles to absolute paths in order to be able to satisfy GetFullHandlePath for the WINUWP target.

flutter/flutter#14967
flutter/flutter#70196

Pre-launch Checklist

• I read the Contributor Guide and followed the process outlined there for submitting PRs.
• I read the Tree Hygiene wiki page, which explains my responsibilities.
• I read and followed the Flutter Style Guide and the C++, Objective-C, Java style guides.
• I listed at least one issue that this PR fixes in the description above.
• I added new tests to check the change I am making or feature I am adding, or Hixie said the PR is test-exempt. See testing the engine for instructions on (WINUWP test infra not yet functional)
  writing and running engine tests.
• I updated/added relevant documentation (doc comments with ///).
• I signed the CLA.
• All existing and new tests are passing.
• The reviewer has submitted any presubmit flakes in this PR using the engine presubmit flakes form before re-triggering the failure.

If you need help, consider asking for advice on the #hackers-new channel on Discord.

[flutter-dashboard]

It looks like this pull request may not have tests. Please make sure to add tests before merging. If you need an exemption to this rule, contact Hixie on the #hackers channel in Chat.

If you are not sure if you need tests, consider this rule of thumb: the purpose of a test is to make sure someone doesn't accidentally revert the fix. Ask yourself, is there anything in your PR that you feel it is important we not accidentally revert back to how it was before your fix?

Reviewers: Read the Tree Hygiene page and make sure this patch meets those guidelines before LGTMing.

[chinmaygarde]

This has a number of issues:

• Access to the map is not thread-safe. These APIs can be accessed on any thread.
• The map may be keyed by handles that have been closed. If handles are reused in the process, this will lead to incorrect results.
• The map will grow indefinitely.

What is the error returned on UWP when the lookup fails? We may be able to tackle this in a more elegant way. If not, a bespoke implementation of file_win or a rethinking of assumptions about being able to lookup file names from handles would likely be necessary.

[clarkezone]

The error returned is access denied. This may be a Windows bug since the directory in question is within the assets path of the app, and win32 operations are supposed to be supported in there but it's a moot point since it is clearly bust and we need to work around it.

1. Making it threadsafe is easy.
2. This is a valid problem. It may be possible to mitigate using GetFileInformationByHandle to validate file handles before dereferencing from the cache
3. Not sure the best way to handle this as it is call pattern dependent. Every file open call calls GetAbsolutePath which in turn calls GetFullHandlePath on the directory's handle. With the caching approach, lifetime is hard.. there is no CloseDirectory so when to purge the cache? We can limit the size by using a stack, depending on typical calling pattern that may work assuming the directory in question has file path resolutions happen within the lifetime of the cache items created.

To stand up UWP, hoping to avoid a big refactor here so looking for the least bad solution.

[clarkezone]

@chinmaygarde to address 3) in your list above I'm considering the following approach

1. Create a UWP specific UniqueFDTraits object
2. Move global handle->absolute path state to that object
3. For UniqueFDTraits::Free impl from 1 remove the item from the map for the given handle

Before I go too far building this, does that in conjunction with the previous commit satisfy your criteria of fixing the issues and sufficiently elegant approach?

[chinmaygarde]

Since this is the bringup of UWP, would you prefer filing bugs for such issues instead? I don't want insist of large API reworks while you are in the process of quickly tackling each minor UWP specific issue. We can file a bug and tag it such that we ensure we get to it before declaring this variant stable. I am not sure how close you are to getting this variant working though.

I am not comfortable with global locks around the map but other than that, this seems to adequately handle the concerns I listed.

[clarkezone]

I didn't get the tests done yesterday and will be offline today. I'll get those done tomorrow. Keen to get this in by EOD tomorrow if at all possible, hence LMK what else needed to land. Thx again for the review.

[clarkezone]

Confirmed that new code is completely covered by existing tests. Please review comments and LMK if you have any more feedback.

[chinmaygarde]

Sorry, I haven't been able to get to this on my Windows workstation to figure out why the static inlines were necessary in the header. I'll address this over the weekend.

[chinmaygarde]

@clarkezone I haven't had a chance to reproduce on Windows yet. @cbracken If you have a setup ready, will be you able to take a look please? If not, depending on the urgency of this issue, we can land it now and followup on a fix for it via a bug.

[googlebot]

All (the pull request submitter and all commit authors) CLAs are signed, but one or more commits were authored or co-authored by someone other than the pull request submitter.

We need to confirm that all authors are ok with their commits being contributed to this project. Please have them confirm that by leaving a comment that contains only @googlebot I consent. in this pull request.

Note to project maintainer: There may be cases where the author cannot leave a comment, or the comment is not properly detected as consent. In those cases, you can manually confirm consent of the commit author(s), and set the cla label to yes (if enabled on your project).

ℹ️ Googlers: Go here for more info.

[google-cla]

All (the pull request submitter and all commit authors) CLAs are signed, but one or more commits were authored or co-authored by someone other than the pull request submitter.

We need to confirm that all authors are ok with their commits being contributed to this project. Please have them confirm that by leaving a comment that contains only @googlebot I consent. in this pull request.

Note to project maintainer: There may be cases where the author cannot leave a comment, or the comment is not properly detected as consent. In those cases, you can manually confirm consent of the commit author(s), and set the cla label to yes (if enabled on your project).

ℹ️ Googlers: Go here for more info.

[google-cla]

A Googler has manually verified that the CLAs look good.

(Googler, please make sure the reason for overriding the CLA status is clearly documented in these comments.)

ℹ️ Googlers: Go here for more info.

[googlebot]

A Googler has manually verified that the CLAs look good.

(Googler, please make sure the reason for overriding the CLA status is clearly documented in these comments.)

ℹ️ Googlers: Go here for more info.

[cbracken]

Added a patch with the fix for the linker issue. Thanks for your patience on this @clarkezone!

[google-cla]

All (the pull request submitter and all commit authors) CLAs are signed, but one or more commits were authored or co-authored by someone other than the pull request submitter.

We need to confirm that all authors are ok with their commits being contributed to this project. Please have them confirm that by leaving a comment that contains only @googlebot I consent. in this pull request.

Note to project maintainer: There may be cases where the author cannot leave a comment, or the comment is not properly detected as consent. In those cases, you can manually confirm consent of the commit author(s), and set the cla label to yes (if enabled on your project).

ℹ️ Googlers: Go here for more info.

[googlebot]

All (the pull request submitter and all commit authors) CLAs are signed, but one or more commits were authored or co-authored by someone other than the pull request submitter.

We need to confirm that all authors are ok with their commits being contributed to this project. Please have them confirm that by leaving a comment that contains only @googlebot I consent. in this pull request.

Note to project maintainer: There may be cases where the author cannot leave a comment, or the comment is not properly detected as consent. In those cases, you can manually confirm consent of the commit author(s), and set the cla label to yes (if enabled on your project).

ℹ️ Googlers: Go here for more info.

[google-cla]

A Googler has manually verified that the CLAs look good.

(Googler, please make sure the reason for overriding the CLA status is clearly documented in these comments.)

ℹ️ Googlers: Go here for more info.

[googlebot]

A Googler has manually verified that the CLAs look good.

(Googler, please make sure the reason for overriding the CLA status is clearly documented in these comments.)

ℹ️ Googlers: Go here for more info.

[cbracken]

Only change was to rebase to tip of tree. CLAs are still fine.

[fluttergithubbot]

This pull request is not suitable for automatic merging in its current state.

• This pull request has changes requested by @chinmaygarde. Please resolve those before re-applying the label.