Switch Chrome Android using v8 arch specific external data name

Switch Chrome Android using v8 arch specific external data name

For 32-bit, natives_blob_32.bin and snapshot_blob_32.bin are used.
For 64-bit, natives_blob_64.bin and natives_blob_64.bin are used.

This patch also removed the existing v8 startup data in
destination directory by using copy_ex to remove directory
first.

BUG=540788
Committed: https://crrev.com/979b03a19f63b1cbe02fa3df99707c3f93f7c9cf
Cr-Commit-Position: refs/heads/master@{#355680}

[michaelbai, 2015-10-09 22:51:40]

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

[commit-bot: I haz the power, 2015-10-09 22:53:41]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1400953002/1
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1400953002/1

[commit-bot: I haz the power, 2015-10-10 01:07:53]

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

[commit-bot: I haz the power, 2015-10-10 01:07:54]

Dry run: This issue passed the CQ dry run.

[michaelbai, 2015-10-15 15:56:40]

michaelbai@chromium.org changed reviewers:
+ abarth@chromium.org, jbudorick@chromium.org

[michaelbai, 2015-10-15 15:58:56]

Adam, would you like review the change in gin?
John, would you like review all gn and gyp change?

Thanks

[michaelbai, 2015-10-19 19:03:30]

michaelbai@chromium.org changed reviewers:
+ jochen@chromium.org
- abarth@chromium.org

[michaelbai, 2015-10-19 19:04:04]

jochen@ could you help to review the change in gn?

[jbudorick, 2015-10-19 22:57:23]

https://codereview.chromium.org/1400953002/diff/60001/build/android/v8_extern...
File build/android/v8_external_startup_data_arch_suffix.gypi (right):

https://codereview.chromium.org/1400953002/diff/60001/build/android/v8_extern...
build/android/v8_external_startup_data_arch_suffix.gypi:10: ['OS=="android" and
(target_arch=="arm" or target_arch=="ia32" or target_arch=="mipsel")', {
What's your reasoning for doing this as two separate conditionals rather than
one joined one, e.g.

  ['<64-bit condition>', {
    'arch_suffix': '64',
   }, {
    'arch_suffix': '32',
  }]

?

https://codereview.chromium.org/1400953002/diff/60001/build_overrides/v8.gni
File build_overrides/v8.gni (right):

https://codereview.chromium.org/1400953002/diff/60001/build_overrides/v8.gni#...
build_overrides/v8.gni:32: } else if (current_cpu == "arm64" || current_cpu ==
"x64" ||
Similarly here.

https://codereview.chromium.org/1400953002/diff/60001/chrome/chrome_android_p...
File chrome/chrome_android_paks.gypi (right):

https://codereview.chromium.org/1400953002/diff/60001/chrome/chrome_android_p...
chrome/chrome_android_paks.gypi:95: 'renaming_sources': [
I'm still not crazy about this interface. I'm wondering if something like

  'rename': [
    ['<(PRODUCT_DIR)/snapshot_blob.bin', 'snapshot_blob_<(arch_suffix).bin'],
    ['<(PRODUCT_DIR)/natives_blob.bin', 'natives_blob_<(arch_suffix).bin'],
  ]

would work...

That said, I suppose it's more important that we get the GN interface right.
(obviously not in this CL)

https://codereview.chromium.org/1400953002/diff/60001/content/content.gyp
File content/content.gyp (left):

https://codereview.chromium.org/1400953002/diff/60001/content/content.gyp#old...
content/content.gyp:633: 'target_name': 'content_v8_external_data',
Why did you merge these two?

[michaelbai, 2015-10-20 00:10:11]

PTAL

https://codereview.chromium.org/1400953002/diff/60001/build/android/v8_extern...
File build/android/v8_external_startup_data_arch_suffix.gypi (right):

https://codereview.chromium.org/1400953002/diff/60001/build/android/v8_extern...
build/android/v8_external_startup_data_arch_suffix.gypi:10: ['OS=="android" and
(target_arch=="arm" or target_arch=="ia32" or target_arch=="mipsel")', {
On 2015/10/19 22:57:23, jbudorick wrote:
> What's your reasoning for doing this as two separate conditionals rather than
> one joined one, e.g.
> 
>   ['<64-bit condition>', {
>     'arch_suffix': '64',
>    }, {
>     'arch_suffix': '32',
>   }]
> 
> ?

This code was copied, it might useful when target_arch is set wrongly or new
arch is added which hardly happen. changed it.

https://codereview.chromium.org/1400953002/diff/60001/build_overrides/v8.gni
File build_overrides/v8.gni (right):

https://codereview.chromium.org/1400953002/diff/60001/build_overrides/v8.gni#...
build_overrides/v8.gni:32: } else if (current_cpu == "arm64" || current_cpu ==
"x64" ||
On 2015/10/19 22:57:23, jbudorick wrote:
> Similarly here.

Done.

https://codereview.chromium.org/1400953002/diff/60001/chrome/chrome_android_p...
File chrome/chrome_android_paks.gypi (right):

https://codereview.chromium.org/1400953002/diff/60001/chrome/chrome_android_p...
chrome/chrome_android_paks.gypi:95: 'renaming_sources': [
On 2015/10/19 22:57:23, jbudorick wrote:
> I'm still not crazy about this interface. I'm wondering if something like
> 
>   'rename': [
>     ['<(PRODUCT_DIR)/snapshot_blob.bin', 'snapshot_blob_<(arch_suffix).bin'],
>     ['<(PRODUCT_DIR)/natives_blob.bin', 'natives_blob_<(arch_suffix).bin'],
>   ]
> 
> would work...
> 
> That said, I suppose it's more important that we get the GN interface right.
> (obviously not in this CL)

I will keep an eye on how to improve this.

https://codereview.chromium.org/1400953002/diff/60001/content/content.gyp
File content/content.gyp (left):

https://codereview.chromium.org/1400953002/diff/60001/content/content.gyp#old...
content/content.gyp:633: 'target_name': 'content_v8_external_data',
On 2015/10/19 22:57:23, jbudorick wrote:
> Why did you merge these two?

I need to copy all assets at a time because the directory is removed before
copying.

[jochen (gone - plz use gerrit), 2015-10-20 08:11:48]

jochen@chromium.org changed reviewers:
+ dpranke@chromium.org, rmcilroy@chromium.org

[jochen (gone - plz use gerrit), 2015-10-20 08:11:49]

+dpranke who touches this variable in gn currently
+rmcilroy who introduced that stuff

[rmcilroy, 2015-10-20 10:15:39]

Overall approach LGTM, but please make sure jbudorick is happy with the build
changes.

[Dirk Pranke, 2015-10-20 21:39:34]

I'm puzzled by this whole CL; see my question in
https://code.google.com/p/chromium/issues/detail?id=540788#c4 (to avoid
duplicating it here ...)

[michaelbai, 2015-10-20 21:45:14]

On 2015/10/20 21:39:34, Dirk Pranke wrote:
> I'm puzzled by this whole CL; see my question in
> https://code.google.com/p/chromium/issues/detail?id=540788#c4 (to avoid
> duplicating it here ...)

Replied in the bug, any question, let me know.

[Dirk Pranke, 2015-10-21 19:31:39]

lgtm for now but I'd still like to talk about how we're supporting side-by-side
more generally.

[jbudorick, 2015-10-21 21:53:35]

lgtm

[michaelbai, 2015-10-21 22:10:37]

michaelbai@chromium.org changed reviewers:
+ mef@chromium.org, miguelg@chromium.org

[michaelbai, 2015-10-21 22:11:18]

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

[michaelbai, 2015-10-21 22:11:50]

Hey owners, please help to review this.

miguelg@ for chrome/
jochen@ for gin/ component/ content/
mef@ for net/

Thanks

[commit-bot: I haz the power, 2015-10-21 22:12:48]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1400953002/80001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1400953002/80001

[michaelbai, 2015-10-21 22:33:07]

michaelbai@chromium.org changed reviewers:
+ gunsch@chromium.org

[mef, 2015-10-21 23:47:55]

net/ lgtm

[commit-bot: I haz the power, 2015-10-21 23:53:42]

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

[commit-bot: I haz the power, 2015-10-21 23:53:43]

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

[gunsch, 2015-10-22 14:31:48]

gunsch@chromium.org changed reviewers:
+ halliwell@chromium.org
- gunsch@chromium.org

[gunsch, 2015-10-22 14:32:36]

gunsch --> halliwell for chromecast/

Luke, at a quick glance this might require some changes in chromecast.mk.

[jochen (gone - plz use gerrit), 2015-10-22 16:30:06]

lgtm

[halliwell, 2015-10-22 20:03:16]

On 2015/10/22 16:30:06, jochen wrote:
> lgtm

chromecast/ lgtm

[David Trainor- moved to gerrit, 2015-10-22 20:59:13]

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

[David Trainor- moved to gerrit, 2015-10-22 20:59:14]

chrome/ lgtm

[michaelbai, 2015-10-22 21:02:03]

The CQ bit was checked by michaelbai@chromium.org

[commit-bot: I haz the power, 2015-10-22 21:02:56]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1400953002/80001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1400953002/80001

[commit-bot: I haz the power, 2015-10-22 21:11:23]

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

[commit-bot: I haz the power, 2015-10-22 21:11:24]

Try jobs failed on following builders:
  chromium_presubmit on tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/chromium_presub...)

[michaelbai, 2015-10-22 21:14:21]

michaelbai@chromium.org changed reviewers:
+ brettw@chromium.org

[michaelbai, 2015-10-22 21:15:39]

brettw@, need your owner approval for src/build_overrides/

[michaelbai, 2015-10-22 22:12:47]

michaelbai@chromium.org changed reviewers:
+ cpu@chromium.org, jam@chromium.org
- brettw@chromium.org, miguelg@chromium.org

[michaelbai, 2015-10-22 22:13:41]

cpu@, jam@ can any of you review src/build_overrides ?

[cpu_(ooo_6.6-7.5), 2015-10-22 22:14:49]

rubber-stamp lgtm given that Dirk is fine with the patch.

[michaelbai, 2015-10-22 22:16:58]

The CQ bit was checked by michaelbai@chromium.org

[commit-bot: I haz the power, 2015-10-22 22:17:17]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1400953002/80001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1400953002/80001

[commit-bot: I haz the power, 2015-10-22 23:59:01]

Committed patchset #5 (id:80001)

[commit-bot: I haz the power, 2015-10-22 23:59:59]

Patchset 5 (id:??) landed as
https://crrev.com/979b03a19f63b1cbe02fa3df99707c3f93f7c9cf
Cr-Commit-Position: refs/heads/master@{#355680}

[DaleCurtis, 2015-10-23 01:05:29]

A revert of this CL (patchset #5 id:80001) has been created in
https://codereview.chromium.org/1419673008/ by dalecurtis@chromium.org.

The reason for reverting is: Looks to have broken various Android Mojo tests:

http://build.chromium.org/p/chromium.linux/buildstatus?builder=Android%20GN&n...
http://build.chromium.org/p/chromium.linux/buildstatus?builder=Android%20GN&n....

[michaelbai, 2015-10-23 02:48:04]

On 2015/10/23 01:05:29, DaleCurtis wrote:
> A revert of this CL (patchset #5 id:80001) has been created in
> https://codereview.chromium.org/1419673008/ by mailto:dalecurtis@chromium.org.
> 
> The reason for reverting is: Looks to have broken various Android Mojo tests:
> 
>
http://build.chromium.org/p/chromium.linux/buildstatus?builder=Android%20GN&n...
>
http://build.chromium.org/p/chromium.linux/buildstatus?builder=Android%20GN&n....

It should only broke html_viewer_unittests, not the mojo_common_unittests