255748 - Mozilla hang when print http://kr.yahoo.com

Status: RESOLVED FIXED

(Core :: Printing: Output, defect)

Description

[Pete Zha]

Steps:
1) Go to http://kr.yahoo.com
2) Click File->Print
3) Click "Print" in the dialog
Mozilla will hang

expect result: not hang and can print the page

Comment 1

[Pete Zha]

It hangs at here:
http://lxr.mozilla.org/seamonkey/source/layout/base/src/nsSpaceManager.cpp#371
371     while (nsnull != band) {
372       if (band->mTop > y) {
373         // The band is below the y-offset. The area between the y-offset and
374         // the top of the band is available
375         aBandData.mCount = 1;
376         aBandData.mTrapezoids[0] =
377           nsRect(0, aYOffset, aMaxSize.width, PR_MIN(band->mTop - y,
aMaxSize.height));
378         aBandData.mTrapezoids[0].mState = nsBandTrapezoid::Available;
379         aBandData.mTrapezoids[0].mFrame = nsnull;
380         break;
381       } else if (y < band->mBottom) {
382         // The band contains the y-offset. Return a list of available and
383         // unavailable rects within the band
384         return GetBandAvailableSpace(band, y, aMaxSize, aBandData);
385       } else {
386         // Skip to the next band
387         band = GetNextBand(band);
388       }
389     }
390   }

Comment 2

[Nathan Strom]

I also see a hang here; "Preparing..." comes up and then firefox is pegged at
100% processor usage. Using Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US;
rv:1.7) Gecko/20040803 Firefox/0.9.3; printer is HP LaserJet 6P on a parallel
port, accessed over a Windows network.

Comment 3

[Ed Catmur]

