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

Issues Search
    My Issues | Starred     Open | Closed | All

Issue 10014005: Add a preference for why an extension is disabled. (Closed)

Created:
8 years, 8 months ago by Yoyo Zhou
Modified:
8 years, 8 months ago
CC:
chromium-reviews, Aaron Boodman, mihaip+watch_chromium.org
Visibility:
Public.

Description

Add a preference for why an extension is disabled. Don't show the permissions upgrade alert for extensions disabled by the user. For existing disabled extensions, guess the disabling reason from whether they exceed their granted permissions. BUG=121436 TEST=ExtensionDisabledGlobalErrorTest.* TBR=csilv@chromium.org,jianli@chromium.org,mark@chromium.org Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=131665

Patch Set 1 #

Patch Set 2 : . #

Total comments: 8

Patch Set 3 : aa's #

Patch Set 4 : ? #

Total comments: 3

Patch Set 5 : . #

Unified diffs Side-by-side diffs Delta from patch set Stats (+220 lines, -61 lines) Patch
M chrome/browser/automation/automation_provider.cc View 1 2 1 chunk +1 line, -1 line 0 comments Download
M chrome/browser/automation/testing_automation_provider.cc View 1 2 1 chunk +1 line, -1 line 0 comments Download
M chrome/browser/extensions/extension_browsertest.cc View 1 2 1 chunk +1 line, -1 line 0 comments Download
M chrome/browser/extensions/extension_context_menu_model.cc View 1 2 1 chunk +2 lines, -1 line 0 comments Download
M chrome/browser/extensions/extension_disabled_ui.cc View 2 chunks +1 line, -1 line 0 comments Download
M chrome/browser/extensions/extension_disabled_ui_browsertest.cc View 1 2 4 chunks +93 lines, -13 lines 0 comments Download
M chrome/browser/extensions/extension_management_api.cc View 1 2 1 chunk +1 line, -1 line 0 comments Download
M chrome/browser/extensions/extension_management_apitest.cc View 1 2 1 chunk +2 lines, -3 lines 0 comments Download
M chrome/browser/extensions/extension_management_browsertest.cc View 1 2 5 chunks +6 lines, -6 lines 0 comments Download
M chrome/browser/extensions/extension_prefs.h View 1 2 1 chunk +6 lines, -0 lines 0 comments Download
M chrome/browser/extensions/extension_prefs.cc View 1 2 2 chunks +24 lines, -0 lines 0 comments Download
M chrome/browser/extensions/extension_service.h View 1 2 1 chunk +2 lines, -1 line 0 comments Download
M chrome/browser/extensions/extension_service.cc View 1 2 4 9 chunks +35 lines, -8 lines 0 comments Download
M chrome/browser/extensions/extension_service_unittest.cc View 1 2 6 chunks +6 lines, -6 lines 0 comments Download
M chrome/browser/notifications/notification_options_menu_model.cc View 1 2 1 chunk +2 lines, -1 line 0 comments Download
M chrome/browser/sync/test/integration/sync_extension_helper.cc View 1 2 1 chunk +2 lines, -1 line 0 comments Download
M chrome/browser/ui/cocoa/extensions/extension_action_context_menu.mm View 1 2 1 chunk +2 lines, -1 line 0 comments Download
M chrome/browser/ui/panels/panel_settings_menu_model.cc View 1 2 1 chunk +1 line, -1 line 0 comments Download
M chrome/browser/ui/webui/extensions/extension_settings_handler.cc View 1 2 1 chunk +2 lines, -1 line 0 comments Download
M chrome/common/extensions/extension.h View 1 2 1 chunk +9 lines, -0 lines 0 comments Download
D chrome/test/data/extensions/permissions-high-v2.crx View 1 2 0 chunks +-1 lines, --1 lines 0 comments Download
D chrome/test/data/extensions/permissions-low-v1.crx View 1 2 0 chunks +-1 lines, --1 lines 0 comments Download
D chrome/test/data/extensions/permissions-low-v1.pem View 1 2 1 chunk +0 lines, -16 lines 0 comments Download
A + chrome/test/data/extensions/permissions_increase/permissions.pem View 1 2 0 chunks +-1 lines, --1 lines 0 comments Download
A chrome/test/data/extensions/permissions_increase/v1/background.html View 1 2 1 chunk +1 line, -0 lines 0 comments Download
A chrome/test/data/extensions/permissions_increase/v1/manifest.json View 1 2 1 chunk +5 lines, -0 lines 0 comments Download
A chrome/test/data/extensions/permissions_increase/v2/background.html View 1 2 1 chunk +1 line, -0 lines 0 comments Download
A chrome/test/data/extensions/permissions_increase/v2/manifest.json View 1 2 1 chunk +8 lines, -0 lines 0 comments Download
A chrome/test/data/extensions/permissions_increase/v3/background.html View 1 2 1 chunk +1 line, -0 lines 0 comments Download
A chrome/test/data/extensions/permissions_increase/v3/manifest.json View 1 2 1 chunk +8 lines, -0 lines 0 comments Download

