Skip to content

For Windows Desktop, pass close code and reason to WebSocket.close() #16

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.

Sign up for GitHub

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

Jump to bottom
Merged
1 commit merged into from
Mar 22, 2019
Merged

For Windows Desktop, pass close code and reason to WebSocket.close() #16

1 commit merged into from
Mar 22, 2019

Conversation

JunielKatarn
Copy link

@JunielKatarn JunielKatarn commented Mar 22, 2019 •
edited by ghost
Loading

Please select one of the following

  • I am removing an existing difference between facebook/react-native and Microsoft/react-native 👍
  • I am cherry-picking a change from Facebook's react-native into Microsoft/react-native 👍
  • I am making a fix / change for the macOS implementation of react-native
  • I am making a change required for Microsoft usage of react-native

Description of changes

See WebSocket.js, near line 227:

if (Platform.OS === 'android') {
  WebSocketModule.close(statusCode, closeReason, this._socketId);

Currently, only Android ReactNative instances pass closeReason and statusCode.
For non-Android platforms, default values are hard-coded.
Windows Desktop (win32) has no reason to use these default values.

This change ensures those parameters are passed on from the JavaScript side.

Microsoft Reviewers: Open in CodeFlow

@acoates-ms
Copy link
Collaborator

/azp run

@azure-pipelines
Copy link

No pipelines are associated with this pull request.

@acoates-ms
Copy link
Collaborator

Should we open an issue on react-native-windows that the UWP version of websocket should support this stuff too? -- May as well track the issue while we know it exists.

@ghost
Copy link

ghost commented Mar 22, 2019

Hello @acoates-ms!

Because this pull request has the AutoMerge label, I will be glad to assist with helping to merge this pull request once all check-in policies pass.

Do note that I've been instructed to only help merge pull-requests of this repository that have been opened for at least 8 hours, a condition that is not currently met. No worries though, I will be back when the time is right! 😉

@ghost ghost merged commit bffa745 into microsoft:master Mar 22, 2019
t-nanava pushed a commit that referenced this pull request Jun 17, 2019
@JunielKatarn
Use all WebSocket.close args for WIN platforms. (#16)
10b3ab3
NickGerleman pushed a commit to NickGerleman/react-native that referenced this pull request Nov 12, 2020
@sammy-SC @facebook-github-bot
Prevent type conversion in Differentiator
81a97de 
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, microsoft#16                     // =16
str     w0, [sp, microsoft#12]
ldr     w8, [sp, microsoft#12]
add     w9, w8, #1                      // =1
mov     w8, w9
sxtw    x0, w8
add     sp, sp, microsoft#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, microsoft#16             // =16
str     x0, [sp, microsoft#8]
ldr     x8, [sp, microsoft#8]
add     x0, x8, #1              // =1
add     sp, sp, microsoft#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
NickGerleman pushed a commit to NickGerleman/react-native that referenced this pull request May 13, 2025
@sammy-SC @facebook-github-bot
fix crash in view culling where differentiator creates create instruc…
34e9cba 
…tion for existing view (facebook#51294)

Summary:
Pull Request resolved: facebook#51294

changelog: [internal]

Fix a crash where a node that is supposed to be culled doesn't get visited because culling context is not updated.
The differentiator would generate a create instruction for a view that already exists.

Stack trace for the crash:
```
* thread #1, queue = 'com.apple.main-thread', stop reason = signal SIGABRT
  * frame #0: 0x0000000111740874 libsystem_kernel.dylib`__pthread_kill + 8
    frame #1: 0x00000001117aa2ec libsystem_pthread.dylib`pthread_kill + 264
    frame #2: 0x0000000180171ea8 libsystem_c.dylib`abort + 100
    frame microsoft#3: 0x00000001802b0144 libc++abi.dylib`abort_message + 128
    frame microsoft#4: 0x000000018029fe4c libc++abi.dylib`demangling_terminate_handler() + 296
    frame microsoft#5: 0x000000018006f220 libobjc.A.dylib`_objc_terminate() + 124
    frame microsoft#6: 0x00000001375d1964 INFRAFramework`meta_terminate() + 5468
    frame microsoft#7: 0x00000001802af570 libc++abi.dylib`std::__terminate(void (*)()) + 12
    frame microsoft#8: 0x00000001802b2498 libc++abi.dylib`__cxxabiv1::failed_throw(__cxxabiv1::__cxa_exception*) + 32
    frame microsoft#9: 0x00000001802b2478 libc++abi.dylib`__cxa_throw + 88
    frame microsoft#10: 0x0000000180093904 libobjc.A.dylib`objc_exception_throw + 384
    frame microsoft#11: 0x0000000180e6999c Foundation`-[NSAssertionHandler handleFailureInFunction:file:lineNumber:description:] + 268
    frame microsoft#12: 0x000000031a3bcfc8 XPLAT_6_Framework`-[RCTComponentViewRegistry dequeueComponentViewWithComponentHandle:tag:] + 528
    frame microsoft#13: 0x000000031a3ccdec XPLAT_6_Framework`RCTPerformMountInstructions(std::__1::vector<facebook::react::ShadowViewMutation, std::__1::allocator<facebook::react::ShadowViewMutation>> const&, RCTComponentViewRegistry*, RCTMountingTransactionObserverCoordinator&, int) + 356
    frame microsoft#14: 0x000000031a3ccc7c XPLAT_6_Framework`-[RCTMountingManager performTransaction:]::$_1::operator()(facebook::react::MountingTransaction const&, facebook::react::SurfaceTelemetry const&) const + 80
    frame microsoft#15: 0x000000031a3ccc20 XPLAT_6_Framework`decltype(std::declval<-[RCTMountingManager performTransaction:]::$_1&>()(std::declval<facebook::react::MountingTransaction const&>(), std::declval<facebook::react::SurfaceTelemetry const&>())) std::__1::__invoke[abi:ne190102]<-[RCTMountingManager performTransaction:]::$_1&, facebook::react::MountingTransaction const&, facebook::react::SurfaceTelemetry const&>(-[RCTMountingManager performTransaction:]::$_1&, facebook::react::MountingTransaction const&, facebook::react::SurfaceTelemetry const&) + 40
    frame microsoft#16: 0x000000031a3ccbc8 XPLAT_6_Framework`void std::__1::__invoke_void_return_wrapper<void, true>::__call[abi:ne190102]<-[RCTMountingManager performTransaction:]::$_1&, facebook::react::MountingTransaction const&, facebook::react::SurfaceTelemetry const&>(-[RCTMountingManager performTransaction:]::$_1&, facebook::react::MountingTransaction const&, facebook::react::SurfaceTelemetry const&) + 40
    frame microsoft#17: 0x000000031a3ccb94 XPLAT_6_Framework`std::__1::__function::__alloc_func<-[RCTMountingManager performTransaction:]::$_1, std::__1::allocator<-[RCTMountingManager performTransaction:]::$_1>, void (facebook::react::MountingTransaction const&, facebook::react::SurfaceTelemetry const&)>::operator()[abi:ne190102](facebook::react::MountingTransaction const&, facebook::react::SurfaceTelemetry const&) + 44
    frame microsoft#18: 0x000000031a3cba1c XPLAT_6_Framework`std::__1::__function::__func<-[RCTMountingManager performTransaction:]::$_1, std::__1::allocator<-[RCTMountingManager performTransaction:]::$_1>, void (facebook::react::MountingTransaction const&, facebook::react::SurfaceTelemetry const&)>::operator()(facebook::react::MountingTransaction const&, facebook::react::SurfaceTelemetry const&) + 44
    frame microsoft#20: 0x000000032f219804 XPLAT_1_Framework`std::__1::function<void (facebook::react::MountingTransaction const&, facebook::react::SurfaceTelemetry const&)>::operator()(this=0x000000016d4f0c78, __arg=0x000000016d4f0a10, __arg=0x000000016d4f0978) const at function.h:989:10
    frame microsoft#21: 0x000000032f219668 XPLAT_1_Framework`facebook::react::TelemetryController::pullTransaction(this=0x00000003f4680f00, willMount=0x000000016d4f0c98, doMount=0x000000016d4f0c78, didMount=0x000000016d4f0c58) const at TelemetryController.cpp:39:3
    frame microsoft#22: 0x000000031a3c5b28 XPLAT_6_Framework`-[RCTMountingManager performTransaction:] + 544
    frame microsoft#23: 0x000000031a3c5864 XPLAT_6_Framework`-[RCTMountingManager initiateTransaction:] + 456
    frame microsoft#24: 0x000000031a3c5240 XPLAT_6_Framework`__42-[RCTMountingManager scheduleTransaction:]_block_invoke + 308
    frame microsoft#25: 0x0000000131f81b84 BOTTOMFramework`__RCTExecuteOnMainQueue_block_invoke + 40
    frame microsoft#26: 0x000000018017c788 libdispatch.dylib`_dispatch_call_block_and_release + 24
    frame microsoft#27: 0x0000000180197278 libdispatch.dylib`_dispatch_client_callout + 12
    frame microsoft#28: 0x00000001801b2fcc libdispatch.dylib`_dispatch_main_queue_drain.cold.7 + 24
    frame microsoft#29: 0x000000018018c1c4 libdispatch.dylib`_dispatch_main_queue_drain + 1184
    frame microsoft#30: 0x000000018018bd14 libdispatch.dylib`_dispatch_main_queue_callback_4CF + 40
    frame microsoft#31: 0x0000000180427fec CoreFoundation`__CFRUNLOOP_IS_SERVICING_THE_MAIN_DISPATCH_QUEUE__ + 12
    frame microsoft#32: 0x00000001804229f8 CoreFoundation`__CFRunLoopRun + 1920
    frame microsoft#33: 0x0000000180421e3c CoreFoundation`CFRunLoopRunSpecific + 536
    frame microsoft#34: 0x0000000190f62d00 GraphicsServices`GSEventRunModal + 164
    frame microsoft#35: 0x0000000185bcec98 UIKitCore`-[UIApplication _run] + 796
    frame microsoft#36: 0x0000000185bd3064 UIKitCore`UIApplicationMain + 124
    frame microsoft#37: 0x0000000115fbf0bc PRODUCTFramework`main + 200
    frame microsoft#38: 0x00000001114293d8 dyld_sim`start_sim + 20
    frame microsoft#39: 0x0000000111506b4c dyld`start + 6000
```

Reviewed By: rubennorte

Differential Revision: D74654157

fbshipit-source-id: 9181bcd28524c71d0ca4620bd630dc0baa172386
This pull request was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@acoates-ms acoates-ms acoates-ms approved these changes

Assignees
No one assigned
Labels
AutoMerge
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@JunielKatarn @acoates-ms