25336 - Loading test4.html (with nsViewManager) makes mozilla loop forever...

Status: VERIFIED WORKSFORME

(Core :: Web Painting, defect, P3)

Description

[jst]

... while assering like crazy at

###!!! Break: at file nsViewManager.cpp, line 1066
###!!! ASSERTION: underflow: '!((PRInt32)pushcnt < 0)',
file nsViewManager.cpp, line 1066

Comment 1

[jst]

Tried backing out kmcclusk's checkins to mozilla/view/src on 01/26/2000 but
that didn't help :(

Comment 2

[jst]

I managed to fix the infinite (almost) loop in nsViewManager.cpp, I created a
patch that fixes the loop, cleans up some assertions in the viewmanager and
removes a duplicate call to SetClipRegion.

I tried to attach the patch but for some reason bugzilla doesn't accept it (!)
so in stead I made it available at

	http://www.citec.fi/~jst/nsViewManager.cpp.diff

With this patch I'm able to open test4.html.

beard, could you have a look at this and comment?

Comment 3

[Patrick C. Beard]

Attachment: aforementioned patch

Comment 4

[Patrick C. Beard]

Thanks, I went ahead and attached the patch to the bug, so you don't have to keep 
it around on your site.

Comment 5

[jst]

Adding beta1 keyword since if Joe User gets ahold of beta1 and test4.html is
listed in the debug menu and loading it locks up the product he's thoughts about
the product won't be very high. And, since test9.html includes test4.html he'll
see the same thing there.

Comment 6

[leger]

Putting on PDT+ radar for beta1. 

Comment 7

[Patrick C. Beard]

This is fixed with nsViewManager2.

Comment 8

[Patrick C. Beard]

Applied the patch. Checking it in.

Comment 9

[leger]

Adding verifyme keyword.

Comment 10

[jst]

beard, you left out the most important part of the patch! It still loops if
with nsViewManager but it's fixed in nsViewManager2. What happends is that we
enter a while loop that is protected with "pushcnt--" in this case with a
pushcnt that is 0 or negative, thus "pushcnt--" will be valid untill we wrap
arround from negative to positive and back down to 0, 2^32 tims, thus it
appears as if mozilla hangs. Here's a patch.

Index: src/nsViewManager.cpp
===================================================================
RCS file: /cvsroot/mozilla/view/src/nsViewManager.cpp,v
retrieving revision 3.146
diff -u -r3.146 nsViewManager.cpp
--- nsViewManager.cpp   2000/02/15 22:11:31     3.146
+++ nsViewManager.cpp   2000/02/18 14:14:59
@@ -1060,7 +1060,7 @@
 
     NS_ASSERTION((pushcnt >= 0), "underflow");
 
-    while (pushcnt--)
+    while (pushcnt-- > 0)
     {
       NS_ASSERTION((pushcnt >= 0), "underflow");
 
What's the deal, is nsViewManager going to be deleted or is it still going to be
used for something?

Since this works with nsViewManager2 (the current default) I'm removing the PDT+
status and beta1 and verifyme keywords but I'm reopening the bug untill
nsViewManger is removed or fixed, depending on what the plan is.

Comment 11

[Patrick C. Beard]

nsViewManager2 has changes in it to fix this bug. Kevin has the files, and should 
check them in.

Comment 12

[Kevin McCluskey (gone)]

Bulk moving to M16

Comment 13

[Kevin McCluskey (gone)]

ViewManager2 has replaced ViewManager as the default. Eventually ViewManager 
will be removed altogether. It is useful to keep both around until it can be 
determined that ViewManager2 has sufficient performance to replace ViewManager, 
and all the bugs have been shaked out of ViewManager2. I will add the fix to 
ViewManager. Thanks..

Comment 14

[Kevin McCluskey (gone)]

Fixed in 4/13/2000 5:08PM build.

Comment 15

[Chris Petersen]

This problem is still occuring in the July 14th Linux build (2000071408). This 
test page 4 is not rendering and throbber continues to cycle but never stops.

Comment 16

[Kevin McCluskey (gone)]

Chris: Are you using view manager 1 or view manager 2? 

Comment 17

[Kevin McCluskey (gone)]

This is working fine on Linux. test4.html doesn't load because test4.html is not 
included with the commercial build on Linux. This bug was about assertions in 
the view manager which caused mozilla to look forever.

Marking as WORKSFORME.

Comment 18

[Chris Petersen]

Marking verified works for me in the Sept 14th build.