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

Issue 265503003: Enable file_handlers and chrome.app.runtime for QuickOffice. (Closed)

Created:
6 years, 7 months ago by benwells
Modified:
6 years, 7 months ago
CC:
chromium-reviews, chromium-apps-reviews_chromium.org, extensions-reviews_chromium.org, chrome-apps-syd-reviews_chromium.org
Base URL:
https://chromium.googlesource.com/chromium/src.git@master
Visibility:
Public.

Description

Enable file_handlers and chrome.app.runtime for QuickOffice. This will enable QuickOffice to use the onLaunched event and file_handlers to implement launching file Files.App and later from the app launcher. To make chrome.app.runtime available to whitelisted extensions, the chrome.app namespace needs to also be removed. This has been enabled by making extension feature files understand blacklisting. The PDF viewer may also use this in future. TBR=sky@chromium.org BUG=329587 Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=268845

Patch Set 1 #

Total comments: 8

Patch Set 2 : Rebase #

Patch Set 3 : With blacklist, test #

Total comments: 8

Patch Set 4 : Feedback #

Patch Set 5 : Reeebase #

Patch Set 6 : Don't autowhitelist all component extensions #

Patch Set 7 : Hacky approach to get around component apps being auto whitelisted to everything #

Total comments: 2

Patch Set 8 : Feedback, less hacky #

Unified diffs Side-by-side diffs Delta from patch set Stats (+243 lines, -21 lines) Patch
M chrome/browser/apps/app_browsertest.cc View 1 2 3 4 1 chunk +10 lines, -0 lines 0 comments Download
M chrome/browser/content_settings/content_settings_internal_extension_provider.cc View 1 2 1 chunk +1 line, -1 line 0 comments Download
M chrome/browser/extensions/api/app_current_window_internal/app_current_window_internal_api.cc View 1 2 1 chunk +1 line, -1 line 0 comments Download
M chrome/browser/extensions/api/tab_capture/tab_capture_api.cc View 1 2 1 chunk +1 line, -1 line 0 comments Download
M chrome/browser/ui/extensions/application_launch.h View 1 2 3 1 chunk +4 lines, -0 lines 0 comments Download
M chrome/browser/ui/extensions/application_launch.cc View 1 2 3 4 3 chunks +10 lines, -1 line 0 comments Download
M chrome/browser/ui/startup/startup_browser_creator_impl.cc View 1 2 3 4 1 chunk +1 line, -1 line 0 comments Download
M chrome/common/extensions/api/_api_features.json View 1 2 3 4 5 6 7 3 chunks +27 lines, -2 lines 0 comments Download
M chrome/common/extensions/api/_manifest_features.json View 1 2 3 4 2 chunks +20 lines, -2 lines 0 comments Download
M chrome/common/extensions/api/file_handlers/file_handlers_parser.cc View 1 chunk +0 lines, -2 lines 0 comments Download
A chrome/test/data/extensions/platform_apps/launch_whitelisted_ext_with_file/manifest.json View 1 2 1 chunk +20 lines, -0 lines 0 comments Download
A + chrome/test/data/extensions/platform_apps/launch_whitelisted_ext_with_file/test.js View 1 2 3 1 chunk +5 lines, -0 lines 0 comments Download
M extensions/common/api/_api_features.json View 1 2 3 4 1 chunk +9 lines, -0 lines 0 comments Download
M extensions/common/api/_manifest_features.json View 1 2 1 chunk +9 lines, -0 lines 0 comments Download
M extensions/common/features/complex_feature.h View 1 2 1 chunk +1 line, -0 lines 0 comments Download
M extensions/common/features/complex_feature.cc View 1 2 3 chunks +14 lines, -0 lines 0 comments Download
M extensions/common/features/feature.h View 1 2 2 chunks +2 lines, -0 lines 0 comments Download
M extensions/common/features/simple_feature.h View 1 2 3 4 5 6 7 4 chunks +6 lines, -2 lines 0 comments Download
M extensions/common/features/simple_feature.cc View 1 2 3 4 5 6 7 7 chunks +24 lines, -8 lines 0 comments Download
M extensions/common/features/simple_feature_unittest.cc View 1 2 1 chunk +78 lines, -0 lines 0 comments Download

Messages

