Summary:
I read my code and one thing stroke me: What if the same event emitter dispatches several events during the same event beat? In the previous implementation, only the first one will be delivered because the first `release` call consumes stored strong reference.
So, I changed the API a bit to make this more explicit: we have `retain` & `release` methods and we have a getter that works (multiple times) only if the object was successfully retained.
Reviewed By: sahrens
Differential Revision: D13668147
fbshipit-source-id: c00af5f15bc7e30aa704d46bd23584b918b6f75a
Summary: See the diff for more details.
Reviewed By: sahrens
Differential Revision: D13642593
fbshipit-source-id: 8bdcc91bcc2ea1e4093bcac03d87167b8901cbb4
Summary: The mutex is a "leaf" mutex, so the code cannot reenter that, so there is no reason to be a recursive one.
Reviewed By: sahrens
Differential Revision: D13642592
fbshipit-source-id: 0fb64d3405c5d3408251dc983c186f6747bc6ee2
Summary:
EventTargetWrapper and EventTarget were merged into one class that controls an `instanceHandle` reference and extracting a strong reference to it.
This diff also decouples the operation of retaining a strong reference (with checking a `enabled` flag) from actual usage of this reference. This allows to wrap into a mutex only first part of this process and avoid possible deadlocks.
Reviewed By: sahrens
Differential Revision: D13616382
fbshipit-source-id: 9907bc12047386fcf027929ae2ae41c0b727cd06
Summary:
For some time we have had a nit trick inside `EventEmitter`: When event emitter is disabled, we switch to store `EventTarget` as a weak pointer instead of complete deleting it. Given some unpredictability of JS GC, it gave us some time to restore the pointer in case if we increment the counter right after decrementing this. Apparently, we found that it doesn't always work (obviously, sigh...), so we implement the `enableCounter_` and left this feature as it is assuming that it can nice optimization that illuminates some unnecessary constraints.
But, apparently, it's actually harmful, assuming that `jsi::WeakObject` (that thing that we use to refer to actual JS target) actually has "unsafe unretained" (not "weak") semantic.
So, we have to remove this.
This change will be particularly important for coming diffs in the stack.
Reviewed By: mdvacca
Differential Revision: D13324480
fbshipit-source-id: 4c4da2984dc6a36b94b564bc9eee144142b430b0
Summary:
Now the event delive pipeline supports `JSI::Value`-based payload. Instead of passing `folly::dynamic`, now we are passing `std::function<jsi::Value(jsi::Runtime &runtime)>` as factory that can build a `JSI::Value` with given `jsi::Runtime` and any captured data.
The old (now legacy) way of calling `EventEmitter::dispatchEvent(..., const folly::dynamic &payload, ...)` is also supported.
Reviewed By: sahrens
Differential Revision: D13123043
fbshipit-source-id: d65348bb215013042abb2fcfe5083a8c697333d0
Summary: Now instead of passing `reactTag` through the whole event pipeline, we store it inside `EventTargetWrapper` (and it does not leave `UIManagerBinding`). It helps with reducing the complexity of `EventEmitter` and will help us in migrating to JSI.
Reviewed By: sahrens
Differential Revision: D13123045
fbshipit-source-id: aa9ee94d5660ff3090369c1e55cf748d2e72b987
Summary: This diff fixes the dispaching of Async Events in Android C++ layer to ensure proper asynchronouns dispatching in the JS thread.
Reviewed By: shergin
Differential Revision: D12988348
fbshipit-source-id: 7aa60b11e2c264c2e68354ed83eb75139060d211
Summary: We are moving to more stable APIs removing all mentiones of the effort name from the codebase.
Reviewed By: mdvacca
Differential Revision: D12912894
fbshipit-source-id: 4a0c6b9e7454b8b14e62d419e9e9311dc0c56e7a
Summary: In the previous approach, when event emitter got disabled for split second, we could lose the EventTarget because JS GC can collect it before we re-enable this. Now we "over-enable" this first, and "under-disable" later.
Reviewed By: mdvacca
Differential Revision: D12990112
fbshipit-source-id: 4e3c0c0e05f03509ec72ca570f59ce16597353f0
Summary: This diff adds systrace support in the C++ side of Fabric
Reviewed By: ejanzer
Differential Revision: D12861373
fbshipit-source-id: 0291f3e406f239bbef3686ac0bba6e9f1c7eac57
Summary:
We double down on JSI in Fabric. So, practically, JSI is now a hard dependency for Fabric. I hope it's for good.
Now `jsi::Runtime` is coupled with scheduling via `EventExecuter`, so we have to make `jsi::Runtime` a part of `EventBeat` to proxy runtime reference to bindgings.
Reviewed By: sahrens
Differential Revision: D12837225
fbshipit-source-id: 98edc33d6a3358e6c2905f2f03ce0004a9ca0503
Summary: Apparently, the standard does not guarantee that the vector is empty after moving from it. So, let's clear it explicitly instead of asserting the emptiness.
Reviewed By: sahrens
Differential Revision: D12837227
fbshipit-source-id: 85dff6848707f4204f4c79be173064547e83c63e
Summary:
The first implementation of EventEmitter's enable/disable feature didn't not provide a way to enable an object after it was disabled.
Apparently, we need this functionality due that fact that all nodes of the same family share same event emitter.
Reviewed By: mdvacca
Differential Revision: D10395849
fbshipit-source-id: 0eba54f0bb7ded35d64afb6559e6e27208c2b577
Summary: missing header and platform. attributedstring builds now, but still fails because of T34990592
Reviewed By: mdvacca
Differential Revision: D10349210
fbshipit-source-id: dcd163df9ac9a4fcb36399cb9f93dbf1b33c062d
Summary: This is the second and the final part of adopting clang-format.
Reviewed By: mdvacca
Differential Revision: D10229624
fbshipit-source-id: d97670b716800ea2488b84bd0aacaf54d8bd2e31
Summary: That's why we need the previous three diffs. Synchonous executor deadlocks if the beat is missing.
Reviewed By: sahrens
Differential Revision: D10081501
fbshipit-source-id: 9986d0a1844e642048b6f37a1fcb5f623a267663
Summary: In some cases we have to have a way to notify a EventBeat consumer that the beat cannot be (and will not be) delivered, so we introducing special API for that.
Reviewed By: mdvacca
Differential Revision: D10081503
fbshipit-source-id: 4c5a392d32572f426e3744bdba797efcd29b8cb4
Summary: This diff unifies the 'handling' of the top prefix in the EventEmitter.cpp class
Reviewed By: shergin
Differential Revision: D10149497
fbshipit-source-id: d0ddbbbeefe3790b414b101da47582161354c971
Summary: This diff exposes the method EventEmitter.dispatchEvent as public in order to be able to access it from the android side.
Reviewed By: shergin
Differential Revision: D10127509
fbshipit-source-id: d6ddf59c654a91fdeed5fba867ca31d6de96d607
Summary: We have to uninstall UIManager synchronously to avoid a race condition when JS is capable to call already deallocated UIManager.
Reviewed By: mdvacca
Differential Revision: D10033406
fbshipit-source-id: 194d1ae2dd5ab09b036b1c165de289ada8e66014
Summary: This is the last step before making JSIUIManagerInstaller a direct dependency of UIManager (and making UIManager installation process completely seamless/platform-agnostic).
Reviewed By: mdvacca
Differential Revision: D9995781
fbshipit-source-id: 6f8c7177495b01ebaac1dbe330f49dce2e2a552c
Summary:
EventBeatBasedExecutor is an executor derived from EventBeat and using EventBeat to ensure proper threading.
Why do we need yet another executor? Because otherwise, we have to make it platform specific-dependency that each platform-specific implementation has to implement and provide. We already have all that we need in already provided EventBeat, so we can just convert that into simple executor in a platform-agnostic way.
Reviewed By: mdvacca
Differential Revision: D9995783
fbshipit-source-id: f8aa72a9744e50ebecbea9ad0e2546f41f5358f2
Summary:
I realized that instead of using shared_ptr's type-erasure feature, we can make the EventHandler's destructor virtual and this itself will allow safe deallocation by a pointer to a base class.
We cannot use the same technic for EventTarget thought because having a weak_ptr to this is another feature of shared_ptr that we need.
Reviewed By: mdvacca
Differential Revision: D9775742
fbshipit-source-id: 3c23a163827e8aa9ec731c89ce87051a93afe4ca
Summary: Instead of relying on explicit `RawEventDispatchable` function, we simply check the existence of the `weak_ptr` to `EventTarget`. This is efficient and sufficient because only an EventEmitter retains an associated EventTarget.
Reviewed By: mdvacca
Differential Revision: D9764858
fbshipit-source-id: 4ac25d925f189d0f8b9002e52388fd51629934a8
Summary:
This diff implements a new model of managing `enabled` flag in EventEmitter.
Now we simply rely on `eventTarget_` is not being `nullptr` (and we reset the pointer to "disable" the event emitter).
Reviewed By: mdvacca
Differential Revision: D9764857
fbshipit-source-id: 1dd3ce0c8589048babbf2dbac9f8359358b31a34
Summary:
As we did in the previous diff, here we implemented `EventEmitter`'s ownership model as a `shared_ptr`. This change fixes problem with leaking `WeakObject`s which happens on hot-reload.
So, in short:
* `EventTargetWrapper` object owns `jsi::WeakObject` that can be converted to actual `jsi::Object` that represent event target in JavaScript realm;
* `EventTargetWrapper` and `jsi::WeakObject` objects must be deallocated as soon as native part does not need them anymore;
* `EventEmitter` objects retain `EventTarget` objects;
* `EventEmitter` can loose event target object in case if assosiated `ShadowNode` got unmounted (not deallocated); in this case `EventEmitter` is loosing possibility to dispatch event even if some mounting-layer code is still retaining it.
Reviewed By: mdvacca
Differential Revision: D9762755
fbshipit-source-id: 96e989767a32914db9f4627fce51b044c71f257a
Summary:
Previously, we used special JSI bindings method to release an event handler (`JSIReleaseFabricEventHandler`). Now we expose this ownership model as a regular `std::shared_ptr`, so when the owner got deallocated, the event handler will be released automatically.
Why not use `unique_ptr`? `unique_ptr` is faster (and simpler) indeed, but it does not provide `type erasure` functionality that we need; to use `unique_ptr` we would have to make JSI an explicit Fabric dependency (we will probably end up with it eventually, but I this particular case is not a good reason for that).
All interactions with `eventHandler_` are done in a non-owning manner, so it's as performant as unique_ptr anyway.
(Please ignore all changes in JSCFabricUIManager.h/cpp files, we will delete them soon.)
Reviewed By: mdvacca
Differential Revision: D9756732
fbshipit-source-id: bffdee0c724dc95855ced7c35e7c13cf1554796e
Summary:
The source of truth has already moved, so now we just need to fix references
This diff is mostly the result of running:
```
$ tools/mobile-unification/loadmod --fixup xplat/configurations/buck/apple/flag_defs.bzl tools/build_defs/apple/
```
Then I committed with `hg commit -I xplat/`
The controller you requested could not be found.
Differential Revision: D9772194
fbshipit-source-id: 93d23ae8e1c62440c7876cad965d963bde960db9
Summary: This change drops the year from the copyright headers and the LICENSE file.
Reviewed By: yungsters
Differential Revision: D9727774
fbshipit-source-id: df4fc1e4390733fe774b1a160dd41b4a3d83302a
Summary:
I was watching a classic magnificent talk about modern C++ by Herb Sutter and I was totally sold on double down on using `auto` in our codebase. Surprisingly, 95% of the code base already follows Herb's guidence; I just changed the last 5% to make it consistent.
All those changes must work *exactly* like it was before.
The talk: https://youtu.be/xnqTKD8uD64?t=28m25s
Reviewed By: mdvacca
Differential Revision: D9753301
fbshipit-source-id: 9629aa485a5d6e51806cc96306c297284d4f90b8
Summary: Unused loads hurt readability and take time to process.
Reviewed By: hramos
Differential Revision: D9494120
fbshipit-source-id: 455b56efadab1cb976344cffcb427772bfda2f71
Summary:
@public
Now, one of the main purposes of `EventEmitter`s is to create RawEvent instances for all specific event invocations.
Reviewed By: mdvacca
Differential Revision: D8886226
fbshipit-source-id: 82a489174efcda097887e70650a2038dc986d149
Summary:
@public
Now, it's not just an abstract class, it's a regular class which unifies event delivery priorities using specific event beats and event pipe.
Reviewed By: mdvacca
Differential Revision: D8886232
fbshipit-source-id: c4360511e5fd477ca7407fc3ebbd99ca578e79cc
Summary:
@public
Instead of having two methods it's easier to have just one which can be abstracted as `EventPipe`.
Reviewed By: mdvacca
Differential Revision: D8886231
fbshipit-source-id: af9fd92dc4afa1219a11acce0aa021a85c94d232
Summary:
@public
EventQueue is a queue of events that synchronizing event dispatching with given Event Beat.
The only difference between UnbatchedEventQueue and BatchedEventQueue is that UnbatchedEventQueue `induce` an Event Beat right after enqueing an event.
Reviewed By: mdvacca
Differential Revision: D8886225
fbshipit-source-id: fedba6fdff2ecb6f3c615cea09b5fdaa58890479
Summary:
@public
EventBeat is an abstraction around proper event scheduling combining proper timing and proper threading. Event Queues use Event Beat to ensure that events are delivered on proper threads and in proper timing (probably batched). Consumers can `request` the next beat and `induce` immediatly beat.
MainRunLoopEventBeat implements particular Event Beat synchronized with the main event loop and calling a callback on the main thread.
Reviewed By: mdvacca
Differential Revision: D8886229
fbshipit-source-id: 1a42fcbf4cd61c6cb4c502890566c98b00226f31
Summary:
@public
We need that because gonna add much more event-related stuff, so it deserves separate buck target.
Reviewed By: mdvacca
Differential Revision: D8831547
fbshipit-source-id: 616581b39b425a49302d5f7f86267e62b0d58389
Valentin Shergin
Taras Tsugrii
David Vacca
Lucian Grijincu
Spencer Ahrens
Scott Rice
Héctor Ramos
Jonathan Kim