RESOLVED FIXED 74653
[Chromium] Implement layoutTestController.workerThreadCount in DRT
https://bugs.webkit.org/show_bug.cgi?id=74653
Summary [Chromium] Implement layoutTestController.workerThreadCount in DRT
Dmitry Lomov
Reported 2011-12-15 15:18:28 PST
It might be either DumpRenderTree-related issue or actual bug in product
Attachments
Fix (5.26 KB, patch)
2012-01-10 16:52 PST, Dmitry Lomov
no flags
More tests pass now - updating expectations (5.59 KB, patch)
2012-01-10 17:00 PST, Dmitry Lomov
no flags
CR feedback (10.17 KB, patch)
2012-01-13 15:30 PST, Dmitry Lomov
fishd: review+
webkit.review.bot: commit-queue-
Rebased (10.22 KB, patch)
2012-01-23 10:57 PST, Dmitry Lomov
no flags
Dmitry Lomov
Comment 1 2011-12-20 17:48:15 PST
The underlying cause is that layoutTestController.workerThreadCount is not implemented in chromium DRT
Dmitry Lomov
Comment 2 2012-01-10 16:52:37 PST
David Levin
Comment 3 2012-01-10 16:54:38 PST
Comment on attachment 121941 [details] Fix Looks good to me. Needs Darin's ok of course.
WebKit Review Bot
Comment 4 2012-01-10 16:56:24 PST
Please wait for approval from fishd@chromium.org before submitting because this patch contains changes to the Chromium public API.
Dmitry Lomov
Comment 5 2012-01-10 17:00:47 PST
Created attachment 121943 [details] More tests pass now - updating expectations
Darin Fisher (:fishd, Google)
Comment 6 2012-01-11 21:23:46 PST
Comment on attachment 121943 [details] More tests pass now - updating expectations View in context: https://bugs.webkit.org/attachment.cgi?id=121943&action=review > Source/WebKit/chromium/src/WebWorkerClientImpl.cpp:229 > +unsigned workerThreadCount() is this returning the count of dedicated worker threads? or does this also include shared workers? if this only returns the number of dedicated worker threads, then i'd make this a static function on WebWorker. WebWorker::count() would probably work. There'd be no need to repeat the word "worker", and no real need to say "thread" since worker implies a thread.
Dmitry Lomov
Comment 7 2012-01-11 21:55:29 PST
(In reply to comment #6) > (From update of attachment 121943 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=121943&action=review > > > Source/WebKit/chromium/src/WebWorkerClientImpl.cpp:229 > > +unsigned workerThreadCount() > > is this returning the count of dedicated worker threads? or does this also include shared workers? It is only dedicated workers. > > if this only returns the number of dedicated worker threads, then i'd make this a static function > on WebWorker. WebWorker::count() would probably work. There'd be no need to repeat the word > "worker", and no real need to say "thread" since worker implies a thread. Class WebWorker is going away soo (there are no real usages of it now). Point taken about Thread. How about function with one of these names: 1. dedicatedWorkerCount 2. webWorkerCount 3. WorkerCount. ?
Darin Fisher (:fishd, Google)
Comment 8 2012-01-12 22:04:11 PST
One idea is to put a method on WebView since all dedicated workers are associated with WebViews. class WebView { public: ... WEBKIT_EXPORT static unsigned dedicatedWorkerCount(); }; Another idea is to introduce a new class that just exists to scope this method (and maybe other worker related methods in the future): class WebWorkerInfo { public: WEBKIT_EXPORT static unsigned dedicatedCount(); };
Dmitry Lomov
Comment 9 2012-01-13 15:30:32 PST
Created attachment 122509 [details] CR feedback
WebKit Review Bot
Comment 10 2012-01-20 22:08:22 PST
Comment on attachment 122509 [details] CR feedback Rejecting attachment 122509 [details] from commit-queue. Failed to run "['/mnt/git/webkit-commit-queue/Tools/Scripts/webkit-patch', '--status-host=queues.webkit.org', '-..." exit_code: 2 Last 500 characters of output: s to file Source/WebKit/chromium/WebKit.gyp.rej patching file Source/WebKit/chromium/public/WebWorkerInfo.h patching file Source/WebKit/chromium/src/WebWorkerInfo.cpp patching file Tools/ChangeLog Hunk #1 succeeded at 1 with fuzz 3. patching file Tools/DumpRenderTree/chromium/LayoutTestController.cpp patching file Tools/DumpRenderTree/chromium/LayoutTestController.h Failed to run "[u'/mnt/git/webkit-commit-queue/Tools/Scripts/svn-apply', u'--reviewer', u'Darin Fisher', u'--for..." exit_code: 1 Full output: http://queues.webkit.org/results/11310334
Dmitry Lomov
Comment 11 2012-01-23 10:57:39 PST
WebKit Review Bot
Comment 12 2012-01-23 20:08:12 PST
Comment on attachment 123575 [details] Rebased Clearing flags on attachment: 123575 Committed r105684: <http://trac.webkit.org/changeset/105684>
WebKit Review Bot
Comment 13 2012-01-23 20:08:18 PST
All reviewed patches have been landed. Closing bug.
Scott Graham
Comment 14 2012-01-24 10:11:11 PST
It looks like the rebase got messed up. The removal of WebWorker.h from WebKit.gyp here https://bugs.webkit.org/show_bug.cgi?id=76512 got reverted.
Note You need to log in before you can comment on or make changes to this bug.