Closed Bug 1229176 Opened 10 years ago Closed 10 years ago

bindings codegen doesn't include nsContentUtils in all cases where necessary

Categories

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

defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla45
Tracking Status
firefox45 --- fixed

People

(Reporter: froydnj, Assigned: froydnj)

References

Details

Attachments

(1 file)

CacheStorageBinding has this function: static bool _constructor(JSContext* cx, unsigned argc, JS::Value* vp) { JS::CallArgs args = JS::CallArgsFromVp(argc, vp); JS::Rooted<JSObject*> obj(cx, &args.callee()); if (!nsContentUtils::ThreadsafeIsCallerChrome()) { return ThrowingConstructor(cx, argc, vp); } ... which comes from this bit of Codegen.py: http://mxr.mozilla.org/mozilla-central/source/dom/bindings/Codegen.py#11770 because CacheStorage is hasInterfaceObject. But CacheStorage has a ChromeConstructor: http://mxr.mozilla.org/mozilla-central/source/dom/webidl/CacheStorage.webidl#16 and we don't check the special ctor() method for whether it's ChromeOnly or not here: http://mxr.mozilla.org/mozilla-central/source/dom/bindings/Codegen.py#12960 We should fix this (it's possible there are other instances, but this is at least one of the things that makes my bug 1218454 patches fail to compile).
If an interface has a {Chrome,}Constructor(), it doesn't show up as a normal member. If the interface has a ChromeConstructor, we need to include nsContentUtils.h in the generated file for a ThreadsafeIsCallerChrome check. There is an existing check for a descriptor's ChromeOnly-ness in CGBindingRoot; this check is used to determine whether nsContentUtils.h is included in the generated file.. But the check in descriptorHasChromeOnly doesn't detect this (ChromeOnly) constructor, and so nsContentUtils.h won't be included if there are no other ChromeOnly members, or if the interface itself is not ChromeOnly. Therefore, we need to take the constructor of the interface (if any) into account when checking for ChromeOnly-ness.
Attachment #8693909 - Flags: review?(bzbarsky)
Comment on attachment 8693909 [details] [diff] [review] make check for ChromeOnly interfaces for header inclusion more complete r=me. Good catch.
Attachment #8693909 - Flags: review?(bzbarsky) → review+
Status: NEW → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla45
See Also: → 1247000
Component: DOM → DOM: Core & HTML
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: