Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(3)

Issue 15091002: Lazily load API schemas from resource files and convert all APIs to features (Closed)

Created:
7 years, 7 months ago by cduvall
Modified:
7 years, 6 months ago
CC:
chromium-reviews, Aaron Boodman, chromium-apps-reviews_chromium.org, Joao da Silva
Visibility:
Public.

Description

Lazily load API schemas from resource files and convert all APIs to features Loading API bindings in web pages had too much memory overhead. This lazily loads the schemas to reduce that overhead. This required converting all APIs to features. BUG=55316 Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=206313

Patch Set 1 #

Total comments: 12

Patch Set 2 : optimize #

Patch Set 3 : #

Patch Set 4 : convert all APIs to features #

Total comments: 22

Patch Set 5 : optimize and "parent" property #

Total comments: 44

Patch Set 6 : comments #

Total comments: 45

Patch Set 7 : more comments and fixed tests #

Total comments: 18

Patch Set 8 : more fixes #

Total comments: 10

Patch Set 9 : changes #

Total comments: 2

Patch Set 10 : rebase and while -> for #

Patch Set 11 : fix chromeos tests #

Unified diffs Side-by-side diffs Delta from patch set Stats (+965 lines, -549 lines) Patch
M chrome/common/extensions/api/_api_features.json View 1 2 3 4 5 6 7 8 9 10 4 chunks +423 lines, -27 lines 0 comments Download
M chrome/common/extensions/api/declarative_content.json View 1 2 3 4 5 6 7 1 chunk +0 lines, -1 line 0 comments Download
M chrome/common/extensions/api/declarative_web_request.json View 1 2 3 4 5 6 7 1 chunk +0 lines, -1 line 0 comments Download
M chrome/common/extensions/api/downloads.idl View 1 2 3 4 5 6 7 1 chunk +0 lines, -1 line 0 comments Download
M chrome/common/extensions/api/downloads_internal.idl View 1 2 3 4 5 6 7 1 chunk +1 line, -1 line 0 comments Download
M chrome/common/extensions/api/events.json View 1 2 3 4 5 6 1 chunk +0 lines, -1 line 0 comments Download
M chrome/common/extensions/api/extension.json View 1 2 3 4 5 6 7 5 chunks +0 lines, -5 lines 0 comments Download
M chrome/common/extensions/api/extension_api.h View 1 2 3 4 5 6 7 5 chunks +11 lines, -42 lines 0 comments Download
M chrome/common/extensions/api/extension_api.cc View 1 2 3 4 5 6 7 8 10 chunks +63 lines, -245 lines 0 comments Download
M chrome/common/extensions/api/extension_api_stub.cc View 1 2 3 4 5 6 7 8 1 chunk +0 lines, -4 lines 0 comments Download
M chrome/common/extensions/api/extension_api_unittest.cc View 1 2 3 4 5 6 7 14 chunks +179 lines, -66 lines 0 comments Download
M chrome/common/extensions/api/font_settings.json View 1 2 3 4 5 6 7 1 chunk +0 lines, -1 line 0 comments Download
M chrome/common/extensions/api/i18n.json View 1 2 3 4 5 6 1 chunk +0 lines, -1 line 0 comments Download
M chrome/common/extensions/api/power.idl View 1 2 3 4 5 6 7 1 chunk +0 lines, -1 line 0 comments Download
M chrome/common/extensions/api/runtime.json View 1 2 3 4 5 6 7 1 chunk +0 lines, -1 line 0 comments Download
M chrome/common/extensions/api/storage.json View 1 2 3 4 5 6 1 chunk +0 lines, -1 line 0 comments Download
M chrome/common/extensions/api/test.json View 1 2 3 4 5 6 7 8 9 19 chunks +0 lines, -22 lines 0 comments Download
M chrome/common/extensions/features/base_feature_provider.h View 1 2 3 4 5 6 7 8 2 chunks +5 lines, -2 lines 0 comments Download
M chrome/common/extensions/features/base_feature_provider.cc View 1 2 3 4 5 6 7 8 5 chunks +66 lines, -11 lines 0 comments Download
M chrome/common/extensions/features/base_feature_provider_unittest.cc View 1 2 3 4 5 6 7 8 2 chunks +7 lines, -0 lines 0 comments Download
M chrome/common/extensions/features/feature.h View 1 2 3 4 5 6 7 8 2 chunks +2 lines, -0 lines 0 comments Download
M chrome/common/extensions/features/feature.cc View 1 2 3 4 5 6 7 8 1 chunk +1 line, -1 line 0 comments Download
M chrome/common/extensions/features/feature_provider.h View 1 2 3 4 5 6 7 8 1 chunk +4 lines, -1 line 0 comments Download
M chrome/common/extensions/features/simple_feature.h View 1 2 3 4 5 6 7 8 1 chunk +2 lines, -0 lines 0 comments Download
M chrome/common/extensions/features/simple_feature.cc View 1 2 3 4 5 6 7 8 6 chunks +22 lines, -4 lines 0 comments Download
M chrome/common/extensions/features/simple_feature_unittest.cc View 1 2 3 4 5 6 7 8 1 chunk +0 lines, -12 lines 0 comments Download
M chrome/common/extensions/manifest.cc View 1 2 3 4 5 6 7 1 chunk +4 lines, -2 lines 0 comments Download
M chrome/renderer/extensions/api_definitions_natives.h View 1 2 3 4 5 6 7 1 chunk +2 lines, -1 line 0 comments Download
M chrome/renderer/extensions/api_definitions_natives.cc View 1 2 3 4 5 6 7 8 2 chunks +18 lines, -7 lines 0 comments Download
M chrome/renderer/extensions/chrome_v8_context.h View 1 2 3 4 5 6 7 8 1 chunk +3 lines, -0 lines 0 comments Download
M chrome/renderer/extensions/chrome_v8_context.cc View 1 2 3 4 5 6 7 8 2 chunks +8 lines, -5 lines 0 comments Download
M chrome/renderer/extensions/dispatcher.cc View 1 2 3 4 5 6 7 8 9 4 chunks +31 lines, -32 lines 0 comments Download
M chrome/renderer/extensions/event_unittest.cc View 1 2 3 4 5 6 7 1 chunk +1 line, -1 line 0 comments Download
M chrome/renderer/extensions/runtime_custom_bindings.cc View 1 2 3 4 5 6 7 8 2 chunks +6 lines, -2 lines 0 comments Download
M chrome/renderer/extensions/v8_schema_registry.h View 1 2 3 4 5 6 7 8 9 2 chunks +2 lines, -2 lines 0 comments Download
M chrome/renderer/extensions/v8_schema_registry.cc View 1 2 3 4 5 6 7 8 9 1 chunk +3 lines, -3 lines 0 comments Download
M chrome/renderer/resources/extensions/test_custom_bindings.js View 1 2 3 4 5 6 7 2 chunks +3 lines, -5 lines 0 comments Download
M chrome/test/data/extensions/api_test/events/background.js View 1 2 3 4 5 6 1 chunk +0 lines, -18 lines 0 comments Download
M chrome/test/data/extensions/api_test/events/manifest.json View 1 2 3 4 5 6 7 1 chunk +1 line, -0 lines 0 comments Download
M chrome/test/data/extensions/api_test/stubs/content_script.js View 1 2 3 4 5 6 7 1 chunk +6 lines, -2 lines 0 comments Download
M chrome/test/data/extensions/extension_api_unittest/api_features.json View 1 2 3 4 5 6 7 5 chunks +41 lines, -1 line 0 comments Download
M chrome/test/data/extensions/extension_api_unittest/privileged_api_features.json View 1 2 3 4 5 6 7 3 chunks +6 lines, -0 lines 0 comments Download
M tools/json_schema_compiler/cpp_bundle_generator.py View 1 2 3 4 5 6 3 chunks +42 lines, -8 lines 0 comments Download
M tools/json_schema_compiler/idl_schema.py View 1 2 3 4 5 6 7 5 chunks +2 lines, -8 lines 0 comments Download

