Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[pull] master from microsoft:master #1

Merged
merged 5 commits into from
Apr 2, 2021
Merged

Conversation

pull[bot]
Copy link

@pull pull bot commented Mar 22, 2021

See Commits and Changes for more details.


Created by pull[bot]

Can you help keep this open source service alive? 💖 Please sponsor : )

appden and others added 5 commits March 1, 2021 13:37
This fixes a regression from when we upgraded to 0.63 where we could no longer call `focus()` or `blur()` on any `View` (including `Text`). This regression was due to a change that dispatches these as commands (to be compatible with Fabric) instead of directly using `UIManager` to change focus. Only the `TextInput` supported these commands.
…ect (#743)

* Update RCTCxxBridge.mm

* Update RCTCxxBridge.mm

* protect against race conditions

* use built in main queue execution
* Update RCTCxxBridge.mm

* make CI xcode versioning more robust to xcode12

* update to xcode 12.4

* use apple variables file

* Update RCTCxxBridge.mm

* add quotes

* switch to catalina

* update pods to 10.15

* make CI xcode versioning more robust to xcode12

* update to xcode 12.4

* use apple variables file

* add quotes

* switch to catalina

* update pods to 10.15

* upgrade version requirements to n-2 for osx

* make CI xcode versioning more robust to xcode12

* update to xcode 12.4

* use apple variables file

* add quotes

* switch to catalina

* make CI xcode versioning more robust to xcode12

* update to xcode 12.4

* use apple variables file

* add quotes

* make CI xcode versioning more robust to xcode12

* update to xcode 12.4

* use apple variables file

* add quotes

* CI should run on PRs to future stable branches (#718)

* Pull in header's C declaration to stop symbol mangling  (#728)

* Update RCTCxxBridge.mm

* Update RCTCxxBridge.mm

* fix undefined extern c symbol

* scroll content with animation (#725)

Summary: please see test plan

Test Plan:
|Before|After|
|{F371503806}|{F371499708}|



Reviewers: skyle

Reviewed By: skyle

Subscribers: zackargyle

Differential Revision: https://phabricator.intern.facebook.com/D26354172

Tasks: T84165504

Signature: 26354172:1612920735:2cd8455b1bae06ee555bd98cfd41c4dfb29c288e

Co-authored-by: Mo Wang <mowang@fb.com>

* Fix missing props on secure text input (#719)

* Enable animated gif playback on Mac (#724)

* enable gif for mac

Summary:
I verify that it works for Zeratul as well. 

It also works regardless turbo module is enabled or not.

Test Plan: 
|{F369886201}|{F369889196}|



Reviewers: skyle

Reviewed By: skyle

Subscribers: zackargyle

Differential Revision: https://phabricator.intern.facebook.com/D26272145

Tasks: T82742678

Signature: 26272145:1612513052:520a8b0b3ab4b211c953b3225c6c96ffb8a29fc5

* bypass setImage logic when it is Mac

Summary: as titled

Test Plan:

{F370138978}

{F370139033}


Reviewers: skyle

Reviewed By: skyle

Subscribers: zackargyle

Differential Revision: https://phabricator.intern.facebook.com/D26288169

Signature: 26288169:1612565769:8f779fe01614e3399ac3e484853bb61246210ff4

* address PR comments

* address PR comments 1

Co-authored-by: Mo Wang <mowang@fb.com>

* Add explicit Tab support to keyboarding (#723)

* Update RCTCxxBridge.mm

* Update RCTCxxBridge.mm

* add explicit tab support

* missing tab validity check

* Fix Deadlock in RCTi18nUtil (iOS) (#733)

* Initial Commit

* Fix typo

* Fix default initialization of isRTLAllowed and documentation

* use BOOL, remove superfluous readwrite decorator

* Fix another typo

* Add TODO Marker

* update to macOS 10.15

* make CI xcode versioning more robust to xcode12

* update to xcode 12.4

* use apple variables file

* add quotes

* switch to catalina

* update pods to 10.15

* upgrade version requirements to n-2 for osx

* make CI xcode versioning more robust to xcode12

* update to xcode 12.4

* use apple variables file

* add quotes

* switch to catalina

* make CI xcode versioning more robust to xcode12

* update to xcode 12.4

* use apple variables file

* add quotes

* make CI xcode versioning more robust to xcode12

* update to xcode 12.4

* use apple variables file

* add quotes

* update to macOS 10.15

* update to xcode 12.4

* use apple variables file

* switch to catalina

* 10.14

* CI has to run on 10.15.4 or later

* fix up pod errors

* upgrade iphone simulator

* iphone 13

* remove old post install script

* iphone 8

* Restore Podfile.lock to see if it fixes CI

* Restore compiler flags in TurboModuleCxx-WinRTPort

* Remove macOS arm64 build divergence

* Remove building all ARCHS temporarily

* Override ONLY_ACTIVE_ARCH in release builds too

* Push iOS 14 snapshots

Co-authored-by: Andrew Coates <30809111+acoates-ms@users.noreply.github.com>
Co-authored-by: Mo <mo.hy.wang@gmail.com>
Co-authored-by: Mo Wang <mowang@fb.com>
Co-authored-by: Scott Kyle <scott@appden.com>
Co-authored-by: Saad Najmi <saadnajmi2@gmail.com>
Co-authored-by: Anand Rajeswaran <anand.rajeswaran@outlook.com>
* Add high contrast API to AccessibilityInfo

* Make RCTAccessibilityManager require main queue setup

* Remove unnecessary removeObserver: call

* Add comment for macOS "high contrast" = "increase contrast"
* add pull yml

* remove yml

* Getting there..

* Closer..

* Remove commented out code

* More simplification

* remove typo

* Make ColorWithSystemEffect it's own sub-object

* It works!

* Add examples of DynamicColorMacOS with a SystemEffect (currently broken)

* Add None Effect too

* Fix typo

* Essentially feature complete

* Rename API

* Missed a file

* Fix the bug where we call processColor too many times

* Replace blah with the issue number

* Change one more title

* Stub out test on other platforms

* Add ifdef macos check, some null checks, and fix up a bunch of the tags

* Fix more of the tags

* Fix lint errors

* ifdef around colorWithEffect method
@Saadnajmi Saadnajmi merged commit ab88c7d into Saadnajmi:master Apr 2, 2021
Saadnajmi pushed a commit that referenced this pull request Sep 3, 2021
Summary:
While in theory we should never delete views before removing them from the hierarchy, there are some exceptions:

(1) Some mysterious cases that don't seem like bugs, but where the child still seems to keep a reference to the parent:
(2) When deleting views as part of stopSurface.

On #1: in the past we had issues when we assumed that ViewManager.getChildCount() would return an accurate count. Sometimes it's just... wrong. Here, I've found at least one case where a View still has a parent after it's removed from the View hierarchy. I assume this is undocumented Android behavior or an Android bug, but either way, there's nothing I can do about it.

On #2: there are valid cases where we want to delete a View without explicitly removing it from the View hierarchy (it will eventually be removed from the hierarchy when the Root view is unmounted, but it may still be "in" a View hierarchy when it's deleted).

Changelog: [Internal]

Reviewed By: mdvacca

Differential Revision: D22321374

fbshipit-source-id: 9667bbe778c418f0216550638dc26ca48a58e5fa
Saadnajmi pushed a commit that referenced this pull request Oct 21, 2021
Summary:
changelog: [internal]

Prevents 2 type converions:
1. int <-> size_t
2. int <-> int32_t

# Why is using size_t better when working with indexes.

## 1. Type conversion isn't for free.

Take this example

```
size_t calculate(int number) {
  return number + 1;
}
```

It generates following assembly (generated with armv8-a clang 10.0.0):

```
calculate(int):                          // calculate(int)
sub     sp, sp, #16                     // =16
str     w0, [sp, #12]
ldr     w8, [sp, #12]
add     w9, w8, #1                      // =1
mov     w8, w9
sxtw    x0, w8
add     sp, sp, #16                     // =16
ret
```

That's 9 instructions.

If we get rid of type conversion:

```
size_t calculate(size_t number) {
  return number + 1;
}
```

Assembly (generated with armv8-a clang 10.0.0):

```
calculate(unsigned long):                          // calculate(unsigned long)
sub     sp, sp, #16             // =16
str     x0, [sp, #8]
ldr     x8, [sp, #8]
add     x0, x8, #1              // =1
add     sp, sp, #16             // =16
ret
```

Compiler now produces only 7 instructions.

## Semantics

When using int for indexing, the type doesn't say much. By using `size_t`, just by looking at the type, it gives the reader more information about where it is coming from.

Reviewed By: JoshuaGross

Differential Revision: D24332248

fbshipit-source-id: 87ef982829ec14906ed9e002ea2e875fda4a0cd8
@Saadnajmi Saadnajmi added this to the Stage 0 (Dummy milestone) milestone Dec 1, 2022
Saadnajmi pushed a commit that referenced this pull request Jan 5, 2023
Merge upstream into fork
Saadnajmi pushed a commit that referenced this pull request Dec 29, 2023
Summary:
Pull Request resolved: facebook#41466

## Context
In open source, all apps use the same turbomodulemanager delegate (i.e: the default delegate).

This diff introduces the buck infra that makes the oss default delegate work for meta apps.

Concretely, we are going to make React Native use the same  delegate for **all** Meta apps.

Each Meta app will:
1. At build time, generate a unique TMProvider map
2. At app init time, initialize the default delegate with the TMProvider map.

## Implementation
**Step #1:** At build time, generate a unique TMProvider map

**Insight:** Buck genrules can accept, as input, the output of a buck query.

So, here's how we get this done:
1. Buck query (i.e: input to Genrule): Given the app's deps, query all the schemas in the app.
2. Genrule: Read the schemas to generate the TMProvider map. The TMProvider map will also contain **all** the app's C++ module codegen.

Concretely:
1. This diff introduces a macro: rn_codegen_appmodules(deps).
2. rn_codegen_appmodules(deps) generates appmodules.so, which contains the TMProvider map.

**Step #2:** At app init time, initialize the default delegate with the TMProvider map.

This is how we'll initialize the DefaultTurboModuleManagerDelegate:
1. DefaultTurboModuleManagerDelegate will load appmodules.so during init.
2. When loaded, appmodules.so will assign the code-generated TMProvider map to DefaultTurboModuleManagerDelegate.

## Impact
This should allow us to:
1. Get one step closer to getting rid of the `js1 build turbomodule-manager-delegates --target <app>` script
3. Remove the TurboModuleManagerDelegate from React Native's public API. (Because we use one delegate for all React Native apps in Meta and OSS)

Changelog: [Internal]

Reviewed By: mdvacca

Differential Revision: D50988397

fbshipit-source-id: 0ca5dec14e2dae89ec97f5d39a182c7937c5c7bf
Saadnajmi pushed a commit that referenced this pull request Feb 14, 2024
…gets

Summary:
Changelog: [Internal]

# This diff

1. Provides all Targets with an `executorFromThis()` method, which can be used from within a Target to access a *`this`-scoped main thread executor* = a `std::function` that will execute a callback asynchronously iff the current Target isn't destroyed first.
2. Refactors how (all) Target objects are constructed and retained, from a plain constructor to `static shared_ptr create()`. This is because `executorFromThis()` relies internally on `enable_shared_from_this` plus two-phase construction to populate the executor.
3. Creates utilities for deriving scoped executors from other executors and `shared_ptr`s.

The concept is very much like `RuntimeExecutor` in reverse: the #1 use case is moving from the JS thread back to the main thread - where "main thread" is defined loosely as "anywhere it's legal to call methods on Target/Agent objects, access session state, etc". The actual dispatching mechanism is left up to the owner of each `PageTarget` object; for now we only have an iOS integration, where we use `RCTExecuteOnMainQueue`.

Coupling the ownership/lifetime semantics with task scheduling is helpful, because it avoids the footgun of accidentally/nondeterministically moving `shared_ptr`s (and destructors!) to a different thread/queue .

# This stack
I'm refactoring the way the Runtime concept works in the modern CDP backend to bring it in line with the Page/Instance concepts.

Overall, this will let us:

* Integrate with engines that require us to instantiate a shared Target-like object (e.g. Hermes AsyncDebuggingAPI) in addition to an per-session Agent-like object.
* Access JSI in a CDP context (both at target setup/teardown time and during a CDP session) to implement our own engine-agnostic functionality (`console` interception, `Runtime.addBinding`, etc).
* Manage CDP execution contexts natively in RN, and (down the line) enable first-class debugging support for multiple Runtimes in an Instance.

The core diffs in this stack:

* ~~Introduce a `RuntimeTarget` class similar to `{Page,Instance}Target`. ~~
* ~~Make runtime registration explicit (`InstanceTarget::registerRuntime` similar to `PageTarget::registerInstance`). ~~
* ~~Rename the existing `RuntimeAgent` interface to `RuntimeAgentDelegate`.~~
* ~~Create a new concrete `RuntimeAgent` class similar to `{Page,Instance}Agent`.~~
* ~~Provide `RuntimeTarget` and `RuntimeAgent` with primitives for safe JSI access, namely a `RuntimeExecutor` for scheduling work on the JS thread.~~
  * Provide RuntimeTarget with mechanism for scheduling work on the "main" thread from the JS thread, for when we need to do more than just send a CDP message (which we can already do with the thread-safe `FrontendChannel`) in response to a JS event. *← This diff*

## Architecture diagrams

Before this stack:
https://pxl.cl/4h7m0

After this stack:
https://pxl.cl/4h7m7

Reviewed By: hoxyq

Differential Revision: D53356953

fbshipit-source-id: 152c784eb64e9b217fc2966743b33f61bd8fd97e
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants