[headless] Use embedded v8 data for headless.

[headless] Use embedded v8 data for headless.

R=alexclarke@chromium.org,skyostil@chromium.org,torne@chromium.org
BUG=610673
Committed: https://crrev.com/f5c8a3b7f61475a02914f955e2f7c83910f92b65
Cr-Commit-Position: refs/heads/master@{#416371}

[altimin, 2016-05-10 17:23:00]

altimin@chromium.org changed reviewers:
+ dpranke@chromium.org

[Sami, 2016-05-10 17:27:41]

Looks good. Do we have a test which blows up if the data file silently stops
getting compiled in?

https://codereview.chromium.org/1962343002/diff/20001/build/args/headless.gn
File build/args/headless.gn (right):

https://codereview.chromium.org/1962343002/diff/20001/build/args/headless.gn#...
build/args/headless.gn:16: # to simplify dependencies.
s/dependencies/deployment/

[altimin, 2016-05-10 17:37:27]

On 2016/05/10 17:27:41, Sami wrote:
> Looks good. Do we have a test which blows up if the data file silently stops
> getting compiled in?

I believe that in silent fail is impossible, we'll fail to link in that case.
 
> https://codereview.chromium.org/1962343002/diff/20001/build/args/headless.gn
> File build/args/headless.gn (right):
> 
>
https://codereview.chromium.org/1962343002/diff/20001/build/args/headless.gn#...
> build/args/headless.gn:16: # to simplify dependencies.
> s/dependencies/deployment/

[altimin, 2016-05-10 17:38:24]

PTAL

https://codereview.chromium.org/1962343002/diff/20001/build/args/headless.gn
File build/args/headless.gn (right):

https://codereview.chromium.org/1962343002/diff/20001/build/args/headless.gn#...
build/args/headless.gn:16: # to simplify dependencies.
On 2016/05/10 17:27:41, Sami wrote:
> s/dependencies/deployment/

Done.

[Sami, 2016-05-10 17:47:20]

On 2016/05/10 17:37:27, altimin wrote:
> On 2016/05/10 17:27:41, Sami wrote:
> > Looks good. Do we have a test which blows up if the data file silently stops
> > getting compiled in?
> 
> I believe that in silent fail is impossible, we'll fail to link in that case.

I was thinking of v8_use_external_startup_data becoming a no-op and reverting
back to an external snapshot. The tests would then load the snapshot at runtime
like they do now and we'd be none the wiser, right?

I suppose whoever changes this would remove v8_use_external_startup_data at the
same time, which would mean we'd get gn failure right away. In that light I
guess this lgtm.

[Dirk Pranke, 2016-05-10 20:33:25]

hm. 

I don't really want to put declare_args() in overrides files. However, if we put
it someplace where it should live, like maybe //v8/v8.gni, I'm not actually sure
how to get there from here yet. I need to think about this more.

[Torne, 2016-05-11 15:38:49]

Looks fine in principle, I'm a bit surprised that headless works by having an
external gn args configuration rather than just itself being a single gn option
that sets everything else internally, but I assume you have some pre-existing
reason for doing it this way.

I think there are one or more bugs open somewhere about removing support for
having these data files externally now that no platform uses them; as discussed
offline, if you can find them can you go note that headless is going to use them
and close them wontfix? :)

[Sami, 2016-05-11 16:02:42]

On 2016/05/11 15:38:49, Torne wrote:
> Looks fine in principle, I'm a bit surprised that headless works by having an
> external gn args configuration rather than just itself being a single gn
option
> that sets everything else internally, but I assume you have some pre-existing
> reason for doing it this way.

FWIW, we started by having an is_headless flag but the gn folks advised us to
move to the separate gn args file which we have now.

[Dirk Pranke, 2016-08-31 23:27:22]

Is this CL still relevant / needed?

[altimin, 2016-09-01 21:48:49]

On 2016/08/31 23:27:22, Dirk Pranke wrote:
> Is this CL still relevant / needed?

Thanks for bumping.
Yes, we still need it. I've been patching Chrome locally for my needs, but I
will update the patch and send it out for review shortly.

[altimin, 2016-09-02 17:55:41]

On 2016/09/01 21:48:49, altimin wrote:
> On 2016/08/31 23:27:22, Dirk Pranke wrote:
> > Is this CL still relevant / needed?
> 
> Thanks for bumping.
> Yes, we still need it. I've been patching Chrome locally for my needs, but I
> will update the patch and send it out for review shortly.

"If you wait by the river long enough, the bodies of your enemies will float
by.".

v8 guys sorted everything out and now we don't need changing build_overrides, we
just need to update build/args/headless.gn.

PTAL.

[Sami, 2016-09-02 17:57:47]

lgtm.

[Dirk Pranke, 2016-09-02 20:21:15]

lgtm

[altimin, 2016-09-02 20:28:44]

The CQ bit was checked by altimin@chromium.org

[commit-bot: I haz the power, 2016-09-02 20:29:33]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-09-02 22:37:02]

Committed patchset #4 (id:60001)

[commit-bot: I haz the power, 2016-09-02 22:40:15]

Description was changed from

==========
[headless] Use embedded v8 data for headless.

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

to

==========
[headless] Use embedded v8 data for headless.

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

Committed: https://crrev.com/f5c8a3b7f61475a02914f955e2f7c83910f92b65
Cr-Commit-Position: refs/heads/master@{#416371}
==========

[commit-bot: I haz the power, 2016-09-02 22:40:16]

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