sizeToContent() crashes on properly sized window

VERIFIED FIXED in mozilla0.9.4

Status

()

--
critical
VERIFIED FIXED
18 years ago
17 years ago

People

(Reporter: colinp, Assigned: waterson)

Tracking

({crash})

Trunk
mozilla0.9.4
x86
Linux
crash
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: fixed-on-trunk, critical for 0.9.4)

Attachments

(3 attachments, 1 obsolete attachment)

(Reporter)

Description

18 years ago
From Bugzilla Helper:
User-Agent: Mozilla/5.0 (X11; U; Linux 2.2.16 i686; en-US; 0.8.1) Gecko/20010314
BuildID:    stable 0.8 from cvs

when I call window.sizeToContent on a window that is already the correct size
(ie, window.sizeToContent doesn't have to redraw anything becuase the window
hasn't changed size), then Mozilla seg faults:
/package/bin/run-mozilla.sh: line 72:  2630 Segmentation fault


Reproducible: Always

Steps to Reproduce:
1. open a window
2. call window.sizeToContent once
3. call window.sizeToContent twice

Actual Results:  segmentation fault

Expected Results:  nothing should happen (the window hasn't changed size, so it
shouldn't redraw anything)
Mozilla build from 2001-03-16 (debug build).  I get no segmentaion fault (that
was fixed, wasn't it?)  I _do_ get GTK is bailing on us (possibly because we
pass it bad data).  After running sizeToContent() three times (two is not
enough) I get:

Gdk-ERROR **: BadAlloc (insufficient resources for operation)
  serial 8530 error_code 11 request_code 53 minor_code 0
Gdk-ERROR **: BadDrawable (invalid Pixmap or Window parameter)
  serial 8536 error_code 9 request_code 66 minor_code 0
Program exited with code 01.

status -> new, severity -> critical, ccing blizzard in case he has any insight.
Severity: minor → critical
Status: UNCONFIRMED → NEW
Ever confirmed: true
Keywords: crash
Summary: sizeToContent() seg faults on properly sized window → sizeToContent() crashes on properly sized window
Could someone create a testcase for this if it's still crashing? Reassigning to
danm since he's the developer who works on this code, I think...
Assignee: jst → danm

Comment 4

18 years ago
Worksforme on windows95 mozilla0.8. Maybe blizzard's latest patch for Gdk events
did some good to this?
still crashes for me in a morning build from cvs, 2001-03-20
(Reporter)

Comment 6

18 years ago
This never crashed for me under windows, though the window changes size quite
bizarrely (if that's a word).  :)  Still crashes for me under linux.

Comment 7

18 years ago
  It won't crash on Windows because Windows is too smart to fall for it. What's 
happening is, sizeToContent is determining that the window should be 71582962 
pixels square. Goofballs that we are, we dutifully try to make the window that 
big, and X falls down, hard. Yay X.
  Seems like, step one, we need a little error checking on the window sizing 
routine. But the main problem is the bad size. Eric? This one yours? I'm in 
DocumentViewerImpl::SizeToContent. After presShell->ResizeReflow, presContext->
GetVisibleArea returns the rect (0, 0, 0x40000000, 0x40000000). It happens on 
Windows, too, you just don't get a crash there. The test case basically goes

<html><body>
<a href="javascript:window.sizeToContent()">click three times to crash</a>
</body></html>
Assignee: danm → evaughan
X uses 16 bit coordinates.  Not much I can do about that and if I do then I'm
lying to layout about my actual size.  I'd rather see the bug fixed for real and
don't try to resize to any unconstrained size, thanks.

Comment 9

18 years ago
I tested this one on 04-10-05 on Linux & i found really bizzare results.
On first click it reduces window size to content size. On second click It 
maximizes window height back to original height, but it maximizes window width 
beyond screen limits. Its maximizes window width to infinity I guess because no 
matter how much you try to drag window to left side, you'll never see end.

Updated

18 years ago
Target Milestone: --- → mozilla0.9.1

Comment 10

18 years ago
This could be applicable for nsbeta1 because 
1] Its a crash with visibility to user. [& crash is really bizzare.]
2] Method sizeToContent()'s main purpose is getting defeated when window is 
allready correct size.

nominating for nsbeta1. Adding keywords mozilla0.9.1, nsbeta1.
Keywords: mozilla0.9.1, nsbeta1

Comment 11

18 years ago
Hmm.  Very few users call window.sizeToContent, even once.  I need to see 
exact steps for users to reproduce this crash (in the current product or on a 
top100 webpage) without writing any code.  Until then, ->0.9.2
Whiteboard: Need steps for user to repro in product
Target Milestone: mozilla0.9.1 → mozilla0.9.2
The bug this blocks involves calling sizeToContent over and over again on a
dialog.  That is not currently being done because sizeToContent crashes;
therefore the dialog is mis-sized and hard to use.

So steps to reproduce:

1)  Write the new folder dialog the way it should be written if this bug were
    not present.
