-
Notifications
You must be signed in to change notification settings - Fork 25
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
CacheOverride StarlingMonkey port #767
Conversation
@@ -737,16 +737,9 @@ routes.set('/btoa', () => { | |||
if (error) { return error; } | |||
error = assertThrows(() => atob("--")) | |||
if (error) { return error; } | |||
error = assertThrows(() => atob("__")) | |||
if (error) { return error; } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This was fixed upstream in StarlingMonkey, so no longer needs separating.
@@ -1527,7 +1522,7 @@ routes.set("/backend/timeout", async () => { | |||
routes.set("/backend/health/called-as-constructor-function", async () => { | |||
let error = assertThrows(() => { | |||
new Backend.health() | |||
}, TypeError, `Backend.health is not a constructor`) | |||
}, TypeError) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is now (intermediate value).health is not a constructor
(but still pointing to the line number source of the error in the original source, so the stack retains the context). This is due to the ad-hoc nature of the bytecode name tracing in https://searchfox.org/mozilla-central/source/js/src/vm/BytecodeUtil.cpp#1590, which doesn't carry through to the expression naming for the builtin module. The same thing happens for arbitrary JS expressions though anyway (eg new getObj().health()
).
Since new Backend.health()
is a pretty extreme edge case, as most JS developers won't attempt to call a function as a class constructor, so I don't think this error message change would be considered breaking.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks good, nice work!
This ports the cache-override builtin to StarlingMonkey, fixing the TODOs on this subsystem and enabling the tests.
A regression in the ehe error lookup function is also fixed.
Integration Test status: 245 / 726 = 34% passing.