787574 - Add a script for creating stub implementations of Web IDL interfaces

Status: RESOLVED FIXED

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

Comment 1

[(no longer active)]

Attachment: Patch (v1)

Comment 2

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 657460 [details] [diff] [review]
Patch (v1)

A few comments:

1) It would be better to put the class in the mozilla::dom namespace.  That's
   the namespace the bindings expect the implementation class to be in by
   default; if you do it that way, you don't have to indicate the nativeType in
   the conf file.
2) Does it make sense to make GetParentObject pure vitrtual so people have to
   actually make sure to implement it correctly?
3) I would prefer we don't do the "moz" thing.  Just SkeletonBinding.  We
   shouldn't really be prefixing DOM APIs anyway, if we can avoid it.
4) The indent on the WrapObject function header is going to be off depending on
   the length of the classname.  Perhaps just put it all on one line?

r=me with the above at least considered.

Comment 3

[(no longer active)]

(In reply to Boris Zbarsky (:bz) from comment #2)
> Comment on attachment 657460 [details] [diff] [review]
> Patch (v1)
> 
> A few comments:
> 
> 1) It would be better to put the class in the mozilla::dom namespace.  That's
>    the namespace the bindings expect the implementation class to be in by
>    default; if you do it that way, you don't have to indicate the nativeType
> in
>    the conf file.

Good point, I'll do that.

> 2) Does it make sense to make GetParentObject pure vitrtual so people have to
>    actually make sure to implement it correctly?

Hmm, I thought about that...  But I don't really want to advertize that GetParentObject needs to be virtual.  How about I make the body of the function not compile by let's say, |return somethingSensible;|?

> 3) I would prefer we don't do the "moz" thing.  Just SkeletonBinding.  We
>    shouldn't really be prefixing DOM APIs anyway, if we can avoid it.

OK, will do.

> 4) The indent on the WrapObject function header is going to be off depending
> on
>    the length of the classname.  Perhaps just put it all on one line?

Sure, I'll put it all on the same line.

Comment 4

[Boris Zbarsky [:bzbarsky]]

> How about I make the body of the function not compile by let's say, |return
> somethingSensible;|?

Sounds good.

Comment 5

[(no longer active)]

Attachment: Patch for landing

Comment 6

[(no longer active)]

https://hg.mozilla.org/integration/mozilla-inbound/rev/0fc505e515aa

Comment 7

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/0fc505e515aa

Comment 8

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

Comment on attachment 658928 [details] [diff] [review]
Patch for landing

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

::: dom/bindings/stubgenerator/Skeleton.h
@@ +7,5 @@
> +#include "nsWrapperCache.h"
> +#include "nsCycleCollectionParticipant.h"
> +#include "mozilla/Attributes.h"
> +
> +class JSContext;

Nit: I believe some compilers will warn here, because JSContext is actually a struct, not a class.

Comment 9

[(no longer active)]

(In reply to :Ms2ger from comment #8)
> Comment on attachment 658928 [details] [diff] [review]
> Patch for landing
> 
> Review of attachment 658928 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> ::: dom/bindings/stubgenerator/Skeleton.h
> @@ +7,5 @@
> > +#include "nsWrapperCache.h"
> > +#include "nsCycleCollectionParticipant.h"
> > +#include "mozilla/Attributes.h"
> > +
> > +class JSContext;
> 
> Nit: I believe some compilers will warn here, because JSContext is actually
> a struct, not a class.

Thanks, fixed. https://hg.mozilla.org/mozilla-central/rev/8f9db0705d2f