[webkit-reviews] review denied: [Bug 75529] [Qt] SharedWorkers do not terminate after tests : [Attachment 121661] The fixing of the tests which use "normal" Workers and the checking in DumpRenderTreeQt.cpp
bugzilla-daemon at webkit.org
bugzilla-daemon at webkit.org
Mon Jan 9 08:14:08 PST 2012
David Levin <levin at chromium.org> has denied Nandor Huszka
<Huszka.Nandor at stud.u-szeged.hu>'s request for review:
Bug 75529: [Qt] SharedWorkers do not terminate after tests
https://bugs.webkit.org/show_bug.cgi?id=75529
Attachment 121661: The fixing of the tests which use "normal" Workers and the
checking in DumpRenderTreeQt.cpp
https://bugs.webkit.org/attachment.cgi?id=121661&action=review
------- Additional Comments from David Levin <levin at chromium.org>
There are several problems with the patch.
1. It is missing a change log which would hopefully explain why some changes
are being done (see other issues below).
2. There are formatting/indenting issues.
3. There is a notifyWhen function which doesn't have a callback so it doesn't
notify.
4. There are a lot of tests which only have this added "<script
src="resources/worker-util.js"></script>" and it is unclear why.
5. It is unclear to me why the tests needed the changes that they got (like the
worker array).
6. This is a rather large change and really does several different things.
Hopefully it would be broken up into different patches that do things
separately and this would be more easily and quickly reviewed. (For example the
drt c++ changes could be one change.)
This is just what I noticed quickly. There may be other issues once these get
addressed.
More information about the webkit-reviews
mailing list