Closed Bug 1270601 Opened 4 years ago Closed 4 years ago

Implement support for the proposed "namespace" IDL syntax

Categories

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

defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla49
Tracking Status
firefox49 --- fixed

People

(Reporter: bzbarsky, Assigned: bzbarsky)

References

(Blocks 1 open bug)

Details

(Whiteboard: btpp-active)

Attachments

(3 files, 2 obsolete files)

The idea is that this:

  namespace console {
    // stuff
  };

should act more or less like this:

  interface console {
    // static stuff
  };

with the following differences:

1)  typeof console === "object", not "function".  The call/construct hooks
    should be null.
2)  console.prototype === undefined.  I think we may already end up doing this
    for static-only stuff if we mark it abstract, fwiw.
3)  console.__proto__ == Object.prototype.
Whiteboard: btpp-active
Attachment #8749393 - Flags: review?(peterv)
Andrea, with those two patches you should be able to do:

  [ClassString="Console"]
  namespace console {
    ....
  };
Attachment #8749392 - Attachment is obsolete: true
Attachment #8749392 - Flags: review?(peterv)
Attachment #8749393 - Attachment is obsolete: true
Attachment #8749393 - Flags: review?(peterv)
Attachment #8752244 - Flags: review?(peterv)
I need these patches in order to land bug 989619.
Flags: needinfo?(peterv)
Comment on attachment 8752243 [details] [diff] [review]
Now with support for partial namespace too, per spec updates

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

::: dom/bindings/parser/WebIDL.py
@@ +800,5 @@
> +                    # Just mark all our methods/attributes as static.  The other
> +                    # option is to duplicate the relevant InterfaceMembers
> +                    # production bits but modified to produce static stuff to
> +                    # start with, but that sounds annoying.
> +                    m.forceStatic()

Would it make sense to do this in IDLNamespace's __init__?

@@ +1360,4 @@
>  
>      def addExtendedAttributes(self, attrs):
> +        # Note that this is only used by IDLInterface.  We could
> +        # consider moving this there...

Yeah, I think we should do that.

@@ +5437,5 @@
> +        if not nonPartialObject:
> +            nonPartialObject = nonPartialConstructor(
> +                # No members, False for isKnownNonPartial
> +                *(nonPartialConstructorArgs + [[], False]))
> +        partialInterface = IDLPartialInterface(*(partialConstructorArgs +

Might make sense to rename IDLPartialInterface (since it's now used for namespace too)?

::: dom/bindings/parser/tests/test_namespace.py
@@ +144,5 @@
> +        results = parser.finish()
> +    except Exception, x:
> +        threw = True
> +    harness.ok(threw, "Should have thrown.")
> +    

Trailing whitespace.

@@ +182,5 @@
> +        results = parser.finish()
> +    except Exception, x:
> +        threw = True
> +    harness.ok(threw, "Should have thrown.")
> +    

Trailing whitespace.

@@ +220,5 @@
> +        results = parser.finish()
> +    except Exception, x:
> +        threw = True
> +    harness.ok(threw, "Should have thrown.")
> +    

Trailing whitespace.
Attachment #8752243 - Flags: review?(peterv) → review+
Attachment #8752244 - Flags: review?(peterv) → review+
Flags: needinfo?(peterv)
Attachment #8758313 - Flags: review?(peterv)
> Would it make sense to do this in IDLNamespace's __init__?

That makes partial stuff hard: the members of partials are just stored on those partials until we get to IDLInterfaceOrNamespace.finish.

> Yeah, I think we should do that.

Done.

> Might make sense to rename IDLPartialInterface (since it's now used for namespace too)?

Done, IDLPartialInterfaceOrNamespace.

Fixed the trailing whitespace.
Comment on attachment 8758313 [details] [diff] [review]
part 3.  Add a way to annotate and IDL namespace as needing a new plain object as its prototype, because using Object.prototype as the prototype of 'console' is not web-compatible

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

::: dom/bindings/BindingUtils.h
@@ +3274,5 @@
>  
> +namespace binding_detail {
> +inline
> +JSObject*
> +GetHackedNamespaceProtoObject(JSContext* aCx, JS::Handle<JSObject*> aForObj)

Can we not name the second argument, to make it clear it's unused?
Attachment #8758313 - Flags: review?(peterv) → review+
> Can we not name the second argument, to make it clear it's unused?

Done.
Pushed by bzbarsky@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/9482e25918a3
part 1.  Add Web IDL parser support for IDL namespace syntax.  r=peterv
https://hg.mozilla.org/integration/mozilla-inbound/rev/1d761dc47c2d
part 2.  Add codegen support for IDL namespaces.  r=peterv
https://hg.mozilla.org/integration/mozilla-inbound/rev/508c43bf5578
part 3.  Add a way to annotate and IDL namespace as needing a new plain object as its prototype, because using Object.prototype as the prototype of 'console' is not web-compatible.  r=peterv
Component: DOM → DOM: Core & HTML
You need to log in before you can comment on or make changes to this bug.