From 08572386332707c99247c4acec6157e3f6e4dd6e Mon Sep 17 00:00:00 2001 From: Samuel Susla Date: Wed, 5 Oct 2022 10:35:33 -0700 Subject: [PATCH] Pass didUserCallbackTimeout argument to Scheduler's callback Summary: changelog: [internal] RuntimeScheduler's callbacks previously did not pass down argument `didUserCallbackTimeout`. This was intentionally left out because React team intended to remove it. But, as it turns out, it was not removed and it is needed for Offscreen to not enter infinite render loop. In React, it is used here: https://github.com/facebook/react/blob/main/packages/react-reconciler/src/ReactFiberWorkLoop.new.js#L1022-L1028 Reviewed By: ryancat Differential Revision: D40060377 fbshipit-source-id: c45719fbbd0275ab2bcf9a2bbf0191aaa96010cc --- .../renderer/runtimescheduler/RuntimeScheduler.cpp | 5 +++-- ReactCommon/react/renderer/runtimescheduler/Task.cpp | 8 ++++---- ReactCommon/react/renderer/runtimescheduler/Task.h | 2 +- .../runtimescheduler/tests/RuntimeSchedulerTest.cpp | 10 ++++------ 4 files changed, 12 insertions(+), 13 deletions(-) diff --git a/ReactCommon/react/renderer/runtimescheduler/RuntimeScheduler.cpp b/ReactCommon/react/renderer/runtimescheduler/RuntimeScheduler.cpp index fd041aca8a4c02..9e21ea3a0c312e 100644 --- a/ReactCommon/react/renderer/runtimescheduler/RuntimeScheduler.cpp +++ b/ReactCommon/react/renderer/runtimescheduler/RuntimeScheduler.cpp @@ -23,6 +23,7 @@ RuntimeScheduler::RuntimeScheduler( void RuntimeScheduler::scheduleWork( std::function callback) const { runtimeAccessRequests_ += 1; + runtimeExecutor_( [this, callback = std::move(callback)](jsi::Runtime &runtime) { runtimeAccessRequests_ -= 1; @@ -95,7 +96,7 @@ void RuntimeScheduler::callExpiredTasks(jsi::Runtime &runtime) { } currentPriority_ = topPriorityTask->priority; - auto result = topPriorityTask->execute(runtime); + auto result = topPriorityTask->execute(runtime, didUserCallbackTimeout); if (result.isObject() && result.getObject(runtime).isFunction(runtime)) { topPriorityTask->callback = @@ -140,7 +141,7 @@ void RuntimeScheduler::startWorkLoop(jsi::Runtime &runtime) const { } currentPriority_ = topPriorityTask->priority; - auto result = topPriorityTask->execute(runtime); + auto result = topPriorityTask->execute(runtime, didUserCallbackTimeout); if (result.isObject() && result.getObject(runtime).isFunction(runtime)) { topPriorityTask->callback = diff --git a/ReactCommon/react/renderer/runtimescheduler/Task.cpp b/ReactCommon/react/renderer/runtimescheduler/Task.cpp index bed71bb4b0326a..475de07cba6bb1 100644 --- a/ReactCommon/react/renderer/runtimescheduler/Task.cpp +++ b/ReactCommon/react/renderer/runtimescheduler/Task.cpp @@ -18,14 +18,14 @@ Task::Task( callback(std::move(callback)), expirationTime(expirationTime) {} -jsi::Value Task::execute(jsi::Runtime &runtime) { +jsi::Value Task::execute(jsi::Runtime &runtime, bool didUserCallbackTimeout) { auto result = jsi::Value::undefined(); // Cancelled task doesn't have a callback. if (callback) { // Callback in JavaScript is expecting a single bool parameter. - // React team plans to remove it and it is safe to pass in - // hardcoded false value. - result = callback.value().call(runtime, {false}); + // React team plans to remove it in the future when a scheduler bug on web + // is resolved. + result = callback.value().call(runtime, {didUserCallbackTimeout}); // Destroying callback to prevent calling it twice. callback.reset(); diff --git a/ReactCommon/react/renderer/runtimescheduler/Task.h b/ReactCommon/react/renderer/runtimescheduler/Task.h index 76c52a9ff5e973..a8b8a69c64caab 100644 --- a/ReactCommon/react/renderer/runtimescheduler/Task.h +++ b/ReactCommon/react/renderer/runtimescheduler/Task.h @@ -33,7 +33,7 @@ struct Task final { std::optional callback; RuntimeSchedulerClock::time_point expirationTime; - jsi::Value execute(jsi::Runtime &runtime); + jsi::Value execute(jsi::Runtime &runtime, bool didUserCallbackTimeout); }; class TaskPriorityComparer { diff --git a/ReactCommon/react/renderer/runtimescheduler/tests/RuntimeSchedulerTest.cpp b/ReactCommon/react/renderer/runtimescheduler/tests/RuntimeSchedulerTest.cpp index 934fedff375455..ed43bab9a25078 100644 --- a/ReactCommon/react/renderer/runtimescheduler/tests/RuntimeSchedulerTest.cpp +++ b/ReactCommon/react/renderer/runtimescheduler/tests/RuntimeSchedulerTest.cpp @@ -116,7 +116,7 @@ TEST_F(RuntimeSchedulerTest, scheduleImmediatePriorityTask) { auto callback = createHostFunctionFromLambda([&didRunTask](bool didUserCallbackTimeout) { didRunTask = true; - EXPECT_FALSE(didUserCallbackTimeout); + EXPECT_TRUE(didUserCallbackTimeout); return jsi::Value::undefined(); }); @@ -137,9 +137,7 @@ TEST_F(RuntimeSchedulerTest, taskExpiration) { auto callback = createHostFunctionFromLambda([&didRunTask](bool didUserCallbackTimeout) { didRunTask = true; - // Task has timed out but the parameter is deprecated and `false` is - // hardcoded. - EXPECT_FALSE(didUserCallbackTimeout); + EXPECT_TRUE(didUserCallbackTimeout); return jsi::Value::undefined(); }); @@ -427,7 +425,7 @@ TEST_F(RuntimeSchedulerTest, scheduleTaskFromTask) { auto secondCallback = createHostFunctionFromLambda( [&didRunSecondTask](bool didUserCallbackTimeout) { didRunSecondTask = true; - EXPECT_FALSE(didUserCallbackTimeout); + EXPECT_TRUE(didUserCallbackTimeout); return jsi::Value::undefined(); }); @@ -508,7 +506,7 @@ TEST_F(RuntimeSchedulerTest, sameThreadTaskCreatesImmediatePriorityTask) { auto callback = createHostFunctionFromLambda( [&didRunSubsequentTask](bool didUserCallbackTimeout) { didRunSubsequentTask = true; - EXPECT_FALSE(didUserCallbackTimeout); + EXPECT_TRUE(didUserCallbackTimeout); return jsi::Value::undefined(); });