Include chrome_elf.dll and ffmpegsumo.dll in Windows isolate for Chromoting tests.

Include chrome_elf.dll and ffmpegsumo.dll in Windows isolate for Chromoting tests.

BUG=425176
Committed: https://crrev.com/d42e0915a0b2691cf481fa39fe8174ae05363901
Cr-Commit-Position: refs/heads/master@{#302735}

[anandc, 2014-11-04 22:07:07]

anandc@chromium.org changed reviewers:
+ maruel@chromium.org

[anandc, 2014-11-04 22:07:08]

Hi M-A, Could you please take a look?

Thanks.

https://codereview.chromium.org/698203005/diff/1/testing/chromoting/chromotin...
File testing/chromoting/chromoting_integration_tests.isolate (right):

https://codereview.chromium.org/698203005/diff/1/testing/chromoting/chromotin...
testing/chromoting/chromoting_integration_tests.isolate:58:
'<(PRODUCT_DIR)/ffmpegsumo.dll',
I'm actually not certain these are sufficient. 
Am rebuilding on Windows right now to see if that will fix it. That will take a
while, so hoping to try adding these and see if that fixes the
dependency-not-found error we've been seeing.

[anandc, 2014-11-05 00:15:47]

On 2014/11/04 22:07:08, anandc wrote:
> Hi M-A, Could you please take a look?
> 
> Thanks.
> 
>
https://codereview.chromium.org/698203005/diff/1/testing/chromoting/chromotin...
> File testing/chromoting/chromoting_integration_tests.isolate (right):
> 
>
https://codereview.chromium.org/698203005/diff/1/testing/chromoting/chromotin...
> testing/chromoting/chromoting_integration_tests.isolate:58:
> '<(PRODUCT_DIR)/ffmpegsumo.dll',
> I'm actually not certain these are sufficient. 
> Am rebuilding on Windows right now to see if that will fix it. That will take
a
> while, so hoping to try adding these and see if that fixes the
> dependency-not-found error we've been seeing.

Just tested this on a swarming Windows bot, and the test succeeded with the 2
additional dependencies included in the isolate:
https://chromium-swarm-dev.appspot.com/user/task/1497d4754916f00
Without it, the test failed:
https://chromium-swarm-dev.appspot.com/user/task/1497d4b1aeabe00.

To be sure, this was with a shared-library build (building static on Windows
doesn't work reliably for me). But the fix looks promising.

[M-A Ruel, 2014-11-05 00:22:40]

lgtm with comment removed.

https://codereview.chromium.org/698203005/diff/20001/testing/chromoting/chrom...
File testing/chromoting/chromoting_integration_tests.isolate (right):

https://codereview.chromium.org/698203005/diff/20001/testing/chromoting/chrom...
testing/chromoting/chromoting_integration_tests.isolate:55: # Copy the VS
runtime DLLs into the isolate so that they
The comment doesn't match the files. Remove it.

IMHO, you should strip this file and instead replace line 77 with
'../../chrome/chrome.isolate'. It contains everything to be able to start
chrome, which includes these dlls.

But if I understand correctly, you don't need chrome.exe at all. In that case
it's better to not include it. It'd still be nice to refactor the includes a bit
eventually to reduce the duplication.

[anandc, 2014-11-05 00:32:33]

https://codereview.chromium.org/698203005/diff/20001/testing/chromoting/chrom...
File testing/chromoting/chromoting_integration_tests.isolate (right):

https://codereview.chromium.org/698203005/diff/20001/testing/chromoting/chrom...
testing/chromoting/chromoting_integration_tests.isolate:55: # Copy the VS
runtime DLLs into the isolate so that they
On 2014/11/05 00:22:40, M-A Ruel wrote:
> The comment doesn't match the files. Remove it.
> 
> IMHO, you should strip this file and instead replace line 77 with
> '../../chrome/chrome.isolate'. It contains everything to be able to start
> chrome, which includes these dlls.
> 
> But if I understand correctly, you don't need chrome.exe at all. In that case
> it's better to not include it. It'd still be nice to refactor the includes a
bit
> eventually to reduce the duplication.

Right, we want browser_tests.exe, but not chrome.exe.

I've not done the stripping to reduce duplication in this CL. Just want to get
the tests working-as-expected first. :-)
Thanks.

[anandc, 2014-11-05 00:32:43]

The CQ bit was checked by anandc@chromium.org

[commit-bot: I haz the power, 2014-11-05 00:34:06]

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

[commit-bot: I haz the power, 2014-11-05 01:30:50]

Committed patchset #3 (id:40001)

[commit-bot: I haz the power, 2014-11-05 01:31:58]

Patchset 3 (id:??) landed as
https://crrev.com/d42e0915a0b2691cf481fa39fe8174ae05363901
Cr-Commit-Position: refs/heads/master@{#302735}