Skip to content
This repository was archived by the owner on Feb 25, 2025. It is now read-only.

Conversation

@liyuqian
Copy link
Contributor

@liyuqian liyuqian commented Sep 4, 2020

Fixes flutter/flutter#65258

The following devicelab tests should pass after this patch:

  • flutter_gallery_sksl_warmup__transition_perf_e2e_ios32
  • flutter_gallery_sksl_warmup_ios32__transition_perf

Fixes flutter/flutter#65258

The following devicelab tests should pass after this patch:
- flutter_gallery_sksl_warmup__transition_perf_e2e_ios32
- flutter_gallery_sksl_warmup_ios32__transition_perf
@flutter-dashboard
Copy link

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.

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

@auto-assign auto-assign bot requested a review from flar September 4, 2020 20:43
@jason-simmons
Copy link
Member

What is the behavior of rewinddir when it fails?

If calling rewinddir on the duped directory file descriptor is unreliable on some platforms, then would it be worthwhile to create a version of VisitFiles that takes a directory path instead of an FD?

@liyuqian
Copy link
Contributor Author

liyuqian commented Sep 8, 2020

I don't see anything when it fails. There's no return value or error defined according to this doc.

As for taking a directory path, currently we don't have a unified way of getting that path. In some cases, we use GetCachesDirectory and let the platform return an arbitrary directory without giving us a path. It seems that unless the directory is invalid, we can always get a string path representation. Do you think it's a good idea to add a std::string GetCachesDirectoryPath() function that returns a valid path or an empty string on all platforms?

@jason-simmons
Copy link
Member

I'd like to get a better understanding of what happens when rewinddir isn't working as expected.

But if rewinddir is broken, then I'd rather add the GetCachesDirectoryPath function and a path-based VisitFiles instead of having an FD-based VisitFiles that doesn't work reliably.

@liyuqian
Copy link
Contributor Author

liyuqian commented Sep 9, 2020

@jason-simmons : it turns out that we don't need to open from the root directory. Just reopen the last sub-directory is enough so the fix could be very short. I'm closing this PR in favor of #21050.

@liyuqian liyuqian closed this Sep 9, 2020
@liyuqian liyuqian deleted the ios_file_fix branch September 28, 2020 23:41
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

iPhone 4s didn't properly rewinddir which causes SkSL tests to be flaky

3 participants