Commit Graph

33 Commits

Author SHA1 Message Date
Janic Duplessis c47759a9ae Fix potential retain cycles in Animated iOS
Summary:
Fixes potential retain cycles detected by an internal fb tool.

```
First:

__NSDictionaryM
-> RCTPropsAnimatedNode
-> _parentNodes -> __NSDictionaryM
-> RCTStyleAnimatedNode
-> _childNodes -> __NSDictionaryM

Second:

RCTScrollView
-> _eventDispatcher -> RCTEventDispatcher
-> _observers -> __NSArrayM
-> RCTNativeAnimatedModule
-> _nodesManager -> RCTNativeAnimatedNodesManager
-> _uiManager -> RCTUIManager
-> _viewRegistry -> __NSDictionaryM
-> RCTScrollView
```

First fix:
Use weak map for parent and child nodes, strong refs are managed by RCTNativeAnimatedNodesManager

Second fix:
Make RCTEventDispatcher observers a weak array and make sure we don't keep strong refs to UIManager in RCTNativeAnimatedNodesManager and RCTPropsAnimatedNode.

Tested that native animations still work in UIExplorer

[IOS] [BUGFIX] [NativeAnimated] - Fix potential retain cycles in Animated iOS
Closes https://github.com/facebook/react-native/pull/16506

Differential Revision: D6126400

Pulled By: shergin

fbshipit-source-id: 1ac5083f8ab79a806305edc23ae4796ed428f78b
2017-10-23 13:20:59 -07:00
Janic Duplessis b49e7afe47 Dispatch native handled events to JS
Summary:
When native events where handled they were not sent to JS as an optimization but this caused some issues. One of the major one is touches are not handled properly inside a ScrollView with an Animated.event because it doesn't receive scroll events so it can't cancel the touch if the user scrolled.
Closes https://github.com/facebook/react-native/pull/10981

Differential Revision: D4226403

Pulled By: astreet

fbshipit-source-id: 41278d3ed4b684af142d9e273b11b974eb679879
2016-11-23 05:43:35 -08:00
James Ide fc11a5fde8 Add support for native animated events on iOS
Summary:
This adds native support for `Animated.event` on iOS.

**Test plan**
Tested in the native animated UIExplorer example that it works properly like on Android.
Closes https://github.com/facebook/react-native/pull/9598

Differential Revision: D4110331

fbshipit-source-id: 15748d23d0f475f2bcd1040ca3dca33e2620f058
2016-11-01 03:58:53 -07:00
Pieter De Baets e762d961cd Use new enqueueJSCall method everywhere
Reviewed By: majak

Differential Revision: D3605263

fbshipit-source-id: 215f896d675b937593c8b796ed6ec5261ac74dbf
2016-08-02 11:14:06 -07:00
Nick Lockwood d9737571c4 Updated AppState module to use new emitter system
Summary: AppState now subclasses NativeEventEmitter instead of using global RCTDeviceEventEmitter.

Reviewed By: javache

Differential Revision: D3310488

fbshipit-source-id: f0116599223f4411307385c0dab683659d8d63b6
2016-05-23 09:13:37 -07:00
Pieter De Baets feda8ce2ee Clean up RCTEventDispatcher code
Reviewed By: majak

Differential Revision: D3175583

fb-gh-sync-id: c751c9dc79edfd9cb6073a2ff5dd743a03334bc4
fbshipit-source-id: c751c9dc79edfd9cb6073a2ff5dd743a03334bc4
2016-04-28 05:48:21 -07:00
Martin Kralik 3874c01292 make `RCTEventDispatcher` dispatch events in incoming order
Reviewed By: javache

Differential Revision: D3138215

