[Extensions] Use compiled feature files

[Extensions] Use compiled feature files

Use generated compiled feature files instead of parsing the JSON at
runtime. In local testing, this shows an order-of-magnitude performance
increase, as well as catching a number of invalid features.

BUG=280286
Committed: https://crrev.com/5c684983bc51d5a377fcc34dcdaa9363e9a9d7d3
Cr-Commit-Position: refs/heads/master@{#407511}

[Devlin, 2016-07-21 18:06:14]

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

[commit-bot: I haz the power, 2016-07-21 18:06:50]

Dry run: 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-07-21 18:37:07]

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

[commit-bot: I haz the power, 2016-07-21 18:37:08]

Dry run: Try jobs failed on following builders:
  mac_chromium_compile_dbg_ng on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_comp...)

[Devlin, 2016-07-21 20:08:38]

Description was changed from

==========
Revert "done"

This reverts commit aaf45a1d154467cc3c2a49be9fc0c581ed527a6c.

woohoo


done

BUG=
==========

to

==========
[Extensions] Use compiled feature files

Use generated compiled feature files instead of parsing the JSON at
runtime. In local testing, this shows an order-of-magnitude performance
increase, as well as catching a number of invalid features.

BUG=280286
==========

[Devlin, 2016-07-21 22:32:11]

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

[commit-bot: I haz the power, 2016-07-21 22:32:55]

Dry run: 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-07-21 23:11:20]

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

[commit-bot: I haz the power, 2016-07-21 23:11:21]

Dry run: Try jobs failed on following builders:
  linux_chromium_chromeos_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[Devlin, 2016-07-22 17:10:50]

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

[commit-bot: I haz the power, 2016-07-22 17:11:10]

Dry run: 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-07-22 17:47:02]

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

[commit-bot: I haz the power, 2016-07-22 17:47:03]

Dry run: Try jobs failed on following builders:
  linux_chromium_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[Devlin, 2016-07-22 18:41:23]

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

[commit-bot: I haz the power, 2016-07-22 18:42:02]

Dry run: 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-07-22 19:57:10]

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

[commit-bot: I haz the power, 2016-07-22 19:57:11]

Dry run: This issue passed the CQ dry run.

[Devlin, 2016-07-22 20:13:35]

Patchset #7 (id:110001) has been deleted

[Devlin, 2016-07-22 20:13:42]

Patchset #2 (id:20001) has been deleted

[Devlin, 2016-07-22 20:13:56]

Patchset #2 (id:40001) has been deleted

[Devlin, 2016-07-22 20:14:04]

Patchset #1 (id:1) has been deleted

[Devlin, 2016-07-22 20:14:12]

Patchset #1 (id:60001) has been deleted

[Devlin, 2016-07-22 20:14:19]

Patchset #1 (id:70001) has been deleted

[Devlin, 2016-07-22 20:15:16]

rdevlin.cronin@chromium.org changed reviewers:
+ asargent@chromium.org, dpranke@chromium.org

[Devlin, 2016-07-22 20:15:17]

Woo!

Antony, mind taking a look?  Dirk, can you scan through the gyp/gn files and see
if anything's crazy?

[Dirk Pranke, 2016-07-22 21:44:18]

lgtm w/ suggestion (feel free to ignore).

https://codereview.chromium.org/2165023003/diff/130001/extensions/test/BUILD.gn
File extensions/test/BUILD.gn (right):

https://codereview.chromium.org/2165023003/diff/130001/extensions/test/BUILD....
extensions/test/BUILD.gn:1: # Copyright 2015 The Chromium Authors. All rights
reserved.
I wonder if it make sense to just move these targets into //extensions/BUILD.gn
.

The advantages would be that you wouldn't need "../" in the sources (which is an
uncommon thing in GN, though allowed), and you wouldn't have the stuttering in
the GN labels, e.g.: //extensions:test_api_features vs.
//extensions/test:test_api_features .

Alternatively, you could maybe also rename the targets to just "api_features",
etc.?

[asargent_no_longer_on_chrome, 2016-07-22 23:58:06]

lgtm

[Devlin, 2016-07-23 00:52:48]

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