Messages

Total messages: 30 (0 generated)
cduvall
Loading an empty web page on master takes around 10,150kb. Loading an empty web page ...
7 years, 7 months ago (2013-05-10 02:25:31 UTC) #1
not at google - send to devlin
I'd say lg but wdyt about the schema loading thing? https://codereview.chromium.org/15091002/diff/1/chrome/common/extensions/api/extension_api.cc File chrome/common/extensions/api/extension_api.cc (right): https://codereview.chromium.org/15091002/diff/1/chrome/common/extensions/api/extension_api.cc#newcode332 ...
7 years, 7 months ago (2013-05-10 02:46:45 UTC) #2
cduvall
So I tried to optimize to reduce the memory usage, and it didn't work, it ...
7 years, 7 months ago (2013-05-10 03:31:07 UTC) #3
cduvall
Still working on a couple things, but thought I would upload so you can take ...
7 years, 7 months ago (2013-05-17 03:09:30 UTC) #4
not at google - send to devlin
awesome! haven't checked over the list yet. https://codereview.chromium.org/15091002/diff/19001/chrome/common/extensions/api/_api_features.json File chrome/common/extensions/api/_api_features.json (right): https://codereview.chromium.org/15091002/diff/19001/chrome/common/extensions/api/_api_features.json#newcode59 chrome/common/extensions/api/_api_features.json:59: "runtime.connect": { ...
7 years, 7 months ago (2013-05-20 17:19:04 UTC) #5
cduvall
https://codereview.chromium.org/15091002/diff/19001/chrome/common/extensions/api/_api_features.json File chrome/common/extensions/api/_api_features.json (right): https://codereview.chromium.org/15091002/diff/19001/chrome/common/extensions/api/_api_features.json#newcode59 chrome/common/extensions/api/_api_features.json:59: "runtime.connect": { On 2013/05/20 17:19:04, kalman wrote: > As ...
7 years, 7 months ago (2013-05-21 23:50:28 UTC) #6
not at google - send to devlin
> So then this feature would just take the properties from the parent feature, and ...
7 years, 7 months ago (2013-05-21 23:54:13 UTC) #7
cduvall
I still need to fix up the tests and maybe write some more, but I ...
7 years, 7 months ago (2013-05-22 03:19:56 UTC) #8
not at google - send to devlin
ok lmk when you fix tests. looking good. https://codereview.chromium.org/15091002/diff/19001/chrome/common/extensions/api/extension_api.cc File chrome/common/extensions/api/extension_api.cc (right): https://codereview.chromium.org/15091002/diff/19001/chrome/common/extensions/api/extension_api.cc#newcode456 chrome/common/extensions/api/extension_api.cc:456: std::set<std::string> ...
7 years, 7 months ago (2013-05-23 00:09:40 UTC) #9
cduvall
Still have a few browser test failures that I didn't have time to fix, but ...
7 years, 7 months ago (2013-05-24 03:13:49 UTC) #10
not at google - send to devlin
lg after tests are fixed and this last round of comments. also adding the rest ...
7 years, 7 months ago (2013-05-24 19:09:18 UTC) #11
Matt Perry
https://codereview.chromium.org/15091002/diff/51003/chrome/common/extensions/api/_api_features.json File chrome/common/extensions/api/_api_features.json (right): https://codereview.chromium.org/15091002/diff/51003/chrome/common/extensions/api/_api_features.json#newcode34 chrome/common/extensions/api/_api_features.json:34: "parent": "bookmarks", Can we get rid of the "parent" ...
7 years, 7 months ago (2013-05-24 19:53:23 UTC) #12
not at google - send to devlin
https://codereview.chromium.org/15091002/diff/51003/chrome/common/extensions/api/_api_features.json File chrome/common/extensions/api/_api_features.json (right): https://codereview.chromium.org/15091002/diff/51003/chrome/common/extensions/api/_api_features.json#newcode34 chrome/common/extensions/api/_api_features.json:34: "parent": "bookmarks", On 2013/05/24 19:53:23, Matt Perry wrote: > ...
7 years, 7 months ago (2013-05-24 20:07:59 UTC) #13
Yoyo Zhou
https://codereview.chromium.org/15091002/diff/51003/chrome/common/extensions/api/_api_features.json File chrome/common/extensions/api/_api_features.json (left): https://codereview.chromium.org/15091002/diff/51003/chrome/common/extensions/api/_api_features.json#oldcode32 chrome/common/extensions/api/_api_features.json:32: "channel": "stable", Why remove this? https://codereview.chromium.org/15091002/diff/51003/chrome/common/extensions/api/_api_features.json File chrome/common/extensions/api/_api_features.json (right): ...
7 years, 6 months ago (2013-05-28 23:49:38 UTC) #14
not at google - send to devlin
https://codereview.chromium.org/15091002/diff/51003/chrome/common/extensions/api/_api_features.json File chrome/common/extensions/api/_api_features.json (left): https://codereview.chromium.org/15091002/diff/51003/chrome/common/extensions/api/_api_features.json#oldcode32 chrome/common/extensions/api/_api_features.json:32: "channel": "stable", On 2013/05/28 23:49:38, Yoyo Zhou wrote: > ...
7 years, 6 months ago (2013-05-28 23:53:57 UTC) #15
cduvall
I *think* all the tests are fixed, I'm going to run some try jobs to ...
7 years, 6 months ago (2013-05-30 00:50:51 UTC) #16
not at google - send to devlin
sorry again for taking you back and forth on that parent thing. can you think ...
7 years, 6 months ago (2013-05-30 16:38:51 UTC) #17
not at google - send to devlin
Looking forward to merging in your latest and change and getting this submitted! Another thing ...
7 years, 6 months ago (2013-06-07 15:47:22 UTC) #18
cduvall
I switched this to using "noparent": true to distinguish APIs that should not inherit features ...
7 years, 6 months ago (2013-06-12 01:22:18 UTC) #19
Matt Perry
_api_features.json LGTM
7 years, 6 months ago (2013-06-12 19:22:35 UTC) #20
not at google - send to devlin
https://codereview.chromium.org/15091002/diff/93001/chrome/renderer/extensions/api_definitions_natives.cc File chrome/renderer/extensions/api_definitions_natives.cc (right): https://codereview.chromium.org/15091002/diff/93001/chrome/renderer/extensions/api_definitions_natives.cc#newcode29 chrome/renderer/extensions/api_definitions_natives.cc:29: const std::vector<std::string>& feature_names = On 2013/06/12 01:22:19, cduvall wrote: ...
7 years, 6 months ago (2013-06-12 22:34:18 UTC) #21
cduvall
https://codereview.chromium.org/15091002/diff/119001/chrome/common/extensions/api/extension_api.cc File chrome/common/extensions/api/extension_api.cc (right): https://codereview.chromium.org/15091002/diff/119001/chrome/common/extensions/api/extension_api.cc#newcode40 chrome/common/extensions/api/extension_api.cc:40: const char kUnavailableMessage[] = "You do not have permission ...
7 years, 6 months ago (2013-06-13 01:02:28 UTC) #22
not at google - send to devlin
lgtm, one more comment. make sure the api features stuff is synced up as well, ...
7 years, 6 months ago (2013-06-13 22:23:25 UTC) #23
cduvall
https://codereview.chromium.org/15091002/diff/148001/chrome/renderer/extensions/dispatcher.cc File chrome/renderer/extensions/dispatcher.cc (right): https://codereview.chromium.org/15091002/diff/148001/chrome/renderer/extensions/dispatcher.cc#newcode641 chrome/renderer/extensions/dispatcher.cc:641: while ((parent_feature = feature_provider->GetParent(parent_feature))) { On 2013/06/13 22:23:25, kalman ...
7 years, 6 months ago (2013-06-13 22:43:06 UTC) #24
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/cduvall@chromium.org/15091002/168001
7 years, 6 months ago (2013-06-13 22:43:55 UTC) #25
commit-bot: I haz the power
Retried try job too often on linux_chromeos for step(s) browser_tests http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=linux_chromeos&number=125688
7 years, 6 months ago (2013-06-14 01:12:48 UTC) #26
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/cduvall@chromium.org/15091002/185001
7 years, 6 months ago (2013-06-14 01:52:01 UTC) #27
commit-bot: I haz the power
Sorry for I got bad news for ya. Compile failed with a clobber build on ...
7 years, 6 months ago (2013-06-14 02:11:47 UTC) #28
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/cduvall@chromium.org/15091002/185001
7 years, 6 months ago (2013-06-14 02:13:22 UTC) #29
commit-bot: I haz the power
7 years, 6 months ago (2013-06-14 06:03:53 UTC) #30
Message was sent while issue was closed.
Change committed as 206313

Powered by Google App Engine
This is Rietveld 408576698