From e60b6424a72385e815d688dd7727976f2525c86c Mon Sep 17 00:00:00 2001 From: Jenn Magder Date: Mon, 28 Sep 2020 15:26:18 -0700 Subject: [PATCH 1/3] Add flag to not publish the observatory port over mDNS --- common/settings.cc | 2 + common/settings.h | 5 ++ shell/common/switches.cc | 4 ++ shell/common/switches.h | 3 ++ .../Source/FlutterObservatoryPublisher.mm | 49 +++++++++++-------- 5 files changed, 43 insertions(+), 20 deletions(-) diff --git a/common/settings.cc b/common/settings.cc index 1508e213bcc6c..ec8ddf60f9c8d 100644 --- a/common/settings.cc +++ b/common/settings.cc @@ -46,6 +46,8 @@ std::string Settings::ToString() const { stream << "enable_dart_profiling: " << enable_dart_profiling << std::endl; stream << "disable_dart_asserts: " << disable_dart_asserts << std::endl; stream << "enable_observatory: " << enable_observatory << std::endl; + stream << "enable_observatory_publication: " << enable_observatory_publication + << std::endl; stream << "observatory_host: " << observatory_host << std::endl; stream << "observatory_port: " << observatory_port << std::endl; stream << "use_test_fonts: " << use_test_fonts << std::endl; diff --git a/common/settings.h b/common/settings.h index 46f88399b407e..a14a7cd26d252 100644 --- a/common/settings.h +++ b/common/settings.h @@ -126,6 +126,11 @@ struct Settings { // Whether the Dart VM service should be enabled. bool enable_observatory = false; + // Whether to publish the observatory URL over mDNS. + // On iOS 14 this prompts a local network permission dialog, + // which cannot be accepted or dismissed in a CI environment. + bool enable_observatory_publication = false; + // The IP address to which the Dart VM service is bound. std::string observatory_host; diff --git a/shell/common/switches.cc b/shell/common/switches.cc index 1b2398b942ff0..3f80830136cdc 100644 --- a/shell/common/switches.cc +++ b/shell/common/switches.cc @@ -221,6 +221,10 @@ Settings SettingsFromCommandLine(const fml::CommandLine& command_line) { settings.enable_observatory = !command_line.HasOption(FlagForSwitch(Switch::DisableObservatory)); + // Enable mDNS Observatory Publication + settings.enable_observatory_publication = !command_line.HasOption( + FlagForSwitch(Switch::DisableObservatoryPublication)); + // Set Observatory Host if (command_line.HasOption(FlagForSwitch(Switch::DeviceObservatoryHost))) { command_line.GetOptionValue(FlagForSwitch(Switch::DeviceObservatoryHost), diff --git a/shell/common/switches.h b/shell/common/switches.h index f33e2722403df..87eb522201ce6 100644 --- a/shell/common/switches.h +++ b/shell/common/switches.h @@ -79,6 +79,9 @@ DEF_SWITCH(DisableObservatory, "disable-observatory", "Disable the Dart Observatory. The observatory is never available " "in release mode.") +DEF_SWITCH(DisableObservatoryPublication, + "disable-observatory-publication", + "Disable mDNS Dart Observatory publication.") DEF_SWITCH(IPv6, "ipv6", "Bind to the IPv6 localhost address for the Dart Observatory. " diff --git a/shell/platform/darwin/ios/framework/Source/FlutterObservatoryPublisher.mm b/shell/platform/darwin/ios/framework/Source/FlutterObservatoryPublisher.mm index 072a244a967fc..134cf8b7c32e4 100644 --- a/shell/platform/darwin/ios/framework/Source/FlutterObservatoryPublisher.mm +++ b/shell/platform/darwin/ios/framework/Source/FlutterObservatoryPublisher.mm @@ -43,6 +43,8 @@ @implementation FlutterObservatoryPublisher #include "flutter/fml/platform/darwin/scoped_nsobject.h" #include "flutter/fml/task_runner.h" #include "flutter/runtime/dart_service_isolate.h" +#include "flutter/shell/common/switches.h" +#include "flutter/shell/platform/darwin/common/command_line.h" @protocol FlutterObservatoryPublisherDelegate - (instancetype)initWithOwner:(FlutterObservatoryPublisher*)owner; @@ -219,27 +221,32 @@ - (instancetype)init { self = [super init]; NSAssert(self, @"Super must not return null on init."); - if (@available(iOS 9.3, *)) { - _delegate.reset([[ObservatoryDNSServiceDelegate alloc] initWithOwner:self]); + auto command_line = flutter::CommandLineFromNSProcessInfo(); + auto settings = flutter::SettingsFromCommandLine(command_line); + if (settings.enable_observatory_publication) { + if (@available(iOS 9.3, *)) { + _delegate.reset([[ObservatoryDNSServiceDelegate alloc] initWithOwner:self]); + } else { + _delegate.reset([[ObservatoryNSNetServiceDelegate alloc] initWithOwner:self]); + } + _weakFactory = std::make_unique>(self); + + fml::MessageLoop::EnsureInitializedForCurrentThread(); + _callbackHandle = flutter::DartServiceIsolate::AddServerStatusCallback( + [weak = _weakFactory->GetWeakPtr(), + runner = fml::MessageLoop::GetCurrent().GetTaskRunner()](const std::string& uri) { + if (!uri.empty()) { + runner->PostTask([weak, uri]() { + if (weak) { + [[weak.get() delegate] + publishServiceProtocolPort:[NSString stringWithUTF8String:uri.c_str()]]; + } + }); + } + }); } else { - _delegate.reset([[ObservatoryNSNetServiceDelegate alloc] initWithOwner:self]); + FML_LOG(INFO) << "Skipping mDNS obsesrvatory publishing"; } - _weakFactory = std::make_unique>(self); - - fml::MessageLoop::EnsureInitializedForCurrentThread(); - - _callbackHandle = flutter::DartServiceIsolate::AddServerStatusCallback( - [weak = _weakFactory->GetWeakPtr(), - runner = fml::MessageLoop::GetCurrent().GetTaskRunner()](const std::string& uri) { - if (!uri.empty()) { - runner->PostTask([weak, uri]() { - if (weak) { - [[weak.get() delegate] - publishServiceProtocolPort:[NSString stringWithUTF8String:uri.c_str()]]; - } - }); - } - }); return self; } @@ -262,7 +269,9 @@ - (NSData*)createTxtData:(NSURL*)url { - (void)dealloc { [_delegate stopService]; - flutter::DartServiceIsolate::RemoveServerStatusCallback(std::move(_callbackHandle)); + if (_callbackHandle) { + flutter::DartServiceIsolate::RemoveServerStatusCallback(std::move(_callbackHandle)); + } [super dealloc]; } @end From 08e8f34ffb976bb88fba2b74ed9b1360d6b0f335 Mon Sep 17 00:00:00 2001 From: Jenn Magder Date: Tue, 6 Oct 2020 14:18:34 -0700 Subject: [PATCH 2/3] Review edits --- common/settings.h | 2 +- .../ios/framework/Source/FlutterEngine.mm | 3 +- .../Source/FlutterObservatoryPublisher.h | 5 + .../Source/FlutterObservatoryPublisher.mm | 136 +++++++----------- 4 files changed, 58 insertions(+), 88 deletions(-) diff --git a/common/settings.h b/common/settings.h index a14a7cd26d252..d85506b946e25 100644 --- a/common/settings.h +++ b/common/settings.h @@ -129,7 +129,7 @@ struct Settings { // Whether to publish the observatory URL over mDNS. // On iOS 14 this prompts a local network permission dialog, // which cannot be accepted or dismissed in a CI environment. - bool enable_observatory_publication = false; + bool enable_observatory_publication = true; // The IP address to which the Dart VM service is bound. std::string observatory_host; diff --git a/shell/platform/darwin/ios/framework/Source/FlutterEngine.mm b/shell/platform/darwin/ios/framework/Source/FlutterEngine.mm index 347b853270e89..b1f5e087da6aa 100644 --- a/shell/platform/darwin/ios/framework/Source/FlutterEngine.mm +++ b/shell/platform/darwin/ios/framework/Source/FlutterEngine.mm @@ -544,7 +544,8 @@ - (BOOL)createShell:(NSString*)entrypoint if (!_platformViewsController) { _platformViewsController.reset(new flutter::FlutterPlatformViewsController()); } - _publisher.reset([[FlutterObservatoryPublisher alloc] init]); + _publisher.reset([[FlutterObservatoryPublisher alloc] + initWithEnableObservatoryPublication:settings.enable_observatory_publication]); [self maybeSetupPlatformViewChannels]; _shell->GetIsGpuDisabledSyncSwitch()->SetSwitch(_isGpuDisabled ? true : false); if (profilerEnabled) { diff --git a/shell/platform/darwin/ios/framework/Source/FlutterObservatoryPublisher.h b/shell/platform/darwin/ios/framework/Source/FlutterObservatoryPublisher.h index 387176f76e25e..a00c17ad89245 100644 --- a/shell/platform/darwin/ios/framework/Source/FlutterObservatoryPublisher.h +++ b/shell/platform/darwin/ios/framework/Source/FlutterObservatoryPublisher.h @@ -9,6 +9,11 @@ @interface FlutterObservatoryPublisher : NSObject +- (instancetype)initWithEnableObservatoryPublication:(BOOL)enableObservatoryPublication + NS_DESIGNATED_INITIALIZER; +- (instancetype)init NS_UNAVAILABLE; ++ (instancetype)new NS_UNAVAILABLE; + @property(nonatomic, readonly) NSURL* url; @end diff --git a/shell/platform/darwin/ios/framework/Source/FlutterObservatoryPublisher.mm b/shell/platform/darwin/ios/framework/Source/FlutterObservatoryPublisher.mm index 134cf8b7c32e4..c54a42400602c 100644 --- a/shell/platform/darwin/ios/framework/Source/FlutterObservatoryPublisher.mm +++ b/shell/platform/darwin/ios/framework/Source/FlutterObservatoryPublisher.mm @@ -37,28 +37,23 @@ @implementation FlutterObservatoryPublisher #include #include "flutter/fml/logging.h" -#include "flutter/fml/make_copyable.h" #include "flutter/fml/memory/weak_ptr.h" #include "flutter/fml/message_loop.h" #include "flutter/fml/platform/darwin/scoped_nsobject.h" -#include "flutter/fml/task_runner.h" #include "flutter/runtime/dart_service_isolate.h" -#include "flutter/shell/common/switches.h" -#include "flutter/shell/platform/darwin/common/command_line.h" @protocol FlutterObservatoryPublisherDelegate -- (instancetype)initWithOwner:(FlutterObservatoryPublisher*)owner; -- (void)publishServiceProtocolPort:(NSString*)uri; +- (void)publishServiceProtocolPort:(NSURL*)uri; - (void)stopService; - -@property(readonly) fml::scoped_nsobject url; @end @interface FlutterObservatoryPublisher () -- (NSData*)createTxtData:(NSURL*)url; ++ (NSData*)createTxtData:(NSURL*)url; -@property(readonly) NSString* serviceName; +@property(readonly, class) NSString* serviceName; @property(readonly) fml::scoped_nsobject> delegate; +@property(nonatomic, readwrite) NSURL* url; +@property(readonly) BOOL enableObservatoryPublication; @end @@ -70,19 +65,9 @@ @interface ObservatoryDNSServiceDelegate : NSObject _owner; DNSServiceRef _dnsServiceRef; } -@synthesize url; - -- (instancetype)initWithOwner:(FlutterObservatoryPublisher*)owner { - self = [super init]; - NSAssert(self, @"Super must not return null on init."); - _owner.reset([owner retain]); - return self; -} - - (void)stopService { if (_dnsServiceRef) { DNSServiceRefDeallocate(_dnsServiceRef); @@ -90,11 +75,7 @@ - (void)stopService { } } -- (void)publishServiceProtocolPort:(NSString*)uri { - // uri comes in as something like 'http://127.0.0.1:XXXXX/' where XXXXX is the port - // number. - url.reset([[NSURL alloc] initWithString:uri]); - +- (void)publishServiceProtocolPort:(NSURL*)url { DNSServiceFlags flags = kDNSServiceFlagsDefault; #if TARGET_IPHONE_SIMULATOR // Simulator needs to use local loopback explicitly to work. @@ -107,11 +88,11 @@ - (void)publishServiceProtocolPort:(NSString*)uri { const char* domain = "local."; // default domain uint16_t port = [[url port] unsignedShortValue]; - NSData* txtData = [_owner createTxtData:url.get()]; - int err = - DNSServiceRegister(&_dnsServiceRef, flags, interfaceIndex, - [_owner.get().serviceName UTF8String], registrationType, domain, NULL, - htons(port), txtData.length, txtData.bytes, registrationCallback, NULL); + NSData* txtData = [FlutterObservatoryPublisher createTxtData:url]; + int err = DNSServiceRegister(&_dnsServiceRef, flags, interfaceIndex, + FlutterObservatoryPublisher.serviceName.UTF8String, registrationType, + domain, NULL, htons(port), txtData.length, txtData.bytes, + registrationCallback, NULL); if (err != 0) { FML_LOG(ERROR) << "Failed to register observatory port with mDNS with error " << err << "."; @@ -124,8 +105,7 @@ - (void)publishServiceProtocolPort:(NSString*)uri { << "to the 'NSBonjourServices' key in your Info.plist for the Debug/" << "Profile configurations. " << "For more information, see " - // Update link to a specific header as needed. - << "https://flutter.dev/docs/development/add-to-app/ios/project-setup"; + << "https://flutter.dev/docs/development/add-to-app/ios/project-setup#local-network-privacy-permissions"; } } else { DNSServiceSetDispatchQueue(_dnsServiceRef, dispatch_get_main_queue()); @@ -164,34 +144,21 @@ static void DNSSD_API registrationCallback(DNSServiceRef sdRef, @end @implementation ObservatoryNSNetServiceDelegate { - fml::scoped_nsobject _owner; fml::scoped_nsobject _netService; } -@synthesize url; - -- (instancetype)initWithOwner:(FlutterObservatoryPublisher*)owner { - self = [super init]; - NSAssert(self, @"Super must not return null on init."); - _owner.reset([owner retain]); - return self; -} - - (void)stopService { [_netService.get() stop]; [_netService.get() setDelegate:nil]; } -- (void)publishServiceProtocolPort:(NSString*)uri { - // uri comes in as something like 'http://127.0.0.1:XXXXX/' where XXXXX is the port - // number. - url.reset([[NSURL alloc] initWithString:uri]); - - NSNetService* netServiceTmp = [[NSNetService alloc] initWithDomain:@"local." - type:@"_dartobservatory._tcp." - name:_owner.get().serviceName - port:[[url port] intValue]]; - [netServiceTmp setTXTRecordData:[_owner createTxtData:url.get()]]; +- (void)publishServiceProtocolPort:(NSURL*)url { + NSNetService* netServiceTmp = + [[NSNetService alloc] initWithDomain:@"local." + type:@"_dartobservatory._tcp." + name:FlutterObservatoryPublisher.serviceName + port:[[url port] intValue]]; + [netServiceTmp setTXTRecordData:[FlutterObservatoryPublisher createTxtData:url]]; _netService.reset(netServiceTmp); [_netService.get() setDelegate:self]; [_netService.get() publish]; @@ -213,49 +180,47 @@ @implementation FlutterObservatoryPublisher { std::unique_ptr> _weakFactory; } -- (NSURL*)url { - return [_delegate.get().url autorelease]; -} - -- (instancetype)init { +- (instancetype)initWithEnableObservatoryPublication:(BOOL)enableObservatoryPublication { self = [super init]; NSAssert(self, @"Super must not return null on init."); - auto command_line = flutter::CommandLineFromNSProcessInfo(); - auto settings = flutter::SettingsFromCommandLine(command_line); - if (settings.enable_observatory_publication) { - if (@available(iOS 9.3, *)) { - _delegate.reset([[ObservatoryDNSServiceDelegate alloc] initWithOwner:self]); - } else { - _delegate.reset([[ObservatoryNSNetServiceDelegate alloc] initWithOwner:self]); - } - _weakFactory = std::make_unique>(self); - - fml::MessageLoop::EnsureInitializedForCurrentThread(); - _callbackHandle = flutter::DartServiceIsolate::AddServerStatusCallback( - [weak = _weakFactory->GetWeakPtr(), - runner = fml::MessageLoop::GetCurrent().GetTaskRunner()](const std::string& uri) { - if (!uri.empty()) { - runner->PostTask([weak, uri]() { - if (weak) { - [[weak.get() delegate] - publishServiceProtocolPort:[NSString stringWithUTF8String:uri.c_str()]]; - } - }); - } - }); + if (@available(iOS 9.3, *)) { + _delegate.reset([[ObservatoryDNSServiceDelegate alloc] init]); } else { - FML_LOG(INFO) << "Skipping mDNS obsesrvatory publishing"; + _delegate.reset([[ObservatoryNSNetServiceDelegate alloc] init]); } + _enableObservatoryPublication = enableObservatoryPublication; + _weakFactory = std::make_unique>(self); + + fml::MessageLoop::EnsureInitializedForCurrentThread(); + + _callbackHandle = flutter::DartServiceIsolate::AddServerStatusCallback( + [weak = _weakFactory->GetWeakPtr(), + runner = fml::MessageLoop::GetCurrent().GetTaskRunner()](const std::string& uri) { + if (!uri.empty()) { + runner->PostTask([weak, uri]() { + // uri comes in as something like 'http://127.0.0.1:XXXXX/' where XXXXX is the port + // number. + if (weak) { + NSURL* url = + [[NSURL alloc] initWithString:[NSString stringWithUTF8String:uri.c_str()]]; + weak.get().url = url; + if (weak.get().enableObservatoryPublication) { + [[weak.get() delegate] publishServiceProtocolPort:url]; + } + } + }); + } + }); return self; } -- (NSString*)serviceName { ++ (NSString*)serviceName { return NSBundle.mainBundle.bundleIdentifier; } -- (NSData*)createTxtData:(NSURL*)url { ++ (NSData*)createTxtData:(NSURL*)url { // Check to see if there's an authentication code. If there is, we'll provide // it as a txt record so flutter tools can establish a connection. NSString* path = [[url path] substringFromIndex:MIN(1, [[url path] length])]; @@ -268,10 +233,9 @@ - (NSData*)createTxtData:(NSURL*)url { - (void)dealloc { [_delegate stopService]; + [_url release]; - if (_callbackHandle) { - flutter::DartServiceIsolate::RemoveServerStatusCallback(std::move(_callbackHandle)); - } + flutter::DartServiceIsolate::RemoveServerStatusCallback(std::move(_callbackHandle)); [super dealloc]; } @end From e446fa6d9b023e5c994615544a2c334cded8a8b5 Mon Sep 17 00:00:00 2001 From: Jenn Magder Date: Wed, 7 Oct 2020 14:58:39 -0700 Subject: [PATCH 3/3] Format --- shell/platform/darwin/ios/framework/Source/FlutterEngine.mm | 2 +- .../darwin/ios/framework/Source/FlutterObservatoryPublisher.mm | 3 ++- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/shell/platform/darwin/ios/framework/Source/FlutterEngine.mm b/shell/platform/darwin/ios/framework/Source/FlutterEngine.mm index b1f5e087da6aa..5704ef29a3d52 100644 --- a/shell/platform/darwin/ios/framework/Source/FlutterEngine.mm +++ b/shell/platform/darwin/ios/framework/Source/FlutterEngine.mm @@ -545,7 +545,7 @@ - (BOOL)createShell:(NSString*)entrypoint _platformViewsController.reset(new flutter::FlutterPlatformViewsController()); } _publisher.reset([[FlutterObservatoryPublisher alloc] - initWithEnableObservatoryPublication:settings.enable_observatory_publication]); + initWithEnableObservatoryPublication:settings.enable_observatory_publication]); [self maybeSetupPlatformViewChannels]; _shell->GetIsGpuDisabledSyncSwitch()->SetSwitch(_isGpuDisabled ? true : false); if (profilerEnabled) { diff --git a/shell/platform/darwin/ios/framework/Source/FlutterObservatoryPublisher.mm b/shell/platform/darwin/ios/framework/Source/FlutterObservatoryPublisher.mm index c54a42400602c..e06ac8ea702a4 100644 --- a/shell/platform/darwin/ios/framework/Source/FlutterObservatoryPublisher.mm +++ b/shell/platform/darwin/ios/framework/Source/FlutterObservatoryPublisher.mm @@ -105,7 +105,8 @@ - (void)publishServiceProtocolPort:(NSURL*)url { << "to the 'NSBonjourServices' key in your Info.plist for the Debug/" << "Profile configurations. " << "For more information, see " - << "https://flutter.dev/docs/development/add-to-app/ios/project-setup#local-network-privacy-permissions"; + << "https://flutter.dev/docs/development/add-to-app/ios/" + "project-setup#local-network-privacy-permissions"; } } else { DNSServiceSetDispatchQueue(_dnsServiceRef, dispatch_get_main_queue());