826740 - Convert HTMLCanvasElement to WebIDL

Status: RESOLVED FIXED

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

Comment 1

[(no longer active)]

I hit this error when converting canvas:

Traceback (most recent call last):
  File "/Users/ehsanakhgari/moz/mozilla-central/config/pythonpath.py", line 56, in <module>
    main(sys.argv[1:])
  File "/Users/ehsanakhgari/moz/mozilla-central/config/pythonpath.py", line 48, in main
    execfile(script, frozenglobals)
  File "/Users/ehsanakhgari/moz/mozilla-central/dom/bindings/BindingGen.py", line 66, in <module>
    main()
  File "/Users/ehsanakhgari/moz/mozilla-central/dom/bindings/BindingGen.py", line 61, in main
    generate_binding_cpp(config, outputPrefix, webIDLFile);
  File "/Users/ehsanakhgari/moz/mozilla-central/dom/bindings/BindingGen.py", line 29, in generate_binding_cpp
    if replaceFileIfChanged(filename, root.define()):
  File "/Users/ehsanakhgari/moz/mozilla-central/dom/bindings/Codegen.py", line 7058, in define
    return stripTrailingWhitespace(self.root.define())
  File "/Users/ehsanakhgari/moz/mozilla-central/dom/bindings/Codegen.py", line 350, in define
    defn = self.child.define()
  File "/Users/ehsanakhgari/moz/mozilla-central/dom/bindings/Codegen.py", line 350, in define
    defn = self.child.define()
  File "/Users/ehsanakhgari/moz/mozilla-central/dom/bindings/Codegen.py", line 350, in define
    defn = self.child.define()
  File "/Users/ehsanakhgari/moz/mozilla-central/dom/bindings/Codegen.py", line 279, in define
    return self.join(child.define() for child in self.children if child is not None)
  File "/Users/ehsanakhgari/moz/mozilla-central/dom/bindings/Codegen.py", line 275, in join
    return self.joiner.join(filter(lambda s: len(s) > 0, (child for child in generator)))
  File "/Users/ehsanakhgari/moz/mozilla-central/dom/bindings/Codegen.py", line 275, in <genexpr>
    return self.joiner.join(filter(lambda s: len(s) > 0, (child for child in generator)))
  File "/Users/ehsanakhgari/moz/mozilla-central/dom/bindings/Codegen.py", line 279, in <genexpr>
    return self.join(child.define() for child in self.children if child is not None)
  File "/Users/ehsanakhgari/moz/mozilla-central/dom/bindings/Codegen.py", line 350, in define
    defn = self.child.define()
  File "/Users/ehsanakhgari/moz/mozilla-central/dom/bindings/Codegen.py", line 350, in define
    defn = self.child.define()
  File "/Users/ehsanakhgari/moz/mozilla-central/dom/bindings/Codegen.py", line 350, in define
    defn = self.child.define()
  File "/Users/ehsanakhgari/moz/mozilla-central/dom/bindings/Codegen.py", line 350, in define
    defn = self.child.define()
  File "/Users/ehsanakhgari/moz/mozilla-central/dom/bindings/Codegen.py", line 350, in define
    defn = self.child.define()
  File "/Users/ehsanakhgari/moz/mozilla-central/dom/bindings/Codegen.py", line 279, in define
    return self.join(child.define() for child in self.children if child is not None)
  File "/Users/ehsanakhgari/moz/mozilla-central/dom/bindings/Codegen.py", line 275, in join
    return self.joiner.join(filter(lambda s: len(s) > 0, (child for child in generator)))
  File "/Users/ehsanakhgari/moz/mozilla-central/dom/bindings/Codegen.py", line 275, in <genexpr>
    return self.joiner.join(filter(lambda s: len(s) > 0, (child for child in generator)))
  File "/Users/ehsanakhgari/moz/mozilla-central/dom/bindings/Codegen.py", line 279, in <genexpr>
    return self.join(child.define() for child in self.children if child is not None)
  File "/Users/ehsanakhgari/moz/mozilla-central/dom/bindings/Codegen.py", line 6460, in define
    return self.cgRoot.define()
  File "/Users/ehsanakhgari/moz/mozilla-central/dom/bindings/Codegen.py", line 350, in define
    defn = self.child.define()
  File "/Users/ehsanakhgari/moz/mozilla-central/dom/bindings/Codegen.py", line 350, in define
    defn = self.child.define()
  File "/Users/ehsanakhgari/moz/mozilla-central/dom/bindings/Codegen.py", line 350, in define
    defn = self.child.define()
  File "/Users/ehsanakhgari/moz/mozilla-central/dom/bindings/Codegen.py", line 279, in define
    return self.join(child.define() for child in self.children if child is not None)
  File "/Users/ehsanakhgari/moz/mozilla-central/dom/bindings/Codegen.py", line 275, in join
    return self.joiner.join(filter(lambda s: len(s) > 0, (child for child in generator)))
  File "/Users/ehsanakhgari/moz/mozilla-central/dom/bindings/Codegen.py", line 275, in <genexpr>
    return self.joiner.join(filter(lambda s: len(s) > 0, (child for child in generator)))
  File "/Users/ehsanakhgari/moz/mozilla-central/dom/bindings/Codegen.py", line 279, in <genexpr>
    return self.join(child.define() for child in self.children if child is not None)
  File "/Users/ehsanakhgari/moz/mozilla-central/dom/bindings/Codegen.py", line 315, in define
    defn = self.child.define()
  File "/Users/ehsanakhgari/moz/mozilla-central/dom/bindings/Codegen.py", line 736, in define
    return "" if self.inline else self._define()
  File "/Users/ehsanakhgari/moz/mozilla-central/dom/bindings/Codegen.py", line 734, in _define
    return self.definition_prologue() + "\n" + self.definition_body() + self.definition_epilogue()
  File "/Users/ehsanakhgari/moz/mozilla-central/dom/bindings/Codegen.py", line 4321, in definition_body
    self.descriptor, self.method).define()
  File "/Users/ehsanakhgari/moz/mozilla-central/dom/bindings/Codegen.py", line 3864, in __init__
    self.cgRoot = CGList([ CGIndenter(getPerSignatureCall(signature)) ])
  File "/Users/ehsanakhgari/moz/mozilla-central/dom/bindings/Codegen.py", line 3855, in getPerSignatureCall
    method, argConversionStartsAt)
  File "/Users/ehsanakhgari/moz/mozilla-central/dom/bindings/Codegen.py", line 3741, in __init__
    static))
  File "/Users/ehsanakhgari/moz/mozilla-central/dom/bindings/Codegen.py", line 3627, in __init__
    resultAlreadyAddRefed)
  File "/Users/ehsanakhgari/moz/mozilla-central/dom/bindings/Codegen.py", line 3593, in getRetvalDeclarationForType
    raise TypeError("Need to sort out ownership model for union retvals");
