chrome/browser/extensions/extension_message_bubble_controller_unittest.cc - Issue 288923004: Add an extension override bubble and warning box for proxy extensions.

Unified Diff: chrome/browser/extensions/extension_message_bubble_controller_unittest.cc

Issue 288923004: Add an extension override bubble and warning box for proxy extensions. (Closed) Base URL: svn://svn.chromium.org/chrome/trunk/src

Patch Set: Address review comments Created 6 years, 7 months ago

Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.

Jump to:

View side-by-side diff with in-line comments

Download patch

« no previous file with comments | « chrome/app/generated_resources.grd ('k') | chrome/browser/extensions/proxy_overridden_bubble_controller.h » ('j') | chrome/browser/extensions/settings_api_helpers.h » ('J')
Expand Comments ('e') | Collapse Comments ('c') | Hide Comments ('s')

Index: chrome/browser/extensions/extension_message_bubble_controller_unittest.cc

diff --git a/chrome/browser/extensions/extension_message_bubble_controller_unittest.cc b/chrome/browser/extensions/extension_message_bubble_controller_unittest.cc

index 77181b7ab6ae5de98ab1ae8bdb537418f2dd3981..d0f4f55fbea9b4a8980715b3db3e0b2c2e1600f7 100644

--- a/chrome/browser/extensions/extension_message_bubble_controller_unittest.cc

+++ b/chrome/browser/extensions/extension_message_bubble_controller_unittest.cc

@@ -10,15 +10,18 @@

#include "chrome/browser/extensions/extension_message_bubble.h"

#include "chrome/browser/extensions/extension_service.h"

#include "chrome/browser/extensions/ntp_overridden_bubble_controller.h"

+#include "chrome/browser/extensions/proxy_overridden_bubble_controller.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/common/chrome_version_info.h"

#include "chrome/test/base/testing_profile.h"

#include "content/public/test/test_browser_thread_bundle.h"

+#include "extensions/browser/extension_registry.h"

#include "extensions/common/extension.h"

#include "extensions/common/extension_builder.h"

#include "extensions/common/feature_switch.h"

+#include "extensions/common/value_builder.h"

namespace {

@@ -158,6 +161,31 @@ class TestNtpOverriddenBubbleController

}

};

+// A test class for the ProxyOverriddenBubbleController.

+class TestProxyOverriddenBubbleController

+ : public ProxyOverriddenBubbleController,

