Fix gpu_unittests.isolate dependencies.

Fix gpu_unittests.isolate dependencies.

The gpu_unittests don't use any ANGLE or native graphics API
logic. Thus the isolate shouldn't include any dependency on things
it doesn't use or build. This should clear up runtime errors on the
bots involving gpu_unittests_run and libGLESv2.dll.

BUG=415981
Committed: https://crrev.com/0a2788afa23b82c428cd2bf00b06d5b510436fef
Cr-Commit-Position: refs/heads/master@{#296233}

[Jamie Madill, 2014-09-23 15:11:03]

jmadill@chromium.org changed reviewers:
+ csharp@chromium.org, kbr@chromium.org

[Jamie Madill, 2014-09-23 15:49:17]

PTAL

[Ken Russell (switch to Gerrit), 2014-09-23 15:58:17]

kbr@chromium.org changed reviewers:
+ bajones@chromium.org, zmo@chromium.org

[Ken Russell (switch to Gerrit), 2014-09-23 15:58:18]

https://codereview.chromium.org/594203003/diff/1/chrome/gpu_unittests.isolate
File chrome/gpu_unittests.isolate (right):

https://codereview.chromium.org/594203003/diff/1/chrome/gpu_unittests.isolate...
chrome/gpu_unittests.isolate:6: '../base/base.isolate',
The gpu_unittests target in src/gpu/gpu.gyp currently depends on
'<(angle_path)/src/build_angle.gyp:translator'. Is this dependency unnecessary?
If so, let's remove it now.

[Jamie Madill, 2014-09-23 16:02:28]

On 2014/09/23 15:58:18, Ken Russell wrote:
> https://codereview.chromium.org/594203003/diff/1/chrome/gpu_unittests.isolate
> File chrome/gpu_unittests.isolate (right):
> 
>
https://codereview.chromium.org/594203003/diff/1/chrome/gpu_unittests.isolate...
> chrome/gpu_unittests.isolate:6: '../base/base.isolate',
> The gpu_unittests target in src/gpu/gpu.gyp currently depends on
> '<(angle_path)/src/build_angle.gyp:translator'. Is this dependency
unnecessary?
> If so, let's remove it now.

Hm... I don't think it's unnecessary. And possibly in the component build we'd
need to package the translator DLL (I had only tested with Release/non
component). Let me check with the component build.. I have a feeling you found a
bug in my CL here.

[Ken Russell (switch to Gerrit), 2014-09-23 16:08:07]

On 2014/09/23 16:02:28, Jamie Madill wrote:
> On 2014/09/23 15:58:18, Ken Russell wrote:
> >
https://codereview.chromium.org/594203003/diff/1/chrome/gpu_unittests.isolate
> > File chrome/gpu_unittests.isolate (right):
> > 
> >
>
https://codereview.chromium.org/594203003/diff/1/chrome/gpu_unittests.isolate...
> > chrome/gpu_unittests.isolate:6: '../base/base.isolate',
> > The gpu_unittests target in src/gpu/gpu.gyp currently depends on
> > '<(angle_path)/src/build_angle.gyp:translator'. Is this dependency
> unnecessary?
> > If so, let's remove it now.
> 
> Hm... I don't think it's unnecessary. And possibly in the component build we'd
> need to package the translator DLL (I had only tested with Release/non
> component). Let me check with the component build.. I have a feeling you found
a
> bug in my CL here.

I think when using isolates with the component build, the isolate driver
implicitly picks up the references by scanning through the Ninja files. I note
that we don't explicitly include the translator in any of the isolates --
probably because of this reason.

Your CL may well be 100% correct. I'd personally like to understand whether the
translator is actually used by gpu_unittests. Regardless, this LGTM if it works.

[Jamie Madill, 2014-09-23 16:45:38]

On 2014/09/23 16:08:07, Ken Russell wrote:
> On 2014/09/23 16:02:28, Jamie Madill wrote:
> > On 2014/09/23 15:58:18, Ken Russell wrote:
> > >
> https://codereview.chromium.org/594203003/diff/1/chrome/gpu_unittests.isolate
> > > File chrome/gpu_unittests.isolate (right):
> > > 
> > >
> >
>
https://codereview.chromium.org/594203003/diff/1/chrome/gpu_unittests.isolate...
> > > chrome/gpu_unittests.isolate:6: '../base/base.isolate',
> > > The gpu_unittests target in src/gpu/gpu.gyp currently depends on
> > > '<(angle_path)/src/build_angle.gyp:translator'. Is this dependency
> > unnecessary?
> > > If so, let's remove it now.
> > 
> > Hm... I don't think it's unnecessary. And possibly in the component build
we'd
> > need to package the translator DLL (I had only tested with Release/non
> > component). Let me check with the component build.. I have a feeling you
found
> a
> > bug in my CL here.
> 
> I think when using isolates with the component build, the isolate driver
> implicitly picks up the references by scanning through the Ninja files. I note
> that we don't explicitly include the translator in any of the isolates --
> probably because of this reason.
> 
> Your CL may well be 100% correct. I'd personally like to understand whether
the
> translator is actually used by gpu_unittests. Regardless, this LGTM if it
works.

Yes, IIRC we broke gpu_unittests once in ANGLE ToT because of a shader
translator
change. The translator doesn't call through to any OS graphics API calls and
there
is a test in the gpu_unittests which tests the shader translator.

I checked and you were right, the translator.dll was packaged automatically.
Seems
to work.

[Ken Russell (switch to Gerrit), 2014-09-23 16:51:09]

OK. Please CQ when ready, and let's re-enable this test on the GPU bots.

[Jamie Madill, 2014-09-23 16:57:46]

On 2014/09/23 16:51:09, Ken Russell wrote:
> OK. Please CQ when ready, and let's re-enable this test on the GPU bots.

Just need LGTM from csharp.. I think he's an owner of this particular file.

[csharp, 2014-09-23 17:00:06]

The CQ bit was checked by csharp@chromium.org

[csharp, 2014-09-23 17:00:07]

lgtm

[Jamie Madill, 2014-09-23 17:51:11]

The CQ bit was unchecked by jmadill@chromium.org

[Jamie Madill, 2014-09-23 17:51:13]

The CQ bit was checked by jmadill@chromium.org

[commit-bot: I haz the power, 2014-09-23 17:59:13]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/594203003/1

[commit-bot: I haz the power, 2014-09-23 20:59:03]

Committed patchset #1 (id:1) as bf6ca4ff4a85dd025040d92dbf1cc4a0716720f3

[commit-bot: I haz the power, 2014-09-23 20:59:31]

Patchset 1 (id:??) landed as
https://crrev.com/0a2788afa23b82c428cd2bf00b06d5b510436fef
Cr-Commit-Position: refs/heads/master@{#296233}