TypeError: Need to sort out ownership model for union retvals

I suspect this is because of the getContext function.

Also, I came across a discrepancy which I filed as bug 827138.

Comment 2

[(no longer active)]

Attachment: What I have so far, pretty simple patch

Comment 3

[:Ms2ger (he/him; ⌚ UTC+1/+2)]

Attachment: Part a: General cleanup for HTMLCanvasElement

Comment 4

[:Ms2ger (he/him; ⌚ UTC+1/+2)]

Attachment: Part b: Remove unused arguments from nsIDOMHTMLCanvasElement

Comment 5

[:Ms2ger (he/him; ⌚ UTC+1/+2)]

Attachment: Part c: Pass a JS::Value instead of an nsIVariant to nsIDOMHTMLCanvasElement.toDataURL

Also tried overloading xpc_qsUnwrapArg, but that's only called for objects, while we need it for primitives; and overloading won't work because DOMString and double aren't distinguishable per spec.

Comment 6

[:Ms2ger (he/him; ⌚ UTC+1/+2)]

Attachment: Part d: Use implicit_jscontext to simplify nsIDOMHTMLCanvasElement.getContext

Comment 7

[:Ms2ger (he/him; ⌚ UTC+1/+2)]

Attachment: Part e: Implement the WebIDL API for HTMLCanvasElement

