New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 680429 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Last visit > 30 days ago
Closed: Jan 2017
Components:
EstimatedDays: ----
NextAction: ----
OS: Linux
Pri: 1
Type: Bug



Sign in to add a comment

Restore disabled component extensions in the profile resetter.

Project Member Reported by mtomasz@chromium.org, Jan 12 2017

Issue description

If a bug happens and they are disabled, then there is no way to enable them again, as components and external components do not have UI for enabling them.
 
We can't really do that as some external component extensions may be removed. This is actually true only for the Hotword extension. Disabling hotword would disable the external component Hotword extension.

We ended up on very convoluted spaghetti code for managing state of external component extensions. Some component extensions can be disabled, some not, some can be reenabled, some cannot.
Project Member

Comment 2 by bugdroid1@chromium.org, Jan 24 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/294bb3e342259c73d2abddd5b759104d34854b17

commit 294bb3e342259c73d2abddd5b759104d34854b17
Author: mtomasz <mtomasz@chromium.org>
Date: Tue Jan 24 02:17:52 2017

Rename confusing method name.

ExtensionsService::DisableUserExtensions(ids) doesn't disable the passed
extensions, but all user extensions except the passed ids.

It's very hard to notice that from the caller side, and the actual
code does completely opposite.

This CL renames this confusing method name.

BUG= 680429 
TEST=Refactoring only.

Review-Url: https://codereview.chromium.org/2646803003
Cr-Commit-Position: refs/heads/master@{#445603}

[modify] https://crrev.com/294bb3e342259c73d2abddd5b759104d34854b17/chrome/browser/extensions/extension_service.cc
[modify] https://crrev.com/294bb3e342259c73d2abddd5b759104d34854b17/chrome/browser/extensions/extension_service.h
[modify] https://crrev.com/294bb3e342259c73d2abddd5b759104d34854b17/chrome/browser/profile_resetter/profile_resetter.cc

Components: Platform>Extensions
Summary: Restore disabled component extensions in the profile resetter. (was: Remove component and external component extensions from syncable preferences.)
Status: Started (was: Assigned)
Project Member

Comment 5 by bugdroid1@chromium.org, Jan 27 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/79fc3e73cfaa2b0b3b51d9e0176f5463cc7bb565

commit 79fc3e73cfaa2b0b3b51d9e0176f5463cc7bb565
Author: mtomasz <mtomasz@chromium.org>
Date: Fri Jan 27 02:34:21 2017

Simplify managing visibility of extensions in settings.

Summary of behavior:
- chrome.management API will expose everything which is not component
  including themes and hosted apps (no change). It will never expose
  components, even with --show-component-extensions-options.
- chrome://extensions will show the same as before this CL, so no
  themes, sometimes hosted documents (only if unpacked), etc (no
  change). What changed is that we will show all components in
  chrome://extensions if --show-component-extensions-options is passed.
  Before we would only show those not hidden from the app launcher
  (sic).

We have too many methods for determining visibility, and they were
contradicting each other. Eg. ShouldNotBeVisible() which is documented
as whether it should be visible *anywhere* can return true, while
ShouldDisplayInAppLauncher could return false in the same time.

In different places we use different of the above methods causing
inconsistent behavior, eg. component extensions would be shown
in chrome://extensions when --show-component-extensions-options is
used, but excluding component extensions which are hidden from
launcher.

Finally, the logic in methods for determining visibility was
too complex, and as a result it was difficult to say whether
something will be visible in some parts of Chrome UI or not.

This CL removes the umbrella ShouldNotBeVisible() method, and introduces
ShouldExposeInManagementAPI.

Also, callers have been updated to use either
ShouldDisplayInExtensionSettings or ShouldDisplayInAppLancher or
ShouldDisplayInNewTabPage or ShouldExposeInManagementAPI depending on
what is the caller.

Ideally, all of the Should* should rather be moved to the callers, as
the callers better know what to display or show, rather than the
Extension class. However, for consistency ShouldExposeInManagementAPI
was added.

Finally, this CL simplifies the logic for determining visibility
by assuming that by default everything is visible, and filtering
out things which we want to hide (no more return true/false every
second line).

BUG= 680429 
TEST=Confirm that all component extensions are visible in
     chrome://extensions when --show-component-extensions-options is
     passed. Also, confirm there are no regressions, so no component
     extensions shown in chrome://extensions by default. Also,
     confirm that chrome.management API doesn't regress by using
     the "Chrome Apps & Extensions Developer Tool".
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation

Review-Url: https://codereview.chromium.org/2645863004
Cr-Commit-Position: refs/heads/master@{#446543}

[modify] https://crrev.com/79fc3e73cfaa2b0b3b51d9e0176f5463cc7bb565/chrome/browser/extensions/extension_ui_util.cc
[modify] https://crrev.com/79fc3e73cfaa2b0b3b51d9e0176f5463cc7bb565/chrome/browser/extensions/extension_ui_util.h
[modify] https://crrev.com/79fc3e73cfaa2b0b3b51d9e0176f5463cc7bb565/extensions/browser/api/management/management_api.cc
[modify] https://crrev.com/79fc3e73cfaa2b0b3b51d9e0176f5463cc7bb565/extensions/common/extension.cc
[modify] https://crrev.com/79fc3e73cfaa2b0b3b51d9e0176f5463cc7bb565/extensions/common/extension.h

Project Member

Comment 6 by bugdroid1@chromium.org, Jan 27 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/622f70d2786a5dd446c45fa8222ce2d14d98cbf5

commit 622f70d2786a5dd446c45fa8222ce2d14d98cbf5
Author: mtomasz <mtomasz@chromium.org>
Date: Fri Jan 27 03:36:26 2017

Reenable disabled component extensions with profile resetter.

Sice disabled extensions are synced and component extensions do not
have a UI for reenabling them, once disabled will stay disabled forever.

It happened that for some reason some component extensions were
disabled. The reason is unknown, but at least users will be able to
recover from this situation by using the profile resetter.

Note, that we allow to disable component extensions programatically,
which was used at least for the Hotword component extension.

TEST=Disable a component extension programatically, then reset the
     profile in chrome://settings.
BUG= 680429 

Review-Url: https://codereview.chromium.org/2647783003
Cr-Commit-Position: refs/heads/master@{#446575}

[modify] https://crrev.com/622f70d2786a5dd446c45fa8222ce2d14d98cbf5/chrome/browser/extensions/extension_service_unittest.cc
[modify] https://crrev.com/622f70d2786a5dd446c45fa8222ce2d14d98cbf5/chrome/browser/profile_resetter/profile_resetter.cc
[modify] https://crrev.com/622f70d2786a5dd446c45fa8222ce2d14d98cbf5/chrome/browser/profile_resetter/profile_resetter_unittest.cc

Status: Fixed (was: Started)

Sign in to add a comment