Messages

Total messages: 10 (0 generated)
Yoyo Zhou
8 years, 8 months ago (2012-04-09 19:32:52 UTC) #1
Aaron Boodman
https://chromiumcodereview.appspot.com/10014005/diff/2002/chrome/browser/extensions/extension_disabled_ui_browsertest.cc File chrome/browser/extensions/extension_disabled_ui_browsertest.cc (right): https://chromiumcodereview.appspot.com/10014005/diff/2002/chrome/browser/extensions/extension_disabled_ui_browsertest.cc#newcode144 chrome/browser/extensions/extension_disabled_ui_browsertest.cc:144: extension, "permissions-higher-v3.crx", 0); These crx files are a maintenance ...
8 years, 8 months ago (2012-04-09 20:30:48 UTC) #2
Yoyo Zhou
https://chromiumcodereview.appspot.com/10014005/diff/2002/chrome/browser/extensions/extension_disabled_ui_browsertest.cc File chrome/browser/extensions/extension_disabled_ui_browsertest.cc (right): https://chromiumcodereview.appspot.com/10014005/diff/2002/chrome/browser/extensions/extension_disabled_ui_browsertest.cc#newcode144 chrome/browser/extensions/extension_disabled_ui_browsertest.cc:144: extension, "permissions-higher-v3.crx", 0); On 2012/04/09 20:30:48, Aaron Boodman wrote: ...
8 years, 8 months ago (2012-04-10 01:57:36 UTC) #3
Yoyo Zhou
sky: please rubber-stamp for chrome/browser/ui zea: please rubber-stamp for chrome/browser/sync, and see the comment in ...
8 years, 8 months ago (2012-04-10 01:59:54 UTC) #4
Yoyo Zhou
sky: please rubber-stamp for chrome/browser/ui zea: please rubber-stamp for chrome/browser/sync, and see the comment in ...
8 years, 8 months ago (2012-04-10 01:59:54 UTC) #5
sky
I'm not an owner of any of the chrome/browser/ui directories you have here. That said, ...
8 years, 8 months ago (2012-04-10 16:17:42 UTC) #6
Yoyo Zhou
On 2012/04/10 16:17:42, sky wrote: > I'm not an owner of any of the chrome/browser/ui ...
8 years, 8 months ago (2012-04-10 16:35:21 UTC) #7
Nicolas Zea
sync LGTM
8 years, 8 months ago (2012-04-10 18:21:11 UTC) #8
Aaron Boodman
lgtm http://codereview.chromium.org/10014005/diff/2003/chrome/browser/extensions/extension_service.cc File chrome/browser/extensions/extension_service.cc (right): http://codereview.chromium.org/10014005/diff/2003/chrome/browser/extensions/extension_service.cc#newcode1514 chrome/browser/extensions/extension_service.cc:1514: DisableExtension(id, Extension::DISABLE_UNKNOWN); On 2012/04/10 01:57:36, Yoyo Zhou wrote: ...
8 years, 8 months ago (2012-04-10 20:37:59 UTC) #9
Yoyo Zhou
8 years, 8 months ago (2012-04-10 22:38:00 UTC) #10 
http://codereview.chromium.org/10014005/diff/2003/chrome/browser/extensions/e...
File chrome/browser/extensions/extension_service.cc (right):

http://codereview.chromium.org/10014005/diff/2003/chrome/browser/extensions/e...
chrome/browser/extensions/extension_service.cc:1514: DisableExtension(id,
Extension::DISABLE_UNKNOWN);
On 2012/04/10 20:37:59, Aaron Boodman wrote:
> On 2012/04/10 01:57:36, Yoyo Zhou wrote:
> > I am not confident this is correct; we don't sync this value, at least. The
> > result would be that we guess from the granted permissions what the disable
> > reason was, as we do for extensions disabled in the past.
> 
> Bummer. I think it would be better to default the other direction - consider
> this a user-initiated disable. This will result in less surprising UI.
> 
> For longer term, can you create a bug to sync this state?

Actually, I think we need to first sync granted extension permissions before it
makes sense to do so. Right now, if you have an extension with too high
permissions, sync will just install it on another profile in the disabled state
assuming that all permissions have been granted. So I think we do have to
default to DISABLE_USER_ACTION here.

Filed crbug.com/122882

Powered by Google App Engine
This is Rietveld 408576698