From 42fc87eb8d504d9771360d898a907d51a5403b7e Mon Sep 17 00:00:00 2001 From: Kevin Gozali Date: Mon, 14 May 2018 20:34:35 -0700 Subject: [PATCH] remove unnecessary ExceptionManager abstraction Reviewed By: sebmarkbage Differential Revision: D8002124 fbshipit-source-id: 4e0bce9686549d0dd7b59b1323efd11ea168855b --- RNTester/Podfile.lock | 16 +++-- React.podspec | 14 ---- React/Base/RCTLog.h | 7 -- React/Base/RCTLog.mm | 65 +++++++++---------- React/CxxExceptions/RCTCxxExceptionManager.h | 27 -------- React/CxxExceptions/RCTCxxExceptionManager.mm | 31 --------- React/Fabric/RCTFabricUIManagerWrapper.h | 2 - React/Fabric/RCTFabricUIManagerWrapper.mm | 8 --- React/Fabric/RCTSurfacePresenter.h | 4 +- React/Fabric/RCTSurfacePresenter.mm | 14 ---- ReactCommon/exceptions/BUCK | 40 ------------ ReactCommon/exceptions/ExceptionManager.h | 28 -------- 12 files changed, 40 insertions(+), 216 deletions(-) delete mode 100644 React/CxxExceptions/RCTCxxExceptionManager.h delete mode 100644 React/CxxExceptions/RCTCxxExceptionManager.mm delete mode 100644 ReactCommon/exceptions/BUCK delete mode 100644 ReactCommon/exceptions/ExceptionManager.h diff --git a/RNTester/Podfile.lock b/RNTester/Podfile.lock index 57ad2bcedb..a52cf43480 100644 --- a/RNTester/Podfile.lock +++ b/RNTester/Podfile.lock @@ -14,9 +14,6 @@ PODS: - Folly (= 2016.10.31.00) - React/Core - React/cxxreact - - React/CxxExceptions (1000.0.0): - - React/CxxBridge - - React/exceptions - React/cxxreact (1000.0.0): - boost-for-react-native (= 1.63.0) - Folly (= 2016.10.31.00) @@ -25,19 +22,27 @@ PODS: - React/DevSupport (1000.0.0): - React/Core - React/RCTWebSocket - - React/exceptions (1000.0.0) - React/fabric (1000.0.0): + - React/fabric/attributedstring (= 1000.0.0) - React/fabric/core (= 1000.0.0) - React/fabric/debug (= 1000.0.0) - React/fabric/graphics (= 1000.0.0) + - React/fabric/text (= 1000.0.0) + - React/fabric/textlayoutmanager (= 1000.0.0) - React/fabric/uimanager (= 1000.0.0) - React/fabric/view (= 1000.0.0) + - React/fabric/attributedstring (1000.0.0): + - Folly (= 2016.10.31.00) - React/fabric/core (1000.0.0): - Folly (= 2016.10.31.00) - React/fabric/debug (1000.0.0): - Folly (= 2016.10.31.00) - React/fabric/graphics (1000.0.0): - Folly (= 2016.10.31.00) + - React/fabric/text (1000.0.0): + - Folly (= 2016.10.31.00) + - React/fabric/textlayoutmanager (1000.0.0): + - Folly (= 2016.10.31.00) - React/fabric/uimanager (1000.0.0): - Folly (= 2016.10.31.00) - React/fabric/view (1000.0.0): @@ -61,7 +66,6 @@ PODS: - React/RCTFabric (1000.0.0): - Folly (= 2016.10.31.00) - React/Core - - React/CxxExceptions - React/fabric - React/RCTGeolocation (1000.0.0): - React/Core @@ -131,7 +135,7 @@ SPEC CHECKSUMS: DoubleConversion: e22e0762848812a87afd67ffda3998d9ef29170c Folly: 9a8eea4725a0b6ba3256ebf206c21e352c23abf8 glog: 1de0bb937dccdc981596d3b5825ebfb765017ded - React: e3382e39c51a5e5889248e97c6b9b1b3ca443a08 + React: 512b17e881b5d04d703cd292caa6f4fe12984688 yoga: bdd268c5812f00bdb52cc2b58f129797e97935eb PODFILE CHECKSUM: 1a96172007b66aa74825c234f17139dd9c3d3cd7 diff --git a/React.podspec b/React.podspec index 9305d94ea4..1c590b4ff5 100644 --- a/React.podspec +++ b/React.podspec @@ -74,13 +74,6 @@ Pod::Spec.new do |s| ss.compiler_flags = folly_compiler_flags ss.private_header_files = "React/Cxx*/*.h" ss.source_files = "React/Cxx*/*.{h,m,mm}" - ss.exclude_files = "React/CxxExceptions/**/*" - end - - s.subspec "CxxExceptions" do |ss| - ss.dependency "React/CxxBridge" - ss.dependency "React/exceptions" - ss.source_files = "React/CxxExceptions/*.{h,m,mm}" end s.subspec "DevSupport" do |ss| @@ -93,7 +86,6 @@ Pod::Spec.new do |s| s.subspec "RCTFabric" do |ss| ss.dependency "Folly", folly_version ss.dependency "React/Core" - ss.dependency "React/CxxExceptions" ss.dependency "React/fabric" ss.compiler_flags = folly_compiler_flags ss.source_files = "React/Fabric/**/*.{c,h,m,mm,S,cpp}" @@ -142,12 +134,6 @@ Pod::Spec.new do |s| ss.pod_target_xcconfig = { "HEADER_SEARCH_PATHS" => "\"$(PODS_TARGET_SRCROOT)/ReactCommon\" \"$(PODS_ROOT)/boost-for-react-native\" \"$(PODS_ROOT)/DoubleConversion\" \"$(PODS_ROOT)/Folly\"" } end - s.subspec "exceptions" do |ss| - ss.source_files = "ReactCommon/exceptions/*.{cpp,h}" - ss.header_dir = "cxxreact" - ss.pod_target_xcconfig = { "HEADER_SEARCH_PATHS" => "\"$(PODS_TARGET_SRCROOT)/ReactCommon\"" } - end - s.subspec "fabric" do |ss| ss.subspec "attributedstring" do |sss| sss.dependency "Folly", folly_version diff --git a/React/Base/RCTLog.h b/React/Base/RCTLog.h index 6ef09d620c..9f9317da41 100644 --- a/React/Base/RCTLog.h +++ b/React/Base/RCTLog.h @@ -120,13 +120,6 @@ RCT_EXTERN void RCTPerformBlockWithLogFunction(void (^block)(void), RCTLogFuncti */ RCT_EXTERN void RCTPerformBlockWithLogPrefix(void (^block)(void), NSString *prefix); -/** - * This method computes the current call stack, useful for error reporting. - */ -RCT_EXTERN NSArray *RCTGetCallStack(const char *fileName, int lineNumber); - -#define RCT_CALLSTACK RCTGetCallStack(__FILE__, __LINE__) - /** * Private logging function - ignore this. */ diff --git a/React/Base/RCTLog.mm b/React/Base/RCTLog.mm index 4e65fa0cc8..edb92fe014 100644 --- a/React/Base/RCTLog.mm +++ b/React/Base/RCTLog.mm @@ -197,41 +197,6 @@ static NSRegularExpression *nativeStackFrameRegex() return _regex; } -NSArray* RCTGetCallStack(const char *fileName, int lineNumber) -{ - NSArray *stackSymbols = [NSThread callStackSymbols]; - NSMutableArray *stack = - [NSMutableArray arrayWithCapacity:(stackSymbols.count - 1)]; - [stackSymbols enumerateObjectsUsingBlock:^(NSString *frameSymbols, NSUInteger idx, __unused BOOL *stop) { - if (idx == 0) { - // don't include the current frame - return; - } - - NSRange range = NSMakeRange(0, frameSymbols.length); - NSTextCheckingResult *match = [nativeStackFrameRegex() firstMatchInString:frameSymbols options:0 range:range]; - if (!match) { - return; - } - - NSString *methodName = [frameSymbols substringWithRange:[match rangeAtIndex:1]]; - char *demangledName = abi::__cxa_demangle([methodName UTF8String], NULL, NULL, NULL); - if (demangledName) { - methodName = @(demangledName); - free(demangledName); - } - - if (idx == 1 && fileName) { - NSString *file = [@(fileName) componentsSeparatedByString:@"/"].lastObject; - [stack addObject:@{@"methodName": methodName, @"file": file, @"lineNumber": @(lineNumber)}]; - } else { - [stack addObject:@{@"methodName": methodName}]; - } - }]; - - return [stack copy]; -} - void _RCTLogNativeInternal(RCTLogLevel level, const char *fileName, int lineNumber, NSString *format, ...) { RCTLogFunction logFunction = RCTGetLocalLogFunction(); @@ -252,7 +217,35 @@ void _RCTLogNativeInternal(RCTLogLevel level, const char *fileName, int lineNumb // Log to red box in debug mode. if (RCTSharedApplication() && level >= RCTLOG_REDBOX_LEVEL) { - NSArray *stack = RCT_CALLSTACK; + NSArray *stackSymbols = [NSThread callStackSymbols]; + NSMutableArray *stack = + [NSMutableArray arrayWithCapacity:(stackSymbols.count - 1)]; + [stackSymbols enumerateObjectsUsingBlock:^(NSString *frameSymbols, NSUInteger idx, __unused BOOL *stop) { + if (idx == 0) { + // don't include the current frame + return; + } + + NSRange range = NSMakeRange(0, frameSymbols.length); + NSTextCheckingResult *match = [nativeStackFrameRegex() firstMatchInString:frameSymbols options:0 range:range]; + if (!match) { + return; + } + + NSString *methodName = [frameSymbols substringWithRange:[match rangeAtIndex:1]]; + char *demangledName = abi::__cxa_demangle([methodName UTF8String], NULL, NULL, NULL); + if (demangledName) { + methodName = @(demangledName); + free(demangledName); + } + + if (idx == 1 && fileName) { + NSString *file = [@(fileName) componentsSeparatedByString:@"/"].lastObject; + [stack addObject:@{@"methodName": methodName, @"file": file, @"lineNumber": @(lineNumber)}]; + } else { + [stack addObject:@{@"methodName": methodName}]; + } + }]; dispatch_async(dispatch_get_main_queue(), ^{ // red box is thread safe, but by deferring to main queue we avoid a startup diff --git a/React/CxxExceptions/RCTCxxExceptionManager.h b/React/CxxExceptions/RCTCxxExceptionManager.h deleted file mode 100644 index c8e3de44c4..0000000000 --- a/React/CxxExceptions/RCTCxxExceptionManager.h +++ /dev/null @@ -1,27 +0,0 @@ -/** - * Copyright (c) 2015-present, Facebook, Inc. - * - * This source code is licensed under the MIT license found in the - * LICENSE file in the root directory of this source tree. - */ - -#pragma once - -#include - -namespace facebook { -namespace react { - -/** - * Connector class (from C++ to ObjC++) to allow FabricUIManager to invoke native UI operations/updates. - * UIKit-related impl doesn't live here, but this class gets passed to the FabricUIManager C++ impl directly. - */ -class RCTCxxExceptionManager : public ExceptionManager { -public: - - virtual void handleSoftException(const std::exception& e) const override; - virtual void handleFatalException(const std::exception& e) const override; -}; - -} // namespace react -} // namespace facebook diff --git a/React/CxxExceptions/RCTCxxExceptionManager.mm b/React/CxxExceptions/RCTCxxExceptionManager.mm deleted file mode 100644 index 61e636378d..0000000000 --- a/React/CxxExceptions/RCTCxxExceptionManager.mm +++ /dev/null @@ -1,31 +0,0 @@ -/** - * Copyright (c) 2015-present, Facebook, Inc. - * - * This source code is licensed under the MIT license found in the - * LICENSE file in the root directory of this source tree. - */ - -#import "RCTCxxExceptionManager.h" - -#import - -#import -#import -#import -#import - -namespace facebook { -namespace react { - -void RCTCxxExceptionManager::handleSoftException(const std::exception& e) const { - RCTExceptionsManager *manager = [[RCTBridge currentBridge] moduleForClass:[RCTExceptionsManager class]]; - [manager reportSoftException:[NSString stringWithUTF8String:e.what()] stack:RCT_CALLSTACK exceptionId:@0]; -} - -void RCTCxxExceptionManager::handleFatalException(const std::exception& e) const { - RCTExceptionsManager *manager = [[RCTBridge currentBridge] moduleForClass:[RCTExceptionsManager class]]; - [manager reportFatalException:[NSString stringWithUTF8String:e.what()] stack:RCT_CALLSTACK exceptionId:@0]; -} - -} // namespace react -} // namespace facebook diff --git a/React/Fabric/RCTFabricUIManagerWrapper.h b/React/Fabric/RCTFabricUIManagerWrapper.h index 234668bb1b..d40ef78c35 100644 --- a/React/Fabric/RCTFabricUIManagerWrapper.h +++ b/React/Fabric/RCTFabricUIManagerWrapper.h @@ -17,7 +17,6 @@ namespace facebook { namespace react { class FabricUIManager; -class ExceptionManager; } // namespace react } // namespace facebook @@ -30,7 +29,6 @@ using namespace facebook::react; @interface RCTFabricUIManagerWrapper : NSObject - (std::shared_ptr)manager; -- (std::shared_ptr)exceptionManager; @end diff --git a/React/Fabric/RCTFabricUIManagerWrapper.mm b/React/Fabric/RCTFabricUIManagerWrapper.mm index dff3f263f1..0d4613b0c2 100644 --- a/React/Fabric/RCTFabricUIManagerWrapper.mm +++ b/React/Fabric/RCTFabricUIManagerWrapper.mm @@ -7,7 +7,6 @@ #import "RCTFabricUIManagerWrapper.h" -#include #include #include #include @@ -20,7 +19,6 @@ @implementation RCTFabricUIManagerWrapper { std::shared_ptr _manager; - std::shared_ptr _exceptionManager; std::shared_ptr _platformUIOperationManager; } @@ -28,7 +26,6 @@ { self = [super init]; if (self) { - _exceptionManager = std::make_shared(); _platformUIOperationManager = std::make_shared(); auto componentDescriptorRegistry = std::make_shared(); @@ -45,11 +42,6 @@ return _manager; } -- (std::shared_ptr)exceptionManager -{ - return _exceptionManager; -} - - (void)invalidate { } diff --git a/React/Fabric/RCTSurfacePresenter.h b/React/Fabric/RCTSurfacePresenter.h index 65456cbf53..52c44fbda7 100644 --- a/React/Fabric/RCTSurfacePresenter.h +++ b/React/Fabric/RCTSurfacePresenter.h @@ -9,7 +9,6 @@ #import #import -#import #import NS_ASSUME_NONNULL_BEGIN @@ -56,10 +55,9 @@ NS_ASSUME_NONNULL_BEGIN @interface RCTSurfacePresenter (Deprecated) /** - * We need to expose `exceptionManager` and `uiManager` for registration + * We need to expose `uiManager` for registration * purposes. Eventually, we will move this down to C++ side. */ -- (std::shared_ptr)exceptionManager_DO_NOT_USE; - (std::shared_ptr)uiManager_DO_NOT_USE; @end diff --git a/React/Fabric/RCTSurfacePresenter.mm b/React/Fabric/RCTSurfacePresenter.mm index 1e0164f2b9..8673a2fc70 100644 --- a/React/Fabric/RCTSurfacePresenter.mm +++ b/React/Fabric/RCTSurfacePresenter.mm @@ -8,7 +8,6 @@ #import "RCTSurfacePresenter.h" #import -#import #import #import #import @@ -30,7 +29,6 @@ using namespace facebook::react; @end @implementation RCTSurfacePresenter { - std::shared_ptr _exceptionManager; RCTScheduler *_scheduler; RCTMountingManager *_mountingManager; RCTBridge *_bridge; @@ -44,8 +42,6 @@ using namespace facebook::react; _bridge = bridge; _batchedBridge = [_bridge batchedBridge] ?: _bridge; - _exceptionManager = std::make_shared(); - _scheduler = [[RCTScheduler alloc] init]; _scheduler.delegate = self; @@ -57,11 +53,6 @@ using namespace facebook::react; return self; } -- (std::shared_ptr)exceptionManager -{ - return _exceptionManager; -} - - (void)schedulerDidComputeMutationInstructions:(facebook::react::TreeMutationInstructionList)instructions rootTag:(ReactTag)rootTag { [_mountingManager mutateComponentViewTreeWithMutationInstructions:instructions @@ -167,11 +158,6 @@ using namespace facebook::react; return _scheduler.uiManager_DO_NOT_USE; } -- (std::shared_ptr)exceptionManager_DO_NOT_USE -{ - return _exceptionManager; -} - @end @implementation RCTBridge (RCTSurfacePresenter) diff --git a/ReactCommon/exceptions/BUCK b/ReactCommon/exceptions/BUCK deleted file mode 100644 index 6f1ef1bb93..0000000000 --- a/ReactCommon/exceptions/BUCK +++ /dev/null @@ -1,40 +0,0 @@ -load("//configurations/buck/apple:flag_defs.bzl", "get_debug_preprocessor_flags") -load("//ReactNative:DEFS.bzl", "IS_OSS_BUILD", "react_native_xplat_target", "rn_xplat_cxx_library", "get_apple_inspector_flags") - -APPLE_COMPILER_FLAGS = [] - -if not IS_OSS_BUILD: - load("@xplat//configurations/buck/apple:flag_defs.bzl", "get_static_library_ios_flags", "flags") - APPLE_COMPILER_FLAGS = flags.get_flag_value(get_static_library_ios_flags(), 'compiler_flags') - -rn_xplat_cxx_library( - name = "exceptions", - srcs = glob(["*.cpp"]), - header_namespace = "", - exported_headers = subdir_glob( - [ - ("", "*.h"), - ], - prefix = "cxxreact", - ), - compiler_flags = [ - "-fexceptions", - "-frtti", - "-std=c++14", - "-Wall", - ], - fbobjc_compiler_flags = APPLE_COMPILER_FLAGS, - fbobjc_preprocessor_flags = get_debug_preprocessor_flags() + get_apple_inspector_flags(), - force_static = True, - preprocessor_flags = [ - "-DLOG_TAG=\"ReactNative\"", - "-DWITH_FBSYSTRACE=1", - ], - visibility = [ - "PUBLIC", - ], - deps = [ - "xplat//fbsystrace:fbsystrace", - "xplat//third-party/glog:glog", - ], -) diff --git a/ReactCommon/exceptions/ExceptionManager.h b/ReactCommon/exceptions/ExceptionManager.h deleted file mode 100644 index 278179c67d..0000000000 --- a/ReactCommon/exceptions/ExceptionManager.h +++ /dev/null @@ -1,28 +0,0 @@ -/** - * Copyright (c) 2015-present, Facebook, Inc. - * - * This source code is licensed under the MIT license found in the - * LICENSE file in the root directory of this source tree. - */ - -#pragma once - -#include - -namespace facebook { -namespace react { - -/** - * An abstract class for C++-based exception handling that differentiates - * soft exceptions from fatal exceptions. - */ -class ExceptionManager { -public: - virtual ~ExceptionManager() = default; - - virtual void handleSoftException(const std::exception &e) const = 0; - virtual void handleFatalException(const std::exception &e) const = 0; -}; - -} // namespace react -} // namespace facebook