Simplifying ContentBrowserClient::GetAdditionalMappedFiles...()

Simplifying ContentBrowserClient::GetAdditionalMappedFiles...()

First step in an effort to refactor content::ChildProcessLauncher, by
unifying
ContentBrowserClient::GetAdditionalMappedFilesForChildProcess() on
Android and Linux by moving the region argument to
content::FileDescriptorInfo.

BUG=675294
Review-Url: https://codereview.chromium.org/2613803003
Cr-Commit-Position: refs/heads/master@{#442007}
Committed: https://chromium.googlesource.com/chromium/src/+/e6f55074f173bd0c45d96e98a7d747032ab786bb

[Jay Civelli, 2017-01-05 02:28:11]

The CQ bit was checked by jcivelli@chromium.org to run a CQ dry run

[Jay Civelli, 2017-01-05 02:28:28]

Description was changed from

==========
Simplifying ContentBrowserClient::GetAdditionalMappedFiles...()

First step in an effort to refactor content::ChildProcessLauncher, by
unifying
ContentBrowserClient::GetAdditionalMappedFilesForChildProcess() on
Android and Linux by moving the region argument to
content::FileDescriptorInfo.

BUG=675294
==========

to

==========
Simplifying ContentBrowserClient::GetAdditionalMappedFiles...()

First step in an effort to refactor content::ChildProcessLauncher, by
unifying
ContentBrowserClient::GetAdditionalMappedFilesForChildProcess() on
Android and Linux by moving the region argument to
content::FileDescriptorInfo.

BUG=675294
==========

[Jay Civelli, 2017-01-05 02:28:28]

jcivelli@chromium.org changed reviewers:
+ boliu@chromium.org, jam@chromium.org

[commit-bot: I haz the power, 2017-01-05 02:28:54]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-01-05 02:52:43]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-01-05 02:52:44]

Dry run: Try jobs failed on following builders:
  cast_shell_android on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/cast_shell_a...)

[boliu, 2017-01-05 07:02:09]

https://codereview.chromium.org/2613803003/diff/1/content/browser/child_proce...
File content/browser/child_process_launcher.cc (right):

https://codereview.chromium.org/2613803003/diff/1/content/browser/child_proce...
content/browser/child_process_launcher.cc:177: std::map<int,
base::MemoryMappedFile::Region> regions;
seems incomplete this is still here and the regions from files_to_resgister is
ignored?

[Jay Civelli, 2017-01-05 17:12:04]

The CQ bit was checked by jcivelli@chromium.org to run a CQ dry run

[Jay Civelli, 2017-01-05 17:12:18]

https://codereview.chromium.org/2613803003/diff/1/content/browser/child_proce...
File content/browser/child_process_launcher.cc (right):

https://codereview.chromium.org/2613803003/diff/1/content/browser/child_proce...
content/browser/child_process_launcher.cc:177: std::map<int,
base::MemoryMappedFile::Region> regions;
On 2017/01/05 07:02:08, boliu wrote:
> seems incomplete this is still here and the regions from files_to_resgister is
> ignored?

You are right. This is part of a bigger CL I am breaking down and forgot some
parts. Thanks for catching this.

[commit-bot: I haz the power, 2017-01-05 17:12:32]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-01-05 18:11:31]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-01-05 18:11:31]

Dry run: This issue passed the CQ dry run.

[boliu, 2017-01-05 23:38:46]

lgtm

[jam, 2017-01-06 01:28:45]

lgtm

https://codereview.chromium.org/2613803003/diff/1/content/public/browser/file...
File content/public/browser/file_descriptor_info.h (right):

https://codereview.chromium.org/2613803003/diff/1/content/public/browser/file...
content/public/browser/file_descriptor_info.h:30: virtual void
ShareWithRegion(int id, base::PlatformFile fd,
nit: indentation is off per style guide. if the second line doesn't align with
"int id", then move the "int id" to the next line

[Jay Civelli, 2017-01-06 18:15:29]

The CQ bit was checked by jcivelli@chromium.org

[Jay Civelli, 2017-01-06 18:15:30]

The patchset sent to the CQ was uploaded after l-g-t-m from jam@chromium.org,
boliu@chromium.org
Link to the patchset: https://codereview.chromium.org/2613803003/#ps60001
(title: "Addressed jam@'s comment")

[commit-bot: I haz the power, 2017-01-06 18:16:05]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-01-06 19:29:17]

CQ is committing da patch.

Bot data: {"patchset_id": 60001, "attempt_start_ts": 1483726529969650,
"parent_rev": "9909a9f963c17a4ae2342303fe5971e54e38ef46", "commit_rev":
"e6f55074f173bd0c45d96e98a7d747032ab786bb"}

[commit-bot: I haz the power, 2017-01-06 19:29:44]

Description was changed from

==========
Simplifying ContentBrowserClient::GetAdditionalMappedFiles...()

First step in an effort to refactor content::ChildProcessLauncher, by
unifying
ContentBrowserClient::GetAdditionalMappedFilesForChildProcess() on
Android and Linux by moving the region argument to
content::FileDescriptorInfo.

BUG=675294
==========

to

==========
Simplifying ContentBrowserClient::GetAdditionalMappedFiles...()

First step in an effort to refactor content::ChildProcessLauncher, by
unifying
ContentBrowserClient::GetAdditionalMappedFilesForChildProcess() on
Android and Linux by moving the region argument to
content::FileDescriptorInfo.

BUG=675294

Review-Url: https://codereview.chromium.org/2613803003
Cr-Commit-Position: refs/heads/master@{#442007}
Committed:
https://chromium.googlesource.com/chromium/src/+/e6f55074f173bd0c45d96e98a7d7...
==========

[commit-bot: I haz the power, 2017-01-06 19:29:49]

Committed patchset #4 (id:60001) as
https://chromium.googlesource.com/chromium/src/+/e6f55074f173bd0c45d96e98a7d7...