diff --git a/Gemfile.lock b/Gemfile.lock index 3683a412..ea4b7e14 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -10,7 +10,7 @@ GEM selenium-webdriver (~> 4.2, < 4.6) bugsnag (6.26.0) concurrent-ruby (~> 1.0) - bugsnag-maze-runner (8.4.0) + bugsnag-maze-runner (8.4.1) appium_lib (~> 12.0.0) appium_lib_core (~> 5.4.0) bugsnag (~> 6.24) @@ -70,7 +70,7 @@ GEM faye-websocket (0.11.3) eventmachine (>= 0.12.0) websocket-driver (>= 0.5.1) - ffi (1.15.5) + ffi (1.16.3) hana (1.3.7) json_schemer (0.2.25) ecma-re-validator (~> 0.3) @@ -78,9 +78,9 @@ GEM regexp_parser (~> 2.0) simpleidn (~> 0.2) uri_template (~> 0.7) - mime-types (3.5.0) + mime-types (3.5.1) mime-types-data (~> 3.2015) - mime-types-data (3.2023.0808) + mime-types-data (3.2023.1003) multi_test (0.1.2) nokogiri (1.15.4-arm64-darwin) racc (~> 1.4) @@ -92,7 +92,7 @@ GEM racc (1.7.1) rack (2.2.8) rake (12.3.3) - regexp_parser (2.8.1) + regexp_parser (2.8.2) rexml (3.2.6) rubyzip (2.3.2) selenium-webdriver (4.5.0) @@ -112,13 +112,14 @@ GEM unf_ext (0.0.8.2) uri_template (0.7.0) webrick (1.7.0) - websocket (1.2.9) + websocket (1.2.10) websocket-driver (0.7.6) websocket-extensions (>= 0.1.0) websocket-extensions (0.1.5) PLATFORMS arm64-darwin-21 + arm64-darwin-22 x86_64-darwin-20 DEPENDENCIES diff --git a/Sources/BugsnagPerformance/Private/BugsnagPerformanceImpl.h b/Sources/BugsnagPerformance/Private/BugsnagPerformanceImpl.h index d81de47e..b0c34f4b 100644 --- a/Sources/BugsnagPerformance/Private/BugsnagPerformanceImpl.h +++ b/Sources/BugsnagPerformance/Private/BugsnagPerformanceImpl.h @@ -57,7 +57,7 @@ class BugsnagPerformanceImpl: public PhasedStartup { void onSpanStarted() noexcept; - void setOnViewLoadSpanStarted(void (^onViewLoadSpanStarted)(NSString *className)) noexcept { + void setOnViewLoadSpanStarted(std::function onViewLoadSpanStarted) noexcept { tracer_->setOnViewLoadSpanStarted(onViewLoadSpanStarted); } diff --git a/Sources/BugsnagPerformance/Private/BugsnagPerformanceImpl.mm b/Sources/BugsnagPerformance/Private/BugsnagPerformanceImpl.mm index 18700cd3..e6310183 100644 --- a/Sources/BugsnagPerformance/Private/BugsnagPerformanceImpl.mm +++ b/Sources/BugsnagPerformance/Private/BugsnagPerformanceImpl.mm @@ -21,13 +21,6 @@ return NSSearchPathForDirectoriesInDomains(NSCachesDirectory, NSUserDomainMask, YES).firstObject; } -void (^generateOnSpanStarted(BugsnagPerformanceImpl *impl))(void) { - __block auto blockImpl = impl; - return ^{ - blockImpl->onSpanStarted(); - }; -} - BugsnagPerformanceImpl::BugsnagPerformanceImpl(std::shared_ptr reachability, AppStateTracker *appStateTracker) noexcept : persistence_(std::make_shared(getPersistenceDir())) @@ -36,7 +29,7 @@ , reachability_(reachability) , batch_(std::make_shared()) , sampler_(std::make_shared()) -, tracer_(std::make_shared(spanStackingHandler_, sampler_, batch_, generateOnSpanStarted(this))) +, tracer_(std::make_shared(spanStackingHandler_, sampler_, batch_, ^{this->onSpanStarted();})) , retryQueue_(std::make_unique([persistence_->bugsnagPerformanceDir() stringByAppendingPathComponent:@"retry-queue"])) , appStateTracker_(appStateTracker) , viewControllersToSpans_([NSMapTable mapTableWithKeyOptions:NSMapTableWeakMemory | NSMapTableObjectPointerPersonality diff --git a/Sources/BugsnagPerformance/Private/BugsnagPerformanceLibrary.mm b/Sources/BugsnagPerformance/Private/BugsnagPerformanceLibrary.mm index 6e7905ff..70e7d3e9 100644 --- a/Sources/BugsnagPerformance/Private/BugsnagPerformanceLibrary.mm +++ b/Sources/BugsnagPerformance/Private/BugsnagPerformanceLibrary.mm @@ -51,17 +51,17 @@ : appStateTracker_([[AppStateTracker alloc] init]) , reachability_(std::make_shared()) , bugsnagPerformanceImpl_(std::make_shared(reachability_, appStateTracker_)) -{ - bugsnagPerformanceImpl_->setOnViewLoadSpanStarted(^(NSString *className) { - bugsnagPerformanceImpl_->didStartViewLoadSpan(className); - }); -} +{} void BugsnagPerformanceLibrary::earlyConfigure(BSGEarlyConfiguration *config) noexcept { bugsnagPerformanceImpl_->earlyConfigure(config); } void BugsnagPerformanceLibrary::earlySetup() noexcept { + auto impl = bugsnagPerformanceImpl_; + bugsnagPerformanceImpl_->setOnViewLoadSpanStarted([=](NSString *className) { + impl->didStartViewLoadSpan(className); + }); bugsnagPerformanceImpl_->earlySetup(); } diff --git a/Sources/BugsnagPerformance/Private/Instrumentation/AppStartupInstrumentation.h b/Sources/BugsnagPerformance/Private/Instrumentation/AppStartupInstrumentation.h index 3df72c21..87daad91 100644 --- a/Sources/BugsnagPerformance/Private/Instrumentation/AppStartupInstrumentation.h +++ b/Sources/BugsnagPerformance/Private/Instrumentation/AppStartupInstrumentation.h @@ -23,7 +23,7 @@ class AppStartupInstrumentation: public PhasedStartup { std::shared_ptr spanAttributesProvider) noexcept; void earlyConfigure(BSGEarlyConfiguration *) noexcept {} - void earlySetup() noexcept {} + void earlySetup() noexcept; void configure(BugsnagPerformanceConfiguration *config) noexcept; void start() noexcept {} @@ -35,6 +35,7 @@ class AppStartupInstrumentation: public PhasedStartup { std::shared_ptr tracer_; std::shared_ptr spanAttributesProvider_; CFAbsoluteTime didStartProcessAtTime_{0}; + CFAbsoluteTime didInitializeAtTime_{0}; CFAbsoluteTime didCallMainFunctionAtTime_{0}; CFAbsoluteTime didBecomeActiveAtTime_{0}; CFAbsoluteTime didFinishLaunchingAtTime_{0}; diff --git a/Sources/BugsnagPerformance/Private/Instrumentation/AppStartupInstrumentation.mm b/Sources/BugsnagPerformance/Private/Instrumentation/AppStartupInstrumentation.mm index df84808b..ef2db240 100644 --- a/Sources/BugsnagPerformance/Private/Instrumentation/AppStartupInstrumentation.mm +++ b/Sources/BugsnagPerformance/Private/Instrumentation/AppStartupInstrumentation.mm @@ -64,9 +64,11 @@ static inline bool isActivePrewarm(void) { , tracer_(tracer) , spanAttributesProvider_(spanAttributesProvider) , didStartProcessAtTime_(getProcessStartTime()) -, didCallMainFunctionAtTime_(CFAbsoluteTimeGetCurrent()) +, didInitializeAtTime_(CFAbsoluteTimeGetCurrent()) , isColdLaunch_(isColdLaunch()) -{ +{} + +void AppStartupInstrumentation::earlySetup() noexcept { if (!canInstallInstrumentation()) { disable(); } @@ -86,7 +88,7 @@ static inline bool isActivePrewarm(void) { beginAppStartSpan(); beginPreMainSpan(); - [preMainSpan_ endWithAbsoluteTime:didCallMainFunctionAtTime_]; + [preMainSpan_ endWithAbsoluteTime:didInitializeAtTime_]; beginPostMainSpan(); shouldRespondToAppDidBecomeActive_ = true; @@ -103,6 +105,7 @@ static inline bool isActivePrewarm(void) { CFSTR("UIApplicationDidBecomeActiveNotification"), nullptr, CFNotificationSuspensionBehaviorDeliverImmediately); + didCallMainFunctionAtTime_ = CFAbsoluteTimeGetCurrent(); } void AppStartupInstrumentation::disable() noexcept { diff --git a/Sources/BugsnagPerformance/Private/Instrumentation/NetworkInstrumentation/NSURLSessionTask+Instrumentation.mm b/Sources/BugsnagPerformance/Private/Instrumentation/NetworkInstrumentation/NSURLSessionTask+Instrumentation.mm index 6df37806..e02af1f0 100644 --- a/Sources/BugsnagPerformance/Private/Instrumentation/NetworkInstrumentation/NSURLSessionTask+Instrumentation.mm +++ b/Sources/BugsnagPerformance/Private/Instrumentation/NetworkInstrumentation/NSURLSessionTask+Instrumentation.mm @@ -41,17 +41,16 @@ static void replace_NSURLSessionTask_resume(Class cls, BSGSessionTaskResumeCallback onResume) { __weak BSGSessionTaskResumeCallback weakOnResume = onResume; - SEL selector = @selector(resume); - typedef void (*IMPPrototype)(id, SEL); - __block IMPPrototype originalIMP = (IMPPrototype)ObjCSwizzle::replaceInstanceMethodOverride(cls, - selector, - ^(id self) { + __block SEL selector = @selector(resume); + __block IMP resume = ObjCSwizzle::replaceInstanceMethodOverride(cls, selector, ^(id self) { BSGSessionTaskResumeCallback localOnResume = weakOnResume; if (localOnResume != nil) { localOnResume(self); } - originalIMP(self, selector); - }); + if (resume) { + reinterpret_cast(resume)(self, selector); + } + }, nullptr); } void bsg_installNSURLSessionTaskPerformance(void (^onResume)(NSURLSessionTask *)) noexcept { diff --git a/Sources/BugsnagPerformance/Private/Instrumentation/ViewLoadInstrumentation.h b/Sources/BugsnagPerformance/Private/Instrumentation/ViewLoadInstrumentation.h index c61516de..70edb51b 100644 --- a/Sources/BugsnagPerformance/Private/Instrumentation/ViewLoadInstrumentation.h +++ b/Sources/BugsnagPerformance/Private/Instrumentation/ViewLoadInstrumentation.h @@ -27,7 +27,7 @@ class ViewLoadInstrumentation: public PhasedStartup { void earlyConfigure(BSGEarlyConfiguration *config) noexcept; void earlySetup() noexcept; void configure(BugsnagPerformanceConfiguration *config) noexcept; - void start() noexcept {} + void start() noexcept {}; private: static std::vector imagesToInstrument() noexcept; @@ -35,7 +35,14 @@ class ViewLoadInstrumentation: public PhasedStartup { static bool isViewControllerSubclass(Class subclass) noexcept; void instrument(Class cls) noexcept; - + void instrumentLoadView(Class cls) noexcept; + void instrumentViewDidLoad(Class cls) noexcept; + void instrumentViewWillAppear(Class cls) noexcept; + void instrumentViewDidAppear(Class cls) noexcept; + void instrumentViewWillDisappear(Class cls) noexcept; + void instrumentViewWillLayoutSubviews(Class cls) noexcept; + void instrumentViewDidLayoutSubviews(Class cls) noexcept; + void onLoadView(UIViewController *viewController) noexcept; void onViewDidAppear(UIViewController *viewController) noexcept; void onViewWillDisappear(UIViewController *viewController) noexcept; diff --git a/Sources/BugsnagPerformance/Private/Instrumentation/ViewLoadInstrumentation.mm b/Sources/BugsnagPerformance/Private/Instrumentation/ViewLoadInstrumentation.mm index d993dfb0..220990e2 100644 --- a/Sources/BugsnagPerformance/Private/Instrumentation/ViewLoadInstrumentation.mm +++ b/Sources/BugsnagPerformance/Private/Instrumentation/ViewLoadInstrumentation.mm @@ -60,42 +60,42 @@ @implementation ViewLoadInstrumentationState } } } else { - classToIsObserved_[[UIViewController class]] = true; - __block auto classToIsObserved = &classToIsObserved_; - __block bool *isEnabled = &isEnabled_; - __block auto appPath = NSBundle.mainBundle.bundlePath.UTF8String; - auto initInstrumentation = ^(id self) { - auto viewControllerClass = [self class]; - auto viewControllerBundlePath = [NSBundle bundleForClass: viewControllerClass].bundlePath.UTF8String; - if (strstr(viewControllerBundlePath, appPath) -#if TARGET_OS_SIMULATOR - // and those loaded from BUILT_PRODUCTS_DIR, because Xcode - // doesn't embed them when building for the Simulator. - || strstr(viewControllerBundlePath, "/DerivedData/") -#endif - ) { - if (*isEnabled && !isClassObserved(viewControllerClass)) { - Trace(@"%@ -[%s %s]", self, class_getName(viewControllerClass), sel_getName(selector)); - instrument(viewControllerClass); - (*classToIsObserved)[viewControllerClass] = true; - } - } - }; - SEL selector = @selector(initWithCoder:); - IMP initWithCoder __block = nullptr; - initWithCoder = ObjCSwizzle::replaceInstanceMethodOverride([UIViewController class], selector, ^(id self, NSCoder *coder) { - std::lock_guard guard(vcInitMutex_); - initInstrumentation(self); - reinterpret_cast(initWithCoder)(self, selector, coder); - }); - - selector = @selector(initWithNibName:bundle:); - IMP initWithNibNameBundle __block = nullptr; - initWithNibNameBundle = ObjCSwizzle::replaceInstanceMethodOverride([UIViewController class], selector, ^(id self, NSString *name, NSBundle *bundle) { - std::lock_guard guard(vcInitMutex_); - initInstrumentation(self); - reinterpret_cast(initWithNibNameBundle)(self, selector, name, bundle); - }); +// classToIsObserved_[[UIViewController class]] = true; +// __block auto classToIsObserved = &classToIsObserved_; +// __block bool *isEnabled = &isEnabled_; +// __block auto appPath = NSBundle.mainBundle.bundlePath.UTF8String; +// auto initInstrumentation = ^(id self) { +// auto viewControllerClass = [self class]; +// auto viewControllerBundlePath = [NSBundle bundleForClass: viewControllerClass].bundlePath.UTF8String; +// if (strstr(viewControllerBundlePath, appPath) +//#if TARGET_OS_SIMULATOR +// // and those loaded from BUILT_PRODUCTS_DIR, because Xcode +// // doesn't embed them when building for the Simulator. +// || strstr(viewControllerBundlePath, "/DerivedData/") +//#endif +// ) { +// if (*isEnabled && !isClassObserved(viewControllerClass)) { +// Trace(@"%@ -[%s %s]", self, class_getName(viewControllerClass), sel_getName(selector)); +// instrument(viewControllerClass); +// (*classToIsObserved)[viewControllerClass] = true; +// } +// } +// }; +// SEL selector = @selector(initWithCoder:); +// IMP initWithCoder __block = nullptr; +// initWithCoder = ObjCSwizzle::replaceInstanceMethodOverride([UIViewController class], selector, ^(id self, NSCoder *coder) { +// std::lock_guard guard(vcInitMutex_); +// initInstrumentation(self); +// reinterpret_cast(initWithCoder)(self, selector, coder); +// }, "@@:@"); +// +// selector = @selector(initWithNibName:bundle:); +// IMP initWithNibNameBundle __block = nullptr; +// initWithNibNameBundle = ObjCSwizzle::replaceInstanceMethodOverride([UIViewController class], selector, ^(id self, NSString *name, NSBundle *bundle) { +// std::lock_guard guard(vcInitMutex_); +// initInstrumentation(self); +// reinterpret_cast(initWithNibNameBundle)(self, selector, name, bundle); +// }, "@@:@"); } // We need to instrument UIViewController because not all subclasses will @@ -313,127 +313,181 @@ @implementation ViewLoadInstrumentationState } void -ViewLoadInstrumentation::instrument(Class cls) noexcept { - __block bool *isEnabled = &isEnabled_; - - SEL selector = @selector(loadView); - IMP loadView __block = nullptr; - loadView = ObjCSwizzle::replaceInstanceMethodOverride(cls, selector, ^(id self){ +ViewLoadInstrumentation::instrumentLoadView(Class cls) noexcept { + __block SEL selector = @selector(loadView); + __block bool const * const isEnabled = &isEnabled_; + __block IMP loadView = ObjCSwizzle::replaceInstanceMethodOverride(cls, selector, ^(id self){ if (*isEnabled) { // Prevent replacing an existing span for view controllers that override // loadView and call through to superclass implementation(s). ViewLoadInstrumentationState *instrumentationState = objc_getAssociatedObject(self, &kAssociatedViewLoadInstrumentationState); if (instrumentationState.loadViewPhaseSpanCreated) { - reinterpret_cast(loadView)(self, selector); + if (loadView) { + reinterpret_cast(loadView)(self, selector); + } return; } Trace(@"%@ -[%s %s]", self, class_getName(cls), sel_getName(selector)); onLoadView(self); BugsnagPerformanceSpan *span = startViewLoadPhaseSpan(self, @"loadView"); - reinterpret_cast(loadView)(self, selector); + if (loadView) { + reinterpret_cast(loadView)(self, selector); + } [span end]; } else { - reinterpret_cast(loadView)(self, selector); + if (loadView) { + reinterpret_cast(loadView)(self, selector); + } } - }); - - selector = @selector(viewDidLoad); - IMP viewDidLoad __block = nullptr; - viewDidLoad = ObjCSwizzle::replaceInstanceMethodOverride(cls, selector, ^(id self){ + }, "v@:"); +} + +void +ViewLoadInstrumentation::instrumentViewDidLoad(Class cls) noexcept { + __block SEL selector = @selector(viewDidLoad); + __block bool const * const isEnabled = &isEnabled_; + __block IMP viewDidLoad = ObjCSwizzle::replaceInstanceMethodOverride(cls, selector, ^(id self){ ViewLoadInstrumentationState *instrumentationState = objc_getAssociatedObject(self, &kAssociatedViewLoadInstrumentationState); if (objc_getAssociatedObject(self, &kAssociatedViewLoadSpan) == nil || !(*isEnabled) || instrumentationState.viewDidLoadPhaseSpanCreated) { - reinterpret_cast(viewDidLoad)(self, selector); + if (viewDidLoad) { + reinterpret_cast(viewDidLoad)(self, selector); + } return; } Trace(@"%@ -[%s %s]", self, class_getName(cls), sel_getName(selector)); BugsnagPerformanceSpan *span = startViewLoadPhaseSpan(self, @"viewDidLoad"); instrumentationState.viewDidLoadPhaseSpanCreated = YES; - reinterpret_cast(viewDidLoad)(self, selector); + if (viewDidLoad) { + reinterpret_cast(viewDidLoad)(self, selector); + } [span end]; - }); - - selector = @selector(viewWillAppear:); - IMP viewWillAppear __block = nullptr; - viewWillAppear = ObjCSwizzle::replaceInstanceMethodOverride(cls, selector, ^(id self, BOOL animated) { + }, "v@:"); +} + +void +ViewLoadInstrumentation::instrumentViewWillAppear(Class cls) noexcept { + __block SEL selector = @selector(viewWillAppear:); + __block bool const * const isEnabled = &isEnabled_; + __block IMP viewWillAppear = ObjCSwizzle::replaceInstanceMethodOverride(cls, selector, ^(id self, BOOL animated) { ViewLoadInstrumentationState *instrumentationState = objc_getAssociatedObject(self, &kAssociatedViewLoadInstrumentationState); if (objc_getAssociatedObject(self, &kAssociatedViewLoadSpan) == nil || !(*isEnabled) || instrumentationState.viewWillAppearPhaseSpanCreated) { - reinterpret_cast(viewWillAppear)(self, selector, animated); + if (viewWillAppear) { + reinterpret_cast(viewWillAppear)(self, selector, animated); + } return; } Trace(@"%@ -[%s %s]", self, class_getName(cls), sel_getName(selector)); BugsnagPerformanceSpan *span = startViewLoadPhaseSpan(self, @"viewWillAppear"); instrumentationState.viewWillAppearPhaseSpanCreated = YES; - reinterpret_cast(viewWillAppear)(self, selector, animated); + if (viewWillAppear) { + reinterpret_cast(viewWillAppear)(self, selector, animated); + } [span end]; BugsnagPerformanceSpan *viewAppearingSpan = startViewLoadPhaseSpan(self, @"View appearing"); objc_setAssociatedObject(self, &kAssociatedViewAppearingSpan, viewAppearingSpan, OBJC_ASSOCIATION_RETAIN_NONATOMIC); - }); - - // viewDidAppear may not fire, so as a fallback we use viewWillDisappear. - // https://developer.apple.com/documentation/uikit/uiviewcontroller#1652793 + }, "v@:B"); +} - selector = @selector(viewDidAppear:); - IMP viewDidAppear __block = nullptr; - viewDidAppear = ObjCSwizzle::replaceInstanceMethodOverride(cls, selector, ^(id self, BOOL animated) { +void +ViewLoadInstrumentation::instrumentViewDidAppear(Class cls) noexcept { + __block SEL selector = @selector(viewDidAppear:); + __block bool const * const isEnabled = &isEnabled_; + __block IMP viewDidAppear = ObjCSwizzle::replaceInstanceMethodOverride(cls, selector, ^(id self, BOOL animated) { ViewLoadInstrumentationState *instrumentationState = objc_getAssociatedObject(self, &kAssociatedViewLoadInstrumentationState); if (objc_getAssociatedObject(self, &kAssociatedViewLoadSpan) == nil || !(*isEnabled) || instrumentationState.viewDidAppearPhaseSpanCreated) { - reinterpret_cast(viewDidAppear)(self, selector, animated); + if (viewDidAppear) { + reinterpret_cast(viewDidAppear)(self, selector, animated); + } return; } endViewAppearingSpan(self); BugsnagPerformanceSpan *span = startViewLoadPhaseSpan(self, @"viewDidAppear"); instrumentationState.viewDidAppearPhaseSpanCreated = YES; Trace(@"%@ -[%s %s]", self, class_getName(cls), sel_getName(selector)); - reinterpret_cast(viewDidAppear)(self, selector, animated); + if (viewDidAppear) { + reinterpret_cast(viewDidAppear)(self, selector, animated); + } [span end]; onViewDidAppear(self); - }); + }, "v@:B"); +} - selector = @selector(viewWillDisappear:); - IMP viewWillDisappear __block = nullptr; - viewWillDisappear = ObjCSwizzle::replaceInstanceMethodOverride(cls, selector, ^(id self, BOOL animated) { +void +ViewLoadInstrumentation::instrumentViewWillDisappear(Class cls) noexcept { + __block SEL selector = @selector(viewWillDisappear:); + __block bool const * const isEnabled = &isEnabled_; + __block IMP viewWillDisappear = ObjCSwizzle::replaceInstanceMethodOverride(cls, selector, ^(id self, BOOL animated) { if (*isEnabled) { Trace(@"%@ -[%s %s]", self, class_getName(cls), sel_getName(selector)); onViewWillDisappear(self); } - reinterpret_cast(viewWillDisappear)(self, selector, animated); - }); - - selector = @selector(viewWillLayoutSubviews); - IMP viewWillLayoutSubviews __block = nullptr; - viewWillLayoutSubviews = ObjCSwizzle::replaceInstanceMethodOverride(cls, selector, ^(id self) { + if (viewWillDisappear) { + reinterpret_cast(viewWillDisappear)(self, selector, animated); + } + }, "v@:B"); +} + +void +ViewLoadInstrumentation::instrumentViewWillLayoutSubviews(Class cls) noexcept { + __block SEL selector = @selector(viewWillLayoutSubviews); + __block bool const * const isEnabled = &isEnabled_; + __block IMP viewWillLayoutSubviews = ObjCSwizzle::replaceInstanceMethodOverride(cls, selector, ^(id self) { ViewLoadInstrumentationState *instrumentationState = objc_getAssociatedObject(self, &kAssociatedViewLoadInstrumentationState); if (objc_getAssociatedObject(self, &kAssociatedViewLoadSpan) == nil || !(*isEnabled) || instrumentationState.viewWillLayoutSubviewsPhaseSpanCreated) { - reinterpret_cast(viewWillLayoutSubviews)(self, selector); + if (viewWillLayoutSubviews) { + reinterpret_cast(viewWillLayoutSubviews)(self, selector); + } return; } Trace(@"%@ -[%s %s]", self, class_getName(cls), sel_getName(selector)); BugsnagPerformanceSpan *span = startViewLoadPhaseSpan(self, @"viewWillLayoutSubviews"); instrumentationState.viewWillLayoutSubviewsPhaseSpanCreated = YES; - reinterpret_cast(viewWillLayoutSubviews)(self, selector); + if (viewWillLayoutSubviews) { + reinterpret_cast(viewWillLayoutSubviews)(self, selector); + } [span end]; BugsnagPerformanceSpan *subviewLayoutSpan = startViewLoadPhaseSpan(self, @"Subview layout"); objc_setAssociatedObject(self, &kAssociatedSubviewLayoutSpan, subviewLayoutSpan, OBJC_ASSOCIATION_RETAIN_NONATOMIC); - }); - - selector = @selector(viewDidLayoutSubviews); - IMP viewDidLayoutSubviews __block = nullptr; - viewDidLayoutSubviews = ObjCSwizzle::replaceInstanceMethodOverride(cls, selector, ^(id self) { + }, "v@:"); +} + +void +ViewLoadInstrumentation::instrumentViewDidLayoutSubviews(Class cls) noexcept { + __block SEL selector = @selector(viewDidLayoutSubviews); + __block bool const * const isEnabled = &isEnabled_; + __block IMP viewDidLayoutSubviews = ObjCSwizzle::replaceInstanceMethodOverride(cls, selector, ^(id self) { ViewLoadInstrumentationState *instrumentationState = objc_getAssociatedObject(self, &kAssociatedViewLoadInstrumentationState); if (objc_getAssociatedObject(self, &kAssociatedViewLoadSpan) == nil || !(*isEnabled) || instrumentationState.viewDidLayoutSubviewsPhaseSpanCreated) { - reinterpret_cast(viewDidLayoutSubviews)(self, selector); + if (viewDidLayoutSubviews) { + reinterpret_cast(viewDidLayoutSubviews)(self, selector); + } return; } endSubviewsLayoutSpan(self); Trace(@"%@ -[%s %s]", self, class_getName(cls), sel_getName(selector)); BugsnagPerformanceSpan *span = startViewLoadPhaseSpan(self, @"viewDidLayoutSubviews"); instrumentationState.viewDidLayoutSubviewsPhaseSpanCreated = YES; - reinterpret_cast(viewDidLayoutSubviews)(self, selector); + if (viewDidLayoutSubviews) { + reinterpret_cast(viewDidLayoutSubviews)(self, selector); + } [span end]; - }); + }, "v@:B"); +} + +void +ViewLoadInstrumentation::instrument(Class cls) noexcept { + instrumentLoadView(cls); + instrumentViewDidLoad(cls); + instrumentViewWillAppear(cls); + instrumentViewDidAppear(cls); + // viewDidAppear may not fire, so as a fallback we use viewWillDisappear. + // https://developer.apple.com/documentation/uikit/uiviewcontroller#1652793 + instrumentViewWillDisappear(cls); + instrumentViewWillLayoutSubviews(cls); + instrumentViewDidLayoutSubviews(cls); } // NOLINTEND(cppcoreguidelines-*) diff --git a/Sources/BugsnagPerformance/Private/Swizzle.h b/Sources/BugsnagPerformance/Private/Swizzle.h index fc51b515..c2829561 100644 --- a/Sources/BugsnagPerformance/Private/Swizzle.h +++ b/Sources/BugsnagPerformance/Private/Swizzle.h @@ -22,9 +22,10 @@ class ObjCSwizzle { /** * Replace a class's override of a method (i.e. only if this class overrides the method). No superclass implementation is replaced. - * Returns nil if no method was replaced (either method not found, or this class doesn't overrde the method). + * If the class doesn't implement the method, it's instead injected into the class provided objcCallingSignature is not null. + * Returns the previous method implementation if it exists. You MUST guard calls to the previous IMP for nil! */ - static IMP _Nullable replaceInstanceMethodOverride(Class _Nonnull cls, SEL _Nonnull name, id _Nonnull block) noexcept; + static IMP _Nullable replaceInstanceMethodOverride(Class _Nonnull cls, SEL _Nonnull name, id _Nonnull block, const char* _Nullable objcCallingSignature) noexcept; /** * Get any classes or superclasses that implement the specified selector. diff --git a/Sources/BugsnagPerformance/Private/Swizzle.mm b/Sources/BugsnagPerformance/Private/Swizzle.mm index b2afc880..9f484f7c 100644 --- a/Sources/BugsnagPerformance/Private/Swizzle.mm +++ b/Sources/BugsnagPerformance/Private/Swizzle.mm @@ -22,13 +22,13 @@ } } -IMP ObjCSwizzle::replaceInstanceMethodOverride(Class clazz, SEL selector, id block) noexcept { - Method method = nullptr; +IMP ObjCSwizzle::replaceInstanceMethodOverride(Class cls, SEL selector, id block, const char* objcCallingSignature) noexcept { + Method method = nil; // Not using class_getInstanceMethod because we don't want to modify the // superclass's implementation. auto methodCount = 0U; - Method *methods = class_copyMethodList(clazz, &methodCount); + Method *methods = class_copyMethodList(cls, &methodCount); if (methods) { for (auto i = 0U; i < methodCount; i++) { if (sel_isEqual(method_getName(methods[i]), selector)) { @@ -39,12 +39,13 @@ free(methods); } - if (!method) { - // This is not considered an error. - return nil; + if (method) { + // We found a method to replace, so replace it and return the old IMP + return method_setImplementation(method, imp_implementationWithBlock(block)); } - return method_setImplementation(method, imp_implementationWithBlock(block)); + // We didn't do anything, so there's no old IMP to return + return nil; } NSArray *ObjCSwizzle::getClassesWithSelector(Class cls, SEL selector) noexcept { diff --git a/Sources/BugsnagPerformance/Private/Tracer.h b/Sources/BugsnagPerformance/Private/Tracer.h index ceb451d7..9506e3b3 100644 --- a/Sources/BugsnagPerformance/Private/Tracer.h +++ b/Sources/BugsnagPerformance/Private/Tracer.h @@ -38,7 +38,7 @@ class Tracer: public PhasedStartup { void configure(BugsnagPerformanceConfiguration *configuration) noexcept; void start() noexcept; - void setOnViewLoadSpanStarted(void (^onViewLoadSpanStarted)(NSString *className)) noexcept { + void setOnViewLoadSpanStarted(std::function onViewLoadSpanStarted) noexcept { onViewLoadSpanStarted_ = onViewLoadSpanStarted; } @@ -66,9 +66,7 @@ class Tracer: public PhasedStartup { std::shared_ptr batch_; void (^onSpanStarted_)(){nil}; - void (^onViewLoadSpanStarted_)(NSString *className){ - ^(NSString *) {} - }; + std::function onViewLoadSpanStarted_{}; BugsnagPerformanceNetworkRequestCallback networkRequestCallback_; BugsnagPerformanceSpan *startSpan(NSString *name, SpanOptions options, BSGFirstClass defaultFirstClass) noexcept;