2)  Try to add new folders on some types of IMAP servers.

Comment 13

18 years ago
Since it doesn't seem to crash in current product, moving ->0.9.3, limbo 
candidate.
Target Milestone: mozilla0.9.2 → mozilla0.9.3
(Reporter)

Comment 14

18 years ago
This crashes all the time under 0.9!!!  It's brutal!  If we're going to be
building apps on xul, this has to work no matter how many times in a row it may
get called (like in a recursive function).

Comment 15

17 years ago
Using the testcasein attachment 27902 [details], and clicking where it says 3 times, I got
this stack dump, using a linux cvs build from today:

#0  0x406bff21 in nsWindow::Resize ()
   from mozilla/dist/bin/components/libwidget_gtk.so
#1  0x4111ae2b in nsView::SetDimensions ()
   from mozilla/dist/bin/components/libgkview.so
#2  0x411212d2 in nsViewManager::SetWindowDimensions ()
   from mozilla/dist/bin/components/libgkview.so
#3  0x41123ab9 in nsViewManager::DispatchEvent ()
   from mozilla/dist/bin/components/libgkview.so
#4  0x4111a1ad in HandleEvent ()
   from mozilla/dist/bin/components/libgkview.so
#5  0x406ba32a in nsWidget::DispatchEvent ()
   from mozilla/dist/bin/components/libwidget_gtk.so
#6  0x406ba255 in nsWidget::DispatchWindowEvent ()
   from mozilla/dist/bin/components/libwidget_gtk.so
#7  0x406b92d9 in nsWidget::OnResize ()
   from mozilla/dist/bin/components/libwidget_gtk.so
#8  0x406c0231 in nsWindow::Resize ()
   from mozilla/dist/bin/components/libwidget_gtk.so
#9  0x4111ae2b in nsView::SetDimensions ()
   from mozilla/dist/bin/components/libgkview.so
#10 0x411212d2 in nsViewManager::SetWindowDimensions ()
   from mozilla/dist/bin/components/libgkview.so
#11 0x41123ab9 in nsViewManager::DispatchEvent ()
   from mozilla/dist/bin/components/libgkview.so
#12 0x4111a1ad in HandleEvent ()
   from mozilla/dist/bin/components/libgkview.so
#13 0x406ba32a in nsWidget::DispatchEvent ()
   from mozilla/dist/bin/components/libwidget_gtk.so
#14 0x406ba255 in nsWidget::DispatchWindowEvent ()
   from mozilla/dist/bin/components/libwidget_gtk.so
#15 0x406b92d9 in nsWidget::OnResize ()
   from mozilla/dist/bin/components/libwidget_gtk.so
Repeats until I got bored at #5500 or so.
(Reporter)

Comment 16

17 years ago
I am still seeing the same thing as Prashant Desale was seeing in April.  If you
create a page that resizes smaller upon the first click, and then back to
original size on the second click, this bug appears.  When the window is resized
the first time it works, however, when you want to resize it back to it's
original size, the window becomes (apparently) infinitely large.

