5 Commits

Author SHA1 Message Date
Sebastian Markbage
5d9326be29 Remove instanceHandle, pass event target instead + add dispatchToEmptyTarget
Summary:
Removes the concept of instance handle. Instead we pass the event target
to createNode and don't pass it to subsequent clones.

The life time of the event target is managed by native (the event emitter).
It has to be released manually.

Reviewed By: shergin

Differential Revision: D8688330

fbshipit-source-id: e11b61f147ea9ca4dfb453fe07063ed06f24b7ac
2018-06-29 15:32:27 -07:00
Valentin Shergin
eabf29e320 Fabric: Getting rid of many auto &&
Summary:
@public
After reading about move-semantic and rvalue refs I realized that we (I) definitely overuse  `auto &&` (aka universal reference) construction. Even if this is harmless, does not look good and idiomatic.
Whenever I used that from a semantical point of view I always meant  "I need an alias for this" which is actually "read-only reference" which is `const auto &`.
This is also fit good to our policy where "everything is const (immutable) by default".
Hence I change that to how it should be.

Reviewed By: fkgozali

Differential Revision: D8475637

fbshipit-source-id: 0a691ededa0e798db8ffa053bff0f400913ab7b8
2018-06-22 07:32:49 -07:00
Kevin Gozali
20a8673b48 iOS: create EventTarget when creating EventEmitter and keep it until the emitter is deallocated.
Summary:
@public
There are some race conditions between VM objects getting deallocated and the instanceHandle held by the eventEmitter can point to deallocated memory space, causing undefined behavior like a crash.
For now, keep a strong ref to the eventTarget inside EventEmitter to avoid that scenario. This is a temporary workaround.

Reviewed By: shergin

Differential Revision: D8576785

fbshipit-source-id: 87ef36f716270ceca906b32bb86e0046ceaca19e
2018-06-21 14:35:39 -07:00
Kevin Gozali
2ca4770011 fabric: send events via JS thread
Summary: Calling the event emitters on the main thread seems to be problematic, so let's dispatch it via the JS thread. This requires some changes to make "eventTarget" single-use because otherwise the binding would need to synchronize the actual JS call with the act of releasing the target.

Reviewed By: shergin

Differential Revision: D8375291

fbshipit-source-id: bd2b42731176ae209f4a19c232309c163fb1c01b
2018-06-15 11:02:17 -07:00
Valentin Shergin
d49ebbcf62 Fabric: All *EventHandlers were renamed to *EventEmitter
Summary:
Using `EventHandlers` name was a bad idea, and I cannot tolerate it anymore.
The worst part of it is that when you have a collection of `EventHandlers` objects you cannot use plural word to describe it because `EventHandlers` is an already plural word.

And, this object is actually an event emitter, the thing on which we call events.

Reviewed By: fkgozali

Differential Revision: D8247723

fbshipit-source-id: b3303a4b9529bd6d32bb8ca0378287ebefaedda8
2018-06-09 13:16:45 -07:00