Android: Get rid of extra dup()s on launching child processes

Android: Get rid of extra dup()s on launching child processes

This is a regression from [1] which added a dup() call on
the renderer launching process. This CL removes these calls
by reveiling the subtle ownership of the file descriptors.

The original intention here was to completely hide and simplify
the notion of the ownership, at the cost of a few dup() calls.
However, the crash report on the reported bug indicates that
the dup() can fail and it lets the renderer initialization fail,
probably due to some per-process limit of the number of opened files.

[1] https://codereview.chromium.org/585203002

R=mdempsky@chromium.org, jam@chromium.org
BUG=455364
Committed: https://crrev.com/f7302eb74f6e79d5b4e26ea5e9aa4e4eb81a52f1
Cr-Commit-Position: refs/heads/master@{#315353}

[Hajime Morrita, 2015-02-06 23:03:52]

jam: PTAL at content/public/browser/file_descriptor_info.h?
mdempsky: PTAL at overall CL?

[jam, 2015-02-06 23:58:18]

lgtm

[mdempsky, 2015-02-07 02:37:05]

lgtm

https://codereview.chromium.org/909553003/diff/1/content/browser/android/chil...
File content/browser/android/child_process_launcher_android.cc (right):

https://codereview.chromium.org/909553003/diff/1/content/browser/android/chil...
content/browser/android/child_process_launcher_android.cc:134:
file_auto_close[i] = files_to_register->OwnsFD(file_fds[i]);
OwnsFD and ReleaseFD seem like clunky APIs, but unfortunately I don't have a
better suggestion, so this seems okay.

Out of curiosity, why does ownership need to be transferred here at all?  Why
can't files_to_register continue to own its FDs, and set all of file_auto_close
to false?

[Hajime Morrita, 2015-02-09 18:44:36]

Thanks for the review!

On 2015/02/07 02:37:05, mdempsky wrote:
> lgtm
> 
>
https://codereview.chromium.org/909553003/diff/1/content/browser/android/chil...
> File content/browser/android/child_process_launcher_android.cc (right):
> 
>
https://codereview.chromium.org/909553003/diff/1/content/browser/android/chil...
> content/browser/android/child_process_launcher_android.cc:134:
> file_auto_close[i] = files_to_register->OwnsFD(file_fds[i]);
> OwnsFD and ReleaseFD seem like clunky APIs, but unfortunately I don't have a
> better suggestion, so this seems okay.
> 
> Out of curiosity, why does ownership need to be transferred here at all?  Why
> can't files_to_register continue to own its FDs, and set all of
file_auto_close
> to false?

This is exactly the point.
The passed file descriptor is outlive after the function scope and used
asynchronously
so we cannot just let FileDescriptorInfo close them, as it dies at the end of
StartChildProcess().

[Hajime Morrita, 2015-02-09 18:44:41]

The CQ bit was checked by morrita@chromium.org

[commit-bot: I haz the power, 2015-02-09 18:46:00]

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

[commit-bot: I haz the power, 2015-02-09 18:53:34]

Committed patchset #1 (id:1)

[commit-bot: I haz the power, 2015-02-09 18:54:10]

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