[webkit-reviews] review granted: [Bug 21970] Make MessagePort event dispatch work in workers : [Attachment 24772] updated patch
bugzilla-daemon at webkit.org
bugzilla-daemon at webkit.org
Thu Oct 30 11:44:53 PDT 2008
Darin Adler <darin at apple.com> has granted Alexey Proskuryakov <ap at webkit.org>'s
request for review:
Bug 21970: Make MessagePort event dispatch work in workers
https://bugs.webkit.org/show_bug.cgi?id=21970
Attachment 24772: updated patch
https://bugs.webkit.org/attachment.cgi?id=24772&action=edit
------- Additional Comments from Darin Adler <darin at apple.com>
> +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
More information about the webkit-reviews
mailing list