Skip to content

Commit

Permalink
Fix crash in slider with custom image
Browse files Browse the repository at this point in the history
Summary:
Changelog: [Internal]

# Why was it crashing?
Crash was caused by `data.getTrackImageRequest()` returning NULL. The crash happened inside `getCoordinator` method. If you look at the implementation of the method below

```
if (request) {
    return &request->getObserverCoordinator();
} else {
    return nullptr;
}
```

you might notice that we check for NULL, why is it crashing then? And why is it only crashing in production?

The reason is compiler's optimiser, which looks at `data.getTrackImageRequest()`, this method returns `ImageRequest const &` and sees that this method will always return a value, never NULL. Compiler then looks at our `getCoordinator` function, sees that we check there for nullity but since it can never be null, removes that branch.

# Solution
Create new SliderState with non null image observer.

Reviewed By: shergin

Differential Revision: D20491367

fbshipit-source-id: 17b7cf31feabbe6f8ece324a3d329902b2ef6a2d
  • Loading branch information
sammy-SC authored and facebook-github-bot committed Mar 17, 2020
1 parent 79490d9 commit 678f1c4
Showing 1 changed file with 15 additions and 0 deletions.
15 changes: 15 additions & 0 deletions ReactCommon/fabric/components/slider/SliderShadowNode.h
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,21 @@ class SliderShadowNode final : public ConcreteViewShadowNode<
void setSliderMeasurementsManager(
const std::shared_ptr<SliderMeasurementsManager> &measurementsManager);

static SliderState initialStateData(
ShadowNodeFragment const &fragment,
SurfaceId const surfaceId,
ComponentDescriptor const &componentDescriptor) {
auto imageSource = ImageSource{ImageSource::Type::Invalid};
return {imageSource,
{imageSource, nullptr},
imageSource,
{imageSource, nullptr},
imageSource,
{imageSource, nullptr},
imageSource,
{imageSource, nullptr}};
}

#pragma mark - LayoutableShadowNode

Size measure(LayoutConstraints layoutConstraints) const override;
Expand Down

0 comments on commit 678f1c4

Please sign in to comment.