Please update either Debugger.Frame.md or Debugger.Object.md to mention the option, with a description that covers:

• which combination of binding kind this actually allows
  • existing var binding + new lexical binding combination is not allowed
• when this option is allowed and when not allowed
  • This option is only for executeInGlobalMethod*, not for Frame's eval*.

Also, incompatible combination of the options should throw an error if any.
For example, this conflicts with useInnerBindings.

Let's put the same types in the same group. It allows packing fields more efficiently, and also making the further modification easier.

Currently there's 2 bytes hole after bypassCSP_ field.
https://searchfox.org/firefox-main/query/default?q=field-layout%3A%27js%3A%3AEvalOptions%27

Please move this below the setBypassCSP method, to make the order consistent.

I'm sorry that the setUseInnerBindings doesn't follow the order and grouping.
It could be fixed in a separate bug.

Here too, please move this below the bypassCSP method, so that the getters and setters are grouped separately.

So, the allowRedeclaringExistingLexicalBinding option is consumed only in Global and GlobalWithExtraOuterBindings kind.
Passing the option in other cases should throw an error, to avoid unexpected behavior.

Please pass the option only when actually performing. the redeclaration.
Here and other places.

Is there any reason for this rewrite?

Please add a testcase with block-level function declaration ({ function f() {} ) too.
It has a special behavior around the conflict.

Please add the following testcases:

• an opposite situation where only the redeclaration closes over the binding
• the first declaration is a const and it's closed over, and ensure that the modified value is reflected.
• ensure the the mutation is reflected even after a JIT compilation

You can test the JIT situation by the following:

• declare a function that closes over a lexical variable and returns it
• call the function many times, so that the function hits the JIT compilation threshold (usually, just call the function 2000 times in a loop)
• perform the redeclartion and mutate the value
• call the function again, and verify the result reflects the mutation

As mentioned above, this JIT situation fails.

The introduction of the above two JS::Rooted instances actually de-optimizes the code in two ways.

The first one is the removal of the RootedTuple.
The combination of the RootedTuple+RootedField is an optimization to merge multiple JS::Rooted handling.
See RootingAPI.h for more details.

The other one is that this patch moves the variable into a loop, which means, the constructor and destructor are called for each time.

Please revert the change to declRoots etc below, and let DefineLexicalBinding function take uninitialized and id.

Shouldn't this check the outer binding as well?
The info message implies that the outer binding is unmodified, but this testcase doesn't verify that.

Theoretically, discarding the JIT script here is not sufficient.

A debugger hook onEnterFrame can run in between here and the JSOp::GlobalOrEvalDeclInstantiation opcode handling (which calls InitGlobalOrEvalDeclarations), at the js::DebugAPI::onEnterFrame call.

Which means, you can JIT-compile the function after this line, and before setting the binding to JS_UNINITIALIZED_LEXICAL.

The following testcase fails, in the same way as the previous testcase, where the access to x is optimized away, and the redeclaration's value is not reflected.

var g = newGlobal({ newCompartment: true });
var dbg = new Debugger;
var gw = dbg.addDebuggee(g);
gw.executeInGlobal(`
let x = 1;
function getX() {
  return x;
}
`);

// When entering the `let x = 2;` script's frame, JIT compile getX.
let first = true;
dbg. => {
  if (!first) {
    return;
  }
  first = false;
  gw.executeInGlobal(`
for (var i = 0; i < 2000; i++) {
  r = getX();
}
`);
};

// Mutate the binding.
gw.executeInGlobal(`let x = 2;`, {
  allowRedeclaringExistingLexicalBinding: true,
});

// Ensure that the mutation is reflected to the JIT-compiled function.
assertEq(gw.executeInGlobal(`getX()`).return, 2);

Also, given that the allowRedeclaringExistingLexicalBinding option is stored into the script flag, handling the option only here is error-prone, where a new consumer can be overlooked.

My understanding is that, you need to discard JIT code after setting the binding to JS_UNINITIALIZED_LEXICAL, in JSOp::GlobalOrEvalDeclInstantiation, so that the property access results in ReferenceError for TDZ until the initialization, and thus there's no chance for JIT compiler to optimize away the property access until then.

The other possible place to discard the JIT code is JSOp::InitGLexical, which is the place where the actual assignment is performed, but given the above ReferenceError from TDZ, I assume that's not necessary and overly complex.

Also, this basically makes the DevTools console evaluation invalidate the JIT code for every each input, even if the code doesn't contain redeclaration.
This should be performed only when there's any actual redeclaration.

in terms of accessibility, "below" is not a good link text, given that it doesn't provide sufficient meaning.
https://www.w3.org/WAI/tips/writing/#make-link-text-meaningful

This can be reworded to "the executeInGlobalWithBindings section" or something along that line.

Here too, but it's slightly different situation than the previous one.
The option section is near enough, so, you can just remove the "see ... for details" part here.

"has no effect" is not true.
Frame#eval throws an error.

The options should be copied by the copy constructor, in the same way as other fields.
JS::TransitiveCompileOptions::copyPODTransitiveOptions

The previous comment should be preserved.
and the "// executeInGlobal is strict by default." comment should be put immediately before the line 10, with an empty line before it

This block should be put immediately after the existing block, so that the "By contrast" makes sense.
(and also "the above" below makes sense)

So, please put the newly added tests at the end of the existing testcases.

The following testcase expects the method to throw, which is different than "not recognized".
It recognizes and throws.
(so, this contradicts with the document)

This keeps the const-ness even if the redeclaration uses different declaration.

The following test fails, due to the x = 3 assignment fails:

var g = newGlobal({ newCompartment: true });
var dbg = new Debugger;
var gw = dbg.addDebuggee(g);
gw.executeInGlobal(`const x = 1;`, {
  allowRedeclaringExistingLexicalBinding: true,
});
gw.executeInGlobal(`let x = 2; x = 3`, {
  allowRedeclaringExistingLexicalBinding: true,
});
assertEq(gw.executeInGlobal(`x`).return, 3);

Same for the opposite way, where the assignment on const succeeds when the original declaration is let.

If we just want to allow re-running similar input again, we could reject redeclaration with different kind (let vs const), so that you don't have to worry about the assignment after the declaration.

This change is unnecessary. It was already in the correct position, where it matches the field's order.

Please mention the restriction where the redeclarations with different const-ness are not allowed.

Please move this block below line 10, so that the newly added tests are added after existing tests.
So that the "By contrast" makes sense.

The last sentence is unnecessary, given that the comment is explaining what the code does, which is obvious from the code.
If you want to add comment about the option, the comment should explain the reasoning why the code is doing so,

accepted, or allowed, or something.
"not recognize" usually means ignored.

Please also assert that the all attributes match, especially the const-ness.

(optional) This is performed for each redeclaration binding.
Assuming the watchGlobalLexicalRedeclaration is not costly operation when performed multiple times (given that the dependencies will be empty in js::ObjectFuse::invalidateAllDependentIonScripts on subsequent calls), performing it here would be okay.
But otherwise it would be nice to move the call after the loop.