Commit Graph

101 Commits

Author SHA1 Message Date
Valentin Shergin 10181f31bd Fabric: Fixed possible crash due race condition during Surface unmounting
Summary:
@public
Now we simply skip `uiManagerDidFinishTransaction` calls if they refer to unregister surfaces. In the future, after we have proper asynchronous scheduling and sync unmounting (and if we chose to have sync unmounting), we can avoid this situation (and assert in this cases).

Reviewed By: sahrens

Differential Revision: D9652731

fbshipit-source-id: e376ea1ae4f93960a903e6397d843bd7c4b72400
2018-09-07 23:48:02 -07:00
Valentin Shergin f341541899 Fabric: Simplified `RootShadowNode` instantiation
Summary:
@public
Trivial.
`children = ShadowNode::emptySharedShadowNodeSharedList()` is already a default.

Reviewed By: rsnara

Differential Revision: D9650218

fbshipit-source-id: ad5c17776866b1dc800f68b0a54b9ac3f70de3f6
2018-09-07 21:46:59 -07:00
Valentin Shergin 1183d82884 Fabric: Removing `ShadowNode::operator==`
Summary:
@public
We don't need this anymore.
The same functionality is now implemented as `ShadowView::operator==` in much more reasonable way.

Reviewed By: sahrens

Differential Revision: D9649821

fbshipit-source-id: 8cd5f3cb4f583fd10d2d1e060aba914541341b5b
2018-09-07 21:46:59 -07:00
Valentin Shergin a3954ed5cb Fabric: Virtual destructors for all abstract interface-like classes
Summary:
@public
Apparently, it's how it should be.

Reviewed By: rsnara

Differential Revision: D9631870

fbshipit-source-id: 46f58270104d699fbc9abe21062c12f791460536
2018-09-07 11:17:52 -07:00
Valentin Shergin 294d91b30a Fabric: ShadowTree is now stored as unique_ptr instead of shared_ptr
Summary:
@public
Now it's clear that we don't need to store/handle ShadowTree objects as `shared_ptr`s; Scheduler should own it. This diff changes that to using unique_ptr and removes a base class of ShadowTree.

Reviewed By: mdvacca

Differential Revision: D9403567

fbshipit-source-id: 6e411714b632a04233fd5b25c8ab7cdd260105fd
2018-09-03 23:04:20 -07:00
Valentin Shergin 1068da2ec7 Introducing `LayoutableShadowNode::isLayoutOnly` and (theoretical) view-flattening
Summary:
@public
Voalá, this small change actually implements view flattening. Obviously, it does not work right now because there are no `ShadowNode` classes which implement `isLayoutOnly`.
Surprisingly, correct implementing of `isLayoutOnly` is quite tricky, we will work on this in coming diffs.

Reviewed By: mdvacca

Differential Revision: D9403565

fbshipit-source-id: 1f16f912cb5c6841405a1fc3cf36aec28698c11f
2018-09-03 23:04:20 -07:00
Valentin Shergin 0792fba63f Fabric: Using ShadowView instead of ShadowNode in Mutations
Summary:
@public
This is quite a big diff but the actual meaningful change is simple: now we use ShadowView class instead of ShadowNode in mutation instructions.
Note:
 * In some places (especially during diffing) we have to operate with ShadowNodeViewPair objects (which represents a pair of ShadowNode and ShadowView). The reason for that is that we cannot construct child ShadowViews from parent ShadowViews because they don't have any information about children.
 * `ShadowTree::emitLayoutEvents` is now much simpler because ShadowView better represents the specifics of this kind of object.
 * The code in RCTMountingManager also became simpler.

This change will allow us to implement more cool tricks soon.

Reviewed By: mdvacca

Differential Revision: D9403564

fbshipit-source-id: dbc7c61af250144d6c7335a01dc30df0005559a2
2018-09-03 23:04:20 -07:00
Valentin Shergin 5c83855c75 Fabric: Introducting `ShadowView` and `ShadowViewMutation`
Summary:
@public
We need some another object like ShadowNode (but not ShadowNode) to represent an instance of the component in the mutation instructions. This is
the main motivation for introducing ShadowView.

Why not use ShadowNode? ShadowNode is designed to represent a node in ShadowTree, not be a part of a mutation instruction.
 * ShadowNode exposes some APIs that should not be exposed to the mounting layer;
 * ShadowNode is an immutable data structure, so we cannot mutate it in some way which can be meaningful for mounting;
 * We should not add to ShadowNode any functionality which is needed only for mounting;
 * ShadowNode is a bit more heavy object to share that it needs to be; it's exposed (embedded into Mutation) as a `shared_ptr` which is not optimal from the performance perspective;
 * Retaining ShadowNode from mounting code can unnecessarily extend its lifetime which can negatively affect memory usage.

