[Webkit-unassigned] [Bug 75529] [Qt] SharedWorkers do not terminate after tests

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=75529





--- Comment #10 from Nandor Huszka <Huszka.Nandor at stud.u-szeged.hu>  2012-01-10 04:38:22 PST ---
(In reply to comment #9)
> (From update of attachment 121661 [details])
> 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.

The r? really was not justified, EWS analysis what I just needed, because it is a WIP patch. It tries to terminate all the stucked Workers, but it will not resolve the original SharedWorker problem. Whatever, thank you for the comments, the explanations will be added to the ChangeLog too.

> 3. There is a notifyWhen function which doesn't have a callback so it doesn't notify.

The layoutTestController.notifyDone calling can be found in many tests, and before this all the Workers should be terminated. The notifyDoneWhenWorkersFreed function do this exactly, it exits all the used Workers (which are stored in the workerArray), and then calls the layoutTestController.notifyDone method.

> 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).

In my opinion, the global workerArray is needed. Workers frequently declared as a local variable in a function, but they should be terminated in an another function. Also frequently occurs that there is possiblity to terminate workers just in an another JavaScript file. For example a worker is created by dedicated-workers-list.html, but terminating it is needed in inspector-test.js, which is imported by dedicated-workers-list.html. If worker is stored as a standard variable, we cannot know its name in inspector-test.js. A global worker array is recommended for storing workers, with this terminating workers in another files is safer.

> 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.)

In itself the modification in DRT causes some test's failing, because it puts an error message to the output, if a test leaves living worker after it. The purpose of the cpp code: tests authors pay attention to terminate the used workers.

It is not a complete patch this way, because SharedWorkers do not terminate, it is in question yet.

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.