Martijn Wargers (dead) Reporter
	Description • 17 years ago

See upcoming testcase, which crashes current trunk build directly, or after a few reloads.

Regarding the "iframe src needed for testcase", the second binding consists of this:
<bindings xmlns="http://www.mozilla.org/xbl">
<binding id="a">
<implementation>
<constructor>
  window.frameElement.parentNode.removeChild(window.frameElement);
</constructor>
</implementation>
<content>
<children xmlns="http://www.mozilla.org/xbl"/>
</content>
</binding>
</bindings>
The rest are basically just empty bindings.

This regressed between 2007-09-14 and 2007-09-15:
I guess a regression from bug 394014, somehow.

http://crash-stats.mozilla.com/report/index/dde6e91c-6619-11dc-b171-001a4bd43e5c
0  	PresShell::ResizeReflow(int, int)  	 mozilla/layout/base/nsPresShell.cpp:2503
1 	PresShell::ResizeReflow(nsIView*, int, int) 	mozilla/layout/base/nsPresShell.cpp:5806
2 	nsViewManager::DoSetWindowDimensions(int, int) 	mozilla/view/src/nsViewManager.h:279
3 	nsViewManager::SetWindowDimensions(int, int) 	mozilla/view/src/nsViewManager.cpp:365
4 	nsViewManager::DispatchEvent(nsGUIEvent*, nsEventStatus*) 	mozilla/view/src/nsViewManager.cpp:960
5 	HandleEvent 	mozilla/view/src/nsView.cpp:168
6 	nsWindow::DispatchEvent(nsGUIEvent*, nsEventStatus&) 	mozilla/widget/src/windows/nsWindow.cpp:1075
7 	nsWindow::DispatchWindowEvent(nsGUIEvent*) 	mozilla/widget/src/windows/nsWindow.cpp:1095
8 	nsWindow::OnResize(nsRect&) 	mozilla/widget/src/windows/nsWindow.cpp:5798
9 	nsWindow::ProcessMessage(unsigned int, unsigned int, long, long*) 	mozilla/widget/src/windows/nsWindow.cpp:4803
10 	nsWindow::WindowProc(HWND__*, unsigned int, unsigned int, long) 	mozilla/widget/src/windows/nsWindow.cpp:1288
11 	InternalCallWinProc 	
etc..

	Martijn Wargers (dead) Reporter
	Comment 2 • 17 years ago

Maybe bug 396108 is related?

	Boris Zbarsky [:bzbarsky] Assignee
	Comment 4 • 17 years ago

I think this same crash could have been triggered before, with a bit more work.  Fix has a few parts:

1)  Resize the subdoc in a post-reflow callback so we don't run script during
    reflow.
2)  Hold strong ref to document viewer across said resize.
3)  Fix some presshell code to deal well with DidDoReflow() running script
    (as it may).

	Boris Zbarsky [:bzbarsky] Assignee
	Comment 5 • 17 years ago

Fixed.

	Boris Zbarsky [:bzbarsky] Assignee
	Comment 6 • 17 years ago

Need to get the test in too...

	Boris Zbarsky [:bzbarsky] Assignee
	Comment 7 • 17 years ago

I backed this out because it seems to have caused orange in the regression test for bug 299716:

*** exiting
*** CHECK FAILED: 0.1 == 0.2
JS frame :: /Users/robcee/slave/trunk_osx2/mozilla/tools/test-harness/xpcshell-simple/head.js :: do_throw :: line 99
JS frame :: /Users/robcee/slave/trunk_osx2/mozilla/tools/test-harness/xpcshell-simple/head.js :: do_check_eq :: line 114
JS frame :: ../../../../_tests/xpcshell-simple/test_extensionmanager/unit/test_bug299716.js :: do_check_item :: line 307
JS frame :: ../../../../_tests/xpcshell-simple/test_extensionmanager/unit/test_bug299716.js :: run_test_pt4 :: line 445
JS frame :: ../../../../_tests/xpcshell-simple/test_extensionmanager/unit/test_bug299716.js :: onStateChange :: line 188
native frame :: <unknown filename> :: <TOP_LEVEL> :: line 0
JS frame :: file:///Users/robcee/slave/trunk_osx2/mozilla/objdir/dist/bin/components/nsExtensionManager.js :: anonymous :: line 5303
JS frame :: file:///Users/robcee/slave/trunk_osx2/mozilla/objdir/dist/bin/components/nsExtensionManager.js :: anonymous :: line 5491
*** FAIL *** 

I can't make head or tail of that test, or why this patch would affect it, and furthermore the extension manager tests fail spectacularly in any debug build (assert city), so I don't even get this in my own builds.

I doubt I'll be able to reland this without some help with this test.  :(

	Boris Zbarsky [:bzbarsky] Assignee
	Comment 8 • 17 years ago

Oh, and the failure was Mac-only.

	Robert Strong (they/them - no direct email)
	Comment 9 • 17 years ago

Mac OS X and our startup is rather fragile - Bug 396234 for an example. EM tests don't fail in a debug Windows build and I'd appreciate it if you filed a bug for the failures you are seeing in debug builds.

Dave, since you have a Mac could you take a look at this?

	Boris Zbarsky [:bzbarsky] Assignee
	Comment 10 • 17 years ago

Filed bug 396839 on the Linux debug failures.

Thanks for looking into this!

	Dave Townsend [:mossop]
	Comment 11 • 17 years ago

I'm unable to reproduce the test failure locally. This is the log of the failure to save me wading through the tinderbox log in the future. Couple of suspicious looking items in there, no clue why this bug would have affected it though

	Dave Townsend [:mossop]
	Comment 12 • 17 years ago

The test failure is happening because the EM is failing to upgrade 2 extensions to newly downloaded versions. Specifically it appears to fail to move aside the old version of the extension. The failure code though is not anything I'd expect to ever see from how we call that function.

	Boris Zbarsky [:bzbarsky] Assignee
	Comment 13 • 17 years ago

I relanded the patch, and now the tree is green.  No idea what happened the first time... maybe it was a network hiccup or something?  Or does this patch not use the network?

	Martijn Wargers (dead) Reporter
	Comment 14 • 17 years ago

Verified fixed, using:
Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9a8pre) Gecko/2007092105 Minefield/3.0a8pre

	neil@parkwaycc.co.uk
	Comment 15 • 17 years ago

Comment on attachment 281425 [details] [diff] [review]
Same as diff -w.

>+  nsSize innerSize(GetSize());
>+  if (IsInline()) {
>+    nsMargin usedBorderPadding = GetUsedBorderAndPadding();
>+    innerSize.width  -= usedBorderPadding.LeftRight();
>+    innerSize.height -= usedBorderPadding.TopBottom();
>+  }
This can be negative (<xul:iframe collapsed="true" style="padding: 1px;"> perhaps?) but the old code never tried to size its child widget to a negative size.