Reviewed By: mdvacca

Differential Revision: D9403562

fbshipit-source-id: 72ad81ed918157a62cd3d1a03261f14447649d0b
2018-09-03 23:04:20 -07:00
Jonathan Kim 087e2a89fc Clean up xplat relative loads
Reviewed By: scottrice

Differential Revision: D9584163

fbshipit-source-id: 4793b7fa6151c2ec2f8c7fae6271635c9844a50a
2018-08-30 13:04:50 -07:00
Jonathan Kim 2515e4861a Move RN's DEFS.bzl to tools and rename to rn_defs.bzl
Reviewed By: mzlee

Differential Revision: D9553765

fbshipit-source-id: cb65081668ea2726f24d2c9c02661e859cc7a994
2018-08-29 13:21:52 -07:00
David Vacca 575f7d478d Implement HorizontalScrollView component
Summary: This diff implements the HorizontalScrollView component for Android Fabric C++, as part of this diff I also re-named the components AndroidHorizontalScrollContentView for RCTAndroidHorizontalScrollContentView and AndroidHorizontalScrollView for RCTAndroidHorizontalScrollView. This might sound against our plan of removing the RCT preffix, but it is to make it simpler to map components between current implementation of RN and Fabric (otherwise we don't know when to add the RCT preffix in Android side to find the right View Manager), later we can just remove the preffix from C++, Android, iOS and JS.

Reviewed By: shergin, achen1

Differential Revision: D9122729

fbshipit-source-id: e9299552857c6dd0c18abfa5fa49a3d50e221729
2018-08-28 23:03:33 -07:00
Valentin Shergin 961b6aceca Fabric: Unified event pipeline: connecting the dots
Summary:
@public
This diff basically wires everything up.

Reviewed By: mdvacca

Differential Revision: D8886227

fbshipit-source-id: fb1a1e3222b3d693a8c28ed780b14f7315b7c019
2018-08-27 07:32:38 -07:00
Valentin Shergin 0dae893eec Fabric: Rethinking of `EventDispatcher`
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
2018-08-27 07:32:37 -07:00
Valentin Shergin 71295bcdac Fabric: Enabling and disabling EventEmitter at post commit phase
Summary:
@public
We need that to ensure that we will not deliver events to nodes with invalid state.

Reviewed By: mdvacca

Differential Revision: D8886234

fbshipit-source-id: 1d6ca129c97a5dca0411e85909aea48185f46c54
2018-08-27 07:32:36 -07:00
Valentin Shergin b784adc7ae Fabric: `FabricUIManager::dispatchEventToEmptyTarget` got coupled with `dispatchEventToTarget`
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
2018-08-27 07:32:35 -07:00
David Vacca b8a50c7614 Improve error message when a component descriptor is not implemented
Summary: This diff improves the error message that is displayed when a component descriptor is not implemented in C++

Reviewed By: shergin

Differential Revision: D9093562

fbshipit-source-id: 930b381bc66c20af6fa160b09e7484bad4666e28
2018-08-16 16:52:42 -07:00
Valentin Shergin 3770d4df45 Fabric: Using `const &` type for `ShadowNodeFragment`'s fields
Summary:
@public
To avoid unnecessary copying of `shared_ptr`s inside ShadowNodeFragment, now we store them as `const &` references.

Reviewed By: mdvacca

Differential Revision: D8988388

fbshipit-source-id: 0b3582e57ce7577b8fa819392bf33f34e1a60b59
2018-08-04 09:47:31 -07:00
Valentin Shergin 06e62440d3 Fabric: Using `ShadowNodeFragment` in `ComponentDescriptor`
Summary:
@public
Now we use same data structure to specify a shape of shadow node as we use in ShadowNode (sub)clases.

Reviewed By: mdvacca

Differential Revision: D8988387

fbshipit-source-id: 475298b2c71ee7ee2b197db009f7b8313b54f5df
2018-08-04 09:47:30 -07:00
Valentin Shergin 52ed882332 Fabric: Using `const ShadowNode &` as a parameter in ShadowNode copy constructor
Summary:
@public
When we copy-construct ShadowNode, we don't need to retain a source shadow node, so there is no need to pass it as a `shared_ptr`. Passing an argument to constructor as `const &` is also more idiomatic in C++.

Reviewed By: mdvacca

Differential Revision: D8988384

fbshipit-source-id: 1279d9185fa1b4b82fd26e3040bd62fa9495b4d3
2018-08-04 09:47:30 -07:00
Valentin Shergin d74346b616 Fabric: `ShadowNode::getChildren()` now returns `vector`, not `shared_ptr`
Summary: TBD

Reviewed By: mdvacca

Differential Revision: D8988385

fbshipit-source-id: 1d1c7e0b87b32b242c69bbce44cf70fb0899cf93
2018-08-04 09:47:30 -07:00
Valentin Shergin 95074e6c12 Fabric: ShadowNode::Fragment
Summary:
@public
This diff changes a way how we specify a shape of newly created and/or cloned of ShadowNode. Previously we pass those values as a list of arguments, now those values are coupled into a new data structure called ShadowNodeFragment. All that makes suppose to make code much more easy to read and maintain, this is especially important because we want to add a couple of new entities in this set.

Reviewed By: mdvacca

Differential Revision: D8988389

fbshipit-source-id: 1835f646e1ecc6a1f413feaf1900f3d3ad0ebc05
2018-08-04 09:47:30 -07:00
Valentin Shergin 9395485822 Fabric: ContextContainer is now able to store any copyable values
Summary:
@public
Previously, ContextContainer could store only `shared_ptr`s, but now it wraps all values in own `shared_ptr` container.
I wish we can use `unique_ptr` here, but apparently we cannot because `unique_ptr` does not support type-erasure (`std::unique_ptr<void>` is illigal).
Becasue ContextContainer is not supposed to be used in hot paths, the performance aspect of that does not actually matter.

Reviewed By: mdvacca

Differential Revision: D8853446

fbshipit-source-id: e5d0a5595fe44c59f1395d6ffccf9d3fed923c83
2018-07-17 22:53:57 -07:00
Valentin Shergin 07a4a959a7 Fabric: Events related classes were moved to separate buck target
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
2018-07-17 22:53:57 -07:00
Valentin Shergin 58da98149d Fabric: Support for optional `key` parameter to register/retrieve in ContextContainer
Summary:
@public
We need this in case when we want to store several intances of the same class in the container.

Reviewed By: mdvacca

Differential Revision: D8814808

fbshipit-source-id: 78ab15d78cf3878d03bf0a45bc42b968d87435e7
2018-07-17 22:53:56 -07:00
Valentin Shergin e9e20e6c83 Fabric: <Root> component was decoupled from <View>
Summary:
@public
There is no reason to have it inside View; it deserves that.

Reviewed By: mdvacca

Differential Revision: D8757012

fbshipit-source-id: 881b54008b51614cd203ab97811494fa7c30e4ef
2018-07-15 16:52:26 -07:00
Valentin Shergin 57b0e68a2c Fabric: `view` module was moved to `components` subdirectory
Summary:
@public
Trivial. We move all components into `/components/` subdirectory.

Reviewed By: mdvacca

Differential Revision: D8757011

fbshipit-source-id: 6a7da09e01184d41d37a1e1782c20d3c79371ae3
2018-07-15 16:52:26 -07:00
Valentin Shergin b42e674c2f Fabric: `scrollview` module was moved to `components` subdirectory
Summary:
@public
Trivial. We move all components into `/components/` subdirectory.

Reviewed By: mdvacca

Differential Revision: D8757013

fbshipit-source-id: fe3021862b3a4f8f0799b0dfaf6d3039f8582a7f
2018-07-15 16:52:26 -07:00
Valentin Shergin ecbe9acbaa Fabric: `text` module was moved to `components` subdirectory
Summary:
@public
Trivial. We move all components into `/components/` subdirectory.

Reviewed By: mdvacca

Differential Revision: D8757014

fbshipit-source-id: 9db94d38fe027e9125d017a17cbd4cf79f0bcf88
2018-07-15 16:52:26 -07:00
Valentin Shergin 0532e01d69 Fabric: Enhancements in ContextContainer
Summary:
@public
Everything is better with C++ templates.
In this cases templates allow us to remove additional parameters and casts on the callsite.

Reviewed By: mdvacca

Differential Revision: D8754523

fbshipit-source-id: 2340b2cd96ab0a60d54d9aa30dea3c072b951a8a
2018-07-15 16:52:26 -07:00
Valentin Shergin e78bf723bf Fabric: Removed last two plactical usages of `ShadowNode::sourceNode_`
Summary:
@public
 * In case of `ShadowTree` we just pass original old node as a `commit` method argument;
 * In case of `ConcreteViewShadowNode` we just don't need that because diffing algorithm does not use that information anymore.

Reviewed By: mdvacca

Differential Revision: D8753906

fbshipit-source-id: b8555083c7e72e9b3c0f9a8065745946d4cf44c7
2018-07-15 16:52:25 -07:00
Valentin Shergin fcd72bf34a Fabric: Generalizing cloning of YogaLayoutable approach
Summary:
@public
Non-null owner pointer in Yoga node indicates that this node is already being used by some other subtree, so it must be cloned in case of possible (re)layout.
Theoretically, this node must/can be cloned by Yoga right before applying a new layout to this node, but Yoga has a special optimization that uses that fact that Yoga always cloning *all* children of a particular node altogether. This is not true for React; to meet React and Yoga worlds we double check the owner pointer in `addChild` and clone node preliminary if needed.
See also the previous diff for more context.

Reviewed By: mdvacca

Differential Revision: D8709952

fbshipit-source-id: 84ef0faa0f1d9cc9a8136b550cf325bc20508d53
2018-07-15 16:52:25 -07:00
Taras Tsugrii 12d98db901 Fix conditional load usage.
Summary:
Conditional `load` statements are not allowed in new Buck build file parser - Skylark.
https://buckbuild.com/concept/skylark.html

Reviewed By: mzlee

Differential Revision: D8842756

fbshipit-source-id: f22dff00f594978e4cab5736268ad3225182c39b
2018-07-14 18:32:20 -07:00
Valentin Shergin e8ec1cb16a Fabric: The diffing algorithm does not use source nodes anymore
Summary:
@public
... and it's as efficient as it was before.

The previous version of the algorithm used `sourceNode` reference to know the previous state of the node to call the algorithm recursively.
That wasn't so good because of several reasons:
 - It was fragile because we had two different sources of the truth of the "previous state of the tree": committed tree and source node pointer;
 - We had to store weak pointers to source nodes inside cloned nodes. That is not free in terms of performance;
 - The old approach introduced a constraint that all previously used and now reinserted nodes must be cloned to update source node (otherwise, the algorithm would regenerate instructions recreating already existing subtrees);
 - That cloning required access to `isSealed` flag which is supposed to be a debug-only thing (that actually affects performance and must be compile-out for release builds).

The new approach compares nodes with same react tag and naturally cloning-artifacts resilient.

Yes, the new approach uses a map of inserted nodes, but the previous one already had it (otherwise there is no way to tell which nodes should be "deleted"). And anyway, this is a very little map that exists for a very little period of time.

Reviewed By: mdvacca

Differential Revision: D8709953

fbshipit-source-id: 027abb326cf45f00f7bb0bbd7c4e612578268c66
2018-07-06 14:49:07 -07:00
Valentin Shergin e0e9c1549e Fabric: Improved prettyprinting of TreeMutationInstruction
Summary:
@public
Quite trivial... and nice.

Reviewed By: mdvacca

Differential Revision: D8709951

fbshipit-source-id: 63e53eb85361fe3a0a0ecd7f21bf4c7db049d5bf
2018-07-06 14:49:06 -07:00
David Vacca 6292e2707a Revert order of Remove Mount Item operations
Summary: Revert the order of "remove mount items", to ensure views are removed from high index to low index.

Reviewed By: shergin

Differential Revision: D8742796

fbshipit-source-id: 6e04c39386d290bf3958ee83256d4fbe23e2c4ca
2018-07-06 09:17:34 -07:00
Kevin Gozali a09c464585 iOS: avoid crash because of null eventTarget
Summary: We were supposed to pass in proper eventEmitter, but passed in one with null eventTarget instead, causing assertion failures when dispatching event.

Reviewed By: sebmarkbage, shergin

Differential Revision: D8720793

fbshipit-source-id: 891f3b2a2c76a6dd3e40039623c6e86991aad50b
2018-07-02 18:02:35 -07:00
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 62f9ced099 Fabric: Subtle changes that make GCC compiler happy
Summary:
@public
Most of them are legit issues which should not be compilable anyways (but Clang tolerates thems).

Reviewed By: mdvacca

Differential Revision: D8655539

fbshipit-source-id: 645729fb9d6a120ce1ab2b07542abcdacd72320d
2018-06-29 12:18:27 -07:00
Valentin Shergin 712c2ed5d2 Fabric: Getting rid of `std::to_string()`
Summary:
@public
Suddenly, it is not supported on Android.
Luckelly `folly:to<std::string>()` is as good as `std::to_string()`.

Reviewed By: mdvacca

Differential Revision: D8655538

fbshipit-source-id: 2b3b970f6a261253aaa6b22dba8338dc66b7195d
2018-06-29 12:18:27 -07:00
Kevin Gozali 1dced3448a iOS: implement <PerformanceLoggerFlag> component
Summary: This is basic impl of <PerformanceLoggerFlag> component without any layout/mounting computation, just TTI.

Reviewed By: shergin, mdvacca

Differential Revision: D8598983

fbshipit-source-id: b938753d6396088735cbbeab26d69c9aaa45608e
2018-06-24 14:17:37 -07:00
Taras Tsugrii 346ac75ed6 Fix deprecated glob usage.
Summary: https://our.intern.facebook.com/intern/wiki/Buck/python-to-skylark/

Differential Revision: D8595731

fbshipit-source-id: 0e3046a7fd2a25e9b13462713ae9a008ad546770
2018-06-23 18:33:48 -07:00
Valentin Shergin 6942408a47 Fabric: Dispatching `onLayout` events to only nodes which requested it
Summary:
@public
The current Fabric architecture, in general, does not support "subscribing" for events, so all kinds of events are always delivered no matter have JavaScript components `on`-handlers for them or not.
At this point, we are not sure should it be this way or not. But we are sure that for some extremely noisy events (like onLayout) we have to make an exception right now (otherwise overall performance will suffer).
So, this diff implements that for `onLayout`.

Reviewed By: fkgozali

Differential Revision: D8597408

fbshipit-source-id: 6933b7cb96e24f0660bd7850b625ff27e3146a2b
2018-06-22 18:46:39 -07:00
Valentin Shergin 250cc3c594 Fabric: Fixed order of instructions in Differentiator
Summary:
@public
Previously Differentiator might generate some `remove` instruction that refers to already `deleted` node. That diffs fixes that.

Reviewed By: fkgozali

Differential Revision: D8596536

fbshipit-source-id: 88117962f93e52167dbcb6525f2cc36758a367e7
2018-06-22 15:34:47 -07:00
Valentin Shergin 483c45cff0 Fabric: Shadow node shallowing was removed from preventive cloning in UIManager
Summary:
@public
We do preventing cloning in UIManager especially to add a layer to Shadow Node source chain,
so apparently there is no point illuminate that by calling `shallowSourceNode`.

Reviewed By: fkgozali

Differential Revision: D8585163

fbshipit-source-id: 3743edc30bf2183c420fd79ce1e59d68ceaa278b
2018-06-22 11:57:41 -07:00
Valentin Shergin af75d93dad Fabric: Bunch of unimplemented yet component was aliased to <View>
Summary:
@public
We need this temporary for testing until we support them all.

Reviewed By: mdvacca

Differential Revision: D8552361

fbshipit-source-id: 25f48cebcf5a665a24b92803dd7738f947ca74b2
2018-06-22 11:57:41 -07:00
Valentin Shergin b09457b4d2 Fabric: <Image> component, xplat part
Summary:
@public
This diff implements basics of cross-platform part of <Image> component.
Known issues:
- Events does not work yet.
- Some quite specific image source parameters (like custom http headers) are not supported yet.

Reviewed By: fkgozali

Differential Revision: D8526575

fbshipit-source-id: ecc97d9fda2b2e65bb1b079af057f8e176a161e5
2018-06-22 07:32:50 -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
Valentin Shergin c674303dfd Fabric: Introducing `ContextContainer`
Summary:
@public
`ContextContainer` is general purpose DI container for Fabric.
We need this to communicate some enviroment-specific and/or platform-specific modules down to cross-platform C++ code.
The first one will be ImageManager. Soon.

Reviewed By: fkgozali

Differential Revision: D8475636

fbshipit-source-id: 0afc65063f818d0bab736cd2c55c6fdd21b629ac
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 cb19621dfe Implementation of JS reload without crashing
Summary:
On JS reload the FabricUIManager and EventDispatcher didn't get release due to a retain cycle. This breaks the cycle.

In addition, force release the Scheduler on reload so that the stale classes get cleaned up properly, avoiding crashes. Also the surface now remounts the content correctly

Reviewed By: shergin

Differential Revision: D8414916

fbshipit-source-id: 4b14031f29b3bc9987d7aa765dc0d930a7de2b1e
2018-06-15 11:02:17 -07:00