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

stop sending State in view preallocation on Android #44954

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,6 @@
import com.facebook.react.uimanager.ReactRoot;
import com.facebook.react.uimanager.ReactRootViewTagGenerator;
import com.facebook.react.uimanager.RootViewUtil;
import com.facebook.react.uimanager.StateWrapper;
import com.facebook.react.uimanager.ThemedReactContext;
import com.facebook.react.uimanager.UIManagerHelper;
import com.facebook.react.uimanager.ViewManagerPropertyUpdater;
Expand Down Expand Up @@ -749,19 +748,11 @@ private void preallocateView(
int reactTag,
final String componentName,
@Nullable Object props,
@Nullable Object stateWrapper,
@Nullable Object eventEmitterWrapper,
boolean isLayoutable) {

mMountItemDispatcher.addPreAllocateMountItem(
MountItemFactory.createPreAllocateViewMountItem(
rootTag,
reactTag,
componentName,
(ReadableMap) props,
(StateWrapper) stateWrapper,
(EventEmitterWrapper) eventEmitterWrapper,
isLayoutable));
rootTag, reactTag, componentName, (ReadableMap) props, isLayoutable));
}

@SuppressLint("NotInvokedPrivateMethod")
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1237,8 +1237,6 @@ public void preallocateView(
@NonNull String componentName,
int reactTag,
@Nullable ReadableMap props,
@Nullable StateWrapper stateWrapper,
@Nullable EventEmitterWrapper eventEmitterWrapper,
boolean isLayoutable) {
UiThreadUtil.assertOnUiThread();

Expand All @@ -1251,8 +1249,7 @@ public void preallocateView(
return;
}

createViewUnsafe(
componentName, reactTag, props, stateWrapper, eventEmitterWrapper, isLayoutable);
createViewUnsafe(componentName, reactTag, props, null, null, isLayoutable);
}

@AnyThread
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,8 +9,6 @@ package com.facebook.react.fabric.mounting.mountitems

import com.facebook.react.bridge.ReadableArray
import com.facebook.react.bridge.ReadableMap
import com.facebook.react.fabric.events.EventEmitterWrapper
import com.facebook.react.uimanager.StateWrapper

/** Factory class that expose creation of [MountItem] */
public object MountItemFactory {
Expand Down Expand Up @@ -49,12 +47,8 @@ public object MountItemFactory {
reactTag: Int,
component: String,
props: ReadableMap?,
stateWrapper: StateWrapper?,
eventEmitterWrapper: EventEmitterWrapper?,
isLayoutable: Boolean
): MountItem =
PreAllocateViewMountItem(
surfaceId, reactTag, component, props, stateWrapper, eventEmitterWrapper, isLayoutable)
): MountItem = PreAllocateViewMountItem(surfaceId, reactTag, component, props, isLayoutable)

/**
* @return a [MountItem] that will be read and execute a collection of MountItems serialized in
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,10 +16,8 @@
import com.facebook.common.logging.FLog;
import com.facebook.infer.annotation.Nullsafe;
import com.facebook.react.bridge.ReadableMap;
import com.facebook.react.fabric.events.EventEmitterWrapper;
import com.facebook.react.fabric.mounting.MountingManager;
import com.facebook.react.fabric.mounting.SurfaceMountingManager;
import com.facebook.react.uimanager.StateWrapper;

/** {@link MountItem} that is used to pre-allocate views for JS components. */
@Nullsafe(Nullsafe.Mode.LOCAL)
Expand All @@ -29,23 +27,17 @@ final class PreAllocateViewMountItem implements MountItem {
private final int mSurfaceId;
private final int mReactTag;
private final @Nullable ReadableMap mProps;
private final @Nullable StateWrapper mStateWrapper;
private final @Nullable EventEmitterWrapper mEventEmitterWrapper;
private final boolean mIsLayoutable;

PreAllocateViewMountItem(
int surfaceId,
int reactTag,
@NonNull String component,
@Nullable ReadableMap props,
@Nullable StateWrapper stateWrapper,
@Nullable EventEmitterWrapper eventEmitterWrapper,
boolean isLayoutable) {
mComponent = getFabricComponentName(component);
mSurfaceId = surfaceId;
mProps = props;
mStateWrapper = stateWrapper;
mEventEmitterWrapper = eventEmitterWrapper;
mReactTag = reactTag;
mIsLayoutable = isLayoutable;
}
Expand All @@ -64,8 +56,7 @@ public void execute(@NonNull MountingManager mountingManager) {
"Skipping View PreAllocation; no SurfaceMountingManager found for [" + mSurfaceId + "]");
return;
}
surfaceMountingManager.preallocateView(
mComponent, mReactTag, mProps, mStateWrapper, mEventEmitterWrapper, mIsLayoutable);
surfaceMountingManager.preallocateView(mComponent, mReactTag, mProps, mIsLayoutable);
}

@Override
Expand All @@ -82,11 +73,7 @@ public String toString() {
.append(mIsLayoutable);

if (IS_DEVELOPMENT_ENVIRONMENT) {
result
.append(" props: ")
.append(mProps != null ? mProps.toString() : "<null>")
.append(" state: ")
.append(mStateWrapper != null ? mStateWrapper.toString() : "<null>");
result.append(" props: ").append(mProps != null ? mProps.toString() : "<null>");
}

return result.toString();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -817,8 +817,7 @@ void FabricMountingManager::maybePreallocateShadowView(

static auto preallocateView =
JFabricUIManager::javaClassStatic()
->getMethod<void(
jint, jint, jstring, jobject, jobject, jobject, jboolean)>(
->getMethod<void(jint, jint, jstring, jobject, jboolean)>(
"preallocateView");

// Do not hold onto Java object from C
Expand All @@ -831,9 +830,6 @@ void FabricMountingManager::maybePreallocateShadowView(
cStateWrapper->setState(shadowView.state);
}

// Do not hold a reference to javaEventEmitter from the C++ side.
jni::local_ref<EventEmitterWrapper::JavaPart> javaEventEmitter = nullptr;

jni::local_ref<jobject> props = getProps({}, shadowView);

auto component = getPlatformComponentName(shadowView);
Expand All @@ -844,8 +840,6 @@ void FabricMountingManager::maybePreallocateShadowView(
shadowView.tag,
component.get(),
props.get(),
(javaStateWrapper != nullptr ? javaStateWrapper.get() : nullptr),
(javaEventEmitter != nullptr ? javaEventEmitter.get() : nullptr),
isLayoutableShadowNode);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@
#include <react/config/ReactNativeConfig.h>
#include <react/renderer/components/view/HostPlatformViewTraitsInitializer.h>
#include <react/renderer/components/view/primitives.h>
#include <react/utils/CoreFeatures.h>

namespace facebook::react {

Expand Down
Loading