582441 - On loading iframe with data URI: "WARNING: NS_ENSURE_TRUE(chromeWindow) failed: file nsFrameLoader.cpp, line 1905"

Status: NEW

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

Description

[Daniel Holbert [:dholbert]]

Attachment: reduced testcase

* STEPS TO REPRODUCE:
Load attached testcase (or any page with an <iframe src=[data URI]>) in a mozilla-central debug build.

* RESULTS: This warning is spammed to stderr:
> WARNING: NS_ENSURE_TRUE(chromeWindow) failed: file ../../../../mozilla/content/base/src/nsFrameLoader.cpp, line 1905

The code in question is:
> 1869 NS_IMETHODIMP
> 1870 nsFrameLoader::GetMessageManager(nsIChromeFrameMessageManager** aManager)
> 1871 {
[...]
> 1903   nsCOMPtr<nsIDOMChromeWindow> chromeWindow =
> 1904     do_QueryInterface(mOwnerContent->GetOwnerDoc()->GetWindow());
> 1905   NS_ENSURE_STATE(chromeWindow);
http://mxr.mozilla.org/mozilla-central/source/content/base/src/nsFrameLoader.cpp?mark=1903-1905#1870

Setting this as blocking bug 549682, since that bug added this line of code.

Comment 1

[Olli Pettay [:smaug][[e-mail]]]

So? Are you filing bugs because of warnings?

Or is there something actually broken?

Comment 2

[Daniel Holbert [:dholbert]]

I filed this because I have a series of new reftests for bug 276431, which all trigger ~16 lines of this warning for each line of useful output. (and this makes it harder to skim their reftest logs for failures & for other useful output)

In each of the reftests, I have a grid of ~16 of these:
  <img src="[dynamically-generated data URI for a simple SVG document]">
to check a variety of different possibilities all in a single test. (e.g. viewBox present/absent, preserveAspectRatio present/absent, height present/absent on <svg> element, height present/absent on <img> container, & same for width)

But this warning gets spammed for each of my <img> elements (presumably for the same reason that it gets spammed for an <iframe>), and so I get ~16 lines of this warning-noise between each useful line of output.

Of course I could grep for REFTEST when saving the log, but that would also filter out any actually-useful warnings, which I'd like to look for to know whether I'm breaking anything.

Comment 3

[Daniel Holbert [:dholbert]]

(In reply to comment #1)
> Or is there something actually broken?

I don't know the code in question, so I don't actually know whether this warning is scary & indicative of things being broken.  I was hoping you might have a hunch about that. :)

If it's not something to worry about, then I'd advocate for expanding the NS_ENSURE_STATE() to the equivalent null-check + return-statement, because IMHO we shouldn't be using NS_ENSURE_XXX in cases where we know that it's going to fail+spam under normal circumstances.  (particularly when it spams many times for a single useful testcase, as in comment 2)

Comment 4

[Firefox Bug Husbandry Bot]

https://bugzilla.mozilla.org/show_bug.cgi?id=1472046

Move all DOM bugs that haven’t been updated in more than 3 years and has no one currently assigned to P5.

If you have questions, please contact :mdaly.