-
Notifications
You must be signed in to change notification settings - Fork 6k
Started printing out error messages when the observatory won't be reachable because of permissions problems. #20960
Started printing out error messages when the observatory won't be reachable because of permissions problems. #20960
Conversation
reachable because of permissions problems.
|
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. |
| if (err != 0) { | ||
| FML_LOG(ERROR) << "Failed to register observatory port with mDNS."; | ||
| if (@available(iOS 14.0, *)) { | ||
| FML_LOG(ERROR) << "Make sure the 'NSBonjourServices' key is set in your Info.plist for '" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we point out it's only needed for Debug and Profile configuration Info.plists? At some point we should write up some info on the website, but in the meantime...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sg, done.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Debug and Profile, not just Debug.
|
|
I didn't see a guard in the header, but apparently it is iOS 14.0+. |
jmagman
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM with nit that the key should be added for both Debug and Profile.
| if (err != 0) { | ||
| FML_LOG(ERROR) << "Failed to register observatory port with mDNS."; | ||
| if (@available(iOS 14.0, *)) { | ||
| FML_LOG(ERROR) << "Make sure the 'NSBonjourServices' key is set in your Info.plist for '" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Debug and Profile, not just Debug.
|
|
||
| /// TODO(aaclarke): Remove this preprocessor macro once infra is moved to Xcode 12. | ||
| static const DNSServiceErrorType kFlutter_DNSServiceErr_PolicyDenied = | ||
| #if __IPHONE_OS_VERSION_MAX_ALLOWED >= 140000 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you use a @available here? I've needed to use this macro outside of the class (like for imports) but hopefully @available works inline? Not a big deal though.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Na, that's why I messed it up the first time. It's in an enum declaration that doesn't have annotations or guards of what version the operating system is.
| // Found in usr/include/dns_sd.h. | ||
| -65570; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍
…t be reachable because of permissions problems. (flutter/engine#20960)
…t be reachable because of permissions problems. (flutter/engine#20960)
…t be reachable because of permissions problems. (flutter/engine#20960)
Description
iOS 14 has changed their policy for mdns permissions. This provides some guidance if users run afoul of them.
Related Issues
flutter/flutter#60634
flutter/flutter#63893
Tests
I added the following tests:
Checklist
Before you create this PR confirm that it meets all requirements listed below by checking the relevant checkboxes (
[x]). This will ensure a smooth and quick review process.Breaking Change
Did any tests fail when you ran them? Please read handling breaking changes.