Bug 59699

Summary: Global object is recreated on teardown, for no good reason
Product: WebKit Reporter: George Staikos <staikos>
Component: JavaScriptCoreAssignee: Geoffrey Garen <ggaren>
Severity: Normal CC: abarth, ademar, dave+webkit, dimich, ggaren, mjs, oliver
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: PC   
OS: OS X 10.5   
Description Flags
Possible fix
Patch barraclough: review+

Description George Staikos 2011-04-28 06:56:10 PDT
#4  0x00075618 in JSC::Structure::addPropertyTransition(JSC::Structure*, JSC::Identifier const&, unsigned int, JSC::JSCell*, unsigned int&) ()
#5  0x008467c6 in JSC::JSObject::putDirectInternal(JSC::Identifier const&, JSC::JSValue, unsigned int, bool, JSC::PutPropertySlot&, JSC::JSCell*) ()
#6  0x0005284c in JSC::InternalFunction::InternalFunction(JSC::JSGlobalData*, JSC::JSGlobalObject*, WTF::NonNullPassRefPtr<JSC::Structure>, JSC::Identifier const&) ()
#7  0x0006f158 in JSC::StringConstructor::StringConstructor(JSC::ExecState*, JSC::JSGlobalObject*, WTF::NonNullPassRefPtr<JSC::Structure>, JSC::Structure*, JSC::StringPrototype*) ()
#8  0x00059df6 in JSC::JSGlobalObject::reset(JSC::JSValue) ()
#9  0x0005b2b4 in JSC::JSGlobalObject::init(JSC::JSObject*) ()
#10 0x002458c0 in WebCore::JSDOMGlobalObject::JSDOMGlobalObject(WTF::NonNullPassRefPtr<JSC::Structure>, WebCore::JSDOMGlobalObject::JSDOMGlobalObjectData*, JSC::JSObject*) ()
#11 0x00246290 in WebCore::JSDOMWindowBase::JSDOMWindowBase(WTF::NonNullPassRefPtr<JSC::Structure>, WTF::PassRefPtr<WebCore::DOMWindow>, WebCore::JSDOMWindowShell*) ()
#12 0x0019e062 in WebCore::JSDOMWindow::JSDOMWindow(WTF::NonNullPassRefPtr<JSC::Structure>, WTF::PassRefPtr<WebCore::DOMWindow>, WebCore::JSDOMWindowShell*) ()
#13 0x0024a054 in WebCore::JSDOMWindowShell::setWindow(WTF::PassRefPtr<WebCore::DOMWindow>) ()
#14 0x0025cb3c in WebCore::ScriptController::clearWindowShell(bool) ()
#15 0x003aa490 in WebCore::Frame::~Frame() ()

You can see from this trace that frame teardown is creating a new global object for no good reason that I can determine.  We immediately get rid of it all anyway.
Comment 1 George Staikos 2011-04-28 06:57:43 PDT
Created attachment 91486 [details]
Possible fix

Does this cause a possible leak?  I'm not sure yet.
Comment 2 Geoffrey Garen 2011-04-28 11:37:23 PDT
> You can see from this trace that frame teardown is creating a new global object for no good reason that I can determine.  We immediately get rid of it all anyway.

Yeah, this is a pretty big travesty.
Comment 3 Geoffrey Garen 2011-04-28 11:39:10 PDT
Comment on attachment 91486 [details]
Possible fix

I think this patch would crash, because setWindow(JSGlobalData&, JSDOMWindow*) assumes the window is not NULL.

It might work to change setWindow(JSGlobalData&, JSDOMWindow*) to use jsNull() as the window shell's prototype if the JSDOMWindow* is NULL, or you could just change setWindow(JSGlobalData&, JSDOMWindow*) to take an explicit prototype argument.
Comment 4 Geoffrey Garen 2011-05-15 22:04:54 PDT
Created attachment 93605 [details]
Comment 5 Geoffrey Garen 2011-05-15 22:10:08 PDT
Committed r86523: <http://trac.webkit.org/changeset/86523>
Comment 6 George Staikos 2011-05-16 04:51:41 PDT
Thanks!   Just got back from vacation/travel to see this :)
Comment 7 Ademar Reis 2011-05-17 11:12:38 PDT
Revision r86523 cherry-picked into qtwebkit-2.2 with commit 97cb464 <http://gitorious.org/webkit/qtwebkit/commit/97cb464>