818219 - Replace HTMLElement quickstubs with new binding methods

Status: RESOLVED FIXED

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

Comment 1

[Peter Van der Beken [:peterv]]

Attachment: Support castable types in unions v1

Currently compilation fails for castable interfaces in union types because we don't include the binding headers in UnionConversion.h but then then try to use the PrototypeTraits etc.

Comment 2

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 688401 [details] [diff] [review]
Support castable types in unions v1

r=me

Comment 3

[Peter Van der Beken [:peterv]]

Attachment: Remove hardcoded interface list in workers v1

It's annoying to have to change the dom/worker code just because we added an interface that has a longer prototype chain than the current max.

Comment 4

[Peter Van der Beken [:peterv]]

Attachment: Add a constructor to callbacks to allow conversion between callback types v1

Comment 5

[Peter Van der Beken [:peterv]]

Attachment: Replace HTMLElement quickstubs with new binding methods v1

Comment 6

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 688409 [details] [diff] [review]
Remove hardcoded interface list in workers v1

I like.  Well, not the necessity, but the solution.  ;)

Comment 7

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 688411 [details] [diff] [review]
Add a constructor to callbacks to allow conversion between callback types v1

r=me

Comment 8

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 688413 [details] [diff] [review]
Replace HTMLElement quickstubs with new binding methods v1

>+#define EVENT(name_, id_, type_, struct_) /* nothing; handled by the nsINode */

s/the nsINode/nsINode/

>+    nsCOMPtr<nsIScriptGlobalObject> sgo = do_QueryInterface(win);           \
>+    nsGlobalWindow* globalWin = nsGlobalWindow::FromSupports(sgo);          \

Why not just QI to nsISupports, since FromSupports _has_ to work with the canonical nsISupports?

Same in a few more places.

>+     return new EventHandlerNonNull(errorHandler);                         \

I would much rather we had this method return already_AddRefed<EventHandlerNonNull> and addref inside here as needed, to make sure that the caller is not just sticking it into a raw ptr.

As a side note, maybe we should allow resultNotAddRefed for callbacks?

>+  OnErrorEventHandlerNonNull* errorHandler =                                \
>+    new OnErrorEventHandlerNonNull(handler);                                \

nsRefPtr, please, so we're not passing in a 0-refcount object.

>+++ b/content/html/content/src/nsHTMLBodyElement.cpp

Could you file a followup on using some of the existing nsDOMEventTargetHelper bits in this code?

r=me with the above nits fixed.

Comment 9

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

Comment on attachment 688413 [details] [diff] [review]
Replace HTMLElement quickstubs with new binding methods v1

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

::: content/html/content/src/nsGenericHTMLElement.cpp
@@ +1896,5 @@
> +                                                                              \
> +  return nsINode::SetOn##name_(handler, error);                               \
> +}
> +#include "nsEventNameList.h"
> +#undef ERROR_EVENT  

Trailing whitespace

::: dom/base/nsGlobalWindow.cpp
@@ +11330,5 @@
>  #include "nsEventNameList.h"
>  #undef TOUCH_EVENT
>  #undef WINDOW_ONLY_EVENT
> +#undef BEFOREUNLOAD_EVENT  
> +#undef ERROR_EVENT  

And here

::: dom/base/nsGlobalWindow.h
@@ +673,5 @@
> +#include "nsEventNameList.h"
> +#undef TOUCH_EVENT
> +#undef WINDOW_ONLY_EVENT
> +#undef BEFOREUNLOAD_EVENT  
> +#undef ERROR_EVENT  

And here

Comment 10

[Peter Van der Beken [:peterv]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/a0e79206a110
https://hg.mozilla.org/integration/mozilla-inbound/rev/92187bc8ec48
https://hg.mozilla.org/integration/mozilla-inbound/rev/f23cff8ec6c9
https://hg.mozilla.org/integration/mozilla-inbound/rev/d5179738abfe

Comment 11

[Ed Morley [:emorley]]

https://hg.mozilla.org/mozilla-central/rev/a0e79206a110
https://hg.mozilla.org/mozilla-central/rev/92187bc8ec48
https://hg.mozilla.org/mozilla-central/rev/f23cff8ec6c9
https://hg.mozilla.org/mozilla-central/rev/d5179738abfe