Add WorkerThread based methods for start/stop of worker threads.

Add WorkerThread based methods for start/stop of worker threads.

This patch adds notification methods for start/stop of worker threads which accept a WorkerThread parameter. This is needed in order to move the worker threads in blink to use WebThread instead of its own version.
It also adds support for shared timer to WebThreadImpl.

BUG=301515
Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=284008

[nasko, 2014-07-16 10:02:50]

Hey Jochen,
This is a start of a CL to get the chromium side of the code to work with
WebThread objects. I initially thought it might go as is, but it will need a
blink-side CL to add those methods to the blink::Platform interface.
Should I just guard all of these changes through a #define as you mentioned
before and commit it?
Thanks,
Nasko

[nasko, 2014-07-17 09:04:26]

Updated to add shared timer support in WebThread.

[jochen (gone - plz use gerrit), 2014-07-17 09:09:37]

if you leave out the OVERRIDEs, it should work as is, no?

[nasko, 2014-07-17 09:45:42]

Yes, this is what I just did.


On Thu, Jul 17, 2014 at 11:09 AM, <jochen@chromium.org> wrote:

> if you leave out the OVERRIDEs, it should work as is, no?
>
> https://codereview.chromium.org/393283003/
>

To unsubscribe from this group and stop receiving emails from it, send an email
to chromium-reviews+unsubscribe@chromium.org.

[jochen (gone - plz use gerrit), 2014-07-17 09:48:03]

lgtm

https://codereview.chromium.org/393283003/diff/20001/content/child/blink_plat...
File content/child/blink_platform_impl.h (right):

https://codereview.chromium.org/393283003/diff/20001/content/child/blink_plat...
content/child/blink_platform_impl.h:152: blink::WebThread* thread) OVERRIDE;
nit. don't use OVERRIDE on blink API implementations - it'll just make two sided
patches more painful

https://codereview.chromium.org/393283003/diff/20001/content/child/webthread_...
File content/child/webthread_impl.h (right):

https://codereview.chromium.org/393283003/diff/20001/content/child/webthread_...
content/child/webthread_impl.h:28: virtual void
setSharedTimerFiredFunction(SharedTimerFunction timerFunction) {}
why not = 0?

[nasko, 2014-07-17 10:49:33]

https://codereview.chromium.org/393283003/diff/20001/content/child/blink_plat...
File content/child/blink_platform_impl.h (right):

https://codereview.chromium.org/393283003/diff/20001/content/child/blink_plat...
content/child/blink_platform_impl.h:152: blink::WebThread* thread) OVERRIDE;
On 2014/07/17 09:48:03, jochen wrote:
> nit. don't use OVERRIDE on blink API implementations - it'll just make two
sided
> patches more painful

Done.

https://codereview.chromium.org/393283003/diff/20001/content/child/webthread_...
File content/child/webthread_impl.h (right):

https://codereview.chromium.org/393283003/diff/20001/content/child/webthread_...
content/child/webthread_impl.h:28: virtual void
setSharedTimerFiredFunction(SharedTimerFunction timerFunction) {}
On 2014/07/17 09:48:03, jochen wrote:
> why not = 0?

If I make them pure virtual, then I have to provide implementations in
WebThreadImplForMessageLoop. I just opted not to touch that class, as it doesn't
change itself.

[nasko, 2014-07-17 10:49:48]

The CQ bit was checked by nasko@chromium.org

[commit-bot: I haz the power, 2014-07-17 10:51:20]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/nasko@chromium.org/393283003/60001

[commit-bot: I haz the power, 2014-07-17 14:36:53]

FYI, CQ is re-trying this CL (attempt #1).
The failing builders are:
  linux_chromium_gn_rel on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/linux_chromium_gn_rel...)
  win_chromium_compile_dbg on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/win_chromium_compile_...)

[commit-bot: I haz the power, 2014-07-17 16:23:01]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2014-07-17 16:23:03]

Try jobs failed on following builders:
  linux_chromium_gn_rel on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/linux_chromium_gn_rel...)

[nasko, 2014-07-17 19:18:16]

The CQ bit was checked by nasko@chromium.org

[commit-bot: I haz the power, 2014-07-17 19:20:43]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/nasko@chromium.org/393283003/80001

[commit-bot: I haz the power, 2014-07-18 01:45:59]

FYI, CQ is re-trying this CL (attempt #1).
The failing builders are:
  android_dbg_triggered_tests on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/android_dbg_triggered...)

[commit-bot: I haz the power, 2014-07-18 02:09:51]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2014-07-18 02:09:52]

