591157 - CreateAboutBlankContentViewer happens every time we open a chrome window

Status: NEW

(Core :: DOM: Navigation, defect)

Description

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

The following stack reveals all:

#0  nsDocShell::CreateAboutBlankContentViewer (this=0x15a9a00, aPrincipal=0x0, aBaseURI=0x0) at /Users/roc/mozilla-trunk/docshell/base/nsDocShell.cpp:6331
#1  0x14966955 in nsDocShell::EnsureContentViewer (this=0x15a9a00) at /Users/roc/mozilla-trunk/docshell/base/nsDocShell.cpp:6313
#2  0x1497ec31 in nsDocShell::GetInterface (this=0x15a9a00, aIID=@0x133cc34c, aSink=0xbfffa5ac) at /Users/roc/mozilla-trunk/docshell/base/nsDocShell.cpp:882
#3  0x009510c5 in nsGetInterface::operator() (this=0xbfffa610, aIID=@0x133cc34c, aInstancePtr=0xbfffa5ac) at nsIInterfaceRequestorUtils.cpp:52
#4  0x129c5f4b in nsCOMPtr<nsIDOMDocument>::assign_from_helper (this=0xbfffa60c, helper=@0xbfffa610, aIID=@0x133cc34c) at nsCOMPtr.h:1272
#5  0x129c6048 in nsCOMPtr<nsIDOMDocument>::nsCOMPtr (this=0xbfffa60c, helper=@0xbfffa610) at nsCOMPtr.h:644
#6  0x12cbb32f in nsGlobalWindow::GetDocument (this=0x1fd55e20, aDocument=0xbfffa658) at /Users/roc/mozilla-trunk/dom/base/nsGlobalWindow.cpp:2533
#7  0x12d20227 in nsPIDOMWindow::EnsureInnerWindow (this=0x1fd55e20) at nsPIDOMWindow.h:332
#8  0x12cfee7e in nsOuterWindowSH::PreCreate (this=0x1708f770, nativeObj=0x1fd55e58, cx=0x11d9e00, globalObj=0x1e8980c0, parentObj=0xbfffa7fc) at /Users/roc/mozilla-trunk/dom/base/nsDOMClassInfo.cpp:4937
#9  0x1321a713 in XPCWrappedNative::GetNewOrUsed (ccx=@0xbfffad8c, Object=0x1fd55e58, Scope=0x1eee3df0, Interface=0xc54cb0, cache=0x0, isGlobal=0, resultWrapper=0xbfffa978) at /Users/roc/mozilla-trunk/js/src/xpconnect/src/xpcwrappednative.cpp:441
#10 0x131eac80 in XPCConvert::NativeInterface2JSObject (lccx=@0xbfffab8c, d=0xbfffaed8, dest=0x0, src=0x1fd55e58, iid=0xbfffaf68, Interface=0x0, cache=0x0, scope=0x1e8a2fc0, allowNativeWrapper=1, isGlobal=0, pErr=0x0, aHelper=0x0) at /Users/roc/mozilla-trunk/js/src/xpconnect/src/xpcconvert.cpp:1239
#11 0x131ec302 in XPCConvert::NativeData2JS (lccx=@0xbfffab8c, d=0xbfffaed8, s=0xbfffb12c, type=@0xbfffafa1, iid=0xbfffaf68, scope=0x1e8a2fc0, pErr=0x0) at /Users/roc/mozilla-trunk/js/src/xpconnect/src/xpcconvert.cpp:472
#12 0x131f10fb in XPCConvert::NativeData2JS (ccx=@0xbfffad8c, d=0xbfffaed8, s=0xbfffb12c, type=@0xbfffafa1, iid=0xbfffaf68, scope=0x1e8a2fc0, pErr=0x0) at xpcprivate.h:3091
#13 0x1320e1ae in nsXPCWrappedJSClass::CallMethod (this=0xcaffc0, wrapper=0x1ee84960, methodIndex=3, info=0x1014f40, nativeParams=0xbfffb12c) at /Users/roc/mozilla-trunk/js/src/xpconnect/src/xpcwrappedjsclass.cpp:1580
#14 0x13204fd5 in nsXPCWrappedJS::CallMethod (this=0x1ee84960, methodIndex=3, info=0x1014f40, params=0xbfffb12c) at /Users/roc/mozilla-trunk/js/src/xpconnect/src/xpcwrappedjs.cpp:570
#15 0x00a01274 in PrepareAndDispatch (self=0x1ee84710, methodIndex=3, args=0xbfffb254) at /Users/roc/mozilla-trunk/xpcom/reflect/xptcall/src/md/unix/xptcstubs_unixish_x86.cpp:93
#16 0x009fbdb7 in nsXPTCStubBase::Stub3 (this=0x1ee84710) at xptcstubsdef.inc:1
#17 0x00981ffb in nsObserverList::NotifyObservers (this=0x166ab78, aSubject=0x1fd55e58, aTopic=0x14b1cce0 "domwindowopened", someData=0x0) at /Users/roc/mozilla-trunk/xpcom/ds/nsObserverList.cpp:130
#18 0x00983590 in nsObserverService::NotifyObservers (this=0xc37c10, aSubject=0x1fd55e58, aTopic=0x14b1cce0 "domwindowopened", someData=0x0) at /Users/roc/mozilla-trunk/xpcom/ds/nsObserverService.cpp:182
#19 0x14ad77a6 in nsWindowWatcher::AddWindow (this=0xc8e170, aWindow=0x1fd55e20, aChrome=0x0) at /Users/roc/mozilla-trunk/embedding/components/windowwatcher/src/nsWindowWatcher.cpp:1185