(In reply to comment #1)

I think the hang is actually at
http://lxr.mozilla.org/seamonkey/source/layout/html/base/src/nsBlockReflowState.cpp#782
782         for (;;) {
783           // Get the available space at the new Y coordinate
784           mY += mAvailSpaceRect.height;
785           GetAvailableSpace();
786 
787           if (0 == mBand.GetFloatCount()) {
788             // Winner. This band has no floats on it, therefore
789             // there can be no overlap.
790             break;
791           }
792 
793           // Check and make sure the float won't intersect any
794           // floats on this band. The floats starting and ending
795           // coordinates must be entirely in the available space.
796           if ((xa < mAvailSpaceRect.x) || (xb > mAvailSpaceRect.XMost())) {
797             // The float can't go here.
798             result = PR_FALSE;
799             break;
800           }
801 
802           // See if there is now enough height for the float.
803           if (yb < mY + mAvailSpaceRect.height) {
804             // Winner. The bottom Y coordinate of the float is in
805             // this band.
806             break;
807           }
808         }

The problem appears to be that mAvailSpaceRect.height is 0, so the y coordinate
in mY is not incremented and nothing happens in the loop.

Comment 4

[Ed Catmur]

In support of this, hacking nsBlockReflowState.cpp:

diff -u -p -r3.484 nsBlockReflowState.cpp
--- layout/html/base/src/nsBlockReflowState.cpp 31 Jul 2004 23:15:10 -0000     3.484
+++ layout/html/base/src/nsBlockReflowState.cpp 18 Aug 2004 18:10:29 -0000
@@ -782,6 +782,8 @@ nsBlockReflowState::CanPlaceFloat(const
         for (;;) {
           // Get the available space at the new Y coordinate
           mY += mAvailSpaceRect.height;
+          if (mAvailSpaceRect.height == 0)
+            mY += 1;
           GetAvailableSpace();

           if (0 == mBand.GetFloatCount()) {

allows the page to be laid out successfully (as far as I can tell; I can't read
Korean). This is probably not the correct fix, though.

Comment 5

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

mAvailSpaceRect should never have a height of 0.

Comment 6

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

It shouldn't, but sometimes it happens.

I think we should add an assertion and check in Ed's patch.

Comment 7

[Pete Zha]

Yes, this hack fix the hang on trunk, although the layout is a little bit
strange. The first page is almost empty and only have the banner. Most of the
information are on the second and rest pages.

This hack doesn't fix the 1.7 branch. Mozilla will crash at nsBlockFrame.cpp here:
3289 #ifdef DEBUG
3290     spins++;
3291     if (1000 == spins) {
3292       ListTag(stdout);
3293       printf(": yikes! spinning on a line over 1000 times!\n");
3294       NS_ABORT();
3295     }
3296 #endif
3297 
3298   } while (NS_SUCCEEDED(rv) && LINE_REFLOW_REDO == lineReflowStatus);
3299 
3300   return rv;
in release build, Mozilla will hang here since the lineReflowStatus is always
LINE_REFLOW_REDO with the hack.

I wonder if I can break if the spins is over 1000. Does it has any side effect?

Comment 8

[Pete Zha]

Attachment: patch

Comment 9

[Pete Zha]

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

This is Ed's patch. Please review

Comment 10

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

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

Ed can't review this.

Add an NS_ASSERTION here to indicate that this should not be happening. With
that, I'd be OK with the patch.

Comment 11

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

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

I don't think it should be hard to fix whatever is causing mAvailSpaceRect to
have a height of 0.

Comment 12

[James Napolitano]

Attachment: testcase

A testcase in case this helps.	Hangs on print preview.

Comment 13

[Mats Palmgren (inactive)]

The testcase works fine in Mozilla 2004-12-16-06 trunk Linux.
URL still hangs though. It also hangs in a debug build and after applying the
patch I instead get a crash:
#7  0x40e66b44 in nsLineBox::IsEmpty() const (this=0x0) at nsLineBox.cpp:286
#8  0x40e66bc3 in nsLineBox::CachedIsEmpty() (this=0x893c240) at nsLineBox.cpp:303
#9  0x40e208ab in nsBlockFrame::ReflowDirtyLines(nsBlockReflowState&)
(this=0x89357fc, aState=@0xbfff895c) at nsBlockFrame.cpp:2301
...

The problem at the URL is still the same, mAvailSpaceRect.height == 0 
in the loop at the end of CanPlaceFloat.

Comment 14

[Mats Palmgren (inactive)]

*** Bug 263800 has been marked as a duplicate of this bug. ***

Comment 15

[Mats Palmgren (inactive)]

Attachment: Testcase #1 (hang in CanPlaceFloat)

Derived from http://kr.yahoo.com/ - assertion before the hang:

###!!! ASSERTION: no prev in flow: 'PR_FALSE', file nsBlockFrame.cpp, line 5691


This testcase spins in nsBlockReflowState::CanPlaceFloat with
mAvailSpaceRect.height == 0

Comment 16

[Mats Palmgren (inactive)]

Attachment: Testcase #2 (hang in FlowAndPlaceFloat)

Derived from http://kr.yahoo.com/ - assertion before the hang:

###!!! ASSERTION: Out of flow frame doesn't have the expected parent:
'outOfFlowFrame->GetParent() == aBlockParent', file nsBlockFrame.cpp, line 7051


This testcase spins in nsBlockReflowState::FlowAndPlaceFloat with
mAvailSpaceRect.height == 0

Comment 17

[Mats Palmgren (inactive)]

Attachment: Testcase #3 (hang in CanPlaceFloat)

Derived from
http://www.creativekitchenonline.com/story/waffles/recipes/belgian/
(bug 263800) - no assertion before the hang.
Spins in CanPlaceFloat with mAvailSpaceRect.height == 0.

Comment 18

[Mats Palmgren (inactive)]

Bailing out of the loops when the available height is zero leads to crashes
at various places, I tried to avoid some of those only to get new crashes...

There was one crash that could be worth fixing, regardless of what is being
done in this bug. When nsBlockFrame::SplitPlaceholder is called on a frame
that has a next-in-flow it will lead to a crash at line 4161:
http://bonsai.mozilla.org/cvsblame.cgi?file=mozilla/layout/generic/nsBlockFrame.cpp&rev=3.680&root=/cvsroot&mark=4155-4161#4152

because nsHTMLContainerFrame::CreateNextInFlow return NS_OK but with a null
frame in that case:
http://bonsai.mozilla.org/cvsblame.cgi?file=mozilla/layout/generic/nsHTMLContainerFrame.cpp&rev=3.205&root=/cvsroot&mark=329,330,350#322

Maybe we should add an assert and return early in SplitPlaceholder instead?

For this bug though, I think David's suggestion in comment 11 to find
the reason mAvailSpaceRect.height is zero is the way forward...

Comment 19

[Mats Palmgren (inactive)]

The SplitPlaceholder problem as described in comment 18 seems to be the 
cause for the crash in bug 265867.

Comment 20

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

I'm thinking that it might be best to get rid of placeholder splitting. I think
David was in favour of getting rid of it but I resisted, but now I'm not sure
I'm right.

Comment 21

[Martijn Wargers (dead)]

I think this is fixed by bug 263825. Not any of the testcases or the url crashes
on print preview for me in the 20050223 build.
I've tested testcase 1 in the 2005022 build on print preview and in that build
it crashed.

Comment 22

[Martijn Wargers (dead)]

Oops! I meant the 20050323 build and the 20050322 build of course in comment 21.

Comment 23

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

That makes sense. I added code to handle the availableHeight == 0 cases in float
placement.