732652 - Remove JS_GetScriptedCaller

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect)

Description

[David Anderson [:dvander] - inactive, e-mail if emergency]

This API call is incompatible with IonMonkey, which does not create JSStackFrames. Most callers of this in the browser just need the topmost script/filename, which we can get easily in other ways.

Comment 1

[David Anderson [:dvander] - inactive, e-mail if emergency]

Attachment: part 1: introduce new API

Comment 2

[David Anderson [:dvander] - inactive, e-mail if emergency]

Attachment: fix websockets

Comment 3

[David Anderson [:dvander] - inactive, e-mail if emergency]

Attachment: fix workers

Comment 4

[David Anderson [:dvander] - inactive, e-mail if emergency]

Attachment: fix xpcshell

Comment 5

[David Anderson [:dvander] - inactive, e-mail if emergency]

Attachment: fix nsJSEnvironment

Comment 6

[David Anderson [:dvander] - inactive, e-mail if emergency]

Attachment: remove js_GetScriptedCaller

For the places where we can't get rid of the API outright, the details have been moved into the JS engine where they can be easily maintained.

Sending to try...

Comment 7

[David Anderson [:dvander] - inactive, e-mail if emergency]

Comment on attachment 603101 [details] [diff] [review]
fix nsJSEnvironment

Note this patch removes the call to SetFrameReturnValue. Luke and I couldn't think of a use case for it - if there is one, I can add it back, but it's unlikely to work with IonMonkey anytime soon.

Comment 8

[David Anderson [:dvander] - inactive, e-mail if emergency]

Attachment: patch

New API addition and removal of the old API.

Comment 9

[Bobby Holley (:bholley)]

Comment on attachment 603100 [details] [diff] [review]
fix xpcshell

I'm not familiar with this stuff but I'll trust dvander.

rs=bholley

Comment 10

[Luke Wagner [:luke]]

Comment on attachment 603162 [details] [diff] [review]
patch

Review of attachment 603162 [details] [diff] [review]:
-----------------------------------------------------------------

::: js/src/jsapi.cpp
@@ +6668,5 @@
> +        *lineno = 0;
> +
> +    for (FrameRegsIter i(cx); !i.done(); ++i) {
> +        if (script)
> +            *script = i.script();

Instead of the loop, how about:

  FrameRegsIter i(cx);
  if (!i.done())
      return false;

?

::: js/src/jsapi.h
@@ +5407,5 @@
> + * Return the current script and line number of the most currently running
> + * frame. Returns true if a scripted frame was found, false otherwise.
> + */
> +extern JS_PUBLIC_API(JSBool)
> +JS_DescribeTopFrame(JSContext *cx, JSScript **script, unsigned *lineno);

I'd like to avoid any mention of "frame" in JSAPI.  How about JS_DescribeScriptedCaller?

::: js/src/jsfun.cpp
@@ +783,5 @@
>              return true;
>  
> +        StackFrame *frame = fp->prev();
> +        if (!frame)
> +            frame = js_GetTopStackFrame(cx, FRAME_EXPAND_ALL);

In view of the "if (!fp->prev())" above, this 'if' can be removed.

Comment 11

[Olli Pettay [:smaug][[e-mail]]]

Comment on attachment 603098 [details] [diff] [review]
fix websockets

rs=me

Comment 12

[David Anderson [:dvander] - inactive, e-mail if emergency]

http://hg.mozilla.org/integration/mozilla-inbound/rev/0215639e611a

Comment 13

[Marco Bonardo [:mak]]

https://hg.mozilla.org/mozilla-central/rev/0215639e611a