871177 - Fix the rooting hazard in IsNotDateOrRegExp

Status: RESOLVED FIXED

(Core :: DOM: Core & HTML, defect)

Description

[(no longer active)]

Attachment: Patch (v1)

Comment 1

[Boris Zbarsky [:bzbarsky]]

The fact that this is a hazard is really unfortunate, actually: this function can be reasonably hot...

Can we make this take a handle, by any chance?

Comment 2

[(no longer active)]

(In reply to comment #1)
> The fact that this is a hazard is really unfortunate, actually: this function
> can be reasonably hot...
> 
> Can we make this take a handle, by any chance?

I can do that, but I have no idea whether that would fix the hazard, since I don't know how to run the hazard detection script locally.

Comment 3

[(no longer active)]

Comment on attachment 748461 [details] [diff] [review]
Patch (v1)

Till, please see comment 1?  Please let me know if I can do something better.

Comment 4

[Till Schneidereit [:till]]

Comment on attachment 748461 [details] [diff] [review]
Patch (v1)

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

> Till, please see comment 1?  Please let me know if I can do something better.

I was wondering about that but thought that there was a good reason for not turning it into a handle.

Doing that will definitely fix the hazard: a handle is sort of a lent root. If you fix(ed) the rooting hazards in all callees, changing the signature to take a HandleObject (or JS::Handle<JSObject>, if HandleObject isn't available outside the js engine) should just work.

Comment 5

[(no longer active)]

IsConvertibleToDictionary accepts a JS::Value, and passes the result of toObject() on that argument to its other override which calls IsNotDateOrRegExp.  How do I correctly root that kind of thing?

Comment 6

[(no longer active)]

Attachment: WIP

So I came up with this and then gave up since this triggers compiler errors in the Web IDL generated bindings, and I don't know enough about the codegen to fix them.

Hopefully somebody else can take this.

Comment 7

[Boris Zbarsky [:bzbarsky]]

I'll give this a shot on Monday.

Comment 8

[(no longer active)]

(In reply to comment #7)
> I'll give this a shot on Monday.

Thanks!

Comment 9

[Boris Zbarsky [:bzbarsky]]

Attachment: Fix rooting hazard in IsNotDateOrRegexp.

Comment 10

[Boris Zbarsky [:bzbarsky]]

Attachment: diff -w for ease of review

Comment 11

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

Comment on attachment 749660 [details] [diff] [review]
Fix rooting hazard in IsNotDateOrRegexp.

-w is indeed a lot easier to review

Comment 12

[Boris Zbarsky [:bzbarsky]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/62b22c58b27b

Comment 13

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/62b22c58b27b