Try jobs failed on following builders:
  android_dbg_triggered_tests on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/android_dbg_triggered...)

[nasko, 2014-07-18 05:06:17]

The CQ bit was checked by nasko@chromium.org

[commit-bot: I haz the power, 2014-07-18 05:07:54]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/nasko@chromium.org/393283003/80001

[commit-bot: I haz the power, 2014-07-18 05:33:02]

Change committed as 284008

[sof, 2014-07-18 08:30:38]

This is causing some cross bot redness, e.g.,


http://build.chromium.org/p/chromium.webkit/builders/WebKit%20Linux/builds/34272

A Blink CL is referred to in the above; where is it?

[nasko, 2014-07-18 08:49:07]

On 2014/07/18 08:30:38, sof wrote:
> This is causing some cross bot redness, e.g.,
> 
> 
>
http://build.chromium.org/p/chromium.webkit/builders/WebKit%20Linux/builds/34272
> 
> A Blink CL is referred to in the above; where is it?

It is currently work-in-progress. I will be uploading it shortly.

[sof, 2014-07-18 08:50:29]

On 2014/07/18 08:49:07, nasko (in Munich) wrote:
> On 2014/07/18 08:30:38, sof wrote:
> > This is causing some cross bot redness, e.g.,
> > 
> > 
> >
>
http://build.chromium.org/p/chromium.webkit/builders/WebKit%20Linux/builds/34272
> > 
> > A Blink CL is referred to in the above; where is it?
> 
> It is currently work-in-progress. I will be uploading it shortly.

ok, revert the Chromium side CL until it is ready?

[nasko, 2014-07-18 08:59:24]

On 2014/07/18 08:50:29, sof wrote:
> On 2014/07/18 08:49:07, nasko (in Munich) wrote:
> > On 2014/07/18 08:30:38, sof wrote:
> > > This is causing some cross bot redness, e.g.,
> > > 
> > > 
> > >
> >
>
http://build.chromium.org/p/chromium.webkit/builders/WebKit%20Linux/builds/34272
> > > 
> > > A Blink CL is referred to in the above; where is it?
> > 
> > It is currently work-in-progress. I will be uploading it shortly.
> 
> ok, revert the Chromium side CL until it is ready?

The above URL doesn't include this CL in the blamelist. Is it certain that it is
the cause of the redness?

[jochen (gone - plz use gerrit), 2014-07-18 09:02:43]

On 2014/07/18 at 08:59:24, nasko wrote:
> On 2014/07/18 08:50:29, sof wrote:
> > On 2014/07/18 08:49:07, nasko (in Munich) wrote:
> > > On 2014/07/18 08:30:38, sof wrote:
> > > > This is causing some cross bot redness, e.g.,
> > > > 
> > > > 
> > > >
> > >
> >
http://build.chromium.org/p/chromium.webkit/builders/WebKit%20Linux/builds/34272
> > > > 
> > > > A Blink CL is referred to in the above; where is it?
> > > 
> > > It is currently work-in-progress. I will be uploading it shortly.
> > 
> > ok, revert the Chromium side CL until it is ready?
> 
> The above URL doesn't include this CL in the blamelist. Is it certain that it
is the cause of the redness?

webkit builders don't show chromium commits in the blame list :-/

[sof, 2014-07-18 09:03:35]

On 2014/07/18 08:59:24, nasko (in Munich) wrote:
> On 2014/07/18 08:50:29, sof wrote:
> > On 2014/07/18 08:49:07, nasko (in Munich) wrote:
> > > On 2014/07/18 08:30:38, sof wrote:
> > > > This is causing some cross bot redness, e.g.,
> > > > 
> > > > 
> > > >
> > >
> >
>
http://build.chromium.org/p/chromium.webkit/builders/WebKit%20Linux/builds/34272
> > > > 
> > > > A Blink CL is referred to in the above; where is it?
> > > 
> > > It is currently work-in-progress. I will be uploading it shortly.
> > 
> > ok, revert the Chromium side CL until it is ready?
> 
> The above URL doesn't include this CL in the blamelist. Is it certain that it
is
> the cause of the redness?

Confirmed locally + if you go back in build history you'll see it showing,

 
http://build.chromium.org/p/chromium.webkit/builders/WebKit%20Mac10.9/builds/...

[nasko, 2014-07-18 09:05:12]

A revert of this CL has been created in
https://codereview.chromium.org/404753002/ by nasko@chromium.org.

The reason for reverting is: Breaking blink bots..