Skip to content

Commit

Permalink
Fix RefreshControl race condition
Browse files Browse the repository at this point in the history
Summary:
There was a race condition with `SwipeRefreshLayout` that cause the `RefreshControl` to keep refreshing when it shouldn't.

It was caused because we have to use `post` to set the refreshing state otherwise it doesn't work when setting `refreshing=true` on initial mount. What happened is that `post` doesn't guarantee the order the runnables will be called so calling post with `refreshing=true` followed by `refreshing=false` caused the `resfreshing=false` runnable to be called before the `resfreshing=true` one. This made it stay in refreshing state when it should not.

```
D/test    ( 6171): setRefreshing true
W/ReactNativeJS( 6171): false
D/test    ( 6171): setRefreshing false
D/test    ( 6171): setRefreshing post false
D/test    ( 6171): setRefreshing post true
```

This change adds an instance variable and uses it in the `post` runnable to make sure the last set value is always used.

**Test plan (required)**
Tested that it fixed the issue in the [original issue app](https://github.com/digisqu
Closes #7317

Differential Revision: D3290464

Pulled By: andreicoman11

fbshipit-source-id: 15cabcfc6d2f191443be96e8845b924ce66c369f
  • Loading branch information
janicduplessis authored and Facebook Github Bot 8 committed May 12, 2016
1 parent 20c8899 commit 8fbce30
Show file tree
Hide file tree
Showing 2 changed files with 19 additions and 9 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -20,10 +20,27 @@
*/
public class ReactSwipeRefreshLayout extends SwipeRefreshLayout {

private boolean mRefreshing = false;

public ReactSwipeRefreshLayout(ReactContext reactContext) {
super(reactContext);
}

@Override
public void setRefreshing(boolean refreshing) {
if (mRefreshing != refreshing) {
mRefreshing = refreshing;
// Use `post` otherwise the control won't start refreshing if refreshing is true when
// the component gets mounted.
post(new Runnable() {
@Override
public void run() {
ReactSwipeRefreshLayout.super.setRefreshing(mRefreshing);
}
});
}
}

@Override
public boolean onInterceptTouchEvent(MotionEvent ev) {
if (super.onInterceptTouchEvent(ev)) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -74,15 +74,8 @@ public void setSize(ReactSwipeRefreshLayout view, int size) {
}

@ReactProp(name = "refreshing")
public void setRefreshing(final ReactSwipeRefreshLayout view, final boolean refreshing) {
// Use `post` otherwise the control won't start refreshing if refreshing is true when
// the component gets mounted.
view.post(new Runnable() {
@Override
public void run() {
view.setRefreshing(refreshing);
}
});
public void setRefreshing(ReactSwipeRefreshLayout view, boolean refreshing) {
view.setRefreshing(refreshing);
}

@ReactProp(name = "progressViewOffset", defaultFloat = 0)
Expand Down

0 comments on commit 8fbce30

Please sign in to comment.