"ASSERTION: number doesn't implement required interface"

RESOLVED FIXED in mozilla2.0b7

Status

()

Core
SVG
--
critical
RESOLVED FIXED
7 years ago
7 years ago

People

(Reporter: Jesse Ruderman, Assigned: longsonr)

Tracking

(Blocks: 2 bugs, {assertion, hang, testcase})

Trunk
mozilla2.0b7
x86
Mac OS X
assertion, hang, testcase
Points:
---
Dependency tree / graph
Bug Flags:
in-testsuite +

Firefox Tracking Flags

(blocking2.0 final+)

Details

Attachments

(3 attachments, 2 obsolete attachments)

(Reporter)

Description

7 years ago
Created attachment 480254 [details]
testcase (asserts and hangs)

###!!! ASSERTION: number doesn't implement required interface: 'val', 
file content/svg/content/src/nsSVGNumberList.cpp, line 177
(Reporter)

Comment 1

7 years ago
Created attachment 480256 [details]
stack trace
We need to audit all these SVG methods where objects can be passed in and fix them. Jesse, if you find more issues with passing in a JS object literal can you just add the tests here rather than filing new bugs? An audit should sweep them all up, but it would be good to still have the tests to check.
(Assignee)

Comment 3

7 years ago
This is actually different to the other interface stuff Jesse has found. We do have a test for the right object but it does the wrong thing. I think I have a patch for this one.
Ah, okay. Thanks Robert.
(Assignee)

Comment 5

7 years ago
OK I was half right. The cause is the same as usual but it triggers a different set of symptoms as code elsewhere is different. That's why we get into a loop.
(Assignee)

Comment 6

7 years ago
Created attachment 480503 [details] [diff] [review]
patch
Attachment #480503 - Flags: review?(jwatt)
(Assignee)

Comment 7

7 years ago
This patch does numbers and point lists. That's all the list types done except for LengthList which works differently.
Comment on attachment 480503 [details] [diff] [review]
patch

You shouldn't use NS_ENSURE_SUCCESS since that will assert if it fails. I don't think you need to do the rv thing either, you can just null check. Something like this:

#define NS_ENSURE_NATIVE_NUMBER(obj, retval)                 \
  {                                                          \
    nsCOMPtr<nsISVGValue> val = do_QueryInterface(obj);      \
    if (!val) {                                              \
      *retval = nsnull;                                      \
      return NS_ERROR_DOM_SVG_WRONG_TYPE_ERR);               \
    }                                                        \
  }

r=jwatt with the replacement of NS_ENSURE_SUCCESS with suitable code, whether the above or not. Thanks Robert!
Attachment #480503 - Flags: review?(jwatt) → review+
(Assignee)

Comment 9

7 years ago
Created attachment 480708 [details] [diff] [review]
patch

This fixes the existing macros per the review comment. I can land the testcase as a crashtest.
Assignee: nobody → longsonr
Attachment #480503 - Attachment is obsolete: true
Attachment #480708 - Flags: approval2.0?
(Assignee)

Comment 10

7 years ago
Should probably block since this causes the browser to hang.
blocking2.0: --- → ?

Updated

7 years ago
blocking2.0: ? → final+
(Assignee)

Updated

7 years ago
Attachment #480708 - Flags: approval2.0?
(Assignee)

Updated

7 years ago
Blocks: 603145
(Assignee)

Comment 11

7 years ago
Created attachment 482238 [details] [diff] [review]
with crashtests and check-in comment
Attachment #480708 - Attachment is obsolete: true
(Assignee)

Updated

7 years ago
Blocks: 499879
(Assignee)

Updated

7 years ago
Keywords: checkin-needed
Whiteboard: [needs landing]
Landed: http://hg.mozilla.org/mozilla-central/rev/1792ad44c9d3
Status: NEW → RESOLVED
Last Resolved: 7 years ago
Keywords: checkin-needed
Resolution: --- → FIXED
Whiteboard: [needs landing]
Target Milestone: --- → mozilla2.0b8
(Assignee)

Updated

7 years ago
Flags: in-testsuite+
Target Milestone: mozilla2.0b8 → mozilla2.0b7
You need to log in before you can comment on or make changes to this bug.