Comment 8

[:Ms2ger (he/him; ⌚ UTC+1/+2)]

Attachment: Part f: Move HTMLCanvasElement to Paris bindings

Comment 9

[:Ms2ger (he/him; ⌚ UTC+1/+2)]

Attachment: Part g: Remove quickstubs

Comment 10

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

Comment on attachment 731860 [details] [diff] [review]
Part b: Remove unused arguments from nsIDOMHTMLCanvasElement

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

r=me with comments addressed

::: dom/interfaces/html/nsIDOMHTMLCanvasElement.idl
@@ -61,5 @@
>    //  toDataURL();              -- defaults to image/png
>    //  toDataURL(type);          -- uses given type
>    //  toDataURL(type, params);  -- uses given type, and any valid parameters
> -  [optional_argc] DOMString toDataURL([optional] in DOMString type,
> -                                      [optional] in nsIVariant params);

This needs an IID change.

Comment 11

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

Comment on attachment 731866 [details] [diff] [review]
Part f: Move HTMLCanvasElement to Paris bindings

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

::: dom/webidl/HTMLCanvasElement.webidl
@@ +23,5 @@
> +  [Pure, SetterThrows]
> +           attribute unsigned long height;
> +
> +  [Throws]
> +  nsISupports? getContext(DOMString contextId, optional any contextOptions);

Why doesn't this return "RenderingContext?"?

@@ +29,5 @@
> +  [Throws]
> +  DOMString toDataURL(optional DOMString type = "",
> +                      optional any encoderOptions);
> +  [Throws]
> +  void toBlob(FileCallback _callback, optional DOMString? type = null);

The spec doesn't have type as a nullable string ... is the spec wrong or are we?

@@ +33,5 @@
> +  void toBlob(FileCallback _callback, optional DOMString? type = null);
> +};
> +
> +// Mozilla specific bits
> +partial interface HTMLCanvasElement {

This is a neat trick.

Comment 12

[:Ms2ger (he/him; ⌚ UTC+1/+2)]

(In reply to Kyle Huey [:khuey] ([e-mail]) from comment #11)
> Comment on attachment 731866 [details] [diff] [review]
> Part f: Move HTMLCanvasElement to Paris bindings
> 
> Review of attachment 731866 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> ::: dom/webidl/HTMLCanvasElement.webidl
> @@ +23,5 @@
> > +  [Pure, SetterThrows]
> > +           attribute unsigned long height;
> > +
> > +  [Throws]
> > +  nsISupports? getContext(DOMString contextId, optional any contextOptions);
> 
> Why doesn't this return "RenderingContext?"?

Bug 858217.

> @@ +29,5 @@
> > +  [Throws]
> > +  DOMString toDataURL(optional DOMString type = "",
> > +                      optional any encoderOptions);
> > +  [Throws]
> > +  void toBlob(FileCallback _callback, optional DOMString? type = null);
> 
> The spec doesn't have type as a nullable string ... is the spec wrong or are
> we?

It doesn't matter, but I'll make this non-nullable.

Comment 13

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

Comment on attachment 731866 [details] [diff] [review]
Part f: Move HTMLCanvasElement to Paris bindings

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

r=me with what we agreed to on irc

Comment 14

[:Ms2ger (he/him; ⌚ UTC+1/+2)]

https://hg.mozilla.org/mozilla-central/rev/a2326f2d2fea
https://hg.mozilla.org/mozilla-central/rev/be3d2fce568d
https://hg.mozilla.org/mozilla-central/rev/61b2c7597a8f
https://hg.mozilla.org/mozilla-central/rev/69516514842f
https://hg.mozilla.org/mozilla-central/rev/cdd115e6d58e
https://hg.mozilla.org/mozilla-central/rev/72844eee2060
https://hg.mozilla.org/mozilla-central/rev/49b4328c90ff