From aac30ecbd611a0929c42347011549dcb0a7d9c41 Mon Sep 17 00:00:00 2001 From: treib Date: Wed, 10 Jun 2015 02:18:09 -0700 Subject: [PATCH] Remove ExtensionPrefs::SetDidExtensionEscalatePermissions. The "DidExtensionEscalatePermissions" flag is basically equivalent to HasDisableReason(DISABLE_PERMISSIONS_INCREASE | DISABLE_REMOTE_INSTALL), so check that instead of storing a separate pref. BUG=493610 Review URL: https://codereview.chromium.org/1164603003 Cr-Commit-Position: refs/heads/master@{#333706} --- chrome/browser/apps/ephemeral_app_browsertest.cc | 7 ------- chrome/browser/apps/ephemeral_app_launcher_browsertest.cc | 7 ------- chrome/browser/extensions/extension_prefs_unittest.cc | 3 ++- chrome/browser/extensions/extension_service.cc | 15 +++++---------- chrome/browser/extensions/extension_service_unittest.cc | 6 ++---- extensions/browser/extension_prefs.cc | 14 +++----------- extensions/browser/extension_prefs.h | 6 ------ 7 files changed, 12 insertions(+), 46 deletions(-) diff --git a/chrome/browser/apps/ephemeral_app_browsertest.cc b/chrome/browser/apps/ephemeral_app_browsertest.cc index 07e01b5707ea..de35c13fde5b 100644 --- a/chrome/browser/apps/ephemeral_app_browsertest.cc +++ b/chrome/browser/apps/ephemeral_app_browsertest.cc @@ -293,13 +293,6 @@ void EphemeralAppTestBase::PromoteEphemeralApp( void EphemeralAppTestBase::DisableEphemeralApp( const Extension* app, Extension::DisableReason disable_reason) { - ExtensionPrefs* prefs = ExtensionPrefs::Get(profile()); - - // Disabling due to a permissions increase also involves setting the - // DidExtensionEscalatePermissions flag. - if (disable_reason == Extension::DISABLE_PERMISSIONS_INCREASE) - prefs->SetDidExtensionEscalatePermissions(app, true); - ExtensionSystem::Get(profile())->extension_service()->DisableExtension( app->id(), disable_reason); diff --git a/chrome/browser/apps/ephemeral_app_launcher_browsertest.cc b/chrome/browser/apps/ephemeral_app_launcher_browsertest.cc index 5affa8a469e3..63e6bc966862 100644 --- a/chrome/browser/apps/ephemeral_app_launcher_browsertest.cc +++ b/chrome/browser/apps/ephemeral_app_launcher_browsertest.cc @@ -287,13 +287,6 @@ class EphemeralAppLauncherTest : public WebstoreInstallerTest { ExtensionSystem::Get(profile())->extension_service(); service->DisableExtension(app->id(), disable_reason); - if (disable_reason == Extension::DISABLE_PERMISSIONS_INCREASE) { - // When an extension is disabled due to a permissions increase, this - // flag needs to be set too, for some reason. - ExtensionPrefs::Get(profile()) - ->SetDidExtensionEscalatePermissions(app, true); - } - EXPECT_TRUE( ExtensionRegistry::Get(profile())->disabled_extensions().Contains( app->id())); diff --git a/chrome/browser/extensions/extension_prefs_unittest.cc b/chrome/browser/extensions/extension_prefs_unittest.cc index 9b2b74e0d43c..61d8093ce3c1 100644 --- a/chrome/browser/extensions/extension_prefs_unittest.cc +++ b/chrome/browser/extensions/extension_prefs_unittest.cc @@ -142,7 +142,8 @@ class ExtensionPrefsEscalatePermissions : public ExtensionPrefsTest { public: void Initialize() override { extension = prefs_.AddExtension("test"); - prefs()->SetDidExtensionEscalatePermissions(extension.get(), true); + prefs()->AddDisableReason(extension->id(), + Extension::DISABLE_PERMISSIONS_INCREASE); } void Verify() override { diff --git a/chrome/browser/extensions/extension_service.cc b/chrome/browser/extensions/extension_service.cc index 77e29c867c89..b9cdc1849a12 100644 --- a/chrome/browser/extensions/extension_service.cc +++ b/chrome/browser/extensions/extension_service.cc @@ -1002,7 +1002,6 @@ void ExtensionService::GrantPermissionsAndEnableExtension( const Extension* extension) { GrantPermissions(extension); RecordPermissionMessagesHistogram(extension, "ReEnable"); - extension_prefs_->SetDidExtensionEscalatePermissions(extension, false); EnableExtension(extension->id()); } @@ -1682,17 +1681,14 @@ void ExtensionService::CheckPermissionsIncrease(const Extension* extension, // Extension has changed permissions significantly. Disable it. A // notification should be sent by the caller. If the extension is already // disabled because it was installed remotely, don't add another disable - // reason, but instead always set the "did escalate permissions" flag, to - // ensure enabling it will always show a warning. - if (disable_reasons == Extension::DISABLE_REMOTE_INSTALL) { - extension_prefs_->SetDidExtensionEscalatePermissions(extension, true); - } else if (is_privilege_increase) { + // reason. + if (is_privilege_increase && + disable_reasons != Extension::DISABLE_REMOTE_INSTALL) { disable_reasons |= Extension::DISABLE_PERMISSIONS_INCREASE; if (!extension_prefs_->DidExtensionEscalatePermissions(extension->id())) { RecordPermissionMessagesHistogram(extension, "AutoDisable"); } extension_prefs_->SetExtensionState(extension->id(), Extension::DISABLED); - extension_prefs_->SetDidExtensionEscalatePermissions(extension, true); #if defined(ENABLE_SUPERVISED_USERS) // If a custodian-installed extension is disabled for a supervised user due @@ -1799,7 +1795,7 @@ void ExtensionService::OnExtensionInstalled( // Installation of a blacklisted extension can happen from sync, policy, // etc, where to maintain consistency we need to install it, just never // load it (see AddExtension). Usually it should be the job of callers to - // incercept blacklisted extension earlier (e.g. CrxInstaller, before even + // intercept blacklisted extensions earlier (e.g. CrxInstaller, before even // showing the install dialogue). extension_prefs_->AcknowledgeBlacklistedExtension(id); UMA_HISTOGRAM_ENUMERATION("ExtensionBlacklist.SilentInstall", @@ -1827,8 +1823,7 @@ void ExtensionService::OnExtensionInstalled( } if (disable_reasons) - extension_prefs_->AddDisableReason(id, - static_cast(disable_reasons)); + extension_prefs_->AddDisableReasons(id, disable_reasons); const Extension::State initial_state = disable_reasons == Extension::DISABLE_NONE ? Extension::ENABLED diff --git a/chrome/browser/extensions/extension_service_unittest.cc b/chrome/browser/extensions/extension_service_unittest.cc index 07ab1658cf38..919f925b850c 100644 --- a/chrome/browser/extensions/extension_service_unittest.cc +++ b/chrome/browser/extensions/extension_service_unittest.cc @@ -2760,8 +2760,6 @@ TEST_F(ExtensionServiceTest, UpdateExtensionPreservesState) { // over to the updated version. service()->DisableExtension(good->id(), Extension::DISABLE_USER_ACTION); extensions::util::SetIsIncognitoEnabled(good->id(), profile(), true); - ExtensionPrefs::Get(profile()) - ->SetDidExtensionEscalatePermissions(good, true); path = data_dir().AppendASCII("good2.crx"); UpdateExtension(good_crx, path, INSTALLED); @@ -2769,8 +2767,8 @@ TEST_F(ExtensionServiceTest, UpdateExtensionPreservesState) { const Extension* good2 = service()->GetExtensionById(good_crx, true); ASSERT_EQ("1.0.0.1", good2->version()->GetString()); EXPECT_TRUE(extensions::util::IsIncognitoEnabled(good2->id(), profile())); - EXPECT_TRUE(ExtensionPrefs::Get(profile()) - ->DidExtensionEscalatePermissions(good2->id())); + EXPECT_EQ(Extension::DISABLE_USER_ACTION, + ExtensionPrefs::Get(profile())->GetDisableReasons(good2->id())); } // Tests that updating preserves extension location. diff --git a/extensions/browser/extension_prefs.cc b/extensions/browser/extension_prefs.cc index 460e0de7297f..79bd54725553 100644 --- a/extensions/browser/extension_prefs.cc +++ b/extensions/browser/extension_prefs.cc @@ -83,9 +83,6 @@ const char kPrefBlacklistAcknowledged[] = "ack_blacklist"; // run of this profile. const char kPrefExternalInstallFirstRun[] = "external_first_run"; -// Indicates whether to show an install warning when the user enables. -const char kExtensionDidEscalatePermissions[] = "install_warning_on_enable"; - // DO NOT USE, use kPrefDisableReasons instead. // Indicates whether the extension was updated while it was disabled. const char kDeprecatedPrefDisableReason[] = "disable_reason"; @@ -729,14 +726,9 @@ bool ExtensionPrefs::SetAlertSystemFirstRun() { bool ExtensionPrefs::DidExtensionEscalatePermissions( const std::string& extension_id) const { - return ReadPrefAsBooleanAndReturn(extension_id, - kExtensionDidEscalatePermissions); -} - -void ExtensionPrefs::SetDidExtensionEscalatePermissions( - const Extension* extension, bool did_escalate) { - UpdateExtensionPref(extension->id(), kExtensionDidEscalatePermissions, - new base::FundamentalValue(did_escalate)); + return HasDisableReason(extension_id, + Extension::DISABLE_PERMISSIONS_INCREASE) || + HasDisableReason(extension_id, Extension::DISABLE_REMOTE_INSTALL); } int ExtensionPrefs::GetDisableReasons(const std::string& extension_id) const { diff --git a/extensions/browser/extension_prefs.h b/extensions/browser/extension_prefs.h index 9cd810e7ea6a..80507895954f 100644 --- a/extensions/browser/extension_prefs.h +++ b/extensions/browser/extension_prefs.h @@ -251,12 +251,6 @@ class ExtensionPrefs : public ExtensionScopedPrefs, public KeyedService { // Did the extension ask to escalate its permission during an upgrade? bool DidExtensionEscalatePermissions(const std::string& id) const; - // If |did_escalate| is true, the preferences for |extension| will be set to - // require the install warning when the user tries to enable. - void SetDidExtensionEscalatePermissions( - const Extension* extension, - bool did_escalate); - // Getters and setters for disabled reason. int GetDisableReasons(const std::string& extension_id) const; bool HasDisableReason(const std::string& extension_id, -- 2.11.4.GIT