diff options
10 files changed, 443 insertions, 202 deletions
diff --git a/chrome/browser/extensions/api/settings_overrides/settings_overrides_api.cc b/chrome/browser/extensions/api/settings_overrides/settings_overrides_api.cc index 73a3d60..fe31090 100644 --- a/chrome/browser/extensions/api/settings_overrides/settings_overrides_api.cc +++ b/chrome/browser/extensions/api/settings_overrides/settings_overrides_api.cc @@ -111,16 +111,21 @@ SettingsOverridesAPI::GetFactoryInstance() { void SettingsOverridesAPI::SetPref(const std::string& extension_id, const std::string& pref_key, base::Value* value) { - PreferenceAPI::Get(profile_)->SetExtensionControlledPref( - extension_id, - pref_key, - kExtensionPrefsScopeRegular, - value); + PreferenceAPI* prefs = PreferenceAPI::Get(profile_); + if (!prefs) + return; // Expected in unit tests. + prefs->SetExtensionControlledPref(extension_id, + pref_key, + kExtensionPrefsScopeRegular, + value); } void SettingsOverridesAPI::UnsetPref(const std::string& extension_id, const std::string& pref_key) { - PreferenceAPI::Get(profile_)->RemoveExtensionControlledPref( + PreferenceAPI* prefs = PreferenceAPI::Get(profile_); + if (!prefs) + return; // Expected in unit tests. + prefs->RemoveExtensionControlledPref( extension_id, pref_key, kExtensionPrefsScopeRegular); diff --git a/chrome/browser/extensions/extension_message_bubble_controller_unittest.cc b/chrome/browser/extensions/extension_message_bubble_controller_unittest.cc index 656ae77..f84e31a 100644 --- a/chrome/browser/extensions/extension_message_bubble_controller_unittest.cc +++ b/chrome/browser/extensions/extension_message_bubble_controller_unittest.cc @@ -9,13 +9,23 @@ #include "chrome/browser/extensions/extension_function_test_utils.h" #include "chrome/browser/extensions/extension_message_bubble.h" #include "chrome/browser/extensions/extension_service.h" +#include "chrome/browser/extensions/settings_api_bubble_controller.h" #include "chrome/browser/extensions/suspicious_extension_bubble_controller.h" #include "chrome/browser/extensions/test_extension_system.h" #include "chrome/test/base/testing_profile.h" #include "content/public/test/test_browser_thread_bundle.h" #include "extensions/common/extension.h" +#include "extensions/common/extension_builder.h" #include "extensions/common/feature_switch.h" +namespace { + +const char kId1[] = "iccfkkhkfiphcjdakkmcjmkfboccmndk"; +const char kId2[] = "ajjhifimiemdpmophmkkkcijegphclbl"; +const char kId3[] = "ioibbbfddncmmabjmpokikkeiofalaek"; + +} // namespace + namespace extensions { class TestDelegate { @@ -97,6 +107,30 @@ class TestDevModeBubbleController } }; +// A test class for the SettingsApiBubbleController. +class TestSettingsApiBubbleController : public SettingsApiBubbleController, + public TestDelegate { + public: + TestSettingsApiBubbleController(Profile* profile, + SettingsApiOverrideType type) + : SettingsApiBubbleController(profile, type) {} + + virtual void OnBubbleAction() OVERRIDE { + ++action_button_callback_count_; + SettingsApiBubbleController::OnBubbleAction(); + } + + virtual void OnBubbleDismiss() OVERRIDE { + ++dismiss_button_callback_count_; + SettingsApiBubbleController::OnBubbleDismiss(); + } + + virtual void OnLinkClicked() OVERRIDE { + ++link_click_callback_count_; + SettingsApiBubbleController::OnLinkClicked(); + } +}; + // A fake bubble used for testing the controller. Takes an action that specifies // what should happen when the bubble is "shown" (the bubble is actually not // shown, the corresponding action is taken immediately). @@ -145,12 +179,76 @@ class FakeExtensionMessageBubble : public ExtensionMessageBubble { class ExtensionMessageBubbleTest : public testing::Test { public: - ExtensionMessageBubbleTest() { + ExtensionMessageBubbleTest() {} + + void LoadGenericExtension(const std::string& index, + const std::string& id, + Manifest::Location location) { + extensions::ExtensionBuilder builder; + builder.SetManifest(extensions::DictionaryBuilder() + .Set("name", std::string("Extension " + index)) + .Set("version", "1.0") + .Set("manifest_version", 2)); + builder.SetLocation(location); + builder.SetID(id); + service_->AddExtension(builder.Build().get()); + } + + void LoadExtensionWithAction(const std::string& index, + const std::string& id, + Manifest::Location location) { + extensions::ExtensionBuilder builder; + builder.SetManifest(extensions::DictionaryBuilder() + .Set("name", std::string("Extension " + index)) + .Set("version", "1.0") + .Set("manifest_version", 2) + .Set("browser_action", + extensions::DictionaryBuilder().Set( + "default_title", "Default title"))); + builder.SetLocation(location); + builder.SetID(id); + service_->AddExtension(builder.Build().get()); + } + + void LoadExtensionOverridingHome(const std::string& index, + const std::string& id, + Manifest::Location location) { + extensions::ExtensionBuilder builder; + builder.SetManifest(extensions::DictionaryBuilder() + .Set("name", std::string("Extension " + index)) + .Set("version", "1.0") + .Set("manifest_version", 2) + .Set("chrome_settings_overrides", + extensions::DictionaryBuilder().Set( + "homepage", "http://www.google.com"))); + builder.SetLocation(location); + builder.SetID(id); + service_->AddExtension(builder.Build().get()); + } + + void LoadExtensionOverridingStart(const std::string& index, + const std::string& id, + Manifest::Location location) { + extensions::ExtensionBuilder builder; + builder.SetManifest(extensions::DictionaryBuilder() + .Set("name", std::string("Extension " + index)) + .Set("version", "1.0") + .Set("manifest_version", 2) + .Set("chrome_settings_overrides", + extensions::DictionaryBuilder().Set( + "startup_pages", + extensions::ListBuilder().Append( + "http://www.google.com")))); + builder.SetLocation(location); + builder.SetID(id); + service_->AddExtension(builder.Build().get()); + } + + void Init() { // The two lines of magical incantation required to get the extension // service to work inside a unit test and access the extension prefs. thread_bundle_.reset(new content::TestBrowserThreadBundle); profile_.reset(new TestingProfile); - static_cast<TestExtensionSystem*>( ExtensionSystem::Get(profile()))->CreateExtensionService( CommandLine::ForCurrentProcess(), @@ -158,49 +256,8 @@ class ExtensionMessageBubbleTest : public testing::Test { false); service_ = profile_->GetExtensionService(); service_->Init(); - - std::string basic_extension = - "{\"name\": \"Extension #\"," - "\"version\": \"1.0\"," - "\"manifest_version\": 2}"; - std::string basic_extension_with_action = - "{\"name\": \"Extension #\"," - "\"version\": \"1.0\"," - "\"browser_action\": {" - " \"default_title\": \"Default title\"" - "}," - "\"manifest_version\": 2}"; - - std::string extension_data; - base::ReplaceChars(basic_extension_with_action, "#", "1", &extension_data); - scoped_refptr<Extension> my_test_extension1( - CreateExtension( - Manifest::COMMAND_LINE, - extension_data, - "Autogenerated 1")); - - base::ReplaceChars(basic_extension, "#", "2", &extension_data); - scoped_refptr<Extension> my_test_extension2( - CreateExtension( - Manifest::UNPACKED, - extension_data, - "Autogenerated 2")); - - base::ReplaceChars(basic_extension, "#", "3", &extension_data); - scoped_refptr<Extension> regular_extension( - CreateExtension( - Manifest::EXTERNAL_POLICY, - extension_data, - "Autogenerated 3")); - - extension_id1_ = my_test_extension1->id(); - extension_id2_ = my_test_extension2->id(); - extension_id3_ = regular_extension->id(); - - service_->AddExtension(regular_extension); - service_->AddExtension(my_test_extension1); - service_->AddExtension(my_test_extension2); } + virtual ~ExtensionMessageBubbleTest() { // Make sure the profile is destroyed before the thread bundle. profile_.reset(NULL); @@ -226,9 +283,6 @@ class ExtensionMessageBubbleTest : public testing::Test { } ExtensionService* service_; - std::string extension_id1_; - std::string extension_id2_; - std::string extension_id3_; private: scoped_ptr<CommandLine> command_line_; @@ -246,8 +300,13 @@ class ExtensionMessageBubbleTest : public testing::Test { #endif TEST_F(ExtensionMessageBubbleTest, MAYBE_WipeoutControllerTest) { - // The test base class adds three extensions, and we control two of them in - // this test (ids are: extension_id1_ and extension_id2_). + Init(); + // Add three extensions, and control two of them in this test (extension 1 + // and 2). + LoadExtensionWithAction("1", kId1, Manifest::COMMAND_LINE); + LoadGenericExtension("2", kId2, Manifest::UNPACKED); + LoadGenericExtension("3", kId3, Manifest::EXTERNAL_POLICY); + scoped_ptr<TestSuspiciousExtensionBubbleController> controller( new TestSuspiciousExtensionBubbleController(profile())); FakeExtensionMessageBubble bubble; @@ -256,8 +315,8 @@ TEST_F(ExtensionMessageBubbleTest, MAYBE_WipeoutControllerTest) { // Validate that we don't have a suppress value for the extensions. ExtensionPrefs* prefs = ExtensionPrefs::Get(profile()); - EXPECT_FALSE(prefs->HasWipeoutBeenAcknowledged(extension_id1_)); - EXPECT_FALSE(prefs->HasWipeoutBeenAcknowledged(extension_id2_)); + EXPECT_FALSE(prefs->HasWipeoutBeenAcknowledged(kId1)); + EXPECT_FALSE(prefs->HasWipeoutBeenAcknowledged(kId2)); EXPECT_FALSE(controller->ShouldShow()); std::vector<base::string16> suspicious_extensions = @@ -267,11 +326,10 @@ TEST_F(ExtensionMessageBubbleTest, MAYBE_WipeoutControllerTest) { EXPECT_EQ(0U, controller->dismiss_click_count()); // Now disable an extension, specifying the wipeout flag. - service_->DisableExtension(extension_id1_, - Extension::DISABLE_NOT_VERIFIED); + service_->DisableExtension(kId1, Extension::DISABLE_NOT_VERIFIED); - EXPECT_FALSE(prefs->HasWipeoutBeenAcknowledged(extension_id1_)); - EXPECT_FALSE(prefs->HasWipeoutBeenAcknowledged(extension_id2_)); + EXPECT_FALSE(prefs->HasWipeoutBeenAcknowledged(kId1)); + EXPECT_FALSE(prefs->HasWipeoutBeenAcknowledged(kId2)); controller.reset(new TestSuspiciousExtensionBubbleController( profile())); SuspiciousExtensionBubbleController::ClearProfileListForTesting(); @@ -283,15 +341,14 @@ TEST_F(ExtensionMessageBubbleTest, MAYBE_WipeoutControllerTest) { EXPECT_EQ(0U, controller->link_click_count()); EXPECT_EQ(1U, controller->dismiss_click_count()); // Now the acknowledge flag should be set only for the first extension. - EXPECT_TRUE(prefs->HasWipeoutBeenAcknowledged(extension_id1_)); - EXPECT_FALSE(prefs->HasWipeoutBeenAcknowledged(extension_id2_)); + EXPECT_TRUE(prefs->HasWipeoutBeenAcknowledged(kId1)); + EXPECT_FALSE(prefs->HasWipeoutBeenAcknowledged(kId2)); // Clear the flag. - prefs->SetWipeoutAcknowledged(extension_id1_, false); - EXPECT_FALSE(prefs->HasWipeoutBeenAcknowledged(extension_id1_)); + prefs->SetWipeoutAcknowledged(kId1, false); + EXPECT_FALSE(prefs->HasWipeoutBeenAcknowledged(kId1)); // Now disable the other extension and exercise the link click code path. - service_->DisableExtension(extension_id2_, - Extension::DISABLE_NOT_VERIFIED); + service_->DisableExtension(kId2, Extension::DISABLE_NOT_VERIFIED); bubble.set_action_on_show( FakeExtensionMessageBubble::BUBBLE_ACTION_CLICK_LINK); @@ -306,7 +363,7 @@ TEST_F(ExtensionMessageBubbleTest, MAYBE_WipeoutControllerTest) { controller->Show(&bubble); // Simulate showing the bubble. EXPECT_EQ(1U, controller->link_click_count()); EXPECT_EQ(0U, controller->dismiss_click_count()); - EXPECT_TRUE(prefs->HasWipeoutBeenAcknowledged(extension_id1_)); + EXPECT_TRUE(prefs->HasWipeoutBeenAcknowledged(kId1)); } // The feature this is meant to test is only implemented on Windows. @@ -319,10 +376,14 @@ TEST_F(ExtensionMessageBubbleTest, MAYBE_WipeoutControllerTest) { TEST_F(ExtensionMessageBubbleTest, MAYBE_DevModeControllerTest) { FeatureSwitch::ScopedOverride force_dev_mode_highlighting( FeatureSwitch::force_dev_mode_highlighting(), true); - // The test base class adds three extensions, and we control two of them in - // this test (ids are: extension_id1_ and extension_id2_). Extension 1 is a - // regular extension, Extension 2 is UNPACKED so it counts as a DevMode - // extension. + Init(); + // Add three extensions, and control two of them in this test (extension 1 + // and 2). Extension 1 is a regular extension, Extension 2 is UNPACKED so it + // counts as a DevMode extension. + LoadExtensionWithAction("1", kId1, Manifest::COMMAND_LINE); + LoadGenericExtension("2", kId2, Manifest::UNPACKED); + LoadGenericExtension("3", kId3, Manifest::EXTERNAL_POLICY); + scoped_ptr<TestDevModeBubbleController> controller( new TestDevModeBubbleController(profile())); @@ -345,8 +406,8 @@ TEST_F(ExtensionMessageBubbleTest, MAYBE_DevModeControllerTest) { EXPECT_EQ(0U, controller->link_click_count()); EXPECT_EQ(0U, controller->action_click_count()); EXPECT_EQ(1U, controller->dismiss_click_count()); - EXPECT_TRUE(service_->GetExtensionById(extension_id1_, false) != NULL); - EXPECT_TRUE(service_->GetExtensionById(extension_id2_, false) != NULL); + EXPECT_TRUE(service_->GetExtensionById(kId1, false) != NULL); + EXPECT_TRUE(service_->GetExtensionById(kId2, false) != NULL); // Do it again, but now press different button (Disable). bubble.set_action_on_show( @@ -361,12 +422,12 @@ TEST_F(ExtensionMessageBubbleTest, MAYBE_DevModeControllerTest) { EXPECT_EQ(0U, controller->link_click_count()); EXPECT_EQ(1U, controller->action_click_count()); EXPECT_EQ(0U, controller->dismiss_click_count()); - EXPECT_TRUE(service_->GetExtensionById(extension_id1_, false) == NULL); - EXPECT_TRUE(service_->GetExtensionById(extension_id2_, false) == NULL); + EXPECT_TRUE(service_->GetExtensionById(kId1, false) == NULL); + EXPECT_TRUE(service_->GetExtensionById(kId2, false) == NULL); // Re-enable the extensions (disabled by the action button above). - service_->EnableExtension(extension_id1_); - service_->EnableExtension(extension_id2_); + service_->EnableExtension(kId1); + service_->EnableExtension(kId2); // Show the dialog a third time, but now press the learn more link. bubble.set_action_on_show( @@ -381,12 +442,12 @@ TEST_F(ExtensionMessageBubbleTest, MAYBE_DevModeControllerTest) { EXPECT_EQ(1U, controller->link_click_count()); EXPECT_EQ(0U, controller->action_click_count()); EXPECT_EQ(0U, controller->dismiss_click_count()); - EXPECT_TRUE(service_->GetExtensionById(extension_id1_, false) != NULL); - EXPECT_TRUE(service_->GetExtensionById(extension_id2_, false) != NULL); + EXPECT_TRUE(service_->GetExtensionById(kId1, false) != NULL); + EXPECT_TRUE(service_->GetExtensionById(kId2, false) != NULL); // Now disable the unpacked extension. - service_->DisableExtension(extension_id1_, Extension::DISABLE_USER_ACTION); - service_->DisableExtension(extension_id2_, Extension::DISABLE_USER_ACTION); + service_->DisableExtension(kId1, Extension::DISABLE_USER_ACTION); + service_->DisableExtension(kId2, Extension::DISABLE_USER_ACTION); controller.reset(new TestDevModeBubbleController( profile())); @@ -396,4 +457,128 @@ TEST_F(ExtensionMessageBubbleTest, MAYBE_DevModeControllerTest) { EXPECT_EQ(0U, dev_mode_extensions.size()); } +// The feature this is meant to test is only implemented on Windows. +#if defined(OS_WIN) +#define MAYBE_SettingsApiControllerTest SettingsApiControllerTest +#else +#define MAYBE_SettingsApiControllerTest DISABLED_SettingsApiControllerTest +#endif + +TEST_F(ExtensionMessageBubbleTest, MAYBE_SettingsApiControllerTest) { + Init(); + extensions::ExtensionPrefs* prefs = + extensions::ExtensionPrefs::Get(profile()); + + for (int i = 0; i < 3; ++i) { + switch (static_cast<SettingsApiOverrideType>(i)) { + case BUBBLE_TYPE_HOME_PAGE: + // Load two extensions overriding home page and one overriding something + // unrelated (to check for interference). Extension 2 should still win + // on the home page setting. + LoadExtensionOverridingHome("1", kId1, Manifest::UNPACKED); + LoadExtensionOverridingHome("2", kId2, Manifest::UNPACKED); + LoadExtensionOverridingStart("3", kId3, Manifest::UNPACKED); + break; + case BUBBLE_TYPE_SEARCH_ENGINE: + // We deliberately skip testing the search engine since it relies on + // TemplateURLServiceFactory that isn't available while unit testing. + // This test is only simulating the bubble interaction with the user and + // that is more or less the same for the search engine as it is for the + // others. + continue; + case BUBBLE_TYPE_STARTUP_PAGES: + // Load two extensions overriding start page and one overriding + // something unrelated (to check for interference). Extension 2 should + // still win on the startup page setting. + LoadExtensionOverridingStart("1", kId1, Manifest::UNPACKED); + LoadExtensionOverridingStart("2", kId2, Manifest::UNPACKED); + LoadExtensionOverridingHome("3", kId3, Manifest::UNPACKED); + break; + default: + NOTREACHED(); + break; + } + + scoped_ptr<TestSettingsApiBubbleController> controller( + new TestSettingsApiBubbleController( + profile(), static_cast<SettingsApiOverrideType>(i))); + + // The list will contain one enabled unpacked extension (ext 2). + EXPECT_TRUE(controller->ShouldShow(kId2)); + std::vector<base::string16> override_extensions = + controller->GetExtensionList(); + ASSERT_EQ(1U, override_extensions.size()); + EXPECT_TRUE(base::ASCIIToUTF16("Extension 2") == + override_extensions[0].c_str()); + EXPECT_EQ(0U, controller->link_click_count()); + EXPECT_EQ(0U, controller->dismiss_click_count()); + EXPECT_EQ(0U, controller->action_click_count()); + + // Simulate showing the bubble and dismissing it. + FakeExtensionMessageBubble bubble; + bubble.set_action_on_show( + FakeExtensionMessageBubble::BUBBLE_ACTION_CLICK_DISMISS_BUTTON); + controller->Show(&bubble); + EXPECT_EQ(0U, controller->link_click_count()); + EXPECT_EQ(0U, controller->action_click_count()); + EXPECT_EQ(1U, controller->dismiss_click_count()); + // No extension should have become disabled. + EXPECT_TRUE(service_->GetExtensionById(kId1, false) != NULL); + EXPECT_TRUE(service_->GetExtensionById(kId2, false) != NULL); + EXPECT_TRUE(service_->GetExtensionById(kId3, false) != NULL); + // Only extension 2 should have been acknowledged. + EXPECT_FALSE(prefs->HasSettingsApiBubbleBeenAcknowledged(kId1)); + EXPECT_TRUE(prefs->HasSettingsApiBubbleBeenAcknowledged(kId2)); + EXPECT_FALSE(prefs->HasSettingsApiBubbleBeenAcknowledged(kId3)); + // Clean up after ourselves. + prefs->SetSettingsApiBubbleBeenAcknowledged(kId2, false); + + // Simulate clicking the learn more link to dismiss it. + bubble.set_action_on_show( + FakeExtensionMessageBubble::BUBBLE_ACTION_CLICK_LINK); + controller.reset(new TestSettingsApiBubbleController( + profile(), static_cast<SettingsApiOverrideType>(i))); + controller->Show(&bubble); + EXPECT_EQ(1U, controller->link_click_count()); + EXPECT_EQ(0U, controller->action_click_count()); + EXPECT_EQ(0U, controller->dismiss_click_count()); + // No extension should have become disabled. + EXPECT_TRUE(service_->GetExtensionById(kId1, false) != NULL); + EXPECT_TRUE(service_->GetExtensionById(kId2, false) != NULL); + EXPECT_TRUE(service_->GetExtensionById(kId3, false) != NULL); + // Only extension 2 should have been acknowledged. + EXPECT_FALSE(prefs->HasSettingsApiBubbleBeenAcknowledged(kId1)); + EXPECT_TRUE(prefs->HasSettingsApiBubbleBeenAcknowledged(kId2)); + EXPECT_FALSE(prefs->HasSettingsApiBubbleBeenAcknowledged(kId3)); + // Clean up after ourselves. + prefs->SetSettingsApiBubbleBeenAcknowledged(kId2, false); + + // Do it again, but now opt to disable the extension. + bubble.set_action_on_show( + FakeExtensionMessageBubble::BUBBLE_ACTION_CLICK_ACTION_BUTTON); + controller.reset(new TestSettingsApiBubbleController( + profile(), static_cast<SettingsApiOverrideType>(i))); + EXPECT_TRUE(controller->ShouldShow(kId2)); + override_extensions = controller->GetExtensionList(); + EXPECT_EQ(1U, override_extensions.size()); + controller->Show(&bubble); // Simulate showing the bubble. + EXPECT_EQ(0U, controller->link_click_count()); + EXPECT_EQ(1U, controller->action_click_count()); + EXPECT_EQ(0U, controller->dismiss_click_count()); + // Only extension 2 should have become disabled. + EXPECT_TRUE(service_->GetExtensionById(kId1, false) != NULL); + EXPECT_TRUE(service_->GetExtensionById(kId2, false) == NULL); + EXPECT_TRUE(service_->GetExtensionById(kId3, false) != NULL); + // No extension should have been acknowledged (it got disabled). + EXPECT_FALSE(prefs->HasSettingsApiBubbleBeenAcknowledged(kId1)); + EXPECT_FALSE(prefs->HasSettingsApiBubbleBeenAcknowledged(kId2)); + EXPECT_FALSE(prefs->HasSettingsApiBubbleBeenAcknowledged(kId3)); + + // Clean up after ourselves. + service_->UninstallExtension(kId1, false, NULL); + service_->UninstallExtension(kId2, false, NULL); + service_->UninstallExtension(kId3, false, NULL); + } +} + } // namespace extensions diff --git a/chrome/browser/extensions/settings_api_bubble_controller.cc b/chrome/browser/extensions/settings_api_bubble_controller.cc index a5819bc..f10ccc9 100644 --- a/chrome/browser/extensions/settings_api_bubble_controller.cc +++ b/chrome/browser/extensions/settings_api_bubble_controller.cc @@ -6,6 +6,7 @@ #include "base/metrics/histogram.h" #include "chrome/browser/extensions/extension_service.h" +#include "chrome/browser/extensions/settings_api_helpers.h" #include "chrome/browser/profiles/profile.h" #include "chrome/browser/ui/startup/startup_browser_creator.h" #include "chrome/common/extensions/manifest_handlers/settings_overrides_handler.h" @@ -93,35 +94,34 @@ bool SettingsApiBubbleDelegate::ShouldIncludeExtension( if (prefs->HasSettingsApiBubbleBeenAcknowledged(extension_id)) return false; - const SettingsOverrides* settings = SettingsOverrides::Get(extension); - if (!settings) - return false; - - bool should_include = false; + const extensions::Extension* override = NULL; switch (type_) { case extensions::BUBBLE_TYPE_HOME_PAGE: - should_include = settings->homepage != NULL; + override = extensions::OverridesHomepage(profile_, NULL); break; case extensions::BUBBLE_TYPE_STARTUP_PAGES: - should_include = !settings->startup_pages.empty(); + override = extensions::OverridesStartupPages(profile_, NULL); break; case extensions::BUBBLE_TYPE_SEARCH_ENGINE: - should_include = settings->search_engine != NULL; + override = extensions::OverridesSearchEngine(profile_, NULL); break; } - if (should_include && extension_id_ != extension_id) { - DCHECK(extension_id_.empty()); - extension_id_ = extension_id; - } - return should_include; + if (!override || override->id() != extension->id()) + return false; + + extension_id_ = extension_id; + return true; } void SettingsApiBubbleDelegate::AcknowledgeExtension( const std::string& extension_id, ExtensionMessageBubbleController::BubbleAction user_action) { - extensions::ExtensionPrefs* prefs = extensions::ExtensionPrefs::Get(profile_); - prefs->SetSettingsApiBubbleBeenAcknowledged(extension_id, true); + if (user_action != ExtensionMessageBubbleController::ACTION_EXECUTE) { + extensions::ExtensionPrefs* prefs = + extensions::ExtensionPrefs::Get(profile_); + prefs->SetSettingsApiBubbleBeenAcknowledged(extension_id, true); + } } void SettingsApiBubbleDelegate::PerformAction( diff --git a/chrome/browser/extensions/settings_api_helpers.cc b/chrome/browser/extensions/settings_api_helpers.cc new file mode 100644 index 0000000..839098cb --- /dev/null +++ b/chrome/browser/extensions/settings_api_helpers.cc @@ -0,0 +1,103 @@ +// Copyright (c) 2014 The Chromium Authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +#include "chrome/browser/extensions/settings_api_helpers.h" + +#include "chrome/browser/extensions/api/preference/preference_api.h" +#include "chrome/common/pref_names.h" +#include "extensions/browser/extension_registry.h" +#include "extensions/common/extension_set.h" + +namespace extensions { + +const extensions::SettingsOverrides* FindOverridingExtension( + content::BrowserContext* browser_context, + SettingsApiOverrideType type, + const Extension** extension) { + const extensions::ExtensionSet& extensions = + extensions::ExtensionRegistry::Get(browser_context)->enabled_extensions(); + + for (extensions::ExtensionSet::const_iterator it = extensions.begin(); + it != extensions.end(); + ++it) { + const extensions::SettingsOverrides* settings = + extensions::SettingsOverrides::Get(*it); + if (settings) { + if (type == BUBBLE_TYPE_HOME_PAGE && !settings->homepage) + continue; + if (type == BUBBLE_TYPE_STARTUP_PAGES && settings->startup_pages.empty()) + continue; + if (type == BUBBLE_TYPE_SEARCH_ENGINE && !settings->search_engine) + continue; + + std::string key; + switch (type) { + case BUBBLE_TYPE_HOME_PAGE: + key = prefs::kHomePage; + break; + case BUBBLE_TYPE_STARTUP_PAGES: + key = prefs::kRestoreOnStartup; + break; + case BUBBLE_TYPE_SEARCH_ENGINE: + key = prefs::kDefaultSearchProviderEnabled; + break; + } + + // Found an extension overriding the current type, check if primary. + PreferenceAPI* preference_api = PreferenceAPI::Get(browser_context); + if (preference_api && // Expected to be NULL in unit tests. + !preference_api->DoesExtensionControlPref((*it)->id(), key, NULL)) + continue; // Not primary. + + // Found the primary extension, return its setting. + *extension = *it; + return settings; + } + } + + return NULL; +} + +const Extension* OverridesHomepage(content::BrowserContext* browser_context, + GURL* home_page_url) { + const extensions::Extension* extension = NULL; + const extensions::SettingsOverrides* settings = + FindOverridingExtension( + browser_context, BUBBLE_TYPE_HOME_PAGE, &extension); + + if (settings && home_page_url) + *home_page_url = *settings->homepage; + return extension; +} + +const Extension* OverridesStartupPages(content::BrowserContext* browser_context, + std::vector<GURL>* startup_pages) { + const extensions::Extension* extension = NULL; + const extensions::SettingsOverrides* settings = + FindOverridingExtension( + browser_context, BUBBLE_TYPE_STARTUP_PAGES, &extension); + if (settings && startup_pages) { + startup_pages->clear(); + for (std::vector<GURL>::const_iterator it = settings->startup_pages.begin(); + it != settings->startup_pages.end(); + ++it) + startup_pages->push_back(GURL(*it)); + } + return extension; +} + +const Extension* OverridesSearchEngine( + content::BrowserContext* browser_context, + api::manifest_types::ChromeSettingsOverrides::Search_provider* + search_provider) { + const extensions::Extension* extension = NULL; + const extensions::SettingsOverrides* settings = + FindOverridingExtension( + browser_context, BUBBLE_TYPE_SEARCH_ENGINE, &extension); + if (settings && search_provider) + search_provider = settings->search_engine.get(); + return extension; +} + +} // namespace extensions diff --git a/chrome/browser/extensions/settings_api_helpers.h b/chrome/browser/extensions/settings_api_helpers.h new file mode 100644 index 0000000..db2ba36 --- /dev/null +++ b/chrome/browser/extensions/settings_api_helpers.h @@ -0,0 +1,48 @@ +// Copyright (c) 2014 The Chromium Authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +#ifndef CHROME_BROWSER_EXTENSIONS_SETTINGS_API_HELPERS_H_ +#define CHROME_BROWSER_EXTENSIONS_SETTINGS_API_HELPERS_H_ + +#include "chrome/common/extensions/manifest_handlers/settings_overrides_handler.h" + +namespace content { +class BrowserContext; +} + +namespace extensions { + +struct SettingsOverrides; + +// Find which |extension| is overriding a particular |type| of setting. Returns +// the SettingsOverride object, or NULL if no |extension| is overriding that +// particular setting. +const extensions::SettingsOverrides* FindOverridingExtension( + content::BrowserContext* browser_context, + SettingsApiOverrideType type, + const Extension** extension); + +// Returns which extension is overriding the homepage in a given +// |browser_context|. |home_page_url|, if non-NULL, will contain the home_page +// value the extension has set. +const Extension* OverridesHomepage(content::BrowserContext* browser_context, + GURL* home_page_url); + +// Returns which extension is overriding the homepage in a given +// |browser_context|. |startup_pages|, if non-NULL, will contain the vector of +// startup page URLs the extension has set. +const Extension* OverridesStartupPages(content::BrowserContext* browser_context, + std::vector<GURL>* startup_pages); + +// Returns which extension is overriding the search engine in a given +// |browser_context|. |search_provider|, if non-NULL, will contain the search +// provider the extension has set. +const Extension* OverridesSearchEngine( + content::BrowserContext* browser_context, + api::manifest_types::ChromeSettingsOverrides::Search_provider* + search_provider); + +} // namespace extensions + +#endif // CHROME_BROWSER_EXTENSIONS_SETTINGS_API_HELPERS_H_ diff --git a/chrome/browser/ui/views/extensions/extension_message_bubble_view.cc b/chrome/browser/ui/views/extensions/extension_message_bubble_view.cc index 05aaaf9..2e58aae4 100644 --- a/chrome/browser/ui/views/extensions/extension_message_bubble_view.cc +++ b/chrome/browser/ui/views/extensions/extension_message_bubble_view.cc @@ -12,10 +12,10 @@ #include "chrome/browser/extensions/extension_message_bubble_controller.h" #include "chrome/browser/extensions/extension_service.h" #include "chrome/browser/extensions/settings_api_bubble_controller.h" +#include "chrome/browser/extensions/settings_api_helpers.h" #include "chrome/browser/extensions/suspicious_extension_bubble_controller.h" #include "chrome/browser/profiles/profile.h" #include "chrome/browser/ui/views/frame/browser_view.h" -#include "chrome/browser/ui/views/settings_api_bubble_helper_views.h" #include "chrome/browser/ui/views/toolbar/browser_actions_container.h" #include "chrome/browser/ui/views/toolbar/browser_actions_container_observer.h" #include "chrome/browser/ui/views/toolbar/toolbar_view.h" diff --git a/chrome/browser/ui/views/settings_api_bubble_helper_views.cc b/chrome/browser/ui/views/settings_api_bubble_helper_views.cc index 5a91d26..8927342 100644 --- a/chrome/browser/ui/views/settings_api_bubble_helper_views.cc +++ b/chrome/browser/ui/views/settings_api_bubble_helper_views.cc @@ -5,8 +5,7 @@ #include "chrome/browser/ui/views/settings_api_bubble_helper_views.h" #include "chrome/browser/extensions/settings_api_bubble_controller.h" -#include "chrome/browser/profiles/profile.h" -#include "chrome/browser/ui/browser.h" +#include "chrome/browser/extensions/settings_api_helpers.h" #include "chrome/browser/ui/browser_finder.h" #include "chrome/browser/ui/views/extensions/extension_message_bubble_view.h" #include "chrome/browser/ui/views/frame/browser_view.h" @@ -14,8 +13,6 @@ #include "chrome/browser/ui/views/toolbar/home_button.h" #include "chrome/browser/ui/views/toolbar/toolbar_view.h" #include "chrome/common/extensions/manifest_handlers/settings_overrides_handler.h" -#include "content/public/browser/browser_context.h" -#include "extensions/browser/extension_registry.h" namespace { @@ -88,70 +85,4 @@ void MaybeShowExtensionControlledSearchNotification( } } -const extensions::SettingsOverrides* FindOverridingExtension( - content::BrowserContext* browser_context, - SettingsApiOverrideType type, - const Extension** extension) { - const extensions::ExtensionSet& extensions = - extensions::ExtensionRegistry::Get(browser_context)->enabled_extensions(); - - for (extensions::ExtensionSet::const_iterator it = extensions.begin(); - it != extensions.end(); - ++it) { - const extensions::SettingsOverrides* settings = - extensions::SettingsOverrides::Get(*it); - if (settings) { - if ((type == BUBBLE_TYPE_HOME_PAGE && settings->homepage) || - (type == BUBBLE_TYPE_STARTUP_PAGES && - !settings->startup_pages.empty()) || - (type == BUBBLE_TYPE_SEARCH_ENGINE && settings->search_engine)) { - *extension = *it; - return settings; - } - } - } - - return NULL; -} - -const Extension* OverridesHomepage(content::BrowserContext* browser_context, - GURL* home_page_url) { - const extensions::Extension* extension = NULL; - const extensions::SettingsOverrides* settings = - FindOverridingExtension( - browser_context, BUBBLE_TYPE_HOME_PAGE, &extension); - if (settings && home_page_url) - *home_page_url = *settings->homepage; - return extension; -} - -const Extension* OverridesStartupPages(content::BrowserContext* browser_context, - std::vector<GURL>* startup_pages) { - const extensions::Extension* extension = NULL; - const extensions::SettingsOverrides* settings = - FindOverridingExtension( - browser_context, BUBBLE_TYPE_STARTUP_PAGES, &extension); - if (settings && startup_pages) { - startup_pages->clear(); - for (std::vector<GURL>::const_iterator it = settings->startup_pages.begin(); - it != settings->startup_pages.end(); - ++it) - startup_pages->push_back(GURL(*it)); - } - return extension; -} - -const Extension* OverridesSearchEngine( - content::BrowserContext* browser_context, - api::manifest_types::ChromeSettingsOverrides::Search_provider* - search_provider) { - const extensions::Extension* extension = NULL; - const extensions::SettingsOverrides* settings = - FindOverridingExtension( - browser_context, BUBBLE_TYPE_SEARCH_ENGINE, &extension); - if (settings && search_provider) - search_provider = settings->search_engine.get(); - return extension; -} - } // namespace extensions diff --git a/chrome/browser/ui/views/settings_api_bubble_helper_views.h b/chrome/browser/ui/views/settings_api_bubble_helper_views.h index a36a806..a475ed7 100644 --- a/chrome/browser/ui/views/settings_api_bubble_helper_views.h +++ b/chrome/browser/ui/views/settings_api_bubble_helper_views.h @@ -5,14 +5,11 @@ #ifndef CHROME_BROWSER_UI_VIEWS_SETTINGS_API_BUBBLE_HELPER_VIEWS_H_ #define CHROME_BROWSER_UI_VIEWS_SETTINGS_API_BUBBLE_HELPER_VIEWS_H_ -#include "chrome/common/extensions/manifest_handlers/settings_overrides_handler.h" - struct AutocompleteMatch; class Browser; class Profile; namespace content { -class BrowserContext; class WebContents; } @@ -31,34 +28,6 @@ void MaybeShowExtensionControlledSearchNotification( content::WebContents* web_contents, const AutocompleteMatch& match); -// Find which |extension| is overriding a particular |type| of setting. Returns -// the SettingsOverride object, or NULL if no |extension| is overriding that -// particular setting. -const extensions::SettingsOverrides* FindOverridingExtension( - content::BrowserContext* browser_context, - SettingsApiOverrideType type, - const Extension** extension); - -// Returns which extension is overriding the homepage in a given -// |browser_context|. |home_page_url|, if non-NULL, will contain the home_page -// value the extension has set. -const Extension* OverridesHomepage(content::BrowserContext* browser_context, - GURL* home_page_url); - -// Returns which extension is overriding the homepage in a given -// |browser_context|. |startup_pages|, if non-NULL, will contain the vector of -// startup page URLs the extension has set. -const Extension* OverridesStartupPages(content::BrowserContext* browser_context, - std::vector<GURL>* startup_pages); - -// Returns which extension is overriding the search engine in a given -// |browser_context|. |search_provider|, if non-NULL, will contain the search -// provider the extension has set. -const Extension* OverridesSearchEngine( - content::BrowserContext* browser_context, - api::manifest_types::ChromeSettingsOverrides::Search_provider* - search_provider); - } // namespace extensions #endif // CHROME_BROWSER_UI_VIEWS_SETTINGS_API_BUBBLE_HELPER_VIEWS_H_ diff --git a/chrome/chrome_browser_extensions.gypi b/chrome/chrome_browser_extensions.gypi index 6b36a4c..381d294 100644 --- a/chrome/chrome_browser_extensions.gypi +++ b/chrome/chrome_browser_extensions.gypi @@ -823,6 +823,8 @@ 'browser/extensions/sandboxed_unpacker.h', 'browser/extensions/script_executor.cc', 'browser/extensions/script_executor.h', + 'browser/extensions/settings_api_helpers.cc', + 'browser/extensions/settings_api_helpers.h', 'browser/extensions/settings_api_bubble_controller.cc', 'browser/extensions/settings_api_bubble_controller.h', 'browser/extensions/standard_management_policy_provider.cc', diff --git a/extensions/browser/extension_prefs.cc b/extensions/browser/extension_prefs.cc index 24f7e58..53d800c 100644 --- a/extensions/browser/extension_prefs.cc +++ b/extensions/browser/extension_prefs.cc @@ -1897,10 +1897,8 @@ void ExtensionPrefs::SetInstallSignature( std::string ExtensionPrefs::GetInstallParam( const std::string& extension_id) const { const base::DictionaryValue* extension = GetExtensionPref(extension_id); - if (!extension) { - NOTREACHED(); + if (!extension) // Expected during unit testing. return std::string(); - } std::string install_parameter; if (!extension->GetString(kPrefInstallParam, &install_parameter)) return std::string(); |