[webkit-reviews] review denied: [Bug 27612] handling scripts can block UI : [Attachment 33349] the patch

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Thu Aug 6 20:31:16 PDT 2009


Eric Seidel <eric at webkit.org> has denied Yong Li <yong.li at torchmobile.com>'s
request for review:
Bug 27612: handling scripts can block UI
https://bugs.webkit.org/show_bug.cgi?id=27612

Attachment 33349: the patch
https://bugs.webkit.org/attachment.cgi?id=33349&action=review

------- Additional Comments from Eric Seidel <eric at webkit.org>
For example.  When a script tag finishes loading, I think js can get a "on
load" event sent.  If that's true, then that event will be sent before
execution instead of after execution in your port.  I would have to look to
confirm if there is an onload for <script> tags.

Seems 120 vs. 0 should be a constant defined with an #ifdef at one location in
the file.

When would this ever be hit?
5     if (!m_clients.contains(c))
 166	     return;
I would think that would be an error.

Style:
80	       if (!m_clientsToNotify.isEmpty()) {
 181		     m_notifyTimer.startOneShot(0);
 182		 } else {
 183		     m_decodedDataDeletionTimer.startOneShot(120);
 184		 }


The change looks technically correct.  But I'm not sure this will function
correctly from a web page's perspective.

The change need more comment as to why this will be OK from a web page's
perspective.


More information about the webkit-reviews mailing list