Closed
Bug 1230092
Opened 8 years ago
Closed 8 years ago
"ASSERTION: QueryInterface needed" when printing fails
Categories
(Core :: DOM: Events, defect)
Core
DOM: Events
Tracking
()
RESOLVED
FIXED
mozilla45
Tracking | Status | |
---|---|---|
firefox45 | --- | fixed |
People
(Reporter: jruderman, Assigned: bzbarsky)
References
Details
(Keywords: assertion, testcase)
Attachments
(3 files, 1 obsolete file)
###!!! ASSERTION: QueryInterface needed: 'query_result.get() == mRawPtr', file nsCOMPtr.h, line 415 Based on the stack, I think there are two things going wrong here: 1) When there's a <canvas height="0">, printing fails. 2) When printing fails, "ASSERTION: QueryInterface needed". This bug is about #2. I'll file a bug on #1 once this is fixed.
Reporter | ||
Comment 1•8 years ago
|
||
Assignee | ||
Comment 2•8 years ago
|
||
The printing code is creating an event via NS_NewDOMCustomEvent. This asserts when assigned to nsIDOMEvent. The reason for that is that CustomEvent multiply-inherits from nsIDOMEvent. One chain goes through nsIDOMCustomEvent. The other chain goes through Event. The QI impl for CustomEvent delegates everything except nsIDOMCustomEvent to Event. So the canonical nsIDOMEvent for a CustomEvent is the one Event inherits from. But the printing code directly passes its nsIDOMCustomEvent to AsyncEventDispatcher, which effectively static_casts it to the non-canonical nsIDOMEvent. We should either make nsIDOMCustomEvent not inherit from nsIDOMEvent or ... something. Changing the QI impl of CustomEvent would help with this case, but not with a case where someone else has an Event and assigns it to nsIDOMEvent and that Event turns out to be a CustomEvent.
Component: DOM → DOM: Events
Comment 3•8 years ago
|
||
All the nsIDOM*Event interfaces inherit from nsIDOMEvent. The printing code should just use nsRefPtr<CustomEvent> event.
Comment 4•8 years ago
|
||
(it is still a bit fragile, but hopefully we'll get rid of nsIDOM*Event interfaces at some point)
Assignee | ||
Comment 5•8 years ago
|
||
> The printing code should just use nsRefPtr<CustomEvent> event.
Well, plus explicit casting to nsIDOMEvent via Event or explicit QI to nsIDOMEvent, right? I have a patch that just nixes the inheritance (and uses CustomEvent directly in the print code).
Assignee | ||
Comment 6•8 years ago
|
||
Attachment #8695340 -
Flags: review?(bugs)
Assignee | ||
Updated•8 years ago
|
Assignee: nobody → bzbarsky
Status: NEW → ASSIGNED
Comment 7•8 years ago
|
||
In other cases we just seem to static_cast<Event*>() when passing CustomEvent to some dispatcher
Assignee | ||
Comment 8•8 years ago
|
||
> In other cases we just seem to static_cast<Event*>()
Sure. I can remove that noise if you like, with the IDL change above...
Comment 9•8 years ago
|
||
yeah, please. it is just that other nsIDOM*Events inherit nsIDOMEvent, so making a special case there feels odd.
Assignee | ||
Comment 10•8 years ago
|
||
> it is just that other nsIDOM*Events inherit nsIDOMEvent
We should, imo, fix that too, but some of the others might take a bit more work.
Assignee | ||
Comment 11•8 years ago
|
||
Attachment #8695349 -
Flags: review?(bugs)
Assignee | ||
Updated•8 years ago
|
Attachment #8695340 -
Attachment is obsolete: true
Attachment #8695340 -
Flags: review?(bugs)
Comment 12•8 years ago
|
||
Comment on attachment 8695349 [details] [diff] [review] Make nsIDOMCustomEvent no longer inherit from nsIDOMEvent, so that CustomEvent doesn't end up with multiple inheritance from nsIDOMEvent ok, fine. I'll file a followup to clean up other interfaces or get rid of nsIDOM*Events
Attachment #8695349 -
Flags: review?(bugs) → review+
Reporter | ||
Comment 13•8 years ago
|
||
> The reason for that is that CustomEvent multiply-inherits from nsIDOMEvent.
Can we catch these bugs earlier? Or show a more precise error message than "QueryInterface needed"? Is it always a bug when a class multiply-inherits from an XPIDL or WebIDL interface?
Assignee | ||
Comment 14•8 years ago
|
||
> Can we catch these bugs earlier? Not very easily. > Or show a more precise error message than "QueryInterface needed"? Well. We can show the type of interface involved, but you can see that anyway from the nsCOMPtr<whatever> listed in the stack. Showing the concrete type of the thing we got assigned is hard without RTTI, right? > Is it always a bug when a class multiply-inherits from an XPIDL No. In fact, it's quite common with nsISupports, for example (though nsCOMPtr<nsISupports> is, I believe, specialized to not assert this way). In most other cases it's not great, but also pretty hard to avoid for some of the old nsIDOM* APIs. The right answer is to get rid of all those APIs. > or WebIDL interface? There is no concept of a C++ class inheriting from a WebIDL interface. ;)
Comment 16•8 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/ac9aa8dab115
Status: ASSIGNED → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla45
You need to log in
before you can comment on or make changes to this bug.
Description
•