Switch ChromeOS and ChromeCast to use external V8 snapshot files.

Switch ChromeOS and ChromeCast to use external V8 snapshot files.

BUG=421063
Committed: https://crrev.com/65212281c5bd5c8f85304232e3f66b8ff876082c
Cr-Commit-Position: refs/heads/master@{#324052}

[rmcilroy, 2015-03-18 22:10:52]

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

[rmcilroy, 2015-03-18 22:10:53]

lcwu@chromium.org: Please review changes in chromecast
jochen@chromium.org: Please review changes in build and content

[lcwu1, 2015-03-19 00:26:31]

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

[jochen (gone - plz use gerrit), 2015-03-19 10:26:04]

lgtm

[gunsch, 2015-03-19 16:52:44]

On 2015/03/19 10:26:04, jochen (slow) wrote:
> lgtm

I'm a little unclear what's going on here. The bug says this affects Linux and
Android. For Chromecast, the change affects only our Android case.

Does this mean Chromecast Linux also needs to bundle these snapshot files
alongside icudtl.dat?

Are the snapshot files automatically built if we have a (direct or indirect)
dependency on v8?

[rmcilroy, 2015-03-19 18:30:32]

On 2015/03/19 16:52:44, gunsch wrote:
> On 2015/03/19 10:26:04, jochen (slow) wrote:
> > lgtm
> 
> I'm a little unclear what's going on here. The bug says this affects Linux and
> Android. For Chromecast, the change affects only our Android case.

Sorry the first line of the bug is out-of-date. We now use external V8 snapshots
on all platforms except ChromeOS and ChromeCast (which should move over with
this CL).
 
> Does this mean Chromecast Linux also needs to bundle these snapshot files
> alongside icudtl.dat?

Yes - is there somewhere in the tree which does this bundling? I couldn't find
it and this change worked on both the cast_shell and cast_shell_apk trybots so I
 wasn't sure what else would need changing. 
 
> Are the snapshot files automatically built if we have a (direct or indirect)
> dependency on v8?

They are build automatically when you have any dependency on V8 (although I've
explicitly added the ../v8/tools/gyp/v8.gyp:v8_external_snapshot dependency
before the copy of the files to the APK to ensure that happens at the correct
time).

[gunsch, 2015-03-19 18:35:04]

On 2015/03/19 18:30:32, rmcilroy wrote:
> On 2015/03/19 16:52:44, gunsch wrote:
> > On 2015/03/19 10:26:04, jochen (slow) wrote:
> > > lgtm
> > 
> > I'm a little unclear what's going on here. The bug says this affects Linux
and
> > Android. For Chromecast, the change affects only our Android case.
> 
> Sorry the first line of the bug is out-of-date. We now use external V8
snapshots
> on all platforms except ChromeOS and ChromeCast (which should move over with
> this CL).
>  
> > Does this mean Chromecast Linux also needs to bundle these snapshot files
> > alongside icudtl.dat?
> 
> Yes - is there somewhere in the tree which does this bundling? I couldn't find
> it and this change worked on both the cast_shell and cast_shell_apk trybots so
I
>  wasn't sure what else would need changing. 
>  
> > Are the snapshot files automatically built if we have a (direct or indirect)
> > dependency on v8?
> 
> They are build automatically when you have any dependency on V8 (although I've
> explicitly added the ../v8/tools/gyp/v8.gyp:v8_external_snapshot dependency
> before the copy of the files to the APK to ensure that happens at the correct
> time).

lgtm, thanks for the explanation!

The other bundling code is in our internal repository, outside of the GYP part
of process---we'll handle changing that.

[rmcilroy, 2015-04-07 14:29:15]

On 2015/03/19 18:35:04, gunsch wrote:
> On 2015/03/19 18:30:32, rmcilroy wrote:
> > On 2015/03/19 16:52:44, gunsch wrote:
> > > On 2015/03/19 10:26:04, jochen (slow) wrote:
> > > > lgtm
> > > 
> > > I'm a little unclear what's going on here. The bug says this affects Linux
> and
> > > Android. For Chromecast, the change affects only our Android case.
> > 
> > Sorry the first line of the bug is out-of-date. We now use external V8
> snapshots
> > on all platforms except ChromeOS and ChromeCast (which should move over with
> > this CL).
> >  
> > > Does this mean Chromecast Linux also needs to bundle these snapshot files
> > > alongside icudtl.dat?
> > 
> > Yes - is there somewhere in the tree which does this bundling? I couldn't
find
> > it and this change worked on both the cast_shell and cast_shell_apk trybots
so
> I
> >  wasn't sure what else would need changing. 
> >  
> > > Are the snapshot files automatically built if we have a (direct or
indirect)
> > > dependency on v8?
> > 
> > They are build automatically when you have any dependency on V8 (although
I've
> > explicitly added the ../v8/tools/gyp/v8.gyp:v8_external_snapshot dependency
> > before the copy of the files to the APK to ensure that happens at the
correct
> > time).
> 
> lgtm, thanks for the explanation!
> 
> The other bundling code is in our internal repository, outside of the GYP part
> of process---we'll handle changing that.

gunsch@: Just a heads-up that I am going to try to land this now (now that the
dependencies have landed) so you will need to update your bundling process in
your internal repo once it has landed. Thanks.

[rmcilroy, 2015-04-07 14:29:22]

The CQ bit was checked by rmcilroy@chromium.org

[rmcilroy, 2015-04-07 14:29:22]

The patchset sent to the CQ was uploaded after l-g-t-m from jochen@chromium.org,
gunsch@chromium.org
Link to the patchset: https://codereview.chromium.org/1019123002/#ps20001
(title: "Rebase")

[commit-bot: I haz the power, 2015-04-07 14:29:43]

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

[commit-bot: I haz the power, 2015-04-07 16:15:14]

Committed patchset #2 (id:20001)

[commit-bot: I haz the power, 2015-04-07 16:17:19]

Patchset 2 (id:??) landed as
https://crrev.com/65212281c5bd5c8f85304232e3f66b8ff876082c
Cr-Commit-Position: refs/heads/master@{#324052}

[gunsch, 2015-04-08 21:18:05]

On 2015/04/07 16:17:19, I haz the power (commit-bot) wrote:
> Patchset 2 (id:??) landed as
> https://crrev.com/65212281c5bd5c8f85304232e3f66b8ff876082c
> Cr-Commit-Position: refs/heads/master@{#324052}

Hey Ross,
 I'm actually unable to run cast_shell from the out/ directory after the
external snapshot changes (x86 build, not using our private repo or anything
fancy). I made sure the "v8_external_snapshot" target is built, but still crash
at startup:

[5156:5156:FATAL:scoped_file.cc(29)] Check failed: 0 == IGNORE_EINTR(close(fd)).
: Bad file descriptor
#0 0x00000057080e base::debug::StackTrace::StackTrace()
#1 0x000000583f32 logging::LogMessage::~LogMessage()
#2 0x000000584485 logging::ErrnoLogMessage::~ErrnoLogMessage()
#3 0x00000058226f base::internal::ScopedFDCloseTraits::Free()
#4 0x000000462094 base::ScopedGeneric<>::FreeIfNecessary()
#5 0x0000004619fd base::ScopedGeneric<>::reset()
#6 0x00000067dcde base::File::MemoryCheckingScopedFD::reset()
#7 0x00000067ba94 base::File::Close()
#8 0x00000067e9f5 base::MemoryMappedFile::CloseHandles()
#9 0x00000067e28c base::MemoryMappedFile::Initialize()
#10 0x000003c9c664 gin::(anonymous namespace)::MapV8Files()
#11 0x000003c9cb0f gin::V8Initializer::LoadV8SnapshotFromFD()
#12 0x0000004cb4e8 content::ContentMainRunnerImpl::Initialize()
#13 0x0000004c92d9 content::ContentMain()
#14 0x00000041e29c main
#15 0x7f7bac20eec5 __libc_start_main
#16 0x00000041e17c <unknown>

I'm digging into it now, but any obvious pointers?

[gunsch, 2015-04-09 01:01:25]

Ah! We also fail on Android, a slightly different way but wouldn't be surprised
if it's the same root cause.

Stack:
04-08 19:57:50.585 F/chromium(31816): [31816:31816:FATAL:file_posix.cc(406)]
Check failed: IsValid(). 
logging::LogMessage::~LogMessage()
/usr/local/google/code/chromium/src/out_cast_android/Debug/../../base/logging.cc:543
base::File::GetLength()
/usr/local/google/code/chromium/src/out_cast_android/Debug/../../base/files/file_posix.cc:406
(discriminator 10)
base::MemoryMappedFile::MapFileRegionToMemory(base::MemoryMappedFile::Region
const&)
/usr/local/google/code/chromium/src/out_cast_android/Debug/../../base/files/memory_mapped_file_posix.cc:29
base::MemoryMappedFile::Initialize(base::File, base::MemoryMappedFile::Region
const&)
/usr/local/google/code/chromium/src/out_cast_android/Debug/../../base/files/memory_mapped_file.cc:64
MapV8Files
/usr/local/google/code/chromium/src/out_cast_android/Debug/../../gin/v8_initializer.cc:76
(discriminator 3)
gin::V8Initializer::LoadV8Snapshot()
/usr/local/google/code/chromium/src/out_cast_android/Debug/../../gin/v8_initializer.cc:135
(discriminator 6)
content::ContentMainRunnerImpl::Initialize(content::ContentMainParams const&)
/usr/local/google/code/chromium/src/out_cast_android/Debug/../../content/app/content_main_runner.cc:725
Start
/usr/local/google/code/chromium/src/out_cast_android/Debug/../../content/app/android/content_main.cc:46
(discriminator 2)

Verifying the APK has the new binary blobs:
gunsch@powerthirst$/usr/local/google/code/chromium/src$ unzip -l
out_cast_android/Debug/apks/CastShell.apk  | grep assets
  3133230  2015-04-08 17:55   assets/cast_shell.pak
  6267808  2015-04-08 17:55   assets/icudtl.dat
   414655  2015-04-08 17:55   assets/natives_blob.bin
   509188  2015-04-08 17:55   assets/snapshot_blob.bin