Skip to content

Fix ReactSurfaceView-backed roots not reporting the end of pending transactions correctly#46676

Closed
javache wants to merge 2 commits into
facebook:mainfrom
javache:export-D63466672
Closed

Fix ReactSurfaceView-backed roots not reporting the end of pending transactions correctly#46676
javache wants to merge 2 commits into
facebook:mainfrom
javache:export-D63466672

Conversation

@javache
Copy link
Copy Markdown
Member

@javache javache commented Sep 26, 2024

Summary:

Context

We recently "fixed" a problem in MountingCoordinator on Android where it would report that it doesn't have any pending transactions when, in fact, it does. The fix introduces a new method in that class to delay marking transactions as done until a mount hook is invoked for that surface.

That fixed the issue... by always reporting that there were pending transactions accidentally.

The reason for this bug is that the mount hook doesn't have access to the mounting coordinator of the surface if the surface is registered through some of the methods in Binding.cpp that don't add the surface to a registry. In that case, we can never mark the transactions as done and the mounting coordinator for those surfaces always report pending transactions incorrectly.

NOTE: this bug only affects apps that have the fixMountingCoordinatorReportedPendingTransactionsOnAndroid feature flag enabled.

Changes

This fixes the issue by making sure that surfaces are always registered in the registry and that we can access their mounting coordinators in the mount hook to report the transactions as done.

Differential Revision: D63466672

Differential Revision: D63463521
@facebook-github-bot facebook-github-bot added CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. p: Facebook Partner: Facebook Partner labels Sep 26, 2024
@facebook-github-bot
Copy link
Copy Markdown
Contributor

This pull request was exported from Phabricator. Differential Revision: D63466672

…ansactions correctly (facebook#46676)

Summary:
Pull Request resolved: facebook#46676

Changelog: [internal]

## Context

We recently "fixed" a problem in `MountingCoordinator` on Android where it would report that it doesn't have any pending transactions when, in fact, it does. The fix introduces a new method in that class to delay marking transactions as done until a mount hook is invoked for that surface.

That fixed the issue... by always reporting that there were pending transactions accidentally.

The reason for this bug is that the mount hook doesn't have access to the mounting coordinator of the surface if the surface is registered through some of the methods in `Binding.cpp` that don't add the surface to a registry. In that case, we can never mark the transactions as done and the mounting coordinator for those surfaces always report pending transactions incorrectly.

NOTE: this bug only affects apps that have the `fixMountingCoordinatorReportedPendingTransactionsOnAndroid` feature flag enabled.

## Changes

This fixes the issue by making sure that surfaces are always registered in the registry and that we can access their mounting coordinators in the mount hook to report the transactions as done.

Reviewed By: rubennorte

Differential Revision: D63466672
@facebook-github-bot
Copy link
Copy Markdown
Contributor

This pull request was exported from Phabricator. Differential Revision: D63466672

@facebook-github-bot facebook-github-bot added the Merged This PR has been merged. label Sep 26, 2024
@facebook-github-bot
Copy link
Copy Markdown
Contributor

This pull request has been merged in 3fbf5c7.

@javache javache deleted the export-D63466672 branch September 27, 2024 09:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. fb-exported Merged This PR has been merged. p: Facebook Partner: Facebook Partner

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants