601251 - "ASSERTION: number doesn't implement required interface"

Status: RESOLVED FIXED

(Core :: SVG, defect)

Description

[Jesse Ruderman]

Attachment: testcase (asserts and hangs)

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

Comment 1

[Jesse Ruderman]

Attachment: stack trace

Comment 2

[Jonathan Watt [:jwatt]]

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.

Comment 3

[Robert Longson [:longsonr]]

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.

Comment 4

[Jonathan Watt [:jwatt]]

Ah, okay. Thanks Robert.

Comment 5

[Robert Longson [:longsonr]]

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.

Comment 6

[Robert Longson [:longsonr]]

Attachment: patch

Comment 7

[Robert Longson [:longsonr]]

This patch does numbers and point lists. That's all the list types done except for LengthList which works differently.

Comment 8

[Jonathan Watt [:jwatt]]

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!

Comment 9

[Robert Longson [:longsonr]]

Attachment: patch

This fixes the existing macros per the review comment. I can land the testcase as a crashtest.

Comment 10

[Robert Longson [:longsonr]]

Should probably block since this causes the browser to hang.

Comment 11

[Robert Longson [:longsonr]]

Attachment: with crashtests and check-in comment

Comment 12

[Daniel Holbert [:dholbert]]

Landed: http://hg.mozilla.org/mozilla-central/rev/1792ad44c9d3