364801 - ASSERTION: Some frame destructors were not called with this testcase that makes scrollbars disappear

Status: VERIFIED FIXED

(Core :: Layout, defect, P5)

Description

[Martijn Wargers (dead)]

Marking security sensitive because similar assertions because are also security sensitive.

See upcoming testcase.
When hovering one of the scrollbars, they disappear. When you reload the page, you get the assertion:
###!!! ASSERTION: Some frame destructors were not called: 'mFrameCount == 0', fi
le c:/mozilla/mozilla/layout/base/nsPresShell.cpp, line 623

Comment 1

[Martijn Wargers (dead)]

Attachment: testcase

Comment 2

[Jesse Ruderman]

I can reproduce this bug on Mac trunk.  The assertion text has changed since the bug was filed, but the assertion still fires.

Is this instance of the assertion an indicator of a security hole?

Comment 3

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

So the immediate problem here is that we should not be allowing mousemove events targeted at scrollbar parts to bubble up to Web content. Olli, any suggestions about how to fix that? Or if it's already supposed to be fixed, what might have broken?

Comment 4

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

We should probably fix the underlying bug too, of course. In the testcase we actually remove the slider first (because it's on top) and then once that's gone we generally get another mouseover on the scrollbar and remove that. It's removing the scrollbar that triggers the frame-destructor-not-called assertions.

Comment 5

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

Attachment: fix the frame destructors warning

This turned out to be simple ... nsHTMLScrollFrame::RemoveFrame did not destroy the frame as it's required to. (nsXULScrollFrame::RemoveFrame is fine though.)

Comment 6

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

We should probably spin off another bug about scrollbar components being exposed to Web content, although I have a feeling one already exists...

Comment 7

[Martijn Wargers (dead)]

Yeah, I think that's bug 251197.

Comment 9

[Mats Palmgren (inactive)]

Comment on attachment 286245 [details] [diff] [review]
fix the frame destructors warning

r+sr=mats

Comment 10

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

checked in

Comment 11

[Daniel Veditz [:dveditz]]

Do we need this patch on the 1.8 branch?
What severity should we rate this as a security bug? (didn't see signs of crashing of other exploitable conditions)

Comment 12

[Daniel Veditz [:dveditz]]

guessing possibly sg:critical based on the symptoms of dupe 399411

Comment 13

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

Comment on attachment 286245 [details] [diff] [review]
fix the frame destructors warning

 The patch should apply to branch very easily.

Comment 14

[Daniel Veditz [:dveditz]]

Comment on attachment 286245 [details] [diff] [review]
fix the frame destructors warning

approved for 1.8.1.12, a=dveditz for release-drivers

Comment 15

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

checked in

Comment 16

[Smokey Ardisson (offline for a while; not following bugs - do not email)]

The MOZILLA_1_8_BRANCH is still burning from this patch, even after roc checked in a bustage fix :(

Comment 17

[Mats Palmgren (inactive)]

I checked in a bustage fix, should go green next cycle.

Comment 18

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

Sorry about that

Comment 19

[Carsten Book [:Tomcat]]

verified fixed on trunk with the testcase and Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.5; en-US; rv:1.9b3pre) Gecko/2008012917 Firefox/3.0b3pre ID:2008012917

Comment 20

[Stephen Donner [:stephend] Not actively reading bugmail]

The scrollbars disappear for me with the testcase at https://bugzilla.mozilla.org/attachment.cgi?id=249508 with both:

Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.8.1.12) Gecko/20080128 Firefox/2.0.0.12

-and-

Mozilla/5.0 (Windows; U; Windows NT 6.0; en-US; rv:1.8.1.11) Gecko/20071127 Firefox/2.0.0.11

Per comment 6, I think this means that the scrollbars are expected to disappear on hover even with the 1.8.1.12's 2.0.0.12 build; if so, cool, but I still can't verify on opt, since this is an assertion.

Martijn, do you have cycles to verify this on the 1.8.1.12 branch, with a debug build?

Comment 21

[Martijn Wargers (dead)]

Bug 334514 didn't go on the branch, so I wouldn't get that assertion anyway in my debug branch build, but I verified that the change was in my source, so this really should be fixed on branch.

Comment 22

[Alexander Sack]

Comment on attachment 286245 [details] [diff] [review]
fix the frame destructors warning

a=asac for 1.8.0.15

Comment 23

[Alexander Sack]

Attachment: 1.8.0 version of patch

caillon, please review. this has just a tiny adaption because of changed API.

Comment 24

[Alexander Sack]

err, sign off i mean :)

Comment 25

[Christopher Aillon (sabbatical, not receiving bugmail)]

Comment on attachment 310232 [details] [diff] [review]
1.8.0 version of patch

a=caillon for 1.8.0.15

Comment 26

[Christopher Aillon (sabbatical, not receiving bugmail)]

Committed to 1.8.0 branch