Closed Bug 1216819 Opened 4 years ago Closed 4 years ago

Allow JSAPI SavedFrame accessors to skip past self-hosted frames

Categories

(Core :: JavaScript Engine, defect)

defect
Not set

Tracking

()

RESOLVED FIXED
mozilla44
Tracking Status
firefox44 --- fixed

People

(Reporter: fitzgen, Assigned: fitzgen)

References

Details

Attachments

(1 file, 1 obsolete file)

No description provided.
Assignee: nobody → nfitzgerald
Status: NEW → ASSIGNED
I opted to make the default be to include self-hosted frames, as that is the current behavior.
Looks like I can't just add a default param and have existing code continue to compile because of GetValueIfNotCached. Looks like I will be touching a bunch of dom code.

In light of that, I will make the default be to exclude self-hosted frames, as most consumers probably don't want to consider them.
Comment on attachment 8676553 [details] [diff] [review]
Allow JSAPI SavedFrame accessors to skip past self-hosted frames

>+UnwrapSavedFrame(JSContext* cx, HandleObject obj, SavedFrameSelfHosted selfHosted,

Why the change from assert to check here?  I thought callers were only expected to call this with actual saved frames (which might need unwrapping)...

r=me.  You'll want to change the DOM bits to make it a 4-arg function in that template and pass the exclude value of the enum, as you note.
Attachment #8676553 - Flags: review?(bzbarsky) → review+
Oh, and you don't need to touch a "bunch" of DOM code.  Just the declaration of GetValueIfNotCached and the one line in it where it calls aPropGetter.
(In reply to Boris Zbarsky [:bz] from comment #5)
> Comment on attachment 8676553 [details] [diff] [review]
> Allow JSAPI SavedFrame accessors to skip past self-hosted frames
> 
> >+UnwrapSavedFrame(JSContext* cx, HandleObject obj, SavedFrameSelfHosted selfHosted,
> 
> Why the change from assert to check here?  I thought callers were only
> expected to call this with actual saved frames (which might need
> unwrapping)...

When I wrote this code, I was under the (incorrect) impression that CheckedUnwrap could *not* return nullptr. I was bitten by the same bug in another place, and I figured I would fix it while I was here.
Attachment #8676553 - Attachment is obsolete: true
https://hg.mozilla.org/mozilla-central/rev/dbfe8e642f56
Status: ASSIGNED → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla44
You need to log in before you can comment on or make changes to this bug.