REGRESSION (r271119): Object methods defined with shorthand notation cannot access “caller” in non-strict mode

• Component: JavaScriptCore
• Status: RESOLVED FIXED

[Jonathan Grimes]

In a script that is not in strict mode defining a object property that has a function as a value using shorthand notation will not be able to access its caller property.

[Jonathan Grimes]

Here is an example:
https://jsfiddle.net/jsg2021/4voxyca2/

[Jonathan Grimes]

If I change object-shorthand notation to old-skewl object notation it works...
ie:
{ constructor: function() {
      this.callParent(arguments);
}}

vs

{ constructor() {
      this.callParent(arguments);
}}

[Radar WebKit Bug Importer]

<rdar://problem/77633889>

[Alexey Shvayka]

Created attachment 428644 [details]
Patch

[Alexey Shvayka]

(In reply to Jonathan Grimes from comment #1)
> Here is an example:
> https://jsfiddle.net/jsg2021/4voxyca2/

Hey Jonathan, thank you for nice repro and sorry for the breakage.

[Jonathan Grimes]

thank you :)

[Darin Adler]

Comment on attachment 428644 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=428644&action=review

> Source/JavaScriptCore/runtime/JSFunction.cpp:413
> +    SourceParseMode parseMode = function->jsExecutable()->parseMode();

Dueling coding styles: This would be better with auto. We don’t want to name the type, risking that if we named the wrong one we could cause type conversion, just to get the function result and pass it along to predicate functions like isGeneratorParseMode.

But you have told me that auto is less in fashion in JavaScriptCore, as coding style and project culture subtly drifts apart.

[Alexey Shvayka]

Comment on attachment 428644 [details]
Patch

(In reply to Darin Adler from comment #7)

Thank you for review!

> > Source/JavaScriptCore/runtime/JSFunction.cpp:413
> > +    SourceParseMode parseMode = function->jsExecutable()->parseMode();
> 
> Dueling coding styles: This would be better with auto. We don’t want to name
> the type, risking that if we named the wrong one we could cause type
> conversion, just to get the function result and pass it along to predicate
> functions like isGeneratorParseMode.

Coming from JS background, I would personally prefer `auto` since in most modern IDEs, type inference is available on :hover.
However, across the JSC codebase, result of parseMode() is explicitly typed.

Darin, is there a protocol to get this patch cherry-picked into Safari 14.5.X?

[EWS]

Committed r277613 (237828@main): <https://commits.webkit.org/237828@main>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 428644 [details].

[Darin Adler]

(In reply to Alexey Shvayka from comment #8)
> Darin, is there a protocol to get this patch cherry-picked into Safari
> 14.5.X?

Need to bring this to the attention of the people inside Apple who pull bugs into release branches. I’m not sure it fits the mission of those kinds of releases. Helpful comment about the impact of the bug could make difference.

[Alexey Shvayka]

(In reply to Darin Adler from comment #10)
> Need to bring this to the attention of the people inside Apple who pull bugs
> into release branches. I’m not sure it fits the mission of those kinds of
> releases. Helpful comment about the impact of the bug could make difference.

With best intentions, r271119 made `function.caller` return `null` for callers that are getters / setters, arrow functions, and ES6 methods. The old code using `function.caller` is not aware it's being called from modern JavaScript. ExtJS 4.x (maybe even later versions) class implementation relies on `function.caller` to perform super(), and is currently broken for userland code that uses new methods syntax. Given that this bug was reported days after 14.5 release, it appears that r271119 has high chances of breaking the web.