Total messages: 44 (0 generated)
benwells
kalman, please take a first look. As discussed doing this the other way (move chrome.app.runtime ...
6 years, 7 months ago (2014-04-30 07:16:45 UTC) #1
not at google - send to devlin
The problem isn't that the APIs will fight over being defined. The problem is that ...
6 years, 7 months ago (2014-04-30 15:52:54 UTC) #2
not at google - send to devlin
P.S. maybe the REAL answer here is that apps should be launchable in a browser ...
6 years, 7 months ago (2014-04-30 15:54:25 UTC) #3
benwells
On 2014/04/30 15:54:25, kalman wrote: > P.S. maybe the REAL answer here is that apps ...
6 years, 7 months ago (2014-05-01 01:17:34 UTC) #4
benwells
https://codereview.chromium.org/265503003/diff/1/chrome/common/extensions/api/_api_features.json File chrome/common/extensions/api/_api_features.json (right): https://codereview.chromium.org/265503003/diff/1/chrome/common/extensions/api/_api_features.json#newcode62 chrome/common/extensions/api/_api_features.json:62: "whitelist": [ On 2014/04/30 15:52:55, kalman wrote: > I ...
6 years, 7 months ago (2014-05-01 01:17:46 UTC) #5
benwells
https://codereview.chromium.org/265503003/diff/1/chrome/common/extensions/api/_api_features.json File chrome/common/extensions/api/_api_features.json (right): https://codereview.chromium.org/265503003/diff/1/chrome/common/extensions/api/_api_features.json#newcode58 chrome/common/extensions/api/_api_features.json:58: "noparent": true, On 2014/04/30 15:52:55, kalman wrote: > nit: ...
6 years, 7 months ago (2014-05-01 08:19:21 UTC) #6
not at google - send to devlin
lgtm, and also mention the blacklist thing in the CL description. https://codereview.chromium.org/265503003/diff/40001/chrome/browser/ui/extensions/application_launch.h File chrome/browser/ui/extensions/application_launch.h (right): ...
6 years, 7 months ago (2014-05-01 20:55:06 UTC) #7
benwells
https://codereview.chromium.org/265503003/diff/40001/chrome/browser/ui/extensions/application_launch.h File chrome/browser/ui/extensions/application_launch.h (right): https://codereview.chromium.org/265503003/diff/40001/chrome/browser/ui/extensions/application_launch.h#newcode99 chrome/browser/ui/extensions/application_launch.h:99: bool LaunchesViaEvent(const extensions::Extension* extension); On 2014/05/01 20:55:06, kalman wrote: ...
6 years, 7 months ago (2014-05-02 00:58:52 UTC) #8
benwells
+tbr sky for refactoring fallout
6 years, 7 months ago (2014-05-02 01:00:26 UTC) #9
benwells
The CQ bit was checked by benwells@chromium.org
6 years, 7 months ago (2014-05-02 01:09:20 UTC) #10
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/benwells@chromium.org/265503003/50001
6 years, 7 months ago (2014-05-02 01:09:40 UTC) #11
commit-bot: I haz the power
The CQ bit was unchecked by commit-bot@chromium.org
6 years, 7 months ago (2014-05-02 01:13:03 UTC) #12
commit-bot: I haz the power
Try jobs failed on following builders: linux_chromium_chromeos_rel on tryserver.chromium
6 years, 7 months ago (2014-05-02 01:13:03 UTC) #13
benwells
The CQ bit was checked by benwells@chromium.org
6 years, 7 months ago (2014-05-02 01:31:04 UTC) #14
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/benwells@chromium.org/265503003/50001
6 years, 7 months ago (2014-05-02 01:31:45 UTC) #15
commit-bot: I haz the power
The CQ bit was unchecked by commit-bot@chromium.org
6 years, 7 months ago (2014-05-02 02:02:30 UTC) #16
commit-bot: I haz the power
Try jobs failed on following builders: linux_chromium_rel on tryserver.chromium
6 years, 7 months ago (2014-05-02 02:02:31 UTC) #17
benwells
The CQ bit was checked by benwells@chromium.org
6 years, 7 months ago (2014-05-02 02:48:16 UTC) #18
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/benwells@chromium.org/265503003/50001
6 years, 7 months ago (2014-05-02 02:48:59 UTC) #19
commit-bot: I haz the power
The CQ bit was unchecked by commit-bot@chromium.org
6 years, 7 months ago (2014-05-02 02:59:05 UTC) #20
commit-bot: I haz the power
Try jobs failed on following builders: linux_chromium_chromeos_rel on tryserver.chromium
6 years, 7 months ago (2014-05-02 02:59:06 UTC) #21
benwells
The CQ bit was checked by benwells@chromium.org
6 years, 7 months ago (2014-05-02 06:30:23 UTC) #22
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/benwells@chromium.org/265503003/50001
6 years, 7 months ago (2014-05-02 06:30:34 UTC) #23
commit-bot: I haz the power
The CQ bit was unchecked by commit-bot@chromium.org
6 years, 7 months ago (2014-05-02 06:53:18 UTC) #24
commit-bot: I haz the power
Try jobs failed on following builders: linux_chromium_rel on tryserver.chromium
6 years, 7 months ago (2014-05-02 06:53:19 UTC) #25
benwells
The CQ bit was checked by benwells@chromium.org
6 years, 7 months ago (2014-05-02 06:59:30 UTC) #26
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/benwells@chromium.org/265503003/50001
6 years, 7 months ago (2014-05-02 06:59:47 UTC) #27
commit-bot: I haz the power
The CQ bit was unchecked by commit-bot@chromium.org
6 years, 7 months ago (2014-05-02 07:04:04 UTC) #28
commit-bot: I haz the power
Try jobs failed on following builders: mac_chromium_rel on tryserver.chromium
6 years, 7 months ago (2014-05-02 07:04:05 UTC) #29
benwells
The CQ bit was checked by benwells@chromium.org
6 years, 7 months ago (2014-05-05 00:10:59 UTC) #30
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/benwells@chromium.org/265503003/50001
6 years, 7 months ago (2014-05-05 00:11:05 UTC) #31
commit-bot: I haz the power
The CQ bit was unchecked by commit-bot@chromium.org
6 years, 7 months ago (2014-05-05 00:57:00 UTC) #32
commit-bot: I haz the power
Try jobs failed on following builders: linux_chromium_rel on tryserver.chromium
6 years, 7 months ago (2014-05-05 00:57:01 UTC) #33
benwells
kalman - as you can see from some earlier try jobs the approach we discussed ...
6 years, 7 months ago (2014-05-06 12:58:48 UTC) #34
not at google - send to devlin
lgtm https://codereview.chromium.org/265503003/diff/100001/extensions/common/features/simple_feature.cc File extensions/common/features/simple_feature.cc (right): https://codereview.chromium.org/265503003/diff/100001/extensions/common/features/simple_feature.cc#newcode300 extensions/common/features/simple_feature.cc:300: // See http://crbug.com/370375 for more details. this is ...
6 years, 7 months ago (2014-05-06 23:04:02 UTC) #35
benwells
On 2014/05/06 23:04:02, kalman wrote: > lgtm > > https://codereview.chromium.org/265503003/diff/100001/extensions/common/features/simple_feature.cc > File extensions/common/features/simple_feature.cc (right): > ...
6 years, 7 months ago (2014-05-07 06:33:31 UTC) #36
benwells
The CQ bit was checked by benwells@chromium.org
6 years, 7 months ago (2014-05-07 06:33:39 UTC) #37
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/benwells@chromium.org/265503003/120001
6 years, 7 months ago (2014-05-07 06:34:42 UTC) #38
commit-bot: I haz the power
FYI, CQ is re-trying this CL (attempt #1). Please consider checking whether the failures are ...
6 years, 7 months ago (2014-05-07 12:40:58 UTC) #39
not at google - send to devlin
The CQ bit was unchecked by kalman@chromium.org
6 years, 7 months ago (2014-05-07 15:35:29 UTC) #40
not at google - send to devlin
The CQ bit was checked by kalman@chromium.org
6 years, 7 months ago (2014-05-07 15:35:36 UTC) #41
not at google - send to devlin
oops lgtm
6 years, 7 months ago (2014-05-07 15:36:08 UTC) #42
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/benwells@chromium.org/265503003/120001
6 years, 7 months ago (2014-05-07 15:40:58 UTC) #43
commit-bot: I haz the power
6 years, 7 months ago (2014-05-07 17:47:52 UTC) #44
Message was sent while issue was closed.
Change committed as 268845

Powered by Google App Engine
This is Rietveld 408576698