Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
2074283002:
Remove natives_blob.bin's arch dependence in Android. (Closed)
|
Created:
4 years, 6 months ago by michaelbai Modified:
4 years, 6 months ago CC:
v8-reviews_googlegroups.com, Torne Base URL:
https://chromium.googlesource.com/v8/v8.git@master Target Ref:
refs/pending/heads/master Project:
v8 Visibility:
Public. |
DescriptionRemove natives_blob.bin's arch dependence in Android.
BUG=620855
Committed: https://crrev.com/8d830a5aab1c688dc0c3a1690e2ceab145b23bc3
Cr-Commit-Position: refs/heads/master@{#37200}
Patch Set 1 #
Total comments: 1
Patch Set 2 : embedded Android logic #
Total comments: 2
Messages
Total messages: 20 (7 generated)
Description was changed from ========== Remove natives_blob.bin's arch dependence in Android. BUG=620855 ========== to ========== Remove natives_blob.bin's arch dependence in Android. BUG=620855 ==========
michaelbai@chromium.org changed reviewers: + agrieve@chromium.org, yangguo@chromium.org
lgtm whichever way you decide to land https://codereview.chromium.org/2074283002/diff/1/BUILD.gn File BUILD.gn (right): https://codereview.chromium.org/2074283002/diff/1/BUILD.gn#newcode511 BUILD.gn:511: sources = ["$root_out_dir/natives_blob.bin"] I suppose you'll need to manually roll v8 once this lands (with your other change). It might be simpler instead to just keep this in renaming sources but give it the same name in src & dst.
On 2016/06/17 23:56:03, agrieve wrote: > lgtm whichever way you decide to land > > https://codereview.chromium.org/2074283002/diff/1/BUILD.gn > File BUILD.gn (right): > > https://codereview.chromium.org/2074283002/diff/1/BUILD.gn#newcode511 > BUILD.gn:511: sources = ["$root_out_dir/natives_blob.bin"] > I suppose you'll need to manually roll v8 once this lands (with your other > change). > > It might be simpler instead to just keep this in renaming sources but give it > the same name in src & dst. lgtm.
Thanks, agrieve@ I will change the chrome side patch as your suggestion.
Yang, pls help to launch this patch
Patchset #2 (id:20001) has been deleted
I remove v8_external_startup_data_renaming_sources and v8_external_startup_data_renaming_destinations in patch #2, because they are only used by this file now. The following patch to remove the definition on chromium side is https://codereview.chromium.org/2088973002
On 2016/06/21 22:14:45, michaelbai wrote: > I remove v8_external_startup_data_renaming_sources and > v8_external_startup_data_renaming_destinations in patch #2, because they are > only used by this file now. > > > The following patch to remove the definition on chromium side is > https://codereview.chromium.org/2088973002 Yang, please to help launch the patch if it is LGTY
https://codereview.chromium.org/2074283002/diff/40001/BUILD.gn File BUILD.gn (right): https://codereview.chromium.org/2074283002/diff/40001/BUILD.gn#newcode524 BUILD.gn:524: renaming_destinations = [ "snapshot_blob_32.bin" ] Does this actually work? in src/startup-data-util.cc we hard-code the default snapshot file names.
https://codereview.chromium.org/2074283002/diff/40001/BUILD.gn File BUILD.gn (right): https://codereview.chromium.org/2074283002/diff/40001/BUILD.gn#newcode524 BUILD.gn:524: renaming_destinations = [ "snapshot_blob_32.bin" ] On 2016/06/22 12:59:50, Yang wrote: > Does this actually work? in src/startup-data-util.cc we hard-code the default > snapshot file names. This target "v8_external_startup_data_assets" isn't used by v8 itself. The way to load snapshot in startup-data-util.cc doesn't work in Android, Android loads the snapshot from APK in gin/v8_initializer.cc.
On 2016/06/22 16:00:37, michaelbai wrote: > https://codereview.chromium.org/2074283002/diff/40001/BUILD.gn > File BUILD.gn (right): > > https://codereview.chromium.org/2074283002/diff/40001/BUILD.gn#newcode524 > BUILD.gn:524: renaming_destinations = [ "snapshot_blob_32.bin" ] > On 2016/06/22 12:59:50, Yang wrote: > > Does this actually work? in src/startup-data-util.cc we hard-code the default > > snapshot file names. > > This target "v8_external_startup_data_assets" isn't used by v8 itself. > > The way to load snapshot in startup-data-util.cc doesn't work in Android, > Android loads the snapshot from APK in gin/v8_initializer.cc. lgtm
The CQ bit was checked by yangguo@chromium.org
The patchset sent to the CQ was uploaded after l-g-t-m from agrieve@chromium.org Link to the patchset: https://codereview.chromium.org/2074283002/#ps40001 (title: "embedded Android logic")
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/2074283002/40001
Message was sent while issue was closed.
Description was changed from ========== Remove natives_blob.bin's arch dependence in Android. BUG=620855 ========== to ========== Remove natives_blob.bin's arch dependence in Android. BUG=620855 ==========
Message was sent while issue was closed.
Committed patchset #2 (id:40001)
Message was sent while issue was closed.
Description was changed from ========== Remove natives_blob.bin's arch dependence in Android. BUG=620855 ========== to ========== Remove natives_blob.bin's arch dependence in Android. BUG=620855 Committed: https://crrev.com/8d830a5aab1c688dc0c3a1690e2ceab145b23bc3 Cr-Commit-Position: refs/heads/master@{#37200} ==========
Message was sent while issue was closed.
Patchset 2 (id:??) landed as https://crrev.com/8d830a5aab1c688dc0c3a1690e2ceab145b23bc3 Cr-Commit-Position: refs/heads/master@{#37200} |
