fix: Align TimerManager sequential ids and function error handling with web standard (#51500)

Summary:
Calls to create timers should return sequential ids (integers greater than zero in the spec's words). This regressed in the `TimerManager` implementation, which instead starts at zero inclusively.

This has two side-effects for code assuming a spec-compliant implementation of `setTimeout` and `setInterval`:
- Calls to `clearTimeout(0)` or `clearInterval(0)` will potentially cancel scheduled timers, although it's supposed to be a noop
- Predicates like `if (timeoutId)` will fail since they assume non-negative ids

The change in this PR is to align with WHATWG HTML 8.6.2 (Timers): https://html.spec.whatwg.org/multipage/timers-and-user-prompts.html#timers

> otherwise, let id be an [implementation-defined](https://infra.spec.whatwg.org/#implementation-defined) integer that is **greater than zero** and does not already [exist](https://infra.spec.whatwg.org/#map-exists) in global's [map of setTimeout and setInterval IDs](https://html.spec.whatwg.org/multipage/timers-and-user-prompts.html#map-of-settimeout-and-setinterval-ids).

Specifically,
- we should return `0` to indicate that no timer was scheduled
- we should start generating timer IDs at `1` instead of `0`

This was previously raised in review comments here: https://github.com/facebook/react-native/pull/45092/files#r1650790008

The spec-incompliant behaviour was raised in an issue here: apollographql/apollo-client#12632 (comment)

This PR does not,
- add bounds checking on `timerIndex_` and add a search of an available id that isn't in the unordered map
- exclude `0` from being an accepted `TimerHandle` in `TimerManager::createTimer` or `TimerManager::deleteTimer` since the above bounds checking hasn't been added either

## Changelog:

[GENERAL] [FIXED] - Align timer IDs and timer function argument error handling with web standards.

Pull Request resolved: #51500

Test Plan:
- Run `setTimeout` / `setInterval`; before applied changes the timeout for the first timer will be `0`
- Run `setTimeout(null)`; before applied changes the timer ID will be non-zero
- Run `setInterval(null)`; before applied changes an error will be thrown rather than `0` being returned

Reviewed By: cipolleschi

Differential Revision: D75145909

Pulled By: rshest

fbshipit-source-id: 6646439abd29cf3cfa9e5cf0a57448e3b7cd1b48

Author: kitten

packages/react-native/ReactCommon/react/runtime/TimerManager.cpp

@@ -242,8 +242,9 @@ void TimerManager::attachGlobals(jsi::Runtime& runtime) {
             }
 
             if (!args[0].isObject() || !args[0].asObject(rt).isFunction(rt)) {
-              // Do not throw any error to match web spec
-              return timerIndex_++;
+              // Do not throw any error to match web spec; instead return 0, an
+              // invalid timer id
+              return 0;
             }
 
             auto callback = args[0].getObject(rt).getFunction(rt);
@@ -300,8 +301,9 @@ void TimerManager::attachGlobals(jsi::Runtime& runtime) {
             }
 
             if (!args[0].isObject() || !args[0].asObject(rt).isFunction(rt)) {
-              throw jsi::JSError(
-                  rt, "The first argument to setInterval must be a function.");
+              // Do not throw any error to match web spec; instead return 0, an
+              // invalid timer id
+              return 0;
             }
             auto callback = args[0].getObject(rt).getFunction(rt);
             auto delay = count > 1

packages/react-native/ReactCommon/react/runtime/TimerManager.h

@@ -93,7 +93,9 @@ class TimerManager {
 
   // Each timeout that is registered on this queue gets a sequential id.  This
   // is the global count from which those are assigned.
-  TimerHandle timerIndex_{0};
+  // As per WHATWG HTML 8.6.1 (Timers) ids must be greater than zero, i.e. start
+  // at 1
+  TimerHandle timerIndex_{1};
 
   // The React Native microtask queue is used to back public APIs including
   // `queueMicrotask`, `clearImmediate`, and `setImmediate` (which is used by

packages/react-native/ReactCommon/react/runtime/tests/cxx/ReactInstanceTest.cpp

@@ -267,7 +267,9 @@ TEST_F(ReactInstanceTest, testSetTimeoutWithoutDelay) {
   EXPECT_CALL(
       *mockRegistry_,
       createTimer(_, 0)); // If delay is not provided, it should use 0
-  eval("setTimeout(() => {});");
+  auto val = eval("setTimeout(() => {});");
+  expectNoError();
+  EXPECT_EQ(val.asNumber(), 1); // First timer id should start at 1
 }
 
 TEST_F(ReactInstanceTest, testSetTimeoutWithPassThroughArgs) {
@@ -299,8 +301,9 @@ TEST_F(ReactInstanceTest, testSetTimeoutWithInvalidArgs) {
       getErrorMessage("setTimeout();"),
       "setTimeout must be called with at least one argument (the function to call).");
 
-  eval("setTimeout('invalid');");
+  auto val = eval("setTimeout('invalid')");
   expectNoError();
+  EXPECT_EQ(val.asNumber(), 0);
 
   eval("setTimeout(() => {}, 'invalid');");
   expectNoError();
@@ -417,9 +420,10 @@ TEST_F(ReactInstanceTest, testSetIntervalWithInvalidArgs) {
   EXPECT_EQ(
       getErrorMessage("setInterval();"),
       "setInterval must be called with at least one argument (the function to call).");
-  EXPECT_EQ(
-      getErrorMessage("setInterval('invalid', 100);"),
-      "The first argument to setInterval must be a function.");
+
+  auto val = eval("setInterval('invalid', 100)");
+  expectNoError();
+  EXPECT_EQ(val.asNumber(), 0);
 }
 
 TEST_F(ReactInstanceTest, testClearInterval) {