Remove JS_GetScriptedCaller

RESOLVED FIXED in mozilla13

Status

()

Core
JavaScript Engine
RESOLVED FIXED
6 years ago
6 years ago

People

(Reporter: dvander, Assigned: dvander)

Tracking

(Blocks: 1 bug)

Trunk
mozilla13
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(5 attachments, 2 obsolete attachments)

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.
Created attachment 603096 [details] [diff] [review]
part 1: introduce new API
Created attachment 603098 [details] [diff] [review]
fix websockets
Created attachment 603099 [details] [diff] [review]
fix workers
Created attachment 603100 [details] [diff] [review]
fix xpcshell
Created attachment 603101 [details] [diff] [review]
fix nsJSEnvironment
Created attachment 603109 [details] [diff] [review]
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...
Attachment #603098 - Flags: review?(mcmanus)
Attachment #603099 - Flags: review?(bent.mozilla)
Attachment #603100 - Flags: review?(bobbyholley+bmo)
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.
Attachment #603101 - Flags: review?(mrbkap)
Created attachment 603162 [details] [diff] [review]
patch

New API addition and removal of the old API.
Attachment #603096 - Attachment is obsolete: true
Attachment #603109 - Attachment is obsolete: true
Attachment #603162 - Flags: review?(luke)
Comment on attachment 603100 [details] [diff] [review]
fix xpcshell

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

rs=bholley
Attachment #603100 - Flags: review?(bobbyholley+bmo) → review+
Attachment #603099 - Flags: review?(bent.mozilla) → review+
Attachment #603098 - Flags: review?(mcmanus) → review?(bugs)

Updated

6 years ago
Attachment #603101 - Flags: review?(mrbkap) → review+

Comment 10

6 years ago
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.
Attachment #603162 - Flags: review?(luke) → review+
Comment on attachment 603098 [details] [diff] [review]
fix websockets

rs=me
Attachment #603098 - Flags: review?(bugs) → review+
http://hg.mozilla.org/integration/mozilla-inbound/rev/0215639e611a
Version: unspecified → Trunk
https://hg.mozilla.org/mozilla-central/rev/0215639e611a
Status: ASSIGNED → RESOLVED
Last Resolved: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla13
You need to log in before you can comment on or make changes to this bug.