Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
2647783003:
Reenable disabled component extensions with profile resetter. (Closed)
DescriptionReenable 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}
Committed: https://chromium.googlesource.com/chromium/src/+/622f70d2786a5dd446c45fa8222ce2d14d98cbf5
Patch Set 1 #Patch Set 2 : Limit to external components + add tests. #
Total comments: 4
Patch Set 3 : Cleanup. #Patch Set 4 : Fixed. #
Total comments: 8
Patch Set 5 : Fixed. #Patch Set 6 : Rebase. #
Messages
Total messages: 34 (22 generated)
The CQ bit was checked by mtomasz@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
Description was changed from
==========
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
==========
to
==========
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
==========
mtomasz@chromium.org changed reviewers: + battre@chromium.org, rdevlin.cronin@chromium.org
@battre: PTAL at chrome/browser/profile_resetter/* @rdevlin.cronin: PTAL at extension_service_unittest.cc Thanks.
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
lgtm https://codereview.chromium.org/2647783003/diff/20001/chrome/browser/extensio... File chrome/browser/extensions/extension_service_unittest.cc (right): https://codereview.chromium.org/2647783003/diff/20001/chrome/browser/extensio... chrome/browser/extensions/extension_service_unittest.cc:265: bool installed_by_default) { type and installed_by_default are not used. I suggest to drop them. https://codereview.chromium.org/2647783003/diff/20001/chrome/browser/extensio... chrome/browser/extensions/extension_service_unittest.cc:272: EXPECT_TRUE(extension.get() != NULL) << error; nullptr
Thanks! https://codereview.chromium.org/2647783003/diff/20001/chrome/browser/extensio... File chrome/browser/extensions/extension_service_unittest.cc (right): https://codereview.chromium.org/2647783003/diff/20001/chrome/browser/extensio... chrome/browser/extensions/extension_service_unittest.cc:265: bool installed_by_default) { On 2017/01/20 07:17:05, battre wrote: > type and installed_by_default are not used. I suggest to drop them. Done. https://codereview.chromium.org/2647783003/diff/20001/chrome/browser/extensio... chrome/browser/extensions/extension_service_unittest.cc:272: EXPECT_TRUE(extension.get() != NULL) << error; On 2017/01/20 07:17:05, battre wrote: > nullptr Done.
The CQ bit was checked by mtomasz@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: Try jobs failed on following builders: linux_chromium_chromeos_rel_ng on master.tryserver.chromium.linux (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
mtomasz@chromium.org changed reviewers: + lazyboy@chromium.org - rdevlin.cronin@chromium.org
@lazyboy: PTAL at extension_service_unittest.cc. Thanks.
https://codereview.chromium.org/2647783003/diff/60001/chrome/browser/extensio... File chrome/browser/extensions/extension_service_unittest.cc (right): https://codereview.chromium.org/2647783003/diff/60001/chrome/browser/extensio... chrome/browser/extensions/extension_service_unittest.cc:6424: // As for components, only non-external component extensions can be disabled. external instead of non-external? https://codereview.chromium.org/2647783003/diff/60001/chrome/browser/extensio... chrome/browser/extensions/extension_service_unittest.cc:6429: base::ScopedTempDir temp_dir; Do you need this? https://codereview.chromium.org/2647783003/diff/60001/chrome/browser/profile_... File chrome/browser/profile_resetter/profile_resetter.cc (right): https://codereview.chromium.org/2647783003/diff/60001/chrome/browser/profile_... chrome/browser/profile_resetter/profile_resetter.cc:286: std::vector<std::string> extension_ids_to_reenable; nit: new codes shoudl use ExtensionId instead of string (same thing). https://codereview.chromium.org/2647783003/diff/60001/chrome/browser/profile_... chrome/browser/profile_resetter/profile_resetter.cc:288: if (extension->location() == extensions::Manifest::EXTERNAL_COMPONENT) { nit: no {}
The CQ bit was checked by mtomasz@chromium.org to run a CQ dry run
https://codereview.chromium.org/2647783003/diff/60001/chrome/browser/extensio... File chrome/browser/extensions/extension_service_unittest.cc (right): https://codereview.chromium.org/2647783003/diff/60001/chrome/browser/extensio... chrome/browser/extensions/extension_service_unittest.cc:6424: // As for components, only non-external component extensions can be disabled. On 2017/01/24 03:30:01, lazyboy wrote: > external instead of non-external? Done. https://codereview.chromium.org/2647783003/diff/60001/chrome/browser/extensio... chrome/browser/extensions/extension_service_unittest.cc:6429: base::ScopedTempDir temp_dir; On 2017/01/24 03:30:01, lazyboy wrote: > Do you need this? Nope! Done. https://codereview.chromium.org/2647783003/diff/60001/chrome/browser/profile_... File chrome/browser/profile_resetter/profile_resetter.cc (right): https://codereview.chromium.org/2647783003/diff/60001/chrome/browser/profile_... chrome/browser/profile_resetter/profile_resetter.cc:286: std::vector<std::string> extension_ids_to_reenable; On 2017/01/24 03:30:01, lazyboy wrote: > nit: new codes shoudl use ExtensionId instead of string (same thing). Done. https://codereview.chromium.org/2647783003/diff/60001/chrome/browser/profile_... chrome/browser/profile_resetter/profile_resetter.cc:288: if (extension->location() == extensions::Manifest::EXTERNAL_COMPONENT) { On 2017/01/24 03:30:01, lazyboy wrote: > nit: no {} Done.
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
lgtm
The CQ bit was checked by mtomasz@chromium.org
The patchset sent to the CQ was uploaded after l-g-t-m from battre@chromium.org Link to the patchset: https://codereview.chromium.org/2647783003/#ps80001 (title: "Fixed.")
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Failed to apply patch for chrome/browser/profile_resetter/profile_resetter.cc:
While running git apply --index -p1;
error: patch failed: chrome/browser/profile_resetter/profile_resetter.cc:271
error: chrome/browser/profile_resetter/profile_resetter.cc: patch does not
apply
Patch: chrome/browser/profile_resetter/profile_resetter.cc
Index: chrome/browser/profile_resetter/profile_resetter.cc
diff --git a/chrome/browser/profile_resetter/profile_resetter.cc
b/chrome/browser/profile_resetter/profile_resetter.cc
index
d3e4360baf4398268c9f92efacd6aeddda1a95c9..869fc4fab73d0dde2150dd8548d2cbb7ca91e619
100644
--- a/chrome/browser/profile_resetter/profile_resetter.cc
+++ b/chrome/browser/profile_resetter/profile_resetter.cc
@@ -7,6 +7,7 @@
#include <stddef.h>
#include <string>
+#include <vector>
#include "base/macros.h"
#include "base/synchronization/cancellation_flag.h"
@@ -37,8 +38,11 @@
#include "components/search_engines/template_url_prepopulate_data.h"
#include "components/search_engines/template_url_service.h"
#include "content/public/browser/browser_thread.h"
+#include "extensions/browser/extension_registry.h"
#include "extensions/browser/extension_system.h"
#include "extensions/browser/management_policy.h"
+#include "extensions/common/extension_id.h"
+#include "extensions/common/manifest.h"
#if defined(OS_WIN)
#include "base/base_paths.h"
@@ -271,6 +275,24 @@ void ProfileResetter::ResetExtensions() {
DCHECK(extension_service);
extension_service->DisableUserExtensions(brandcode_extensions);
+ // Reenable all disabled external component extensions.
+ // BrandcodedDefaultSettings does not contain information about component
+ // extensions, so fetch them from the existing registry. This may be not very
+ // robust, as the profile resetter may be invoked when the registry is in
some
+ // iffy state. However, we can't enable an extension which is not in the
+ // registry anyway.
+ extensions::ExtensionRegistry* extension_registry =
+ extensions::ExtensionRegistry::Get(profile_);
+ DCHECK(extension_registry);
+ std::vector<extensions::ExtensionId> extension_ids_to_reenable;
+ for (const auto& extension : extension_registry->disabled_extensions()) {
+ if (extension->location() == extensions::Manifest::EXTERNAL_COMPONENT)
+ extension_ids_to_reenable.push_back(extension->id());
+ }
+ for (const auto& extension_id : extension_ids_to_reenable) {
+ extension_service->EnableExtension(extension_id);
+ }
+
MarkAsDone(EXTENSIONS);
}
The CQ bit was checked by mtomasz@chromium.org
The patchset sent to the CQ was uploaded after l-g-t-m from lazyboy@chromium.org, battre@chromium.org Link to the patchset: https://codereview.chromium.org/2647783003/#ps100001 (title: "Rebase.")
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
CQ is committing da patch.
Bot data: {"patchset_id": 100001, "attempt_start_ts": 1485482830604350,
"parent_rev": "6a5bbaea71e02af0b0bd60c7ba3ed61f77179b65", "commit_rev":
"622f70d2786a5dd446c45fa8222ce2d14d98cbf5"}
Message was sent while issue was closed.
Description was changed from
==========
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
==========
to
==========
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}
Committed:
https://chromium.googlesource.com/chromium/src/+/622f70d2786a5dd446c45fa8222c...
==========
Message was sent while issue was closed.
Committed patchset #6 (id:100001) as https://chromium.googlesource.com/chromium/src/+/622f70d2786a5dd446c45fa8222c...
Message was sent while issue was closed.
A revert of this CL (patchset #6 id:100001) has been created in https://codereview.chromium.org/2974033002/ by mtomasz@chromium.org. The reason for reverting is: It didn't work as expected, so removing dead code.. |
|||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
