878325 - XUL template builder needs to do a better job propagating exceptions

Status: NEW

(Core :: DOM: Core & HTML, defect)

Description

[Bobby Holley (:bholley)]

I'm doing some work that causes WrapNative to throw more often, in cases where it should. But it's uncovered a few consumers that don't do a good job of propagating exceptions all the way up. This stuff is one of them.

Comment 1

[Bobby Holley (:bholley)]

Attachment: Part 1 - Propagate exceptions out of CreateTemplateBuilder and LoadDataSources. v1

Comment 2

[Bobby Holley (:bholley)]

Attachment: Part 2 - Propagate exceptions out of ResolveForwardReferences. v1

Comment 3

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 756837 [details] [diff] [review]
Part 1 - Propagate exceptions out of CreateTemplateBuilder and LoadDataSources. v1

r=me

Comment 4

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 756838 [details] [diff] [review]
Part 2 - Propagate exceptions out of ResolveForwardReferences. v1

Let's hope this doesn't break some weird edge case....

Comment 5

[Bobby Holley (:bholley)]

remote:   https://hg.mozilla.org/integration/mozilla-inbound/rev/d4a39e18e5a7
remote:   https://hg.mozilla.org/integration/mozilla-inbound/rev/803586f0a297

Comment 6

[Ryan VanderMeulen [:RyanVM]]

Backed out for mochitest-other orange.
https://hg.mozilla.org/integration/mozilla-inbound/rev/0a6f4bdb34bf

https://tbpl.mozilla.org/php/getParsedLog.php?id=23726714&tree=Mozilla-Inbound

Comment 7

[Bobby Holley (:bholley)]

Hm, so this busted two tests:

content/xul/templates/tests/chrome/test_tmpl_invalidqp.xul
content/xul/templates/tests/chrome/test_tmpl_storage_baddatasource.xul

It looks like these things are both designed to throw, but are somehow getting tripped up where they weren't before.

I really don't know enough about this code to debug it though. Enn, can you apply the patches and have a look?

Comment 8

[Neil Deakin]

The code here has its own logging which one can enable, The two specific errors being tested for here should really just silently fail when the logging is disabled.

Comment 9

[Neil Deakin]

Attachment: Something like this

However, since opening the test seems to cause the test's page to never stop loading, there is some other bug here as well. I assume an error result from XULDocument::ResolveForwardReferences isn't causing some cleanup to occur properly.

Comment 10

[Bobby Holley (:bholley)]

If you squelch exceptions, you need to call JS_ClearPendingException.