Give toJS a JSDOMGlobalObject parameter This completes the plumbing, and fixes almost all of our "wrong prototype" issues covered by bug 27088.
Created attachment 33398 [details] Compiles, but crashes fast/dom/gc-6.html. --- 74 files changed, 747 insertions(+), 581 deletions(-)
This change is crazy-huge. I need to find a smart way to break it down into smaller chunks.
I wonder if we ever forcefully tear-down global objects. That could explain the 0xbbadbeef I was seeing hit in some lookup table.
In the crash I'm seeing, this call: Structure* getCachedDOMStructure(JSDOMGlobalObject* globalObject, const ClassInfo* classInfo) { JSDOMStructureMap& structures = globalObject->structures(); return structures.get(classInfo).get(); // CRASHING LINE } structures is junk. I'm not sure when structures would be junk. In gc-6.html, the frame is navigated, but the global object should still be alive: function doit() { var frame = document.getElementById("frame"); spanB = frame.contentDocument.getElementById("span-B"); spanB.customProperty = "B"; frame.onload = frameLoaded; frame.src = "about:blank"; } frameLoaded() does a GC and then does: output.innerHTML += spanB.parentNode + "<BR>"; (which is where we're crashing). I expect that now that spanB.parentNode would be (correctly) trying to use the prototype chain off of the previous window. Maybe somehow we've destroyed that previous window?
OK. I've verified that the subframe's global object is getting collected, even though the JSHTMLSpanElement is still alive with a mark() function which i know is marking the frame's global object.
I found the bug. :( JSNode::mark() is calling DOMObject::mark(); direclty instead of DOMObjectWithGlobalPointer::mark()! At least I think so. Will fix tomrorow!
Fixing the various DOMObject::mark() calls to DOMObjectWithGlobalPointer::mark() calls did not fix the crash. :( Investigating.
Actually, now I"m just hitting an ASSERT in gc-6. I"m hitting this ASSERT: DOMObjectWithGlobalPointer(PassRefPtr<JSC::Structure> structure, JSDOMGlobalObject* globalObject) : DOMObject(structure) , m_globalObject(globalObject) { ASSERT(globalObject->scriptExecutionContext()); } Caused by this FIXME: Document* DOMWindow::document() const { // FIXME: This function shouldn't need a frame to work. if (!m_frame) return 0; I'll comment out the ASSERT for now. DOMWindow should always be able to reach the Document! Just like the Document should always be able to reach the JSDOMWindow. :(
Created attachment 33407 [details] Fully working patch --- 78 files changed, 764 insertions(+), 593 deletions(-)
Comment on attachment 33407 [details] Fully working patch This patch is ready for review. I just figure it's large enough no one will dare. Feel welcome to make comments or r+/r- however :) I will attempt to split it up tomorrow.
Comment on attachment 33407 [details] Fully working patch Hum.. Nevermind. I havne' tadded ChangeLogs, so it's not really ready. :(
Comment on attachment 33407 [details] Fully working patch Most of this patch has been taken care of by sub-bugs. I need to write a bunch more tests and upload a new copy with the remaining bits tomorrow.
Created attachment 33462 [details] Remaining changes, mostly needs a few tests --- 29 files changed, 105 insertions(+), 99 deletions(-)
Any progress here Eric?
This patch is correct, it just needs tests. The testing will take a little effort because it has to be x-frame, making very specific calls and in some cases may need to be careful that the calls aren't already cached.
I haven't looked at this bug in a long time, no.
The JSDOMGlobalObject parameter seems to now be present in every toJS declaration and implementation. Can this be closed?
rniwa@webkit.org - All dependent bugs are closed? Is this patch needed still and have to rebase? or Webkit is now different path and this is WONTFIX. Appreciate if you can confirm. Thanks!
We don't need this bug holding an obsolete patch, but should check bug 27088 which tracks the observable issue. Also, yes, it does look like all of this has been done over the years.