Closed
Bug 830879
Opened 11 years ago
Closed 11 years ago
HTML Element WebIDL bindings require classinfo
Categories
(Core :: DOM: Core & HTML, defect)
Core
DOM: Core & HTML
Tracking
()
RESOLVED
FIXED
mozilla21
People
(Reporter: humph, Assigned: peterv)
References
(Blocks 1 open bug)
Details
Attachments
(1 file, 2 obsolete files)
3.08 KB,
patch
|
bzbarsky
:
review+
|
Details | Diff | Splinter Review |
I've been trying to update the patch in bug 629350, and at the suggestion of Ms2ger, I tried doing it with just HTMLTrackElement.webidl and no nsIDOMHTMLTrackElement.idl, or classinfo additions. I was able to get close, but not close enough. Here is what I ended-up with: HTMLTrackElement.h: ------------------- #include "nsIDOMHTMLElement.h" #include "nsIDOMEventTarget.h" #include "nsGenericHTMLElement.h" #include "nsGkAtoms.h" namespace mozilla { namespace dom { class HTMLTrackElement : public nsGenericHTMLElement, public nsIDOMHTMLElement { public: HTMLTrackElement(already_AddRefed<nsINodeInfo> aNodeInfo) : nsGenericHTMLElement(aNodeInfo) { SetIsDOMBinding(); } virtual ~HTMLTrackElement(); NS_DECL_ISUPPORTS_INHERITED NS_FORWARD_NSIDOMNODE_TO_NSINODE NS_FORWARD_NSIDOMELEMENT_TO_GENERIC NS_FORWARD_NSIDOMHTMLELEMENT_TO_GENERIC ... virtual nsresult Clone(nsINodeInfo *aNodeInfo, nsINode **aResult) const; virtual nsXPCClassInfo* GetClassInfo(); virtual nsIDOMNode* AsDOMNode() { return this; } protected: virtual JSObject* WrapNode(JSContext *aCx, JSObject *aScope, bool *aTriedToWrap) MOZ_OVERRIDE; }; } // namespace dom } // namespace mozilla HTMLTrackElement.cpp: --------------------- #include "mozilla/dom/HTMLTrackElement.h" #include "mozilla/dom/HTMLTrackElementBinding.h" #include "mozilla/ErrorResult.h" #include "nsContentUtils.h" NS_IMPL_NS_NEW_HTML_ELEMENT(Track) namespace mozilla { namespace dom { HTMLTrackElement::~HTMLTrackElement() { } NS_IMPL_ADDREF_INHERITED(HTMLTrackElement, Element) NS_IMPL_RELEASE_INHERITED(HTMLTrackElement, Element) // QueryInterface implementation for HTMLTrackElement NS_INTERFACE_TABLE_HEAD(HTMLTrackElement) NS_HTML_CONTENT_INTERFACE_TABLE1(HTMLTrackElement, nsIDOMHTMLElement) NS_HTML_CONTENT_INTERFACE_TABLE_TO_MAP_SEGUE(HTMLTrackElement, nsGenericHTMLElement) NS_HTML_CONTENT_INTERFACE_MAP_END NS_IMPL_ELEMENT_CLONE(HTMLTrackElement) JSObject* HTMLTrackElement::WrapNode(JSContext* cx, JSObject* scope, bool* triedToWrap) { return HTMLTrackElementBinding::Wrap(cx, scope, this, triedToWrap); } ... This fails to build: 12:27 < humph> is it the 12:27 < humph> virtual nsXPCClassInfo* GetClassInfo(); 12:28 < bz> Yes 12:28 < bz> nuke that Removing that line then produces: 1:00.87 HTMLTrackElement.cpp 1:02.43 /Users/dave/Sites/repos/mozilla-central/content/html/content/src/HTMLTrackElement.cpp:32:23: error: allocating an object of abstract class type 'mozilla::dom::HTMLTrackElement' 1:02.43 NS_IMPL_ELEMENT_CLONE(HTMLTrackElement) 1:02.43 ^ 1:02.43 ../../../../dist/include/mozilla/dom/Element.h:1175:26: note: expanded from macro 'NS_IMPL_ELEMENT_CLONE' 1:02.43 _elementName *it = new _elementName(ni.forget()); \ 1:02.43 ^ 1:02.43 ../../../../dist/include/nsINode.h:1495:27: note: unimplemented pure virtual method 'GetClassInfo' in 'HTMLTrackElement' 1:02.43 virtual nsXPCClassInfo* GetClassInfo() = 0; 1:02.43 ^ 1:02.51 In file included from /Users/dave/Sites/repos/mozilla-central/content/html/content/src/HTMLTrackElement.cpp:6: 1:02.51 In file included from ../../../../dist/include/mozilla/dom/HTMLTrackElement.h:11: 1:02.51 /Users/dave/Sites/repos/mozilla-central/content/html/content/src/nsGenericHTMLElement.h:1805:16: error: allocating an object of abstract class type 'mozilla::dom::HTMLTrackElement' 1:02.51 return new U(aNodeInfo); 1:02.51 ^ 1:02.51 /Users/dave/Sites/repos/mozilla-central/content/html/content/src/HTMLTrackElement.cpp:11:1: note: in instantiation of function template specialization 'mozilla::dom::NewHTMLElementHelper::Create<nsHTMLTrackElement, mozilla::dom::HTMLTrackElement>' requested here 1:02.51 NS_IMPL_NS_NEW_HTML_ELEMENT(Track) 1:02.51 ^ 1:02.51 /Users/dave/Sites/repos/mozilla-central/content/html/content/src/nsGenericHTMLElement.h:1837:10: note: expanded from macro 'NS_IMPL_NS_NEW_HTML_ELEMENT' 1:02.51 return mozilla::dom::NewHTMLElementHelper:: \ 1:02.51 ^ 1:02.51 2 errors generated. 12:29 < Ms2ger> Does anything implement it on the parent chain? 12:29 < humph> new problem 12:30 < Ms2ger> Yeah 12:30 < humph> the clone bit is wrong now 12:30 < Ms2ger> No, it's just new HTMLTrackElement() failing 12:30 < Ms2ger> Because you removed GetClassInfo() :) 12:30 < bz> er.. 12:30 < bz> Why does GetClassInfo affect HTMLTrackElement()? 12:30 < bz> Is it pure virtual? 12:31 < Ms2ger> Yeah 12:31 < bz> damn 12:31 * bz thinks 12:32 < Ms2ger> Would returning null be an issue? 12:32 < bz> So one option is to instead inherit from HTMLElement 12:32 < bz> instead of nsGenericHTMLEleemnt 12:33 < bz> I don't know 12:33 < bz> the GetClassInfo() caller seems to be in xpcObjectHelper 12:33 < bz> fwiw 12:33 < Ms2ger> Yeah 12:33 < Ms2ger> Do we hit that? 12:33 < bz> Not sure 12:34 < bz> Could, for passing these objects to non-webidl methods 12:34 < bz> Or something
Assignee | ||
Comment 1•11 years ago
|
||
I was already working on making this possible, I think this should help.
Assignee: nobody → peterv
Status: NEW → ASSIGNED
Reporter | ||
Comment 2•11 years ago
|
||
This patch, with my current classinfo-less HTMLTrackElement work, let me build successfully: [15:02:55.348] HTMLTrackElement [15:02:55.358] function HTMLTrackElement() { [native code] } And it lives in the DOM too :)
Assignee | ||
Comment 3•11 years ago
|
||
This replaces a compile-time check (pure virtual GetClassInfo()) with a runtime-assertion, but there's no way around that I think.
Attachment #702428 -
Attachment is obsolete: true
Attachment #707525 -
Flags: review?(bzbarsky)
![]() |
||
Comment 4•11 years ago
|
||
Comment on attachment 707525 [details] [diff] [review] v1.1 The assert makes no sense. That code is only reached when mXPCClassInfo is nonnull. Furthermore, this is going to end up doing a CallQI for webidl-binding nodes, for the same reason. Is that purposeful?
Assignee | ||
Comment 5•11 years ago
|
||
Bleh, this seems more like what I wanted to do.
Attachment #707525 -
Attachment is obsolete: true
Attachment #707525 -
Flags: review?(bzbarsky)
Attachment #707781 -
Flags: review?(bzbarsky)
![]() |
||
Comment 6•11 years ago
|
||
Comment on attachment 707781 [details] [diff] [review] v1.2 Yeah, this I buy. r=me
Attachment #707781 -
Flags: review?(bzbarsky) → review+
Comment 7•11 years ago
|
||
Comment on attachment 707781 [details] [diff] [review] v1.2 Review of attachment 707781 [details] [diff] [review]: ----------------------------------------------------------------- ::: js/xpconnect/src/XPCQuickStubs.cpp @@ +1026,5 @@ > +xpcObjectHelper::AssertGetClassInfoResult() > +{ > + MOZ_ASSERT(mXPCClassInfo || > + static_cast<nsINode*>(GetCanonical())->IsDOMBinding(), > + "GetClassInfo() should only return null for new DOM bindings!"); Also assert mIsNode, please.
Assignee | ||
Comment 8•11 years ago
|
||
https://hg.mozilla.org/integration/mozilla-inbound/rev/b48d4a8141bd
Comment 9•11 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/b48d4a8141bd
Status: ASSIGNED → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla21
You need to log in
before you can comment on or make changes to this bug.
Description
•