Bug 59699

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

George Staikos
Reported 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.
Attachments
Possible fix (1.29 KB, patch)
2011-04-28 06:57 PDT, George Staikos 		no flags
DetailsFormatted DiffDiff
Patch (1.76 KB, patch)
2011-05-15 22:04 PDT, Geoffrey Garen 		barraclough: review+
DetailsFormatted DiffDiff
Show Obsolete (1) View All Add attachment proposed patch, testcase, etc.
George Staikos
Comment 1 2011-04-28 06:57:43 PDT
Created attachment 91486 [details] Possible fix Does this cause a possible leak? I'm not sure yet.
Geoffrey Garen
Comment 2 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.
Geoffrey Garen
Comment 3 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.
Geoffrey Garen
Comment 4 2011-05-15 22:04:54 PDT
Created attachment 93605 [details] Patch
Geoffrey Garen
Comment 5 2011-05-15 22:10:08 PDT
Committed r86523: <http://trac.webkit.org/changeset/86523>
George Staikos
Comment 6 2011-05-16 04:51:41 PDT
Thanks! Just got back from vacation/travel to see this :)
Ademar Reis
Comment 7 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>
Note You need to log in before you can comment on or make changes to this bug.