Split feature definitions into extensions and chrome features.

Split feature definitions into extensions and chrome features.

Features defined in _*_features.json are split into chrome/common/extensions/api and extensions/common/api, according to where those features are implemented.

This also creates extensions_resources.pak.

With support for more than one features source, we can eliminate the AppShell dependency on Chrome features.

BUG=339301
TBR=sergeyberezin@chromium.org

Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=266634

[Yoyo Zhou, 2014-04-22 21:09:54]

https://chromiumcodereview.appspot.com/246423002/diff/20001/chrome/chrome_rep...
File chrome/chrome_repack_resources.gypi (right):

https://chromiumcodereview.appspot.com/246423002/diff/20001/chrome/chrome_rep...
chrome/chrome_repack_resources.gypi:33: # New paks should be added here by
default.
I know it says new paks should be added here, but since this just splits
common_resources.pak, it's safer to put them together.

[not at google - send to devlin, 2014-04-22 21:39:09]

lg, but you're going to need to make the docserver work. ken is the expert on
that.

https://chromiumcodereview.appspot.com/246423002/diff/20001/apps/shell/common...
File apps/shell/common/shell_extensions_client.cc (right):

https://chromiumcodereview.appspot.com/246423002/diff/20001/apps/shell/common...
apps/shell/common/shell_extensions_client.cc:130: extensions::FeatureProvider*
ShellExtensionsClient::CreateFeatureProvider(
scoped_ptr<>  ?

https://chromiumcodereview.appspot.com/246423002/diff/20001/extensions/common...
File extensions/common/features/feature_provider.cc (right):

https://chromiumcodereview.appspot.com/246423002/diff/20001/extensions/common...
extensions/common/features/feature_provider.cc:18: struct Static {
this seems more like a class, and |feature_providers| should be a private
|feature_providers_| variable.

[Ken Rockot(use gerrit already), 2014-04-22 21:47:13]

On 2014/04/22 21:39:09, kalman wrote:
> lg, but you're going to need to make the docserver work. ken is the expert on
> that.

As luck would have it, you can make the docserver work without too much effort
now.

You will want to modify extension_paths.py, probably doing something similar to
API_PATHS for now (i.e. change API_FEATURES and friends to be lists of paths
rather than single paths).

Then you need to update features_bundle.py so that it pulls and merges data from
multiple JSON sources for each feature type.

Every other part of the doc server goes through FeaturesBundle to get features
data, and its the only module in the doc server that uses the API_FEATURES etc
path constants, so your changes should be isolated to these two modules.

[Yoyo Zhou, 2014-04-22 23:17:40]

Made changes to the docserver. It looks good when I run preview.py locally.

https://chromiumcodereview.appspot.com/246423002/diff/20001/apps/shell/common...
File apps/shell/common/shell_extensions_client.cc (right):

https://chromiumcodereview.appspot.com/246423002/diff/20001/apps/shell/common...
apps/shell/common/shell_extensions_client.cc:130: extensions::FeatureProvider*
ShellExtensionsClient::CreateFeatureProvider(
On 2014/04/22 21:39:09, kalman wrote:
> scoped_ptr<>  ?

I think this is silly because it gets immediately turned into linked_ptr. But
done.

https://chromiumcodereview.appspot.com/246423002/diff/20001/extensions/common...
File extensions/common/features/feature_provider.cc (right):

https://chromiumcodereview.appspot.com/246423002/diff/20001/extensions/common...
extensions/common/features/feature_provider.cc:18: struct Static {
On 2014/04/22 21:39:09, kalman wrote:
> this seems more like a class, and |feature_providers| should be a private
> |feature_providers_| variable.

Done.

[Ken Rockot(use gerrit already), 2014-04-22 23:23:16]

LGTM

https://chromiumcodereview.appspot.com/246423002/diff/40001/chrome/common/ext...
File chrome/common/extensions/docs/server2/features_bundle.py (right):

https://chromiumcodereview.appspot.com/246423002/diff/40001/chrome/common/ext...
chrome/common/extensions/docs/server2/features_bundle.py:19:
posixpath.join(EXTENSIONS_API, feature_file)]
nit: It would be nice if this could just map from API_PATHS for each
feature_file. It looks like that should be easy as _FeaturesCache should fail
silently and gracefully when APPS_API/_foo_features.json doesn't exist.

[Yoyo Zhou, 2014-04-23 00:12:09]

The CQ bit was checked by yoz@chromium.org

[commit-bot: I haz the power, 2014-04-23 00:12:28]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/yoz@chromium.org/246423002/60001

[commit-bot: I haz the power, 2014-04-23 00:13:08]

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

[commit-bot: I haz the power, 2014-04-23 00:13:09]

Failed to apply patch for chrome/common/extensions/docs/server2/app.yaml:
While running patch -p1 --forward --force --no-backup-if-mismatch;
  patching file chrome/common/extensions/docs/server2/app.yaml
  Hunk #1 FAILED at 1.
  1 out of 1 hunk FAILED -- saving rejects to file
chrome/common/extensions/docs/server2/app.yaml.rej

Patch:       chrome/common/extensions/docs/server2/app.yaml
Index: chrome/common/extensions/docs/server2/app.yaml
diff --git a/chrome/common/extensions/docs/server2/app.yaml
b/chrome/common/extensions/docs/server2/app.yaml
index
aacbeee56ce386b6923b0e6b266871b8a26e79f7..81862d950ec69615c6df8b88905e30e1484371fa
100644
--- a/chrome/common/extensions/docs/server2/app.yaml
+++ b/chrome/common/extensions/docs/server2/app.yaml
@@ -1,5 +1,5 @@
 application: chrome-apps-doc
-version: 3-18-3
+version: 3-19-0
 runtime: python27
 api_version: 1
 threadsafe: false

[not at google - send to devlin, 2014-04-23 01:22:32]

lgtm

https://chromiumcodereview.appspot.com/246423002/diff/20001/apps/shell/common...
File apps/shell/common/shell_extensions_client.cc (right):

https://chromiumcodereview.appspot.com/246423002/diff/20001/apps/shell/common...
apps/shell/common/shell_extensions_client.cc:130: extensions::FeatureProvider*
ShellExtensionsClient::CreateFeatureProvider(
On 2014/04/22 23:17:40, Yoyo Zhou wrote:
> On 2014/04/22 21:39:09, kalman wrote:
> > scoped_ptr<>  ?
> 
> I think this is silly because it gets immediately turned into linked_ptr. But
> done.

make_linked_ptr should accept a scoped_ptr rvalue.

https://chromiumcodereview.appspot.com/246423002/diff/60001/chrome/common/ext...
File chrome/common/extensions/docs/server2/extensions_paths.py (right):

https://chromiumcodereview.appspot.com/246423002/diff/60001/chrome/common/ext...
chrome/common/extensions/docs/server2/extensions_paths.py:29:
PERMISSION_FEATURES = '_permission_features.json'
this file is supposed to contain full paths, so it looks like these constants
should be moved into FeaturesBundle.

[Yoyo Zhou, 2014-04-24 16:11:09]

The CQ bit was checked by yoz@chromium.org

[Yoyo Zhou, 2014-04-24 16:11:12]

The CQ bit was unchecked by yoz@chromium.org

[Yoyo Zhou, 2014-04-24 16:34:50]

https://chromiumcodereview.appspot.com/246423002/diff/20001/apps/shell/common...
File apps/shell/common/shell_extensions_client.cc (right):

https://chromiumcodereview.appspot.com/246423002/diff/20001/apps/shell/common...
apps/shell/common/shell_extensions_client.cc:130: extensions::FeatureProvider*
ShellExtensionsClient::CreateFeatureProvider(
On 2014/04/23 01:22:32, kalman wrote:
> On 2014/04/22 23:17:40, Yoyo Zhou wrote:
> > On 2014/04/22 21:39:09, kalman wrote:
> > > scoped_ptr<>  ?
> > 
> > I think this is silly because it gets immediately turned into linked_ptr.
But
> > done.
> 
> make_linked_ptr should accept a scoped_ptr rvalue.

Nope

https://chromiumcodereview.appspot.com/246423002/diff/60001/chrome/common/ext...
File chrome/common/extensions/docs/server2/extensions_paths.py (right):

https://chromiumcodereview.appspot.com/246423002/diff/60001/chrome/common/ext...
chrome/common/extensions/docs/server2/extensions_paths.py:29:
PERMISSION_FEATURES = '_permission_features.json'
On 2014/04/23 01:22:32, kalman wrote:
> this file is supposed to contain full paths, so it looks like these constants
> should be moved into FeaturesBundle.

Done.

[Yoyo Zhou, 2014-04-24 16:34:57]

The CQ bit was checked by yoz@chromium.org

[commit-bot: I haz the power, 2014-04-24 16:35:25]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/yoz@chromium.org/246423002/90002

[commit-bot: I haz the power, 2014-04-24 16:53:09]

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

[commit-bot: I haz the power, 2014-04-24 16:53:10]

Try jobs failed on following builders:
  tryserver.chromium on linux_chromium_clang_dbg

[Yoyo Zhou, 2014-04-24 18:59:41]

thakis: can you review resources changes? You're needed as an OWNER on
common_resources.grd but I'd like to make sure extensions_resources.grd looks
acceptable.

[Nico, 2014-04-24 19:02:21]

grd and gyp lgtm

(I prefer having fewer grd files, but for a toplevel directory I don't know of
any alternatives.)

[Yoyo Zhou, 2014-04-24 19:33:42]

The CQ bit was checked by yoz@chromium.org

[Yoyo Zhou, 2014-04-24 19:49:53]

The CQ bit was unchecked by yoz@chromium.org

[Yoyo Zhou, 2014-04-24 20:21:21]

The CQ bit was checked by yoz@chromium.org

[commit-bot: I haz the power, 2014-04-24 21:12:32]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/yoz@chromium.org/246423002/160001

[commit-bot: I haz the power, 2014-04-24 22:49:00]

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

[commit-bot: I haz the power, 2014-04-24 22:49:01]

Try jobs failed on following builders:
  tryserver.chromium on chromium_presubmit

[Yoyo Zhou, 2014-04-25 15:37:10]

The CQ bit was checked by yoz@chromium.org

[commit-bot: I haz the power, 2014-04-25 21:42:10]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/yoz@chromium.org/246423002/160001

[commit-bot: I haz the power, 2014-04-25 23:19:36]

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

[commit-bot: I haz the power, 2014-04-25 23:19:37]

Try jobs failed on following builders:
  tryserver.chromium on chromium_presubmit

[Yoyo Zhou, 2014-04-26 14:03:09]

Grr, this is complaining about a missing owner. TBRing pgervais who is looking
into this.

[Yoyo Zhou, 2014-04-26 14:03:30]

On 2014/04/26 14:03:09, Yoyo Zhou wrote:
> Grr, this is complaining about a missing owner. TBRing pgervais who is looking
> into this.

(To be clear, complaining about a missing owner for files I didn't touch.)

[Yoyo Zhou, 2014-04-26 14:03:37]

The CQ bit was checked by yoz@chromium.org

[commit-bot: I haz the power, 2014-04-26 14:04:12]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/yoz@chromium.org/246423002/160001

[commit-bot: I haz the power, 2014-04-26 19:19:23]

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

[commit-bot: I haz the power, 2014-04-26 19:19:24]

The commit queue went berserk retrying too often for a
seemingly flaky test on builder android_dbg:
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=android_db...
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=android_db...
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=android_db...
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=android_db...
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=android_db...
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=android_db...
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=android_db...
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=android_db...
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=android_db...

[Yoyo Zhou, 2014-04-28 04:07:51]

The CQ bit was checked by yoz@chromium.org

[commit-bot: I haz the power, 2014-04-28 04:09:17]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/yoz@chromium.org/246423002/160001

[commit-bot: I haz the power, 2014-04-28 09:03:45]

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

[commit-bot: I haz the power, 2014-04-28 09:03:46]

Retried try job too often on android_dbg for step(s) slave_steps
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=android_db...

[Yoyo Zhou, 2014-04-28 12:09:13]

The CQ bit was checked by yoz@chromium.org

[commit-bot: I haz the power, 2014-04-28 12:09:38]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/yoz@chromium.org/246423002/160001

[Yoyo Zhou, 2014-04-28 15:19:18]

The CQ bit was checked by yoz@chromium.org

[commit-bot: I haz the power, 2014-04-28 15:19:57]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/yoz@chromium.org/246423002/180001

[commit-bot: I haz the power, 2014-04-28 16:48:18]

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

[commit-bot: I haz the power, 2014-04-28 16:48:19]

Try jobs failed on following builders:
  tryserver.chromium on android_dbg_triggered_tests

[Yoyo Zhou, 2014-04-28 17:03:29]

The CQ bit was checked by yoz@chromium.org

[commit-bot: I haz the power, 2014-04-28 17:04:52]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/yoz@chromium.org/246423002/200001

[Yoyo Zhou, 2014-04-28 17:36:31]

The spurious OWNERS failure is
https://code.google.com/p/chromium/issues/detail?id=366489 - I've TBRed
sergeyberezin.

[commit-bot: I haz the power, 2014-04-28 19:49:33]

Change committed as 266634