534478 - every printing reftest causes assertions "Wrong scope, this is really bad!" and "This will screw up our existing presentation"

Status: RESOLVED FIXED

(Core :: Layout, defect)

Description

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Every reftest-print reftest causes two assertions:

###!!! ASSERTION: This will screw up our existing presentation: '!mPresShell && !mPresContext && !mWindow', file /builds/slave/mozilla-central-linux-debug/build/layout/base/nsDocumentViewer.cpp, line 2427

###!!! ASSERTION: Wrong scope, this is really bad!: 'JS_GetGlobalForObject(cx, obj) == newScope', file /builds/slave/mozilla-central-linux-debug/build/content/base/src/nsDocument.cpp, line 3654


Steps to reproduce:
 cd $(objdir)/_tests/reftest
 python ./runreftest.py $(srcdir)/layout/reftests/bugs/reftest.list

you should hit the assertions first on layout/reftests/bugs/200774-1.html .  (You can trim the rest out of the reftest.list.)


Marking security-sensitive given the second assertion, although it's possible this is actually a reftest harness bug.

This has been happening for at least 3 weeks; it's not new.

Comment 1

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Attachment: assertion stacks

Comment 2

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

I added a hack for this in the reftest harness (for the assertion counting):
http://hg.mozilla.org/mozilla-central/rev/4700e3c42868
that will need to be removed when this is fixed.

Comment 3

[Jesse Ruderman]

Still happens on trunk even now that bug 500882 is fixed.

Comment 5

[Brandon Sterne (:bsterne)]

I'm not sure what severity this deserves.  We've never assigned a severity to any of our other "Wrong scope" assertion bugs.  dbaron, any thoughts?

Comment 6

[Robert O'Callahan (:roc) (email my personal email if necessary)]

smaug, what do you think about this?

Comment 7

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

So reftest is the only one which calls setPageMode, and that method has even
comment that it is only partially working.

Comment 8

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

Attachment: patch

This should do it. One assertion is just wrong, and we don't need
to re-set document to window.

I need to check what to do with reftest.js. For some reason, not clear from
hg blame, there is this var count = (xr.widgetToolkit == "cocoa") ? 3 : 2;

Comment 9

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Comment on attachment 433835 [details] [diff] [review]
patch

r=dbaron

Comment 10

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

David, do you happen to remember why there is the '3' for cocoa?

I'm just building on OSX so that I can check the difference, but I'm not
100% which assertion I should be looking for.

Comment 11

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

I assume the assertion is
##!!! ASSERTION: nsPrintSettingsX::ReadPageFormatFromPrefs()

Comment 12

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

Attachment: + reftest.js change

Comment 13

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

(In reply to comment #10)
> David, do you happen to remember why there is the '3' for cocoa?
> 
> I'm just building on OSX so that I can check the difference, but I'm not
> 100% which assertion I should be looking for.

(In reply to comment #11)
> I assume the assertion is
> ##!!! ASSERTION: nsPrintSettingsX::ReadPageFormatFromPrefs()

Sounds right.  I remember that there was an additional assertion in Mac OS X-specific code on all the printing reftests.

Comment 14

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

http://hg.mozilla.org/mozilla-central/rev/be5a3a430fb1