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

Conversation

@gaaclarke
Copy link
Member

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.

  • I read the contributor guide and followed the process outlined there for submitting PRs.
  • I signed the CLA.
  • I read and followed the C++, Objective-C, Java style guides for the engine.
  • I read the tree hygiene wiki page, which explains my responsibilities.
  • I updated/added relevant documentation.
  • All existing and new tests are passing.
  • I am willing to follow-up on review comments in a timely manner.

Breaking Change

Did any tests fail when you ran them? Please read handling breaking changes.

reachable because of permissions problems.
@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.

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 '"
Copy link
Member

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...

Copy link
Member Author

Choose a reason for hiding this comment

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

Sg, done.

Copy link
Member

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.

@jmagman
Copy link
Member

jmagman commented Sep 2, 2020

build_ios_debug_sim didn't like kDNSServiceErr_PolicyDenied
https://logs.chromium.org/logs/flutter/buildbucket/cr-buildbucket.appspot.com/8870232824019334080/+/steps/build_ios_debug_sim/0/stdout

../../flutter/shell/platform/darwin/ios/framework/Source/FlutterObservatoryPublisher.mm:135:27: error: use of undeclared identifier 'kDNSServiceErr_PolicyDenied'
  } else if (errorCode == kDNSServiceErr_PolicyDenied) {

@gaaclarke
Copy link
Member Author

build_ios_debug_sim didn't like kDNSServiceErr_PolicyDenied
https://logs.chromium.org/logs/flutter/buildbucket/cr-buildbucket.appspot.com/8870232824019334080/+/steps/build_ios_debug_sim/0/stdout

../../flutter/shell/platform/darwin/ios/framework/Source/FlutterObservatoryPublisher.mm:135:27: error: use of undeclared identifier 'kDNSServiceErr_PolicyDenied'
  } else if (errorCode == kDNSServiceErr_PolicyDenied) {

I didn't see a guard in the header, but apparently it is iOS 14.0+.

@gaaclarke gaaclarke requested a review from jmagman September 3, 2020 17:35
Copy link
Member

@jmagman jmagman left a 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 '"
Copy link
Member

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
Copy link
Member

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.

Copy link
Member Author

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.

Comment on lines +131 to +132
// Found in usr/include/dns_sd.h.
-65570;
Copy link
Member

Choose a reason for hiding this comment

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

👍

@gaaclarke gaaclarke merged commit 5047e5a into flutter:master Sep 3, 2020
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Sep 3, 2020
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Sep 4, 2020
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Sep 4, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants