Fabric: Moving [super updateProps:props oldProps:oldProps]; to the end of the methods

Summary:
In some cases, the view class is the only that retains stored `props_` variable. At the same time the `[super updateProps:props oldProps:oldProps];` actually resets the pointer with a pointer to new props value which sometimes causes the deallocation of the old value. All that is okay unless the implementation of `updateProps:oldProps:` in superclasses stores a raw reference to an old value in the very beginning of the method (for convenience and perf reasons).

So, to prevent preliminary deallocation of the old value pointed by `_props` we moved all `[super updateProps:props oldProps:oldProps];` calls to the end of overloaded methods.

Reviewed By: mdvacca

Differential Revision: D15770068

fbshipit-source-id: af36b3e70560ab00846cd26b0963bbc059e977bc
This commit is contained in:
Valentin Shergin
2019-06-12 21:18:56 -07:00
committed by Facebook Github Bot
parent b8bc40e2f0
commit b5cd9e4db1
10 changed files with 19 additions and 18 deletions
@@ -63,8 +63,6 @@ static UIActivityIndicatorViewStyle convertActivityIndicatorViewStyle(const Acti
const auto &oldViewProps = *std::static_pointer_cast<const ActivityIndicatorViewProps>(_props);
const auto &newViewProps = *std::static_pointer_cast<const ActivityIndicatorViewProps>(props);
[super updateProps:props oldProps:oldProps];
if (oldViewProps.animating != newViewProps.animating) {
if (newViewProps.animating) {
[_activityIndicatorView startAnimating];
@@ -85,6 +83,8 @@ static UIActivityIndicatorViewStyle convertActivityIndicatorViewStyle(const Acti
if (oldViewProps.size != newViewProps.size) {
_activityIndicatorView.activityIndicatorViewStyle = convertActivityIndicatorViewStyle(newViewProps.size);
}
[super updateProps:props oldProps:oldProps];
}
@end
@@ -55,8 +55,6 @@
const auto &oldImageProps = *std::static_pointer_cast<const ImageProps>(_props);
const auto &newImageProps = *std::static_pointer_cast<const ImageProps>(props);
[super updateProps:props oldProps:oldProps];
// `resizeMode`
if (oldImageProps.resizeMode != newImageProps.resizeMode) {
if (newImageProps.resizeMode == ImageResizeMode::Repeat) {
@@ -72,6 +70,8 @@
if (oldImageProps.tintColor != newImageProps.tintColor) {
_imageView.tintColor = [UIColor colorWithCGColor:newImageProps.tintColor.get()];
}
[super updateProps:props oldProps:oldProps];
}
- (void)updateLocalData:(SharedLocalData)localData oldLocalData:(SharedLocalData)oldLocalData
@@ -74,8 +74,6 @@ using namespace facebook::react;
const auto &oldScrollViewProps = *std::static_pointer_cast<const ScrollViewProps>(_props);
const auto &newScrollViewProps = *std::static_pointer_cast<const ScrollViewProps>(props);
[super updateProps:props oldProps:oldProps];
#define REMAP_PROP(reactName, localName, target) \
if (oldScrollViewProps.reactName != newScrollViewProps.reactName) { \
target.localName = newScrollViewProps.reactName; \
@@ -113,6 +111,8 @@ using namespace facebook::react;
// MAP_SCROLL_VIEW_PROP(scrollIndicatorInsets);
// MAP_SCROLL_VIEW_PROP(snapToInterval);
// MAP_SCROLL_VIEW_PROP(snapToAlignment);
[super updateProps:props oldProps:oldProps];
}
- (void)updateState:(State::Shared const &)state oldState:(State::Shared const &)oldState
@@ -53,8 +53,6 @@ using namespace facebook::react;
auto const &oldConcreteProps = *std::static_pointer_cast<PullToRefreshViewProps const>(_props);
auto const &newConcreteProps = *std::static_pointer_cast<PullToRefreshViewProps const>(props);
[super updateProps:props oldProps:oldProps];
if (newConcreteProps.refreshing != oldConcreteProps.refreshing) {
if (newConcreteProps.refreshing) {
[_refreshControl beginRefreshing];
@@ -76,6 +74,8 @@ using namespace facebook::react;
if (needsUpdateTitle) {
[self _updateTitle];
}
[super updateProps:props oldProps:oldProps];
}
#pragma mark -
@@ -112,8 +112,6 @@ using namespace facebook::react;
const auto &oldSliderProps = *std::static_pointer_cast<const SliderProps>(_props);
const auto &newSliderProps = *std::static_pointer_cast<const SliderProps>(props);
[super updateProps:props oldProps:oldProps];
// `value`
if (oldSliderProps.value != newSliderProps.value) {
_sliderView.value = newSliderProps.value;
@@ -149,6 +147,8 @@ using namespace facebook::react;
if (oldSliderProps.maximumTrackTintColor != newSliderProps.maximumTrackTintColor) {
_sliderView.maximumTrackTintColor = [UIColor colorWithCGColor:newSliderProps.maximumTrackTintColor.get()];
}
[super updateProps:props oldProps:oldProps];
}
- (void)updateLocalData:(SharedLocalData)localData oldLocalData:(SharedLocalData)oldLocalData
@@ -48,8 +48,6 @@ using namespace facebook::react;
const auto &oldSwitchProps = *std::static_pointer_cast<const SwitchProps>(_props);
const auto &newSwitchProps = *std::static_pointer_cast<const SwitchProps>(props);
[super updateProps:props oldProps:oldProps];
// `value`
if (oldSwitchProps.value != newSwitchProps.value) {
_switchView.on = newSwitchProps.value;
@@ -75,6 +73,8 @@ using namespace facebook::react;
if (oldSwitchProps.thumbTintColor != newSwitchProps.thumbTintColor) {
_switchView.thumbTintColor = [UIColor colorWithCGColor:newSwitchProps.thumbTintColor.get()];
}
[super updateProps:props oldProps:oldProps];
}
- (void)onChange:(UISwitch *)sender
@@ -57,10 +57,10 @@ using namespace facebook::react;
{
const auto &paragraphProps = std::static_pointer_cast<const ParagraphProps>(props);
[super updateProps:props oldProps:oldProps];
assert(paragraphProps);
_paragraphAttributes = paragraphProps->paragraphAttributes;
[super updateProps:props oldProps:oldProps];
}
- (void)updateLocalData:(SharedLocalData)localData oldLocalData:(SharedLocalData)oldLocalData
@@ -50,11 +50,11 @@ using namespace facebook::react;
const auto &oldViewProps = *std::static_pointer_cast<const UnimplementedNativeViewProps>(_props);
const auto &newViewProps = *std::static_pointer_cast<const UnimplementedNativeViewProps>(props);
[super updateProps:props oldProps:oldProps];
if (oldViewProps.name != newViewProps.name) {
_label.text = [NSString stringWithFormat:@"'%s' is not Fabric compatible yet.", newViewProps.name.c_str()];
}
[super updateProps:props oldProps:oldProps];
}
@end
@@ -70,6 +70,7 @@ NS_ASSUME_NONNULL_BEGIN
/**
* Enforcing `call super` semantic for overridden methods from `RCTComponentViewProtocol`.
* The methods update the instance variables.
*/
- (void)updateProps:(facebook::react::Props::Shared const &)props
oldProps:(facebook::react::Props::Shared const &)oldProps NS_REQUIRES_SUPER;
@@ -110,8 +110,6 @@ using namespace facebook::react;
auto const &oldViewProps = *std::static_pointer_cast<ViewProps const>(_props);
auto const &newViewProps = *std::static_pointer_cast<ViewProps const>(props);
_props = std::static_pointer_cast<ViewProps const>(props);
BOOL needsInvalidateLayer = NO;
// `opacity`
@@ -241,6 +239,8 @@ using namespace facebook::react;
}
_needsInvalidateLayer = _needsInvalidateLayer || needsInvalidateLayer;
_props = std::static_pointer_cast<ViewProps const>(props);
}
- (void)updateEventEmitter:(EventEmitter::Shared const &)eventEmitter