1277401 - Support [Func=""] on dictionary members

Status: RESOLVED FIXED

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

Description

[Xidorn Quan [:xidorn] UTC+11]

It would be useful to support controling availability of a dictionary member based on a function (rather than just ChromeOnly), so that we can limit it's usage with functions, e.g. IsChromeOrXBL.

We would want to use this for EventListenerOptions.mozSystemGroup. See bug 1274520 comment 52.

Comment 1

[Boris Zbarsky [:bzbarsky]]

Attachment: part 1. Factor out the condition list generation for pref/func so we can reuse it for dictionary members

Comment 2

[Boris Zbarsky [:bzbarsky]]

Attachment: part 2. Fix up includes for dictionaries that have [ChromeOnly] members

Comment 3

[Boris Zbarsky [:bzbarsky]]

Attachment: part 3. Add IDL parser support for [Func] on dictionary members

Comment 4

[Boris Zbarsky [:bzbarsky]]

Attachment: part 4. Fix up includes for dictionary members with [Func] on them

Comment 5

[Boris Zbarsky [:bzbarsky]]

Attachment: part 5. Change dictionary codegen to output Func stuff as needed

The example codegen from this patch looks like this.  In
DictWithConditionalMembers::Init:

  if (!isNull) {
    if (nsContentUtils::ThreadsafeIsCallerChrome() &&
	nsGenericHTMLElement::TouchEventsEnabled(cx, *object)) {
      if (!JS_GetPropertyById(cx, *object, atomsCache->chromeOnlyFuncControlledMember_id, temp.ptr())) {
	return false;
      }
    } else {
      temp->setUndefined();
    }
  }

...

  if (!isNull) {
    if (nsContentUtils::ThreadsafeIsCallerChrome()) {
      if (!JS_GetPropertyById(cx, *object, atomsCache->chromeOnlyMember_id, temp.ptr())) {
	return false;
      }
    } else {
      temp->setUndefined();
    }
  }

...

  if (!isNull) {
    if (TestFuncControlledMember(cx, *object)) {
      if (!JS_GetPropertyById(cx, *object, atomsCache->funcControlledMember_id, temp.ptr())) {
	return false;
      }
    } else {
      temp->setUndefined();
    }
  }

For comparison, an unconditional member now looks like this:

  if (!isNull) {
    if (!JS_GetPropertyById(cx, *object, atomsCache->a_id, temp.ptr())) {
      return false;
    }
  }
  
And when converting the dictionary to a JS object, we now have:

{
  if (nsContentUtils::ThreadsafeIsCallerChrome() &&
      nsGenericHTMLElement::TouchEventsEnabled(cx, obj)) {
    if (mChromeOnlyFuncControlledMember.WasPassed()) {
      do {
	// the actual conversion here
      }
    }
  }

  if (nsContentUtils::ThreadsafeIsCallerChrome()) {
    if (mChromeOnlyMember.WasPassed()) {
      do {
	// the actual conversion here
      }
    }
  }
      
  if (TestFuncControlledMember(cx, obj)) {
    if (mFuncControlledMember.WasPassed()) {
      do {
	// the actual conversion here
      }
    }
  }

Comment 6

[Peter Van der Beken [:peterv]]

Comment on attachment 8759234 [details] [diff] [review]
part 1.  Factor out the condition list generation for pref/func so we can reuse it for dictionary members

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

::: dom/bindings/Codegen.py
@@ +3339,5 @@
> +    if func:
> +        assert isinstance(func, list) and len(func) == 1
> +        conditions.append("%s(aCx, %s)" % (func[0], objName))
> +    if idlobj.getExtendedAttribute("SecureContext"):
> +        conditions.append("mozilla::dom::IsSecureContextOrObjectIsFromSecureContext(aCx, aObj)")

Shouldn't this use objName instead of aObj?

Comment 7

[Peter Van der Beken [:peterv]]

Comment on attachment 8759248 [details] [diff] [review]
part 5.  Change dictionary codegen to output Func stuff as needed

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

::: dom/bindings/Codegen.py
@@ +3348,2 @@
>      if idlobj.getExtendedAttribute("SecureContext"):
> +        conditions.append("mozilla::dom::IsSecureContextOrObjectIsFromSecureContext(%s, %s)" % (cxName, objName))

Ah, here it's changed to objName, ok.

Comment 8

[Boris Zbarsky [:bzbarsky]]

> Ah, here it's changed to objName, ok.

But you're right: it should move to part 1.  Will do that.

Comment 9

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/8bca03ed2ecb
part 1.  Factor out the condition list generation for pref/func so we can reuse it for dictionary members.  r=peterv
https://hg.mozilla.org/integration/mozilla-inbound/rev/42aa63e1c573
part 2.  Fix up includes for dictionaries that have [ChromeOnly] members.  r=peterv
https://hg.mozilla.org/integration/mozilla-inbound/rev/cd91042c6a8b
part 3.  Add IDL parser support for [Func] on dictionary members.  r=peterv
https://hg.mozilla.org/integration/mozilla-inbound/rev/759fdef36b82
part 4.  Fix up includes for dictionary members with [Func] on them.  r=peterv
https://hg.mozilla.org/integration/mozilla-inbound/rev/44182df66982
part 5.  Change dictionary codegen to output Func stuff as needed.  r=peterv

Comment 11

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/8bca03ed2ecb
https://hg.mozilla.org/mozilla-central/rev/42aa63e1c573
https://hg.mozilla.org/mozilla-central/rev/cd91042c6a8b
https://hg.mozilla.org/mozilla-central/rev/759fdef36b82
https://hg.mozilla.org/mozilla-central/rev/44182df66982