419452 - loadOverlay fails to fire observer when overlay is overlaid

Status: RESOLVED FIXED

(Core :: XUL, defect)

Description

[[e-mail]]

If the overlay loaded document.loadOverlay itself has an overlay, either via a PI or via the chrome registry, then the overlay load observer never fires.

Note that this is not quite the same as bug 330458 as I am not trying to call document.loadOverlay before the previous overlay has loaded.

Comment 1

[[e-mail]]

Attachment: Proposed patch

If you can think of a good way to notify for the original URI I'm all ears.

The only way I could think of was to prohibit multiple outstanding loads.

Comment 2

[[e-mail]]

Comment on attachment 305522 [details] [diff] [review]
Proposed patch

This means that, with care, loadOverlay can always be used usefully.

Comment 3

[Mike Beltzner [:beltzner, not reading bugmail]]

Comment on attachment 305522 [details] [diff] [review]
Proposed patch

a=beltzner for 1.9

Comment 4

[[e-mail]]

Fix checked in.

Comment 5

[Reed Loden [:reed]]

Backed this out as the possible cause of the Txul regression at least on Linux.

Comment 6

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

Didn't seem to be the cause of the Txul regression (appears to bug bug 395609), but still waiting to see if it's the cause of the Windows debug orange (due to an assertion).

Comment 7

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

