Summary: | Make MessagePort event dispatch work in workers | ||||||||
---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Alexey Proskuryakov <ap> | ||||||
Component: | WebCore JavaScript | Assignee: | Alexey Proskuryakov <ap> | ||||||
Status: | RESOLVED FIXED | ||||||||
Severity: | Normal | ||||||||
Priority: | P2 | ||||||||
Version: | 528+ (Nightly build) | ||||||||
Hardware: | Mac | ||||||||
OS: | OS X 10.5 | ||||||||
Attachments: |
|
Description
Alexey Proskuryakov
2008-10-30 09:09:19 PDT
Created attachment 24771 [details]
proposed patch
Created attachment 24772 [details]
updated patch
Removed a little too many unrelated changes...
Comment on attachment 24772 [details] updated patch > +JSDOMGlobalObject::JSDOMGlobalObjectData::JSDOMGlobalObjectData() > + : evt(0) In practice, this "current event" could, and maybe should, be per-thread data, not per-global object data. It's set on entry and then cleared on exit by the code that dispatches events. > +JSDOMGlobalObject::~JSDOMGlobalObject() > +{ > + // Clear any backpointers to the window > + ListenersMap::iterator i2 = d()->jsEventListeners.begin(); > + ListenersMap::iterator e2 = d()->jsEventListeners.end(); A little strange that 2 comes before 1 in this function; but I guess that was copied and pasted from elsewhere. > void JSDOMWindowBase::clearHelperObjectProperties() > { > - d()->evt = 0; > + setCurrentEvent(0); > } I believe this function is unnecessary. I'm not insisting that you remove it, but I believe that it's doing no good at all. > Index: WebCore/dom/Document.h > =================================================================== > --- WebCore/dom/Document.h (revision 37996) > +++ WebCore/dom/Document.h (working copy) > @@ -410,7 +410,7 @@ public: > > bool wellFormed() const { return m_wellFormed; } > > - const KURL& url() const { return m_url; } > + virtual const KURL& url() const ; Extra space before the semicolon. It could be bad for performance to make Document::url() virtual just because ScriptExecutionContext::url() needs to be. Maybe you should consider the style I used for Node::localName() and Element::localName() that allows the latter to be a non-virtual function? I guess it's premature optimization to assume that it's going to be a performance problem, but it does worry me a bit. r=me Committed revision 38033. (In reply to comment #3) > In practice, this "current event" could, and maybe should, be per-thread data, > not per-global object data. I'm not sure about this - per-thread keys are a rather limited process-wide resource, so frameworks shouldn't use them unless there is a significant benefit. > A little strange that 2 comes before 1 in this function; but I guess that was > copied and pasted from elsewhere. It was, fixed now. > It could be bad for performance to make Document::url() virtual just because > ScriptExecutionContext::url() needs to be. Maybe you should consider the style > I used for Node::localName() and Element::localName() that allows the latter to > be a non-virtual function? OK. > r=me Thank you! |