Closed
Bug 1230092
Opened 7 years ago
Closed 7 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•7 years ago
|
||
![]() |
Assignee | |
Comment 2•7 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•7 years ago
|
||
All the nsIDOM*Event interfaces inherit from nsIDOMEvent. The printing code should just use nsRefPtr<CustomEvent> event.
Comment 4•7 years ago
|
||
(it is still a bit fragile, but hopefully we'll get rid of nsIDOM*Event interfaces at some point)
![]() |
Assignee | |
Comment 5•7 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•7 years ago
|
||
Attachment #8695340 -
Flags: review?(bugs)
![]() |
Assignee | |
Updated•7 years ago
|
Assignee: nobody → bzbarsky
Status: NEW → ASSIGNED
Comment 7•7 years ago
|
||
In other cases we just seem to static_cast<Event*>() when passing CustomEvent to some dispatcher
![]() |
Assignee | |
Comment 8•7 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•7 years ago
|
||
yeah, please. it is just that other nsIDOM*Events inherit nsIDOMEvent, so making a special case there feels odd.
![]() |
Assignee | |
Comment 10•7 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•7 years ago
|
||
Attachment #8695349 -
Flags: review?(bugs)
![]() |
Assignee | |
Updated•7 years ago
|
Attachment #8695340 -
Attachment is obsolete: true
Attachment #8695340 -
Flags: review?(bugs)
Comment 12•7 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•7 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•7 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•7 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/ac9aa8dab115
Status: ASSIGNED → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla45
You need to log in
before you can comment on or make changes to this bug.
Description
•