+ public TestDelegate {

+ public:

+ explicit TestProxyOverriddenBubbleController(Profile* profile)

+ : ProxyOverriddenBubbleController(profile) {

+ }

+ virtual void OnBubbleAction() OVERRIDE {

+ ++action_button_callback_count_;

+ ProxyOverriddenBubbleController::OnBubbleAction();

+ }

+ virtual void OnBubbleDismiss() OVERRIDE {

+ ++dismiss_button_callback_count_;

+ ProxyOverriddenBubbleController::OnBubbleDismiss();

+ }

+ virtual void OnLinkClicked() OVERRIDE {

+ ++link_click_callback_count_;

+ ProxyOverriddenBubbleController::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).

@@ -208,84 +236,129 @@ class ExtensionMessageBubbleTest : public testing::Test {

public:

ExtensionMessageBubbleTest() {}

- void LoadGenericExtension(const std::string& index,

- const std::string& id,

- Manifest::Location location) {

- extensions::ExtensionBuilder builder;

- builder.SetManifest(extensions::DictionaryBuilder()

+ testing::AssertionResult LoadGenericExtension(const std::string& index,

+ const std::string& id,

+ Manifest::Location location) {

+ ExtensionBuilder builder;

+ builder.SetManifest(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());

+ if (ExtensionRegistry::Get(profile())->enabled_extensions().GetByID(id))

+ return testing::AssertionSuccess();

+ return testing::AssertionFailure() << "Could not install extension: " << id;

}

- void LoadExtensionWithAction(const std::string& index,

- const std::string& id,

- Manifest::Location location) {

- extensions::ExtensionBuilder builder;

- builder.SetManifest(extensions::DictionaryBuilder()

+ testing::AssertionResult LoadExtensionWithAction(

+ const std::string& index,

+ const std::string& id,

+ Manifest::Location location) {

+ ExtensionBuilder builder;

+ builder.SetManifest(DictionaryBuilder()

.Set("name", std::string("Extension " + index))

.Set("version", "1.0")

.Set("manifest_version", 2)

.Set("browser_action",

- extensions::DictionaryBuilder().Set(

+ DictionaryBuilder().Set(

"default_title", "Default title")));

builder.SetLocation(location);

builder.SetID(id);

service_->AddExtension(builder.Build().get());

+ if (ExtensionRegistry::Get(profile())->enabled_extensions().GetByID(id))

+ return testing::AssertionSuccess();

+ return testing::AssertionFailure() << "Could not install extension: " << id;

}

- void LoadExtensionOverridingHome(const std::string& index,

- const std::string& id,

- Manifest::Location location) {

- extensions::ExtensionBuilder builder;

- builder.SetManifest(extensions::DictionaryBuilder()

+ testing::AssertionResult LoadExtensionOverridingHome(

+ const std::string& index,

+ const std::string& id,

+ Manifest::Location location) {

+ ExtensionBuilder builder;

+ builder.SetManifest(DictionaryBuilder()

.Set("name", std::string("Extension " + index))

.Set("version", "1.0")

.Set("manifest_version", 2)

.Set("chrome_settings_overrides",

- extensions::DictionaryBuilder().Set(

+ DictionaryBuilder().Set(

"homepage", "http://www.google.com")));

builder.SetLocation(location);

builder.SetID(id);

service_->AddExtension(builder.Build().get());

+ if (ExtensionRegistry::Get(profile())->enabled_extensions().GetByID(id))

+ return testing::AssertionSuccess();

+ return testing::AssertionFailure() << "Could not install extension: " << id;

}

- void LoadExtensionOverridingStart(const std::string& index,

- const std::string& id,

- Manifest::Location location) {

- extensions::ExtensionBuilder builder;

- builder.SetManifest(extensions::DictionaryBuilder()

+ testing::AssertionResult LoadExtensionOverridingStart(

+ const std::string& index,

+ const std::string& id,

+ Manifest::Location location) {

+ ExtensionBuilder builder;

+ builder.SetManifest(DictionaryBuilder()

.Set("name", std::string("Extension " + index))

Devlin 2014/05/21 17:00:39 In a separate change, we could probably refactor t

.Set("version", "1.0")

.Set("manifest_version", 2)

.Set("chrome_settings_overrides",

- extensions::DictionaryBuilder().Set(

+ DictionaryBuilder().Set(

"startup_pages",

- extensions::ListBuilder().Append(

+ ListBuilder().Append(

"http://www.google.com"))));

builder.SetLocation(location);

builder.SetID(id);

service_->AddExtension(builder.Build().get());

+ if (ExtensionRegistry::Get(profile())->enabled_extensions().GetByID(id))

+ return testing::AssertionSuccess();

+ return testing::AssertionFailure() << "Could not install extension: " << id;

}

- void LoadExtensionOverridingNtp(const std::string& index,

- const std::string& id,

- Manifest::Location location) {

- extensions::ExtensionBuilder builder;

- builder.SetManifest(extensions::DictionaryBuilder()

+ testing::AssertionResult LoadExtensionOverridingNtp(

+ const std::string& index,

+ const std::string& id,

+ Manifest::Location location) {

+ ExtensionBuilder builder;

+ builder.SetManifest(DictionaryBuilder()

.Set("name", std::string("Extension " + index))

.Set("version", "1.0")

.Set("manifest_version", 2)

.Set("chrome_url_overrides",

- extensions::DictionaryBuilder().Set(

+ DictionaryBuilder().Set(

"newtab", "Default.html")));

builder.SetLocation(location);

builder.SetID(id);

service_->AddExtension(builder.Build().get());

+ if (ExtensionRegistry::Get(profile())->enabled_extensions().GetByID(id))

+ return testing::AssertionSuccess();

+ return testing::AssertionFailure() << "Could not install extension: " << id;

+ }

+ testing::AssertionResult LoadExtensionOverridingProxy(

+ const std::string& index,

+ const std::string& id,

+ Manifest::Location location) {

+ ExtensionBuilder builder;

+ builder.SetManifest(DictionaryBuilder()

+ .Set("name", std::string("Extension " + index))

+ .Set("version", "1.0")

+ .Set("manifest_version", 2)

+ .Set("permissions",

+ ListBuilder().Append("proxy")));

+ builder.SetLocation(location);

+ builder.SetID(id);

+ service_->AddExtension(builder.Build().get());

+ if (ExtensionRegistry::Get(profile())->enabled_extensions().GetByID(id))

+ return testing::AssertionSuccess();

+ return testing::AssertionFailure() << "Could not install extension: " << id;

}

void Init() {

@@ -347,9 +420,9 @@ TEST_F(ExtensionMessageBubbleTest, MAYBE_WipeoutControllerTest) {

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);

+ ASSERT_TRUE(LoadExtensionWithAction("1", kId1, Manifest::COMMAND_LINE));

Devlin 2014/05/21 17:00:39 Thanks for also changing the other test cases. :)

+ ASSERT_TRUE(LoadGenericExtension("2", kId2, Manifest::UNPACKED));

+ ASSERT_TRUE(LoadGenericExtension("3", kId3, Manifest::EXTERNAL_POLICY));

scoped_ptr<TestSuspiciousExtensionBubbleController> controller(

new TestSuspiciousExtensionBubbleController(profile()));

@@ -424,9 +497,9 @@ TEST_F(ExtensionMessageBubbleTest, MAYBE_DevModeControllerTest) {

// 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);

+ ASSERT_TRUE(LoadExtensionWithAction("1", kId1, Manifest::COMMAND_LINE));

+ ASSERT_TRUE(LoadGenericExtension("2", kId2, Manifest::UNPACKED));

+ ASSERT_TRUE(LoadGenericExtension("3", kId3, Manifest::EXTERNAL_POLICY));

scoped_ptr<TestDevModeBubbleController> controller(

new TestDevModeBubbleController(profile()));

@@ -450,8 +523,9 @@ 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(kId1, false) != NULL);

- EXPECT_TRUE(service_->GetExtensionById(kId2, false) != NULL);

+ ExtensionRegistry* registry = ExtensionRegistry::Get(profile());

+ EXPECT_TRUE(registry->enabled_extensions().GetByID(kId1) != NULL);

+ EXPECT_TRUE(registry->enabled_extensions().GetByID(kId2) != NULL);

// Do it again, but now press different button (Disable).

bubble.set_action_on_show(

@@ -466,8 +540,8 @@ 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(kId1, false) == NULL);

- EXPECT_TRUE(service_->GetExtensionById(kId2, false) == NULL);

+ EXPECT_TRUE(registry->disabled_extensions().GetByID(kId1) != NULL);

+ EXPECT_TRUE(registry->disabled_extensions().GetByID(kId2) != NULL);

// Re-enable the extensions (disabled by the action button above).

service_->EnableExtension(kId1);

@@ -486,8 +560,8 @@ 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(kId1, false) != NULL);

- EXPECT_TRUE(service_->GetExtensionById(kId2, false) != NULL);

+ EXPECT_TRUE(registry->enabled_extensions().GetByID(kId1) != NULL);

+ EXPECT_TRUE(registry->enabled_extensions().GetByID(kId2) != NULL);

// Now disable the unpacked extension.

service_->DisableExtension(kId1, Extension::DISABLE_USER_ACTION);

@@ -510,8 +584,7 @@ TEST_F(ExtensionMessageBubbleTest, MAYBE_DevModeControllerTest) {

TEST_F(ExtensionMessageBubbleTest, MAYBE_SettingsApiControllerTest) {

Init();

- extensions::ExtensionPrefs* prefs =

- extensions::ExtensionPrefs::Get(profile());

+ ExtensionPrefs* prefs = ExtensionPrefs::Get(profile());

for (int i = 0; i < 3; ++i) {

switch (static_cast<SettingsApiOverrideType>(i)) {

@@ -519,9 +592,10 @@ TEST_F(ExtensionMessageBubbleTest, MAYBE_SettingsApiControllerTest) {

// 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);

+ ASSERT_TRUE(LoadExtensionOverridingHome("1", kId1, Manifest::UNPACKED));

+ ASSERT_TRUE(LoadExtensionOverridingHome("2", kId2, Manifest::UNPACKED));

+ ASSERT_TRUE(

+ LoadExtensionOverridingStart("3", kId3, Manifest::UNPACKED));

break;

case BUBBLE_TYPE_SEARCH_ENGINE:

// We deliberately skip testing the search engine since it relies on

@@ -534,9 +608,11 @@ TEST_F(ExtensionMessageBubbleTest, MAYBE_SettingsApiControllerTest) {

// 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);

+ ASSERT_TRUE(

+ LoadExtensionOverridingStart("1", kId1, Manifest::UNPACKED));

+ ASSERT_TRUE(

+ LoadExtensionOverridingStart("2", kId2, Manifest::UNPACKED));

+ ASSERT_TRUE(LoadExtensionOverridingHome("3", kId3, Manifest::UNPACKED));

break;

default:

NOTREACHED();

@@ -567,9 +643,10 @@ TEST_F(ExtensionMessageBubbleTest, MAYBE_SettingsApiControllerTest) {

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);

+ ExtensionRegistry* registry = ExtensionRegistry::Get(profile());

+ EXPECT_TRUE(registry->enabled_extensions().GetByID(kId1) != NULL);

+ EXPECT_TRUE(registry->enabled_extensions().GetByID(kId2) != NULL);

+ EXPECT_TRUE(registry->enabled_extensions().GetByID(kId3) != NULL);

// Only extension 2 should have been acknowledged.

EXPECT_FALSE(prefs->HasSettingsApiBubbleBeenAcknowledged(kId1));

EXPECT_TRUE(prefs->HasSettingsApiBubbleBeenAcknowledged(kId2));

@@ -587,9 +664,9 @@ TEST_F(ExtensionMessageBubbleTest, MAYBE_SettingsApiControllerTest) {

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);

+ EXPECT_TRUE(registry->enabled_extensions().GetByID(kId1) != NULL);

+ EXPECT_TRUE(registry->enabled_extensions().GetByID(kId2) != NULL);

+ EXPECT_TRUE(registry->enabled_extensions().GetByID(kId3) != NULL);

// Only extension 2 should have been acknowledged.

EXPECT_FALSE(prefs->HasSettingsApiBubbleBeenAcknowledged(kId1));

EXPECT_TRUE(prefs->HasSettingsApiBubbleBeenAcknowledged(kId2));

@@ -610,9 +687,9 @@ TEST_F(ExtensionMessageBubbleTest, MAYBE_SettingsApiControllerTest) {

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);

+ EXPECT_TRUE(registry->enabled_extensions().GetByID(kId1) != NULL);

+ EXPECT_TRUE(registry->disabled_extensions().GetByID(kId2) != NULL);

+ EXPECT_TRUE(registry->enabled_extensions().GetByID(kId3) != NULL);

// No extension should have been acknowledged (it got disabled).

EXPECT_FALSE(prefs->HasSettingsApiBubbleBeenAcknowledged(kId1));

EXPECT_FALSE(prefs->HasSettingsApiBubbleBeenAcknowledged(kId2));

@@ -634,14 +711,13 @@ TEST_F(ExtensionMessageBubbleTest, MAYBE_SettingsApiControllerTest) {

TEST_F(ExtensionMessageBubbleTest, MAYBE_NtpOverriddenControllerTest) {

Init();

- extensions::ExtensionPrefs* prefs =

- extensions::ExtensionPrefs::Get(profile());

+ ExtensionPrefs* prefs = ExtensionPrefs::Get(profile());

// Load two extensions overriding new tab page and one overriding something

// unrelated (to check for interference). Extension 2 should still win

// on the new tab page setting.

- LoadExtensionOverridingNtp("1", kId1, Manifest::UNPACKED);

- LoadExtensionOverridingNtp("2", kId2, Manifest::UNPACKED);

- LoadExtensionOverridingStart("3", kId3, Manifest::UNPACKED);

+ ASSERT_TRUE(LoadExtensionOverridingNtp("1", kId1, Manifest::UNPACKED));

+ ASSERT_TRUE(LoadExtensionOverridingNtp("2", kId2, Manifest::UNPACKED));

+ ASSERT_TRUE(LoadExtensionOverridingStart("3", kId3, Manifest::UNPACKED));

scoped_ptr<TestNtpOverriddenBubbleController> controller(

new TestNtpOverriddenBubbleController(profile()));

@@ -667,9 +743,10 @@ TEST_F(ExtensionMessageBubbleTest, MAYBE_NtpOverriddenControllerTest) {

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);

+ ExtensionRegistry* registry = ExtensionRegistry::Get(profile());

+ EXPECT_TRUE(registry->enabled_extensions().GetByID(kId1) != NULL);

+ EXPECT_TRUE(registry->enabled_extensions().GetByID(kId2) != NULL);

+ EXPECT_TRUE(registry->enabled_extensions().GetByID(kId3) != NULL);

// Only extension 2 should have been acknowledged.

EXPECT_FALSE(prefs->HasNtpOverriddenBubbleBeenAcknowledged(kId1));

EXPECT_TRUE(prefs->HasNtpOverriddenBubbleBeenAcknowledged(kId2));

@@ -687,9 +764,9 @@ TEST_F(ExtensionMessageBubbleTest, MAYBE_NtpOverriddenControllerTest) {

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);

+ EXPECT_TRUE(registry->enabled_extensions().GetByID(kId1) != NULL);

+ EXPECT_TRUE(registry->enabled_extensions().GetByID(kId2) != NULL);

+ EXPECT_TRUE(registry->enabled_extensions().GetByID(kId3) != NULL);

// Only extension 2 should have been acknowledged.

EXPECT_FALSE(prefs->HasNtpOverriddenBubbleBeenAcknowledged(kId1));

EXPECT_TRUE(prefs->HasNtpOverriddenBubbleBeenAcknowledged(kId2));

@@ -709,9 +786,9 @@ TEST_F(ExtensionMessageBubbleTest, MAYBE_NtpOverriddenControllerTest) {