New issue
Advanced search Search tips

Issue 610673 link

Starred by 7 users

Issue metadata

Status: Fixed
Owner:
Closed: Feb 2017
Cc:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 1
Type: Bug

Blocking:
issue 546953
issue 615047



Sign in to add a comment

Investigate bundling resource files into the headless library

Project Member Reported by skyos...@chromium.org, May 10 2016

Issue description

Investigate if we could bundle the following data files into the headless library to simplify deployment:

- chrome-devel-sandbox
- headless_lib.pak
- icudtl.dat
- snapshot_blob.bin
 
Labels: -Pri-3 Pri-1
Owner: altimin@chromium.org
Status: Started (was: Untriaged)
Blocking: 546953
Project Member

Comment 3 by bugdroid1@chromium.org, May 11 2016

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/c29ab96bddc5a4e964a66cb5ed7872771db8ff59

commit c29ab96bddc5a4e964a66cb5ed7872771db8ff59
Author: altimin <altimin@chromium.org>
Date: Wed May 11 12:15:12 2016

[headless] Use embedded icu data for headless.

R=alexclarke@chromium.org,skyostil@chromium.org
BUG= 610673 

Review-Url: https://codereview.chromium.org/1964893002
Cr-Commit-Position: refs/heads/master@{#392900}

[modify] https://crrev.com/c29ab96bddc5a4e964a66cb5ed7872771db8ff59/build/args/headless.gn

Project Member

Comment 4 by bugdroid1@chromium.org, May 18 2016

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/deps/icu.git/+/d9f48be67ba790adbc59d3ab195b2d8278c9d73b

commit d9f48be67ba790adbc59d3ab195b2d8278c9d73b
Author: Sami Kyostila <skyostil@chromium.org>
Date: Wed May 18 16:19:26 2016

Export correct defines depending on use_icu_data_file flag.

BUG= 610673 
R=alexclarke@chromium.org, dpranke@chromium.org

Review URL: https://codereview.chromium.org/1991833002 .

[modify] https://crrev.com/d9f48be67ba790adbc59d3ab195b2d8278c9d73b/BUILD.gn

FWIW, for  bug 612904  we would need to keep this configurable so that Chrome can pass its own pak file to headless.
Project Member

Comment 6 by bugdroid1@chromium.org, May 19 2016

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/3b1f5303df713c0879ef425e19bd75229c32ef45

commit 3b1f5303df713c0879ef425e19bd75229c32ef45
Author: altimin <altimin@chromium.org>
Date: Thu May 19 12:34:24 2016

Fix icu_use_data_file = false.

- Move variables needed only with ICU_UTIL_DATA_IMPL == ICU_UTIL_DATA_FILE inside corresponding ifdef.
- Move ICU_UTIL_DATA_* defines to header.
- Hide calls to base::i18n::InitializeICUFromRawMemory and base::i18n::GetRawIcuMemory in services/shell/ under ifdef.

R=jshin@chromium.org,
CC=alexclarke@chromium.org,skyostil@chromium.org
BUG= 610673 

Review-Url: https://codereview.chromium.org/1989333002
Cr-Commit-Position: refs/heads/master@{#394750}

[modify] https://crrev.com/3b1f5303df713c0879ef425e19bd75229c32ef45/DEPS
[modify] https://crrev.com/3b1f5303df713c0879ef425e19bd75229c32ef45/base/i18n/icu_util.cc
[modify] https://crrev.com/3b1f5303df713c0879ef425e19bd75229c32ef45/base/i18n/icu_util.h
[modify] https://crrev.com/3b1f5303df713c0879ef425e19bd75229c32ef45/services/shell/public/cpp/lib/initialize_base_and_icu.cc
[modify] https://crrev.com/3b1f5303df713c0879ef425e19bd75229c32ef45/services/shell/runner/init.cc

Project Member

Comment 7 by bugdroid1@chromium.org, May 19 2016

The following revision refers to this bug:
  https://chromium.googlesource.com/v8/v8.git/+/a2956f4243f6336ad772372d97cc497d6177d2a7

commit a2956f4243f6336ad772372d97cc497d6177d2a7
Author: altimin <altimin@chromium.org>
Date: Thu May 19 15:50:14 2016

Do not define use_icu_data_file_flag.

use_icu_data_file from //third_party/icu should be used.
//third_party/icu also defines ICU_UTIL_DATA_IMPL.

BUG= chromium:610673 , chromium:474921 

Review-Url: https://codereview.chromium.org/1996033002
Cr-Commit-Position: refs/heads/master@{#36371}

[modify] https://crrev.com/a2956f4243f6336ad772372d97cc497d6177d2a7/BUILD.gn
[modify] https://crrev.com/a2956f4243f6336ad772372d97cc497d6177d2a7/DEPS
[modify] https://crrev.com/a2956f4243f6336ad772372d97cc497d6177d2a7/build_overrides/v8.gni

Project Member

Comment 8 by bugdroid1@chromium.org, May 24 2016

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/deps/icu.git/+/d9f48be67ba790adbc59d3ab195b2d8278c9d73b

commit d9f48be67ba790adbc59d3ab195b2d8278c9d73b
Author: Sami Kyostila <skyostil@chromium.org>
Date: Wed May 18 16:19:26 2016

Export correct defines depending on use_icu_data_file flag.

BUG= 610673 
R=alexclarke@chromium.org, dpranke@chromium.org

Review URL: https://codereview.chromium.org/1991833002 .

[modify] https://crrev.com/d9f48be67ba790adbc59d3ab195b2d8278c9d73b/BUILD.gn

Blocking: 615047

Comment 10 by js...@chromium.org, Jul 25 2016

Cc: js...@chromium.org
While working on https://codereview.chromium.org/2174993002/, I found that icu_use_data_file=false in GN (icu_use_data_file_flag=0 in gyp) does not work on Mac. (see also  bug 630929 ). 

Comment 11 by js...@chromium.org, Jul 25 2016

altimin@:  what platform is headless (blink) built? Perhaps, Linux? 

Somehow on Linux, it seems to be fine (with my CL applied to third_party/icu/BUILD.gn. Otherwise, gn fails to generate ninja files), but on Mac I have this issue:
 
When compiling files under services/ (services/shell/runner/init.cc and services/shell/public/cpp/lib/initialize_base_and_icu.cc ) with icu_use_data_file=false/icu_use_data_file_flag=0,  ICU_UTIL_DATA_IMPL is not defined (and set to '0' which is equal to ICU_UTIL_DATA_FILE) so that  those files refer to  base::i18n::GetRawIcuMemory() and base::i18n::InitializeICUFromRawMemory().  Because those functions are not defined in base/i18n/icu_util.cc when icu_use_data_file_flag=0 / icu_use_data_file=false, linker complained about undefined references. 

jshin@: Yes, headless is Linux-only at this point. We have plans to start supporting Mac, but it's not our priority now.

About undefined references: please make sure that your targets depend on //third_party/icu and //third_party/icu:icu_config gets applied to them (this icu_config exports ICU_UTIL_DATA_IMPL).

Comment 13 by js...@chromium.org, Jul 25 2016

Thanks, altimin@.  

I guess what happened is that gyp was not updated for targets compiling services/shell/runner/init.cc and services/shell/public/cpp/lib/initialize_base_and_icu.cc  because Linux began to use GN exclusively and icu_use_data_file_flag=0 in gyp was not tested recently on Mac. 

I'll fix  bug 630929  (so that I can generate ninja files with GN) per your suggestion in that bug and see whether GN build can be made to work. Given that Mac is transitioning to GN, gyp build might as well be left 'unfixed' (because icu_use_data_file_flag=0 is NOT used by default). 

Comment 14 Deleted

I believe that icu_use_data_file_flag is used only by headless now (and was broken when we started using it and I had to fix it). 

It's very good that you are starting to use it. I hope that you have some waterfall bots testing this build configuration. We have info-only headless build bot and we are hoping to add it to waterfall at some point.

And yes, gyp pretty much doesn't matter now (for example, headless is gn-only). 
Project Member

Comment 16 by bugdroid1@chromium.org, Sep 2 2016

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/f5c8a3b7f61475a02914f955e2f7c83910f92b65

commit f5c8a3b7f61475a02914f955e2f7c83910f92b65
Author: altimin <altimin@chromium.org>
Date: Fri Sep 02 22:36:39 2016

[headless] Use embedded v8 data for headless.

R=alexclarke@chromium.org,skyostil@chromium.org,torne@chromium.org
BUG= 610673 

Review-Url: https://codereview.chromium.org/1962343002
Cr-Commit-Position: refs/heads/master@{#416371}

[modify] https://crrev.com/f5c8a3b7f61475a02914f955e2f7c83910f92b65/build/args/headless.gn

Are there still other external data files we're using?
Yes, I still haven't landed resource pack bundling.
Project Member

Comment 19 by bugdroid1@chromium.org, Feb 9 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/6ed1a70ef94814ec92966802be57c3a36d71b7f9

commit 6ed1a70ef94814ec92966802be57c3a36d71b7f9
Author: altimin <altimin@chromium.org>
Date: Thu Feb 09 11:59:37 2017

[headless] Embed pak file into binary.

Embed headless_lib.pak into headless chromium binary.
Also make necessary changes to ui::ResourceBundle and ui::DataPack
to make this possible.

R=alexclarke@chromium.org,skyostil@chromium.org
BUG= 610673 

Review-Url: https://codereview.chromium.org/1969313005
Cr-Commit-Position: refs/heads/master@{#449263}

[modify] https://crrev.com/6ed1a70ef94814ec92966802be57c3a36d71b7f9/build/args/headless.gn
[modify] https://crrev.com/6ed1a70ef94814ec92966802be57c3a36d71b7f9/headless/BUILD.gn
[add] https://crrev.com/6ed1a70ef94814ec92966802be57c3a36d71b7f9/headless/headless.gni
[add] https://crrev.com/6ed1a70ef94814ec92966802be57c3a36d71b7f9/headless/lib/embed_data.py
[modify] https://crrev.com/6ed1a70ef94814ec92966802be57c3a36d71b7f9/headless/lib/headless_content_main_delegate.cc
[add] https://crrev.com/6ed1a70ef94814ec92966802be57c3a36d71b7f9/headless/lib/util/embedded_file.h
[modify] https://crrev.com/6ed1a70ef94814ec92966802be57c3a36d71b7f9/ui/base/resource/data_pack.cc
[modify] https://crrev.com/6ed1a70ef94814ec92966802be57c3a36d71b7f9/ui/base/resource/data_pack.h
[modify] https://crrev.com/6ed1a70ef94814ec92966802be57c3a36d71b7f9/ui/base/resource/data_pack_unittest.cc
[modify] https://crrev.com/6ed1a70ef94814ec92966802be57c3a36d71b7f9/ui/base/resource/resource_bundle.cc
[modify] https://crrev.com/6ed1a70ef94814ec92966802be57c3a36d71b7f9/ui/base/resource/resource_bundle.h

Status: Fixed (was: Started)

Sign in to add a comment