Seems like we're leaving some Ts/Twinopen on the table here.

Comment 1

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

It gets worse. CreateAboutBlankContentViewer ends up calling DocumentViewerImpl::MakeWindow twice! The first call creates a viewmanager that we then throw away in the second call and create a new one. First call:

#8  0x1266928b in DocumentViewerImpl::MakeWindow (this=0x170af130, aSize=@0xbfff9fb8, aContainerView=0x0) at /Users/roc/mozilla-trunk/layout/base/nsDocumentViewer.cpp:2305
#9  0x1266dad9 in DocumentViewerImpl::InitInternal (this=0x170af130, aParentWidget=0x1fd85570, aState=0x0, aBounds=@0xbfffa1fc, aDoCreation=1, aInPrintPreview=0, aNeedMakeCX=1) at /Users/roc/mozilla-trunk/layout/base/nsDocumentViewer.cpp:897
#10 0x1266e6a9 in DocumentViewerImpl::Init (this=0x170af130, aParentWidget=0x1fd85570, aBounds=@0xbfffa1fc) at /Users/roc/mozilla-trunk/layout/base/nsDocumentViewer.cpp:694
#11 0x14963f47 in nsDocShell::SetupNewViewer (this=0x1358200, aNewViewer=0x170af130) at /Users/roc/mozilla-trunk/docshell/base/nsDocShell.cpp:7531
#12 0x1497906a in nsDocShell::Embed (this=0x1358200, aContentViewer=0x170af130, aCommand=0x149eca38 "", aExtraInfo=0x0) at /Users/roc/mozilla-trunk/docshell/base/nsDocShell.cpp:5638
#13 0x14965b96 in nsDocShell::CreateAboutBlankContentViewer (this=0x1358200, aPrincipal=0x0, aBaseURI=0x0) at /Users/roc/mozilla-trunk/docshell/base/nsDocShell.cpp:6406

Second call:

#8  0x1266928b in DocumentViewerImpl::MakeWindow (this=0x170af130, aSize=@0xbfffa2d4, aContainerView=0x0) at /Users/roc/mozilla-trunk/layout/base/nsDocumentViewer.cpp:2305
#9  0x1266cc70 in DocumentViewerImpl::SetDOMDocument (this=0x170af130, aDocument=0x1600cd8) at /Users/roc/mozilla-trunk/layout/base/nsDocumentViewer.cpp:1753
#10 0x14965bbe in nsDocShell::CreateAboutBlankContentViewer (this=0x1358200, aPrincipal=0x0, aBaseURI=0x0) at /Users/roc/mozilla-trunk/docshell/base/nsDocShell.cpp:6407

Maybe CreateAboutBlankContentViewer's call to SetDOMDocument is just unnecessary?

Comment 2

[Boris Zbarsky [:bzbarsky]]

Yeah, I'm not sure why that SetDOMDocument call is needed...

Comment 3

[Timothy Nikkel (:tnikkel)]

Note to myself (or any other interested party): I wonder if this is creating the widgets we see in bug 590709.

Comment 4

[Jonas Sicking (:sicking) No longer reading bugmail consistently]

SetDOMDocument has been treated as do-not-touch-it-mostly-seems-to-work code for a long time. I wouldn't assume that anything it does is done for any particularly good reasons any more.

Comment 5

[Boris Zbarsky [:bzbarsky]]

The question is whether we need to call it at all here.

Comment 6

[Timothy Nikkel (:tnikkel)]

Removing that SetDOMDocument call passes try server, whatever that means.

Comment 7

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

It means we should remove the call!

Comment 8

[Timothy Nikkel (:tnikkel)]

Attachment: patch

Comment 9

[Timothy Nikkel (:tnikkel)]

I actually looked at what the Embed and SetDOMDocument calls do specifically. This gave me more confidence the SetDOMDocument call is redundant.

Comment 10

[Boris Zbarsky [:bzbarsky]]

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

r=me.

Comment 11

[Johnny Stenback (:jst)]

Blake, is this change cool with you? I know you've touched plenty of related code lately...

Comment 12

[Blake Kaplan (:mrbkap) (inactive)]

Yeah, as long as we still call nsGlobalWindow::SetNewDocument at least once, this should be fine.

Comment 13

[Timothy Nikkel (:tnikkel)]

Yeah, it still get's called in DocumentViewerImpl::InitInternal.

Comment 14

[Timothy Nikkel (:tnikkel)]

http://hg.mozilla.org/mozilla-central/rev/839282b76e8d

This removes the unneeded SetDOMDocument call, but doesn't resolve what this bug was originally reported as.

Comment 15

[Blake Kaplan (:mrbkap) (inactive)]

We should clone this bug and close this one, actually. Multiple patches per bug in bugzilla tends to confuse me...