Updated

17 years ago
Target Milestone: mozilla0.9.3 → mozilla0.9.4
(Reporter)

Updated

17 years ago
Keywords: oeone

Updated

17 years ago
Status: NEW → ASSIGNED
Whiteboard: Need steps for user to repro in product → have possible fix. Testing

Comment 17

17 years ago
Ok I have a fix for this one. It also removes one resize reflow from every
window that opens so it might improve performance. Please test this patch to
make sure it doesn't break anything.

Whiteboard: have possible fix. Testing → Have fix. Please test

Comment 19

17 years ago
need R and SR
(Reporter)

Comment 20

17 years ago
Is this fix in the latest build?

I'll gladly test it once it's in a build.
Set milestone to mozilla0.9.5 added nsbranch keyword
Keywords: nsbranch
Target Milestone: mozilla0.9.4 → mozilla0.9.5
sr=blizzard

Comment 23

17 years ago
*** Bug 98100 has been marked as a duplicate of this bug. ***

Updated

17 years ago
Blocks: 98993
OK.  I tried applying that last patch.  On startup I get:

Error reading file
/home/bzbarsky/mozilla/debug/mozilla/dist/bin/chrome/skins/classic/linkToolbarOverlay.css
Error reading file
jar:resource:///chrome/comm.jar!/content/navigator/LinkToolbarHandler.js
WARNING: CSSLoaderImpl::DidLoadStyle: Load of URL
'chrome://linktoolbar/skin/linkToolbarOverlay.css' failed.  Error code: 18, file
nsCSSLoader.cpp, line 933
Failed to load
jar:resource:///chrome/comm.jar!/content/navigator/LinkToolbarHandler.js
Error reading file
jar:resource:///chrome/comm.jar!/content/navigator/LinkToolbarItem.js
Failed to load jar:resource:///chrome/comm.jar!/content/navigator/LinkToolbarItem.js
Error reading file
jar:resource:///chrome/comm.jar!/content/navigator/LanguageDictionary.js
Failed to load
jar:resource:///chrome/comm.jar!/content/navigator/LanguageDictionary.js
After which, of course, the toolbar does not work.  I'm not sure why I'm getting
those error messages; the paths looks fine to me....

This is a linux debug build from 2001-09-10

Comment 25

17 years ago
> OK.  I tried applying that last patch.  On startup I get:

Try unapplying the patch and see if you get them same errors. I think you just
have a bad build.

*** Bug 96401 has been marked as a duplicate of this bug. ***
Whiteboard: Have fix. Please test → Have fix. Please test, critical for 0.9.4
This patch seems to fix problems with XUL dialogs in embedding being resized to
enormous sizes.
(Assignee)

Comment 28

17 years ago
I think this patch will break printing: I'm pretty sure that we do a resize
reflow at the same w&h to force form controls to sync up properly. See
nsDocumentViewer.cpp:2917.
(Assignee)

Comment 29

17 years ago
sizeToContent() calls ResizeReflow() with an unbounded width & height. It relies
on SyncFrameViewAfterReflow() causing a recursive resize-reflow at the correct
width/height to get the visible area properly updated in the pres context.
nsView will stifle window resizing when the width and height do not change, so
the unfortunate call to ResizeReflow() that ends up being non-recursive leaves
the display area unconstrained.

Now it turns out (on Linux at least) that the first size-to-content ends up
being slightly different (off by one pixel) than the second size-to-content. So
the first two end up recurring through ResizeReflow(), and the third one doesn't.

I think that the right way to fix this is to copy the pattern from
InitialReflow() and ensure that we reset the visible area to the desired size
that resulted from this activation of ResizeReflow().
(Assignee)

Comment 30