[commit-bot: I haz the power, 2016-07-23 00:53:08]

Dry run: 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-07-23 01:54:07]

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

[commit-bot: I haz the power, 2016-07-23 01:54:08]

Dry run: This issue passed the CQ dry run.

[Devlin, 2016-07-25 06:39:48]

https://codereview.chromium.org/2165023003/diff/130001/extensions/test/BUILD.gn
File extensions/test/BUILD.gn (right):

https://codereview.chromium.org/2165023003/diff/130001/extensions/test/BUILD....
extensions/test/BUILD.gn:1: # Copyright 2015 The Chromium Authors. All rights
reserved.
On 2016/07/22 21:44:18, Dirk Pranke wrote:
> I wonder if it make sense to just move these targets into
//extensions/BUILD.gn
> .
> 
> The advantages would be that you wouldn't need "../" in the sources (which is
an
> uncommon thing in GN, though allowed), and you wouldn't have the stuttering in
> the GN labels, e.g.: //extensions:test_api_features vs.
> //extensions/test:test_api_features .
> 
> Alternatively, you could maybe also rename the targets to just "api_features",
> etc.?

I think I'll save this for a followup.  The gn rule currently uses the target
name to create the file, so it'd be a little more than a quick change.  Also,
even if we did it here, we'd still need the ../s in the
chrome/common/extensions/api version.  It could be worth investigating, but I'd
prefer to land this one like this for now.

[Devlin, 2016-07-25 06:40:41]

rdevlin.cronin@chromium.org changed reviewers:
+ jochen@chromium.org

[Devlin, 2016-07-25 06:40:42]

+jochen for top-level stamp.
asargent@ reviewed extensions and json_schema_compiler and dpranke reviewed
structure of gyp/gn, but need a stamp for:
    chrome/browser/BUILD.gn
    chrome/common/BUILD.gn
    chrome/common/common_resources.grd

Cheers!

[jochen (gone - plz use gerrit), 2016-07-25 15:52:55]

lgtm

[Devlin, 2016-07-25 15:54:23]

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

[commit-bot: I haz the power, 2016-07-25 15:54:44]

Dry run: 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-07-25 16:43:52]

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

[commit-bot: I haz the power, 2016-07-25 16:43:53]

Dry run: This issue passed the CQ dry run.

[Devlin, 2016-07-25 17:59:41]

The CQ bit was checked by rdevlin.cronin@chromium.org

[Devlin, 2016-07-25 17:59:41]

The patchset sent to the CQ was uploaded after l-g-t-m from
dpranke@chromium.org, asargent@chromium.org
Link to the patchset: https://codereview.chromium.org/2165023003/#ps170001
(title: "Rebase")

[commit-bot: I haz the power, 2016-07-25 18:00:03]

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-07-25 18:05:01]

Description was changed from

==========
[Extensions] Use compiled feature files

Use generated compiled feature files instead of parsing the JSON at
runtime. In local testing, this shows an order-of-magnitude performance
increase, as well as catching a number of invalid features.

BUG=280286
==========

to

==========
[Extensions] Use compiled feature files

Use generated compiled feature files instead of parsing the JSON at
runtime. In local testing, this shows an order-of-magnitude performance
increase, as well as catching a number of invalid features.

BUG=280286
==========

[commit-bot: I haz the power, 2016-07-25 18:05:05]

Committed patchset #4 (id:170001)

[commit-bot: I haz the power, 2016-07-25 18:06:50]

Description was changed from

==========
[Extensions] Use compiled feature files

Use generated compiled feature files instead of parsing the JSON at
runtime. In local testing, this shows an order-of-magnitude performance
increase, as well as catching a number of invalid features.

BUG=280286
==========

to

==========
[Extensions] Use compiled feature files

Use generated compiled feature files instead of parsing the JSON at
runtime. In local testing, this shows an order-of-magnitude performance
increase, as well as catching a number of invalid features.

BUG=280286

Committed: https://crrev.com/5c684983bc51d5a377fcc34dcdaa9363e9a9d7d3
Cr-Commit-Position: refs/heads/master@{#407511}
==========

[commit-bot: I haz the power, 2016-07-25 18:06:52]

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