Support [Func=""] on dictionary members

RESOLVED FIXED in Firefox 50

Status

()

defect
RESOLVED FIXED
3 years ago
5 months ago

People

(Reporter: xidorn, Assigned: bzbarsky)

Tracking

unspecified
mozilla50
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox50 fixed)

Details

(Whiteboard: btpp-active)

Attachments

(5 attachments)

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.
Assignee: nobody → bzbarsky
Status: NEW → ASSIGNED
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
      }
    }
  }
Attachment #8759248 - Flags: review?(peterv)
Whiteboard: btpp-active
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?
Attachment #8759234 - Flags: review?(peterv) → review+
Attachment #8759235 - Flags: review?(peterv) → review+
Attachment #8759236 - Flags: review?(peterv) → review+
Attachment #8759237 - Flags: review?(peterv) → review+
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.
Attachment #8759248 - Flags: review?(peterv) → review+
> Ah, here it's changed to objName, ok.

But you're right: it should move to part 1.  Will do that.
Pushed by bzbarsky@mozilla.com:
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
Duplicate of this bug: 1278458
Blocks: 1279991
Component: DOM → DOM: Core & HTML
You need to log in before you can comment on or make changes to this bug.