Update from https://crrev.com/312398

Update from https://crrev.com/312398

Involves adding many //testing/test.gni imports, fixing one
SkSurface::NewRenderTarget invocation inside sky, and fixing up
base::Process usage in the shell.

Committed: https://chromium.googlesource.com/external/mojo/+/5e66a79729a658ce54b1ec33f4d4d108bdfccaff

[jamesr, 2015-01-21 21:09:10]

https://codereview.chromium.org/862133002/diff/1/shell/child_process_host.h
File shell/child_process_host.h (right):

https://codereview.chromium.org/862133002/diff/1/shell/child_process_host.h#n...
shell/child_process_host.h:68: base::Process child_process_;
Trung - could you please review the changes to this .h/.cc?  The base::Process
stuff has been refactored upstream to get rid of the ProcessHandle concept and I
wanna make sure I patched this file correctly.

[jamesr, 2015-01-21 21:09:32]

jamesr@chromium.org changed reviewers:
+ viettrungluu@chromium.org

[jamesr, 2015-01-21 21:10:14]

https://codereview.chromium.org/862133002/diff/1/shell/child_process_host.h
File shell/child_process_host.h (right):

https://codereview.chromium.org/862133002/diff/1/shell/child_process_host.h#n...
shell/child_process_host.h:68: base::Process child_process_;
Trung - could you please review the changes to this .h/.cc?  The base::Process
stuff has been refactored upstream to get rid of the ProcessHandle concept and I
wanna make sure I patched this file correctly.

[viettrungluu, 2015-01-21 22:27:04]

https://codereview.chromium.org/862133002/diff/1/shell/child_process_host.cc
File shell/child_process_host.cc (right):

https://codereview.chromium.org/862133002/diff/1/shell/child_process_host.cc#...
shell/child_process_host.cc:56: // Note: |WaitForExit()| closes the process
handle.
It doesn't, so you should do |child_process_.Close()| afterwards.

https://codereview.chromium.org/862133002/diff/1/shell/child_process_host.cc#...
shell/child_process_host.cc:58: LOG_IF(ERROR, rv != 0) << "Failed to wait for
child process";
You should keep the form of the original LOG_IF:

LOG_IF(ERROR, !child_process_.WaitForExit(&rv))

(or something equivalent).

That is, log an error if you fail to wait, not if the child's exit code is
"failure".

[jamesr, 2015-01-22 00:59:22]

jamesr@chromium.org changed reviewers:
+ qsr@chromium.org

[jamesr, 2015-01-22 00:59:29]

Gonna land since we have to keep up, although this might break some mojo shell
tests on the android bots.  Will deal with fallout if it occurs.

qsr - the fix should be easy if I'm understanding it correctly but it'll require
cross-repo changes.

https://codereview.chromium.org/862133002/diff/10001/shell/BUILD.gn
File shell/BUILD.gn (right):

https://codereview.chromium.org/862133002/diff/10001/shell/BUILD.gn#newcode439
shell/BUILD.gn:439: # TODO(qsr): asset_location = mojo_shell_assets_dir ?
qsr@ = there appears to be a deficiency in the test() template - there's no way
to specify the asset_location for the generated xxx__unittest_apk.

[jamesr, 2015-01-22 01:02:17]

Committed patchset #2 (id:10001) manually as
5e66a79729a658ce54b1ec33f4d4d108bdfccaff.

[qsr, 2015-01-22 12:26:04]

https://codereview.chromium.org/862133002/diff/10001/shell/BUILD.gn
File shell/BUILD.gn (right):

https://codereview.chromium.org/862133002/diff/10001/shell/BUILD.gn#newcode439
shell/BUILD.gn:439: # TODO(qsr): asset_location = mojo_shell_assets_dir ?
On 2015/01/22 00:59:28, jamesr wrote:
> qsr@ = there appears to be a deficiency in the test() template - there's no
way
> to specify the asset_location for the generated xxx__unittest_apk.

 Fixed by https://codereview.chromium.org/862293003/

[jamesr, 2015-01-22 18:20:20]

On 2015/01/22 12:26:04, qsr wrote:
> https://codereview.chromium.org/862133002/diff/10001/shell/BUILD.gn
> File shell/BUILD.gn (right):
> 
> https://codereview.chromium.org/862133002/diff/10001/shell/BUILD.gn#newcode439
> shell/BUILD.gn:439: # TODO(qsr): asset_location = mojo_shell_assets_dir ?
> On 2015/01/22 00:59:28, jamesr wrote:
> > qsr@ = there appears to be a deficiency in the test() template - there's no
> way
> > to specify the asset_location for the generated xxx__unittest_apk.
> 
>  Fixed by https://codereview.chromium.org/862293003/

Thanks!