Take 2: Moved logic for mapping child process FDs for ICU and V8 into child_process_launcher.cc

Take 2: Moved logic for mapping child process FDs for ICU and V8 into child_process_launcher.cc

Used to be defined in each app's ContentBrowserClient, but since
content/ is the one that receives the FDs, it makes sense that it should
be the one to send them.

This also removes ChildProcessLauncher::AppendMappedFileCommandLineSwitches
as it is no longer needed.

Changes MemoryMappedFile::Region to be a POD so that it doesn't create require static initializers.
BUG=394502
Committed: https://crrev.com/fd2d44abf30dcb70c083cbbbb2491e825ab61f41
Cr-Commit-Position: refs/heads/master@{#335207}

[agrieve, 2015-06-17 02:14:59]

agrieve@chromium.org changed reviewers:
+ jam@chromium.org

[agrieve, 2015-06-17 02:15:00]

Confirmed with tools/linux/dump-static-initializers.py that no static
initializers are introduced now. Only difference from before is how the
variables are defined.

[rmcilroy, 2015-06-17 08:00:49]

rmcilroy@chromium.org changed reviewers:
+ rmcilroy@chromium.org

[rmcilroy, 2015-06-17 08:00:50]

https://codereview.chromium.org/1185973003/diff/40001/gin/v8_initializer.cc
File gin/v8_initializer.cc (right):

https://codereview.chromium.org/1185973003/diff/40001/gin/v8_initializer.cc#n...
gin/v8_initializer.cc:54:
*reinterpret_cast<base::MemoryMappedFile::Region*>(g_snapshot_region_data);
This seems a bit hacky personally. I would prefer just to use a pointer, or
maybe you could refactor this (and the PlatformFiles) to be initialized as a
base::LazyInstance (or simply stored in a global struct which is initialized
once as necessary)?

[agrieve, 2015-06-17 13:41:46]

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

[commit-bot: I haz the power, 2015-06-17 13:42:54]

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

[commit-bot: I haz the power, 2015-06-17 13:42:57]

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

[commit-bot: I haz the power, 2015-06-17 13:42:57]

Dry run: No LGTM from a valid reviewer yet. Only full committers are accepted.
Even if an LGTM may have been provided, it was from a non-committer,
_not_ a full super star committer.
See http://www.chromium.org/getting-involved/become-a-committer
Note that this has nothing to do with OWNERS files.

[jam, 2015-06-17 15:38:35]

please get owners of base/ and gin/ to approve the changes (i assume that
patchset 1->3 is the difference after reland?).

then you have my rubberstamp lgtm for the reland

[agrieve, 2015-06-17 18:34:39]

agrieve@chromium.org changed reviewers:
+ jochen@chromium.org, thestig@chromium.org

[agrieve, 2015-06-17 18:37:28]

On 2015/06/17 08:00:50, rmcilroy wrote:
> https://codereview.chromium.org/1185973003/diff/40001/gin/v8_initializer.cc
> File gin/v8_initializer.cc (right):
> 
>
https://codereview.chromium.org/1185973003/diff/40001/gin/v8_initializer.cc#n...
> gin/v8_initializer.cc:54:
> *reinterpret_cast<base::MemoryMappedFile::Region*>(g_snapshot_region_data);
> This seems a bit hacky personally. I would prefer just to use a pointer, or
> maybe you could refactor this (and the PlatformFiles) to be initialized as a
> base::LazyInstance (or simply stored in a global struct which is initialized
> once as necessary)?

Alright, made it less hacky by converting Region to be a POD.

[agrieve, 2015-06-17 18:37:45]

On 2015/06/17 15:38:35, jam wrote:
> please get owners of base/ and gin/ to approve the changes (i assume that
> patchset 1->3 is the difference after reland?).
> 
> then you have my rubberstamp lgtm for the reland

Added owners.

[agrieve, 2015-06-17 20:21:25]

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

[agrieve, 2015-06-17 20:21:25]

The patchset sent to the CQ was uploaded after l-g-t-m from jam@chromium.org
Link to the patchset: https://codereview.chromium.org/1185973003/#ps100001
(title: "zero initialize")

[commit-bot: I haz the power, 2015-06-17 20:24:21]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1185973003/100001

[commit-bot: I haz the power, 2015-06-17 20:43:34]

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

[commit-bot: I haz the power, 2015-06-17 20:43:36]

Dry run: Try jobs failed on following builders:
  linux_chromium_gn_dbg on tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  linux_chromium_gn_rel on tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[Lei Zhang, 2015-06-17 20:50:54]

On 2015/06/17 20:43:36, commit-bot: I haz the power wrote:
> Dry run: Try jobs failed on following builders:
>   linux_chromium_gn_dbg on tryserver.chromium.linux (JOB_FAILED,
>
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
>   linux_chromium_gn_rel on tryserver.chromium.linux (JOB_FAILED,
>
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

Trybots are red...

[agrieve, 2015-06-18 00:24:36]

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

[agrieve, 2015-06-18 00:24:37]

The patchset sent to the CQ was uploaded after l-g-t-m from jam@chromium.org
Link to the patchset: https://codereview.chromium.org/1185973003/#ps120001
(title: "finish Region refactor")

[commit-bot: I haz the power, 2015-06-18 00:26:26]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1185973003/120001

[agrieve, 2015-06-18 02:19:57]

On 2015/06/17 20:50:54, Lei Zhang wrote:
> On 2015/06/17 20:43:36, commit-bot: I haz the power wrote:
> > Dry run: Try jobs failed on following builders:
> >   linux_chromium_gn_dbg on tryserver.chromium.linux (JOB_FAILED,
> >
>
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
> >   linux_chromium_gn_rel on tryserver.chromium.linux (JOB_FAILED,
> >
>
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
> 
> Trybots are red...

Wanted to make sure everyone was okay with making Region a POD before updating
remaining usages of it...

I've gone ahead and done the updates now though and bots are green.

[commit-bot: I haz the power, 2015-06-18 02:25:38]

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

[commit-bot: I haz the power, 2015-06-18 02:25:39]

Dry run: Try jobs failed on following builders:
  win8_chromium_ng on tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win8_chromium_ng/...)

[jochen (gone - plz use gerrit), 2015-06-18 12:59:21]

gin/ lgtm

https://codereview.chromium.org/1185973003/diff/120001/base/files/memory_mapp...
File base/files/memory_mapped_file.h (right):

https://codereview.chromium.org/1185973003/diff/120001/base/files/memory_mapp...
base/files/memory_mapped_file.h:31: bool operator==(const Region& other) const;
not needed anymore, no?

[agrieve, 2015-06-18 13:33:02]

https://codereview.chromium.org/1185973003/diff/120001/base/files/memory_mapp...
File base/files/memory_mapped_file.h (right):

https://codereview.chromium.org/1185973003/diff/120001/base/files/memory_mapp...
base/files/memory_mapped_file.h:31: bool operator==(const Region& other) const;
On 2015/06/18 12:59:21, jochen wrote:
> not needed anymore, no?

I think they are still needed:
http://stackoverflow.com/questions/217911/why-dont-c-compilers-define-operato...

[Lei Zhang, 2015-06-18 19:00:51]

lgtm

https://codereview.chromium.org/1185973003/diff/120001/base/i18n/icu_util.cc
File base/i18n/icu_util.cc (right):

https://codereview.chromium.org/1185973003/diff/120001/base/i18n/icu_util.cc#...
base/i18n/icu_util.cc:126: if (g_icudtl_mapped_file != nullptr) {
just: if (foo_ptr) ?

https://codereview.chromium.org/1185973003/diff/120001/base/i18n/icu_util.cc#...
base/i18n/icu_util.cc:132: g_icudtl_mapped_file = new MemoryMappedFile();
how about:

scoped_ptr<MemoryMappedFile> icudtl_mapped_file(new MemoryMappedFile());
if (!icudtl_mapped_file-Initialize(...)) {
  LOG(ERROR) << "fail";
  return false;
}
g_icudtl_mapped_file = icudtl_mapped_file.release();

[agrieve, 2015-06-18 20:22:14]

https://codereview.chromium.org/1185973003/diff/120001/base/i18n/icu_util.cc
File base/i18n/icu_util.cc (right):

https://codereview.chromium.org/1185973003/diff/120001/base/i18n/icu_util.cc#...
base/i18n/icu_util.cc:126: if (g_icudtl_mapped_file != nullptr) {
On 2015/06/18 19:00:51, Lei Zhang wrote:
> just: if (foo_ptr) ?

Done.

https://codereview.chromium.org/1185973003/diff/120001/base/i18n/icu_util.cc#...
base/i18n/icu_util.cc:132: g_icudtl_mapped_file = new MemoryMappedFile();
On 2015/06/18 19:00:51, Lei Zhang wrote:
> how about:
> 
> scoped_ptr<MemoryMappedFile> icudtl_mapped_file(new MemoryMappedFile());
> if (!icudtl_mapped_file-Initialize(...)) {
>   LOG(ERROR) << "fail";
>   return false;
> }
> g_icudtl_mapped_file = icudtl_mapped_file.release();

Like it. Done.

[agrieve, 2015-06-18 20:22:28]

The CQ bit was checked by agrieve@chromium.org

[agrieve, 2015-06-18 20:22:28]

The patchset sent to the CQ was uploaded after l-g-t-m from
thestig@chromium.org, jochen@chromium.org, jam@chromium.org
Link to the patchset: https://codereview.chromium.org/1185973003/#ps160001
(title: "scoped_ptr for gin/v8_initializer.cc")

[commit-bot: I haz the power, 2015-06-18 20:23:05]

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

[commit-bot: I haz the power, 2015-06-18 21:51:12]

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

[commit-bot: I haz the power, 2015-06-18 21:51:14]

Try jobs failed on following builders:
  win_chromium_x64_rel_ng on tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_x64_...)

[agrieve, 2015-06-19 01:31:29]

The CQ bit was checked by agrieve@chromium.org

[commit-bot: I haz the power, 2015-06-19 01:32:08]

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

[commit-bot: I haz the power, 2015-06-19 01:45:13]

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

[commit-bot: I haz the power, 2015-06-19 01:45:15]

Try jobs failed on following builders:
  win_chromium_x64_rel_ng on tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_x64_...)

[agrieve, 2015-06-19 02:18:41]

The CQ bit was checked by agrieve@chromium.org

[agrieve, 2015-06-19 02:18:41]

The patchset sent to the CQ was uploaded after l-g-t-m from
thestig@chromium.org, jochen@chromium.org, jam@chromium.org
Link to the patchset: https://codereview.chromium.org/1185973003/#ps180001
(title: " ")

[commit-bot: I haz the power, 2015-06-19 02:19:01]

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

[commit-bot: I haz the power, 2015-06-19 04:33:14]

Committed patchset #10 (id:180001)

[commit-bot: I haz the power, 2015-06-19 04:34:06]

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