17 years ago
Created attachment 48950 [details] [diff] [review]
be sure to SetVisibleArea() after getting the desiredSize so failure to recurse when syncing view doesn't leave visible area horked
(Assignee)

Comment 31

17 years ago
Comment on attachment 48950 [details] [diff] [review]
be sure to SetVisibleArea() after getting the desiredSize so failure to recurse when syncing view doesn't leave visible area horked

Oops! Don't look at this!
Attachment #48950 - Attachment is obsolete: true
(Assignee)

Comment 32

17 years ago
Created attachment 48951 [details] [diff] [review]
be sure to SetVisibleArea() after getting the desiredSize so failure to recurse when syncing view doesn't leave visible area horked
(Assignee)

Comment 33

17 years ago
Taking.
Assignee: evaughan → waterson
Status: ASSIGNED → NEW
Target Milestone: mozilla0.9.5 → mozilla0.9.4
r=dbaron.  (InitialReflow does it, and SetVisibleArea isn't used for anything
too scary -- like anything the view system does, which is more like what I
expected from the name.)
Anyone, dbaron in particular: is the off-by-one pixel "jitter" between the first
and second rezize-reflow worth filing as a bug?  I know little about how such
errors arise, but if it's just due to twips-to-pixels and back math, there are
surely fixes.

/be
Chris, your last patch doesn't fix the embedding crash that I'm seeing.  I'm
stell getting resize requests that are way off the scale:

*** size_to_cb 361 214
*** size_to_cb 76695849 76695848
load_finished_cb
*** size_to_cb 76695849 76695848
visibility_cb 1

( of course, the fact that I'm getting three resize requests isn't great either. )
Oh, here's the URL I'm using as a test case:

https://www.theneals.net/irc/irc.cgi
(Assignee)

Comment 38

17 years ago
Using what harness? This seems to work in gtkEmbed.
Yeah, it's probably worth filing a bug on the off-by-one (and attaching a
testcase!).

Updated

17 years ago
Blocks: 99225
(Assignee)

Comment 40

17 years ago
Ok, TestGtkEmbed seems to be fixed with this patch on the trunk; blizzard says
that he's still seeing the problem on the 0.9.4 branch. I'll investigate there...
No longer blocks: 99225
Status: NEW → ASSIGNED
(Assignee)

Comment 41

17 years ago
blizzard, I just tried the patch on the mozilla-0.9.4 branch, and it seems to
fix the problem for me. I ran:

  LD_LIBRARY_PATH=`pwd` ./TestGtkEmbed https://www.theneals.net/irc/irc.cgi

Without the patch, I crash the app, and once wedged the X server so bad I had to
reboot. With the patch, things appear to work properly.
(Assignee)

Updated

17 years ago
Keywords: patch
OK, the patch is working here on the 0.9.4 branch.  I must have been on crack or
something.
Chris, can I check this into the 0.9.4 branch?

Comment 44

17 years ago
sr=attinasi
(Assignee)

Comment 45

17 years ago
Fix checked in on trunk.
Whiteboard: Have fix. Please test, critical for 0.9.4 → fixed-on-trunk, critical for 0.9.4
(Assignee)

Comment 46

17 years ago
Checked in on mozilla-0.9.4 branch, taking blizzard's request as a=.
Status: ASSIGNED → RESOLVED
Last Resolved: 17 years ago
Resolution: --- → FIXED
(Reporter)

Comment 47

17 years ago
what is the complete patch to this bug?
(Assignee)

Comment 48

17 years ago
I believe that attachment 48951 [details] [diff] [review] should be a complete fix.
(Reporter)

Comment 49

17 years ago
really?  a one liner?  almost seems too easy.  :)

Comment 50

17 years ago
Verified on 2001-09-21-04-0.9.4 branch. Needs verification on trunk. Adding 
keyword vtrunk.
Keywords: vtrunk

Comment 51

17 years ago
Verified on trunk 2001092011.

Comment 52

17 years ago
verified on trunk too according to Philip Langdale. Marking verified.
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.