Bug 74653 – [Chromium] Implement layoutTestController.workerThreadCount in DRT

• Component: WebCore Misc.
• Status: RESOLVED FIXED

[Dmitry Lomov]

It might be either DumpRenderTree-related issue or actual bug in product

[Dmitry Lomov]

The underlying cause is that layoutTestController.workerThreadCount  is not implemented in chromium DRT

[Dmitry Lomov]

Created attachment 121941 [details]
Fix

[David Levin]

Comment on attachment 121941 [details]
Fix

Looks good to me. Needs Darin's ok of course.

[WebKit Review Bot]

Please wait for approval from [e-mail] before submitting because this patch contains changes to the Chromium public API.

[Dmitry Lomov]

Created attachment 121943 [details]
More tests pass now - updating expectations

[Darin Fisher (:fishd, Google)]

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]

(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)]

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]

Created attachment 122509 [details]
CR feedback

[WebKit Review Bot]

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]

Created attachment 123575 [details]
Rebased

[WebKit Review Bot]

Comment on attachment 123575 [details]
Rebased

Clearing flags on attachment: 123575

Committed r105684: <http://trac.webkit.org/changeset/105684>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.

[Scott Graham]

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.