940305 - Move the extraWarnings flag from ContextOptions to RuntimeOptions

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect)

Comment 1

[Eddy Bruel [:ejpbruel]]

Attachment: patch

Try run for this patch:

https://tbpl.mozilla.org/?tree=Try&rev=deea483870b5

Comment 2

[Bill McCloskey [inactive unless it's an emergency] (:billm)]

We intentionally only set extraWarnings only for chrome code. It doesn't seem like we can do this.

Comment 3

[Bobby Holley (:bholley)]

Yes, the patch here is wrong. It will need to have a per-compartment override.

Comment 4

[Jeff Walden [:Waldo]]

Shouldn't extraWarnings be in CompartmentOptions, then?

Comment 5

[Bobby Holley (:bholley)]

(In reply to Jeff Walden [:Waldo] (remove +bmo to email) from comment #4)
> Shouldn't extraWarnings be in CompartmentOptions, then?

If we don't care about twiddling the default runtime-wide, then yes.

Comment 6

[Bill McCloskey [inactive unless it's an emergency] (:billm)]

Ideally we'd have some notion of an OptionsGroup or something, and each compartment would just specify its OptionsGroup. Then the runtime would specify the right options for each OptionsGroup.

Comment 7

[Bill McCloskey [inactive unless it's an emergency] (:billm)]

(And the browser would have one group for chrome and one for content.)

Comment 8

[Bill McCloskey [inactive unless it's an emergency] (:billm)]

This turned out to be pretty spammy so I backed it out.
https://hg.mozilla.org/integration/mozilla-inbound/rev/7fec954d58b6

Comment 9

[Bobby Holley (:bholley)]

wrong bug number.

Comment 10

[Bill McCloskey [inactive unless it's an emergency] (:billm)]

Oops. For anyone looking, that checkin was for bug 980558.

Comment 11

[Bobby Holley (:bholley)]

https://tbpl.mozilla.org/?tree=Try&rev=bb86bbc10053

Comment 12

[Bobby Holley (:bholley)]

https://tbpl.mozilla.org/?tree=Try&rev=6eee3d2cb8e3

Comment 13

[Bobby Holley (:bholley)]

Attachment: Move extraWarnings to RuntimeOptions with a per-compartment override. v1

Kyle for the workers stuff, Jan for everything else.

I should have split out the workers stuff, but I didn't realized how involved
it would be until I was already partway through it.

Comment 14

[Bobby Holley (:bholley)]

https://tbpl.mozilla.org/?tree=Try&rev=1e70988afe9d

Comment 15

[Jan de Mooij [:jandem]]

Comment on attachment 8455494 [details] [diff] [review]
Move extraWarnings to RuntimeOptions with a per-compartment override. v1

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

Looks good, sorry for the delay.

Comment 16

[Bill McCloskey [inactive unless it's an emergency] (:billm)]

It might be nice to the strict option apply only for chrome code, as the strict.debug option does now. It's really weird that they behave differently. Also, I can't imagine anyone wanting to turn on extra warnings for all web content. At most you'd want something in devtools to turn it on for specific pages (which would be an amazingly awesome feature by the way).

Comment 17

[Bobby Holley (:bholley)]

(In reply to Bill McCloskey (:billm) from comment #16)
> It might be nice to the strict option apply only for chrome code, as the
> strict.debug option does now. It's really weird that they behave
> differently. Also, I can't imagine anyone wanting to turn on extra warnings
> for all web content. At most you'd want something in devtools to turn it on
> for specific pages (which would be an amazingly awesome feature by the way).

Please file a followup - I'm unlikely to do that in this bug.

Comment 18

[Ben McCann]

Thought I'd check in on this. I think it's just waiting for an okay from Kyle?

Comment 19

[Bobby Holley (:bholley)]

(In reply to Ben McCann from comment #18)
> Thought I'd check in on this. I think it's just waiting for an okay from
> Kyle?

Yes. jst is going to ping kyle about this in person later this week. :-)

Comment 20

[Kyle Huey (Exited; not receiving bugmail, old account, do not use)]

(In reply to Bobby Holley (:bholley) from comment #19)
> (In reply to Ben McCann from comment #18)
> > Thought I'd check in on this. I think it's just waiting for an okay from
> > Kyle?
> 
> Yes. jst is going to ping kyle about this in person later this week. :-)

If you want to take some b2g blockers let me know.

Comment 21

[Bobby Holley (:bholley)]

https://hg.mozilla.org/integration/mozilla-inbound/rev/d4be8edb114b

Comment 22

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/d4be8edb114b