-
Notifications
You must be signed in to change notification settings - Fork 6k
Add flag to not publish the observatory port over mDNS #21632
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -544,7 +544,8 @@ - (BOOL)createShell:(NSString*)entrypoint | |
| if (!_platformViewsController) { | ||
| _platformViewsController.reset(new flutter::FlutterPlatformViewsController()); | ||
| } | ||
| _publisher.reset([[FlutterObservatoryPublisher alloc] init]); | ||
| _publisher.reset([[FlutterObservatoryPublisher alloc] | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. For a test, maybe put this in a separate method, OCMPartialMock it to return a OCMPartialMock of a publisher and make sure its enableObservatoryPublication is never called? We're already injecting ocmocked projects in flutterenginetests There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The hard part is mocking out the There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I got bogged down with this. I want to merge this now to unblock g3 using this flag. |
||
| initWithEnableObservatoryPublication:settings.enable_observatory_publication]); | ||
| [self maybeSetupPlatformViewChannels]; | ||
| _shell->GetIsGpuDisabledSyncSwitch()->SetSwitch(_isGpuDisabled ? true : false); | ||
| if (profilerEnabled) { | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -37,26 +37,23 @@ @implementation FlutterObservatoryPublisher | |
| #include <net/if.h> | ||
|
|
||
| #include "flutter/fml/logging.h" | ||
| #include "flutter/fml/make_copyable.h" | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Unused imports. |
||
| #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" | ||
|
|
||
| @protocol FlutterObservatoryPublisherDelegate | ||
| - (instancetype)initWithOwner:(FlutterObservatoryPublisher*)owner; | ||
| - (void)publishServiceProtocolPort:(NSString*)uri; | ||
| - (void)publishServiceProtocolPort:(NSURL*)uri; | ||
| - (void)stopService; | ||
|
|
||
| @property(readonly) fml::scoped_nsobject<NSURL> url; | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Removed the delegate |
||
| @end | ||
|
|
||
| @interface FlutterObservatoryPublisher () | ||
| - (NSData*)createTxtData:(NSURL*)url; | ||
| + (NSData*)createTxtData:(NSURL*)url; | ||
|
|
||
| @property(readonly) NSString* serviceName; | ||
|
Comment on lines
-56
to
-58
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. By making these class methods, I was able to remove the need for the delegate to hang onto a back pointer ( |
||
| @property(readonly, class) NSString* serviceName; | ||
| @property(readonly) fml::scoped_nsobject<NSObject<FlutterObservatoryPublisherDelegate>> delegate; | ||
| @property(nonatomic, readwrite) NSURL* url; | ||
| @property(readonly) BOOL enableObservatoryPublication; | ||
|
|
||
| @end | ||
|
|
||
|
|
@@ -68,31 +65,17 @@ @interface ObservatoryDNSServiceDelegate : NSObject <FlutterObservatoryPublisher | |
| @end | ||
|
|
||
| @implementation ObservatoryDNSServiceDelegate { | ||
| fml::scoped_nsobject<FlutterObservatoryPublisher> _owner; | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. |
||
| 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); | ||
| _dnsServiceRef = NULL; | ||
| } | ||
| } | ||
|
|
||
| - (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. | ||
|
|
@@ -105,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 << "."; | ||
|
|
@@ -122,8 +105,8 @@ - (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()); | ||
|
|
@@ -162,34 +145,21 @@ static void DNSSD_API registrationCallback(DNSServiceRef sdRef, | |
| @end | ||
|
|
||
| @implementation ObservatoryNSNetServiceDelegate { | ||
| fml::scoped_nsobject<FlutterObservatoryPublisher> _owner; | ||
| fml::scoped_nsobject<NSNetService> _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]; | ||
|
|
@@ -211,19 +181,16 @@ @implementation FlutterObservatoryPublisher { | |
| std::unique_ptr<fml::WeakPtrFactory<FlutterObservatoryPublisher>> _weakFactory; | ||
| } | ||
|
|
||
| - (NSURL*)url { | ||
| return [_delegate.get().url autorelease]; | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Now |
||
| } | ||
|
|
||
| - (instancetype)init { | ||
| - (instancetype)initWithEnableObservatoryPublication:(BOOL)enableObservatoryPublication { | ||
| self = [super init]; | ||
| NSAssert(self, @"Super must not return null on init."); | ||
|
|
||
| if (@available(iOS 9.3, *)) { | ||
| _delegate.reset([[ObservatoryDNSServiceDelegate alloc] initWithOwner:self]); | ||
| _delegate.reset([[ObservatoryDNSServiceDelegate alloc] init]); | ||
| } else { | ||
| _delegate.reset([[ObservatoryNSNetServiceDelegate alloc] initWithOwner:self]); | ||
| _delegate.reset([[ObservatoryNSNetServiceDelegate alloc] init]); | ||
| } | ||
| _enableObservatoryPublication = enableObservatoryPublication; | ||
| _weakFactory = std::make_unique<fml::WeakPtrFactory<FlutterObservatoryPublisher>>(self); | ||
|
|
||
| fml::MessageLoop::EnsureInitializedForCurrentThread(); | ||
|
|
@@ -233,9 +200,15 @@ - (instancetype)init { | |
| 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) { | ||
| [[weak.get() delegate] | ||
| publishServiceProtocolPort:[NSString stringWithUTF8String:uri.c_str()]]; | ||
| NSURL* url = | ||
| [[NSURL alloc] initWithString:[NSString stringWithUTF8String:uri.c_str()]]; | ||
| weak.get().url = url; | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @gaaclarke What's the engine idiomatic way to grab a strong pointer to retain this in scope, instead of getting the weak pointer each time? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I don't believe there is one. The weakptr's are limited to being used one a single thread so there is no risk that it will get deleted or edited in this scope. |
||
| if (weak.get().enableObservatoryPublication) { | ||
| [[weak.get() delegate] publishServiceProtocolPort:url]; | ||
| } | ||
| } | ||
| }); | ||
| } | ||
|
|
@@ -244,11 +217,11 @@ - (instancetype)init { | |
| 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])]; | ||
|
|
@@ -261,6 +234,7 @@ - (NSData*)createTxtData:(NSURL*)url { | |
|
|
||
| - (void)dealloc { | ||
| [_delegate stopService]; | ||
| [_url release]; | ||
|
|
||
| flutter::DartServiceIsolate::RemoveServerStatusCallback(std::move(_callbackHandle)); | ||
| [super dealloc]; | ||
|
|
||
Uh oh!
There was an error while loading. Please reload this page.