fb-gh-sync-id: 430b57d7bb8e3f0f90877b627bfd6a842719ba5d
fbshipit-source-id: 430b57d7bb8e3f0f90877b627bfd6a842719ba5d
2016-04-05 09:17:28 -07:00
Martin Kralik 02b6e38bee better event emitting II: no deadlocks
Summary:D3092867 / 1d3db4c5dc caused deadlock when chrome debugging was turned on, so it was reverted as D3128586 / 144dc30661.
The reason: I was calling `[_bridge dispatchBlock:^{ [self flushEventsQueue]; } queue:RCTJSThread];` from main thread and expecting it will `dispatch_async` to another,
since a held lock was being accessed the dispatched block and was released after the dispatch.
Turns out `RCTWebSocketExecutor` (which is used when chrome debugger is turned on) executes all blocks dispatched this way to `RCTJSThread` synchronously on the main thread.
This resulted in a deadlock. The "dispatched" block was trying to acquired lock which held by the same thread in the dispatching phase.

A fix for this is pretty simple. We will release the lock before dispatching the block.

However it's not super straightforward to see this won't introduce some race condition in a case with two threads where we would end up with events not being processed.
My thinking why that shouldn't happen goes like this: We could get in a bad state if `flushEventsQueue` would run on JS thread while `sendEvent:` is running on MT.
(I don't have a specific example how, maybe it's not possible. However when I show this case is safe we know we are good.)
The way how locking is setup in this diff the only possible scenario where these two threads would execute in these methods concurrently is JS holding the lock and MT going to enqueue another block on JS thread (since that's outside of "locked" zone).
But this scenarion can never happen, since if MT is about to enqueue the block on JS thread it means there cannot be a not yet fully executed block on JS thread.
Therefore nothing bad can happen.

So this diff brings back the reverted diff and adds to it the fix for the deadlock.

Reviewed By: javache

Differential Revision: D3130375

fb-gh-sync-id: 885a166f2f808551d7cd4e4eb98634d26afe6a11
fbshipit-source-id: 885a166f2f808551d7cd4e4eb98634d26afe6a11
2016-04-02 23:31:25 -07:00
Alex Kotliarskyi 144dc30661 Revert [RN][iOS] better event emitting
Reviewed By: sahrens

Differential Revision: D3128586

fb-gh-sync-id: 5c3c79fa983f6c1c43319a6c14049f99e0dfee8a
fbshipit-source-id: 5c3c79fa983f6c1c43319a6c14049f99e0dfee8a
2016-04-01 14:26:20 -07:00
Martin Kralik 1d3db4c5dc better event emitting
Summary:Previously, (mostly touch and scroll) event handling on iOS worked in a hybrid way:
* All incoming coalesce-able events would be pooled and retrieved by js thread in the beginning of its frame (all of this happens on js thread)
* Any non-coalesce-able event would be immediately dispatched on a js thread (triggered from main thread), and if there would be pooled coalesce-able events they would be immediately dispatched at first too.

This behavior has a subtle race condition, where two events are produced (on MT) in one order and received in js in different order.
See https://github.com/facebook/react-native/issues/5246#issuecomment-198326673 for further explanation of this case.

The new event handling is (afaik) what Android already does. When an event comes we add it into a pool of events and dispatch a block on js thread to inform js there are events to be processed. We keep track of whether we did so, so there is at most one of these blocks waiting to be processed. When the block is executed js will process all events that are in pool at that time (NOT at time of enqueuing the block).
This creates a single way of processing events and makes it impossible to process them in different order in js.

The tricky part was making sure we don't coalesce events across gestures/different scrolls. Before this was achieved by knowing that gestures and scrolls start/end with  non-coalesce-able event, so the pool never contained events that shouldn't be coalesced together. That "assumption" doesn't hold now.
I've re-added `coalescingKey` and made touch and scroll events use it to prevent coalescing events of the same type that should remain separate in previous diffs (see dependencies).

On top of it it decreases latency in events processing in case where we get only coalesce-able events. Previously these would be processed at begging of the next js frame, even when js would be free and could process them sooner. This delay is done, since they would get processed as soon as the enqueued block would run.

To illustrate this improvement let's look at these two systraces.
Before: https://cloud.githubusercontent.com/assets/713625/14021417/47b35b7a-f1d3-11e5-93dd-4363edfa1923.png
After: https://cloud.githubusercontent.com/assets/713625/14021415/4798582a-f1d3-11e5-8715-425596e0781c.png

Reviewed By: javache

Differential Revision: D3092867

fb-gh-sync-id: 29071780f00fcddb0b1886a46caabdb3da1d5d84
fbshipit-source-id: 29071780f00fcddb0b1886a46caabdb3da1d5d84
2016-04-01 06:54:50 -07:00
Martin Kralik a496baa68c reintroduced coalescing key for events
Summary: This was previously removed in D2884587, but we will need it going forward. See D3092867 for reasons why it's necessary again.

Reviewed By: javache

Differential Revision: D3092848

fb-gh-sync-id: 0d10dbac4148fcc8e035d32d8eab50f876d99e88
fbshipit-source-id: 0d10dbac4148fcc8e035d32d8eab50f876d99e88
2016-04-01 06:54:49 -07:00
Martin Kralik 91e5829419 flush events queue when an event cannot be coalesced (4/7)
Summary:
Currently only scroll events are send through `sendEvent`, and all of them are can be coalesced. In future (further in the stack) touch events will go through there as well, but they won't support coalescing.
In order to ensure js processes touch and scroll events in the same order as they were created, we will flush the coalesced events when we encounter one that cannot be coalesced.

public
___
//This diff is part of a larger stack. For high level overview what's going on jump to D2884593.//

Reviewed By: nicklockwood

Differential Revision: D2884591

fb-gh-sync-id: a3d0e916843265ec57f16aad2f016a79764dcce8
2016-02-03 05:23:55 -08:00
Martin Kralik 7f2b72528e RCTEvent protocol changes (3/7)
Summary:
I want to use the `RCTEvent` protocol for touch events as well. That's why I'm removing not very well defined `body` property and replacing it with `arguments` method, which will return an array that will be passed directly to the js call.
I think this makes sense because there is no unified arguments format for all events and and the called  js method (`moduleDotMethod`) is already event specific.
This way touch events and scroll events can result in calling a completely different js function with a completely different arguments (what they indeed currently do).

public
___
//This diff is part of a larger stack. For high level overview what's going on jump to D2884593.//

Reviewed By: nicklockwood

Differential Revision: D2884590

fb-gh-sync-id: 2c1885c3414e255d8572c0fbbbfe62a23d94dd06
2016-02-03 05:23:50 -08:00
Martin Kralik 3e89c3ea3b removed `coalescingKey` from events (2/7)
Summary:
This property was never used, so I'm removing it.

public
___
//This diff is part of a larger stack. For high level overview what's going on jump to D2884593.//

Reviewed By: javache

Differential Revision: D2884587

fb-gh-sync-id: acd5e576cd13a02e77225f3b308232f8331d3b61
2016-02-03 05:23:43 -08:00
Martin Kralik ee533037f6 removed unused RCTBaseEvent (1/7)
Summary:
`RCTBaseEvent` was never used. This diff removes it.

public
___
//This diff is part of a larger stack. For high level overview what's going on jump to D2884593.//

Reviewed By: javache

Differential Revision: D2884585

fb-gh-sync-id: 66a6afcda3b5baec7f768682da215570f6d33bb1
2016-02-03 05:23:36 -08:00
Nick Lockwood 060664fd3d Refactored module access to allow for lazy loading
Summary: public

The `bridge.modules` dictionary provides access to all native modules, but this API requires that every module is initialized in advance so that any module can be accessed.

This diff introduces a better API that will allow modules to be initialized lazily as they are needed, and deprecates `bridge.modules` (modules that use it will still work, but should be rewritten to use `bridge.moduleClasses` or `-[bridge moduleForName/Class:` instead.

The rules are now as follows:

* Any module that overrides `init` or `setBridge:` will be initialized on the main thread when the bridge is created
* Any module that implements `constantsToExport:` will be initialized later when the config is exported (the module itself will be initialized on a background queue, but  `constantsToExport:` will still be called on the main thread.
* All other modules will be initialized lazily when a method is first called on them.

These rules may seem slightly arcane, but they have the advantage of not violating any assumptions that may have been made by existing code - any module written under the original assumption that it would be initialized synchronously on the main thread when the bridge is created should still function exactly the same, but modules that avoid overriding `init` or `setBridge:` will now be loaded lazily.

I've rewritten most of the standard modules to take advantage of this new lazy loading, with the following results:

Out of the 65 modules included in UIExplorer:

* 16 are initialized on the main thread when the bridge is created
* A further 8 are initialized when the config is exported to JS
* The remaining 41 will be initialized lazily on-demand

Reviewed By: jspahrsummers

Differential Revision: D2677695

fb-gh-sync-id: 507ae7e9fd6b563e89292c7371767c978e928f33
2015-11-25 04:49:45 -08:00
Nick Lockwood c5b990f65f Added lightweight generic annotations
Summary: public

Added lightweight genarics annotations to make the code more readable and help the compiler catch bugs.

Fixed some type bugs and improved bridge validation in a few places.

Reviewed By: javache

Differential Revision: D2600189

fb-gh-sync-id: f81e22f2cdc107bf8d0b15deec6d5b83aacc5b56
2015-11-03 14:49:30 -08:00
Dave Sibiski 6c7c845145 Implements `onKeyPress`
Summary: - When a key is pressed, it's `key value` is passed as an argument to the callback handler.
 - For `Enter` and `Backspace` keys, I'm using their `key value` as defined [here](https://developer.mozilla.org/en-US/docs/Web/API/KeyboardEvent/key#Key_values). As per JonasJonny & brentvatne's [suggestion](https://github.com/facebook/react-native/issues/1882#issuecomment-123485883).

- Example
```javascript
 _handleKeyPress: function(e) {
      console.log(e.nativeEvent.key);
  },

  render: function() {
    return (
      <View style={styles.container}>
        <TextInput
            style={{width: 150, height: 25, borderWidth: 0.5}}
            onKeyPress={this._handleKeyPress}
        />
        <TextInput
            style={{width: 150, height: 100, borderWidth: 0.5}}
            onKeyPress={this._handleKeyPress}
            multiline={true}
        />
      </View>
    );
  }
```
- Implements [shouldChangeCharactersInRange](https://developer.apple.com/library/prerelease/ios/documentat
Closes https://github.com/facebook/react-native/pull/2082

Reviewed By: javache

Differential Revision: D2280460

Pulled By: nicklockwood

fb-gh-sync-id: 1f824f80649043dc2520c089e2531d428d799405
2015-11-02 09:15:31 -08:00
Alexey Lang e727fc817b Pause JS DisplayLink if nothing to process.
Reviewed By: @jspahrsummers

Differential Revision: D2489107
2015-09-29 09:22:10 -07:00
Pieter De Baets 371aeceb72 Small perf improvement to RCTPerfStats and RCTBridgeModuleNameForClass 2015-08-25 04:48:39 -08:00
Nick Lockwood 88e0bbc469 Ran Convert > To Modern Objective C Syntax 2015-08-25 01:08:49 -08:00
Nick Lockwood 8d1e02b8bd Convert alloc/init to new to please linter 2015-08-17 08:46:00 -07:00
Nick Lockwood 48af214216 Simplified event registration
Summary:
Our events all follow a common pattern, so there's no good reason why the configuration should be so verbose. This diff eliminates that redundancy, and gives us the freedom to simplify the underlying mechanism in future without further churning the call sites.
2015-08-11 06:41:04 -08:00
Spencer Ahrens 961c1eb429 [ReactNative] TextInput bug fixes and features
Summary:
This introduces event counts to make sure JS doesn't set out of date values on
native text inputs, which can cause dropped characters and can mess with
autocomplete, and obviates the need for the input buffering which added lag and
complexity to the component.  Made sure to test simulated super-slow JS text
event processing to make sure characters aren't dropped, as well as typing
obviously correctable words and making sure autocomplete works as expected.

TextInput is now a controlled input by default without causing any issues for
most cases, so I removed the `controlled` prop.

Fixes selection state jumping by restoring it after setting new text values, so
highlighting the middle of some text in the new ReWrite example and hitting
space will replace that selection with an underscore and keep the cursor at a
sensible position as expected, instead of jumping to the end.

Ads `maxLength` prop to support the most commonly needed syncronous behavior:
preventing the user from typing too many characters.  It can also be used to
prevent users from continuing to type after entering special characters by
changing it to the current length after a regex match.  Made sure to verify it
works well with pasted input (including in the middle of existing text),
truncating it and collapsing the selection the same way it does on the web.

Fixes bug in TextEventsExample where it wouldn't show the submit and end events,
even though there were firing correctly.
2015-07-21 12:45:07 -08:00
Tadeu Zagallo d3065fc2e7 [ReactNative] Remove RCT_IMPORT_METHOD macro and generate lookup table dynamically
Summary:
@public

This removes the last piece of data that was still stored on the DATA section,
`RCT_IMPORT_METHOD`. JS calls now dynamically populate a lookup table simultaneously
on JS and Native, instead of creating  a mapping at load time.

Test Plan: Everything still runs, tests are green.
2015-06-15 13:05:52 -08:00
Nick Lockwood 650fc9de4c Increased warning levels to -Wall -Wextra, and fixed Xcode 7 beta issues
Summary:
@public

I've increased the warning levels in the OSS frameworks, which caught a bunch of minor issues. I also fixed some new errors in Xcode 7 relating to designated initializers and TLS security.

Test Plan:
* Test the sample apps and make sure they still work.
* Run tests.
2015-06-15 07:52:50 -08:00
Tadeu Zagallo c2fb21b322 [ReactNative] Properly pause frame update observers when idle
Summary:
@public

`RCTDispatchEvent` and `RCTTiming` weren't being paused when there wasn't any
work left to be done.

Test Plan:
Run the timers example - check everything still works as expected
Test the ListView paging example - check scroll events are still fired as expected
Launch UIExplorer, let it idle, and put a break point on `-[RCTBridge dispatchBlock:moduleID:]`,
it should never fire.
2015-06-07 08:45:18 -08:00
Tadeu Zagallo 777363fdd7 [ReactNative] Use NSDictionary + NSNumber for event coalescing
Summary:
@public

On D2099270 event coalescing was implemented and the event key on the RCTSparseArray
is an uint64_t, but it was declared as NSUInteger.  On a 32 bits architecture
it'll be clipped to 4 bits, meaning that just `reactTag` will be taken into
account, e.g. different types of events can coalesce with each other if they target the same view

Switching to use an NSMutableDictionary instead of RCTSparseArray and NSNumber
as keys instead of uint64_t

Test Plan: Fixed the previous tests and added a new test to RCTEventDispatcherTests
2015-06-02 03:06:06 -08:00
Tadeu Zagallo 4fc15dbf17 [ReactNative] Implement proper event coalescing 2015-05-27 20:41:20 -08:00
Ben Alpert dd56ccb9c7 [react-native] Fix capitalization of "REact" 2015-04-27 13:52:57 -08:00
James Ide 20d95ed129 [Scroll] Include content insets in scroll events
Summary:
When calculating how far the user has scrolled, it is necessary to know the content insets where:

    number of pixels scrolled = content offset + leading content inset for the scroll axis

This diff adds the contentInset field to native scroll events.
Closes https://github.com/facebook/react-native/pull/737
Github Author: James Ide <ide@jameside.com>

Test Plan: Imported from GitHub, without a `Test Plan:` line.
2015-04-09 08:45:58 -08:00
Nick Lockwood bf4868edda Added non-class-scanning-based approach fror registering js methods 2015-04-08 05:45:20 -08:00
Tadeu Zagallo 20291a02df [ReactNative] s/ReactKit/React/g 2015-03-26 02:42:24 -08:00