Make workers functional on OSX and Linux.

Make workers functional on OSX and Linux.

BUG=http://crbug.com/16664
TEST=The same tests run for Web Workers on Windows should be run on Linux and OSX.



Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=20653

[levin, 2009-07-03 07:23:37]

I know I need to add a BUG= and TEST= to the description.

Other than that, how does this look to you?

Thanks,
Dave

[jam, 2009-07-06 19:24:12]

def the right approach, just some nits

http://codereview.chromium.org/155015/diff/31/1010
File chrome/browser/worker_host/worker_process_host.cc (right):

http://codereview.chromium.org/155015/diff/31/1010#newcode106
Line 106: ipcfd, kPrimaryIPCChannel +
base::GlobalDescriptors::kBaseDescriptor));
nit: this formatting is off (2 spaces only, need brace brackets if more than one
line)

http://codereview.chromium.org/155015/diff/31/1011
File chrome/common/child_process_host.cc (right):

http://codereview.chromium.org/155015/diff/31/1011#newcode65
Line 65: if (process_event_.get()) {
This changes the logic slightly, since process_event_ is only set in Windows. 
That means other platforms won't call EnsureProcessTerminated, which we want to
call.  I think this should be changed back to if(handle()) and you can move the
StopWatching() to the Windows ifdef.

http://codereview.chromium.org/155015/diff/31/1014
File chrome/worker/worker_thread.cc (right):

http://codereview.chromium.org/155015/diff/31/1014#newcode57
Line 57: new WebWorkerClientProxy(url, route_id);
nit: spacing is off

[levin, 2009-07-13 22:38:24]

Fixed up.

chrome/common/child_process_host.cc was fixed up for Linux in
http://src.chromium.org/viewvc/chrome?view=rev&revision=20414 so the changes to
that file went away.

[jam, 2009-07-13 23:30:22]

lgtm

http://codereview.chromium.org/155015/diff/34/1033
File chrome/browser/worker_host/worker_process_host.cc (right):

http://codereview.chromium.org/155015/diff/34/1033#newcode111
Line 111: ipcfd, kPrimaryIPCChannel +
base::GlobalDescriptors::kBaseDescriptor));
nit: sorry I wasn't clear least time, two spaces on the first line and 4 spaces
on the second.

[M-A Ruel, 2009-08-04 16:59:02]

On 2009/07/13 23:30:22, John Abd-El-Malek wrote:
> lgtm

YOU FORGOT TO SVN:IGNORE THE GENERATED PROJECT FILES. (I'll do because
you're not alone)

[levin, 2009-08-04 17:50:42]

I don't understand.  What generated files were introduced by this change?
Are you referring to the precompiled headers files for windows that were
there before this change?

Thanks,
Dave


On Tue, Aug 4, 2009 at 9:59 AM, <maruel@chromium.org> wrote:

> On 2009/07/13 23:30:22, John Abd-El-Malek wrote:
>
>> lgtm
>>
>
> YOU FORGOT TO SVN:IGNORE THE GENERATED PROJECT FILES. (I'll do because
> you're not alone)
>
> http://codereview.chromium.org/155015
>

[M-A Ruel, 2009-08-04 18:10:51]

worker.scons. See a random try
job<http://build.chromium.org/buildbot/try-server/builders/linux/builds/11740/steps/gclient/logs/stdio>for
an example.

Note this won't be necessary once (probably me) fix http://crbug.com/15924.

M-A


On Tue, Aug 4, 2009 at 1:50 PM, David Levin <levin@chromium.org> wrote:

> I don't understand.  What generated files were introduced by this change?
> Are you referring to the precompiled headers files for windows that were
> there before this change?
>
> Thanks,
> Dave
>
>
> On Tue, Aug 4, 2009 at 9:59 AM, <maruel@chromium.org> wrote:
>
>> On 2009/07/13 23:30:22, John Abd-El-Malek wrote:
>>
>>> lgtm
>>>
>>
>> YOU FORGOT TO SVN:IGNORE THE GENERATED PROJECT FILES. (I'll do because
>> you're not alone)
>>
>> http://codereview.chromium.org/155015
>>
>
>