Either this bug or bug 395609 (both backed out in the same cycle) was the cause of the orange on fxdebug-win32-tbox due to an assertion during the bloat test:

  WARNING: NS_ENSURE_TRUE(sgo || !hasHadScriptObject) failed: file e:/builds/tinderbox/Fx-Trunk-Memtest/WINNT_5.2_Depend/mozilla/content/base/src/nsContentUtils.cpp, line 2583
  WARNING: NS_ENSURE_TRUE(sgo || !hasHadScriptObject) failed: file e:/builds/tinderbox/Fx-Trunk-Memtest/WINNT_5.2_Depend/mozilla/content/base/src/nsContentUtils.cpp, line 2583
  ###!!! ASSERTION: invalid active window: 'Error', file e:/builds/tinderbox/Fx-Trunk-Memtest/WINNT_5.2_Depend/mozilla/embedding/components/windowwatcher/src/nsWindowWatcher.cpp, line 1086
  gklayout!nsFocusController::UpdateWWActiveWindow+0x000000000000013B (e:\builds\tinderbox\fx-trunk-memtest\winnt_5.2_depend\mozilla\dom\src\base\nsfocuscontroller.cpp, line 592)
  gklayout!nsFocusController::SetActive+0x000000000000002E (e:\builds\tinderbox\fx-trunk-memtest\winnt_5.2_depend\mozilla\dom\src\base\nsfocuscontroller.cpp, line 559)
  appshell!nsWebShellWindow::HandleEvent+0x0000000000000546 (e:\builds\tinderbox\fx-trunk-memtest\winnt_5.2_depend\mozilla\xpfe\appshell\src\nswebshellwindow.cpp, line 473)
  gkwidget!nsWindow::DispatchEvent+0x00000000000000C6 (e:\builds\tinderbox\fx-trunk-memtest\winnt_5.2_depend\mozilla\widget\src\windows\nswindow.cpp, line 972)
  gkwidget!nsWindow::DispatchWindowEvent+0x0000000000000026 (e:\builds\tinderbox\fx-trunk-memtest\winnt_5.2_depend\mozilla\widget\src\windows\nswindow.cpp, line 993)
  gkwidget!nsWindow::DispatchFocus+0x00000000000000D2 (e:\builds\tinderbox\fx-trunk-memtest\winnt_5.2_depend\mozilla\widget\src\windows\nswindow.cpp, line 5976)
  gkwidget!nsWindow::ProcessMessage+0x000000000000101A (e:\builds\tinderbox\fx-trunk-memtest\winnt_5.2_depend\mozilla\widget\src\windows\nswindow.cpp, line 4530)
  gkwidget!nsWindow::WindowProc+0x0000000000000178 (e:\builds\tinderbox\fx-trunk-memtest\winnt_5.2_depend\mozilla\widget\src\windows\nswindow.cpp, line 1187)
  USER32!LoadCursorW+0x0000000000004CF5
  USER32!LoadCursorW+0x0000000000004E86
  USER32!GetMessageW+0x000000000000009F
  USER32!GetClientRect+0x000000000000004A
  ntdll!KiUserCallbackDispatcher+0x000000000000002E
  gklayout!nsView::~nsView+0x00000000000001AA (e:\builds\tinderbox\fx-trunk-memtest\winnt_5.2_depend\mozilla\view\src\nsview.cpp, line 274)
  gklayout!nsView::`scalar deleting destructor'+0x000000000000000F
  gklayout!nsIView::Destroy+0x000000000000002A (e:\builds\tinderbox\fx-trunk-memtest\winnt_5.2_depend\mozilla\view\src\nsview.cpp, line 314)
  gklayout!nsFrame::Destroy+0x00000000000000D5 (e:\builds\tinderbox\fx-trunk-memtest\winnt_5.2_depend\mozilla\layout\generic\nsframe.cpp, line 510)
  gklayout!nsSplittableFrame::Destroy+0x000000000000002D (e:\builds\tinderbox\fx-trunk-memtest\winnt_5.2_depend\mozilla\layout\generic\nssplittableframe.cpp, line 74)
  gklayout!nsContainerFrame::Destroy+0x000000000000017E (e:\builds\tinderbox\fx-trunk-memtest\winnt_5.2_depend\mozilla\layout\generic\nscontainerframe.cpp, line 300)
  gklayout!ViewportFrame::Destroy+0x000000000000001E (e:\builds\tinderbox\fx-trunk-memtest\winnt_5.2_depend\mozilla\layout\generic\nsviewportframe.cpp, line 69)
  gklayout!nsFrameManager::Destroy+0x0000000000000079 (e:\builds\tinderbox\fx-trunk-memtest\winnt_5.2_depend\mozilla\layout\base\nsframemanager.cpp, line 284)
  gklayout!PresShell::Destroy+0x0000000000000401 (e:\builds\tinderbox\fx-trunk-memtest\winnt_5.2_depend\mozilla\layout\base\nspresshell.cpp, line 1676)
  gklayout!DocumentViewerImpl::Destroy+0x000000000000059C (e:\builds\tinderbox\fx-trunk-memtest\winnt_5.2_depend\mozilla\layout\base\nsdocumentviewer.cpp, line 1522)
  docshell!nsDocShell::Destroy+0x0000000000000303 (e:\builds\tinderbox\fx-trunk-memtest\winnt_5.2_depend\mozilla\docshell\base\nsdocshell.cpp, line 3654)
  appshell!nsXULWindow::Destroy+0x000000000000035A (e:\builds\tinderbox\fx-trunk-memtest\winnt_5.2_depend\mozilla\xpfe\appshell\src\nsxulwindow.cpp, line 503)
  appshell!nsWebShellWindow::Destroy+0x000000000000014F (e:\builds\tinderbox\fx-trunk-memtest\winnt_5.2_depend\mozilla\xpfe\appshell\src\nswebshellwindow.cpp, line 840)
  appshell!nsContentTreeOwner::Destroy+0x0000000000000047 (e:\builds\tinderbox\fx-trunk-memtest\winnt_5.2_depend\mozilla\xpfe\appshell\src\nscontenttreeowner.cpp, line 563)
  gklayout!nsGlobalWindow::ReallyCloseWindow+0x000000000000021B (e:\builds\tinderbox\fx-trunk-memtest\winnt_5.2_depend\mozilla\dom\src\base\nsglobalwindow.cpp, line 5412)
  gklayout!nsCloseEvent::Run+0x0000000000000024 (e:\builds\tinderbox\fx-trunk-memtest\winnt_5.2_depend\mozilla\dom\src\base\nsglobalwindow.cpp, line 5220)
  xpcom_core!nsThread::ProcessNextEvent+0x00000000000001FA (e:\builds\tinderbox\fx-trunk-memtest\winnt_5.2_depend\mozilla\xpcom\threads\nsthread.cpp, line 511)
  xpcom_cnsStringStats
   => mAllocCount:          33007
   => mReallocCount:         4184
   => mFreeCount:           27642  --  LEAKED 5365 !!!
   => mShareCount:          29820
   => mAdoptCount:           4329
   => mAdoptFreeCount:       4294  --  LEAKED 35 !!!
  ore!NS_ProcessNextEvent_P+0x0000000000000053 (e:\builds\tinderbox\fx-trunk-memtest\winnt_5.2_depend\mozilla\obj-fx-trunk\xpcom\build\nsthreadutils.cpp, line 227)
  gkwidget!nsBaseAppShell::Run+0x000000000000005D (e:\builds\tinderbox\fx-trunk-memtest\winnt_5.2_depend\mozilla\widget\src\xpwidgets\nsbaseappshell.cpp, line 151)
  tkitcmps!nsAppStartup::Run+0x000000000000006B (e:\builds\tinderbox\fx-trunk-memtest\winnt_5.2_depend\mozilla\toolkit\components\startup\src\nsappstartup.cpp, line 181)
  xul!XRE_main+0x0000000000003350 (e:\builds\tinderbox\fx-trunk-memtest\winnt_5.2_depend\mozilla\toolkit\xre\nsapprunner.cpp, line 3149)
  firefox!NS_internal_main+0x00000000000002B2 (e:\builds\tinderbox\fx-trunk-memtest\winnt_5.2_depend\mozilla\browser\app\nsbrowserapp.cpp, line 158)
  firefox!wmain+0x0000000000000119 (e:\builds\tinderbox\fx-trunk-memtest\winnt_5.2_depend\mozilla\toolkit\xre\nswindowswmain.cpp, line 87)
  firefox!__tmainCRTStartup+0x00000000000001A6 (f:\sp\vctools\crt_bld\self_x86\crt\src\crtexe.c, line 594)
  firefox!wmainCRTStartup+0x000000000000000D (f:\sp\vctools\crt_bld\self_x86\crt\src\crtexe.c, line 414)
  kernel32!ProcessIdToSessionId+0x0000000000000209


From the log, it also looks like more DOMWINDOWs were created than before in those broken cycles; something Jesse also saw in one of his builds, along with a bunch of other brokenness (but that went away when he updated).

Comment 8

[[e-mail]]

Hmm, well this code path shouldn't actually get triggered until we write some code that uses it ;-) Resolving fixed again for now.

Comment 9

[Johnny Stenback (:jst)]

Reopening, this was backed out due to bug 420095.

Comment 10

[Reed Loden [:reed]]

(In reply to comment #9)
> Reopening, this was backed out due to bug 420095.

What bug # did you really mean?

Comment 11

[[e-mail]]

bug 420059.

Comment 12

[[e-mail]]

Attachment: Fixed patch

OK, so I hadn't noticed the while loop last time - this potentially invalidates mCurrentPrototype while looking for a loadable overlay. I also fixed the case where the overlay is already in the XUL cache (i.e. shouldReturn is false).

Comment 13

[[e-mail]]

Attachment: Fixed patch

OK, so I hadn't noticed the while loop last time - this potentially invalidates mCurrentPrototype while looking for a loadable overlay. I also fixed the case where the overlay is already in the XUL cache (i.e. shouldReturn is false).

This time, with the correct reviewer!

Comment 14

[Mike Beltzner [:beltzner, not reading bugmail]]

Comment on attachment 307100 [details] [diff] [review]
Fixed patch

a1.9=beltzner

Comment 15

[[e-mail]]

Fixed fix checked in.