From b642677a97860a9fd31ad8fb55a1bc37420550e8 Mon Sep 17 00:00:00 2001 From: Valentin Shergin Date: Wed, 6 Nov 2019 14:40:51 -0800 Subject: [PATCH] Fabric: Removing `ComponentDescriptorProviderRegistry::remove()` Summary: This diff removes `ComponentDescriptorProviderRegistry::remove()` and two derivative interfaces. First, we don't use that and there is no concrete idea why we would need to use that. Those were originally built only for symmetry with limited knowledge about what exactly we need. Second, those methods are actually dangerous and probably must not be supported by design. Removing a ComponentDescriptorProvider destroys already registered `ComponentDescriptor`s, and at the same time we might have ShadowNodes referring to that (which will cause a crash), and there is no reasonable way to check for the existence of those nodes. Changelog: [Internal] Fabric-specific internal change. Reviewed By: sammy-SC Differential Revision: D18285497 fbshipit-source-id: b461e38b923c217a256e1155689311397a994feb --- React/Fabric/Mounting/RCTComponentViewFactory.h | 5 ----- React/Fabric/Mounting/RCTComponentViewFactory.mm | 9 --------- .../ComponentDescriptorProviderRegistry.cpp | 15 --------------- .../ComponentDescriptorProviderRegistry.h | 3 +-- .../uimanager/ComponentDescriptorRegistry.cpp | 15 --------------- .../uimanager/ComponentDescriptorRegistry.h | 6 +++--- 6 files changed, 4 insertions(+), 49 deletions(-) diff --git a/React/Fabric/Mounting/RCTComponentViewFactory.h b/React/Fabric/Mounting/RCTComponentViewFactory.h index fa4e152c470..ee347d9cf75 100644 --- a/React/Fabric/Mounting/RCTComponentViewFactory.h +++ b/React/Fabric/Mounting/RCTComponentViewFactory.h @@ -30,11 +30,6 @@ NS_ASSUME_NONNULL_BEGIN */ - (void)registerComponentViewClass:(Class)componentViewClass; -/** - * Unregisters a component view class in the factory. - */ -- (void)unregisterComponentViewClass:(Class)componentViewClass; - /** * Creates a component view with given component handle. */ diff --git a/React/Fabric/Mounting/RCTComponentViewFactory.mm b/React/Fabric/Mounting/RCTComponentViewFactory.mm index ef6c07e0f3c..d85ae828d02 100644 --- a/React/Fabric/Mounting/RCTComponentViewFactory.mm +++ b/React/Fabric/Mounting/RCTComponentViewFactory.mm @@ -111,15 +111,6 @@ using namespace facebook::react; } } -- (void)unregisterComponentViewClass:(Class)componentViewClass -{ - std::unique_lock lock(_mutex); - - auto componentDescriptorProvider = [componentViewClass componentDescriptorProvider]; - _componentViewClasses.erase(componentDescriptorProvider.handle); - _providerRegistry.remove(componentDescriptorProvider); -} - - (RCTComponentViewDescriptor)createComponentViewWithComponentHandle:(facebook::react::ComponentHandle)componentHandle { RCTAssertMainQueue(); diff --git a/ReactCommon/fabric/uimanager/ComponentDescriptorProviderRegistry.cpp b/ReactCommon/fabric/uimanager/ComponentDescriptorProviderRegistry.cpp index ec1269e7ea7..6bc6df9d7b5 100644 --- a/ReactCommon/fabric/uimanager/ComponentDescriptorProviderRegistry.cpp +++ b/ReactCommon/fabric/uimanager/ComponentDescriptorProviderRegistry.cpp @@ -38,21 +38,6 @@ void ComponentDescriptorProviderRegistry::add( } } -void ComponentDescriptorProviderRegistry::remove( - ComponentDescriptorProvider provider) const { - std::unique_lock lock(mutex_); - componentDescriptorProviders_.erase(provider.handle); - - for (auto const &weakRegistry : componentDescriptorRegistries_) { - auto registry = weakRegistry.lock(); - if (!registry) { - continue; - } - - registry->remove(provider); - } -} - void ComponentDescriptorProviderRegistry::setComponentDescriptorProviderRequest( ComponentDescriptorProviderRequest componentDescriptorProviderRequest) const { diff --git a/ReactCommon/fabric/uimanager/ComponentDescriptorProviderRegistry.h b/ReactCommon/fabric/uimanager/ComponentDescriptorProviderRegistry.h index dbfdfcf7a12..fb8e5ee4512 100644 --- a/ReactCommon/fabric/uimanager/ComponentDescriptorProviderRegistry.h +++ b/ReactCommon/fabric/uimanager/ComponentDescriptorProviderRegistry.h @@ -28,12 +28,11 @@ using ComponentDescriptorProviderRequest = class ComponentDescriptorProviderRegistry final { public: /* - * Adds (or removes) a `ComponentDescriptorProvider`s and update the managed + * Adds a `ComponentDescriptorProvider`s and update the managed * `ComponentDescriptorRegistry`s accordingly. * The methods can be called on any thread. */ void add(ComponentDescriptorProvider provider) const; - void remove(ComponentDescriptorProvider provider) const; /* * ComponenDescriptorRegistry will call the `request` in case if a component diff --git a/ReactCommon/fabric/uimanager/ComponentDescriptorRegistry.cpp b/ReactCommon/fabric/uimanager/ComponentDescriptorRegistry.cpp index 7dbc86b9e33..d8193511fe1 100644 --- a/ReactCommon/fabric/uimanager/ComponentDescriptorRegistry.cpp +++ b/ReactCommon/fabric/uimanager/ComponentDescriptorRegistry.cpp @@ -47,21 +47,6 @@ void ComponentDescriptorRegistry::add( } } -void ComponentDescriptorRegistry::remove( - ComponentDescriptorProvider componentDescriptorProvider) const { - std::unique_lock lock(mutex_); - - assert( - _registryByHandle.find(componentDescriptorProvider.handle) != - _registryByHandle.end()); - assert( - _registryByName.find(componentDescriptorProvider.name) != - _registryByName.end()); - - _registryByHandle.erase(componentDescriptorProvider.handle); - _registryByName.erase(componentDescriptorProvider.name); -} - void ComponentDescriptorRegistry::registerComponentDescriptor( SharedComponentDescriptor componentDescriptor) const { ComponentHandle componentHandle = componentDescriptor->getComponentHandle(); diff --git a/ReactCommon/fabric/uimanager/ComponentDescriptorRegistry.h b/ReactCommon/fabric/uimanager/ComponentDescriptorRegistry.h index 92ae8d14c2f..f0b0d9f284d 100644 --- a/ReactCommon/fabric/uimanager/ComponentDescriptorRegistry.h +++ b/ReactCommon/fabric/uimanager/ComponentDescriptorRegistry.h @@ -59,13 +59,13 @@ class ComponentDescriptorRegistry { SharedComponentDescriptor componentDescriptor) const; /* - * Adds (or removes) a `ComponentDescriptor ` created using given - * `ComponentDescriptorProvider` and stored `ComponentDescriptorParameters`. + * Creates a `ComponentDescriptor` using specified + * `ComponentDescriptorProvider` and stored `ComponentDescriptorParameters`, + * and then adds that to the registry. * To be used by `ComponentDescriptorProviderRegistry` only. * Thread safe. */ void add(ComponentDescriptorProvider componentDescriptorProvider) const; - void remove(ComponentDescriptorProvider componentDescriptorProvider) const; mutable better::shared_mutex mutex_; mutable better::map