From 8d2de4875667830bd0d90a06b2596f4b0540c451 Mon Sep 17 00:00:00 2001 From: treib Date: Tue, 4 Aug 2015 09:36:48 -0700 Subject: [PATCH] Extensions: Remove the intermediate PermissionMessageStrings interface Switch all callers over to the new system (CoalescedPermissionMessages). Also rename GetCoalescedPermissionMessages to GetPermissionMessages - it's the only way to get permission messages now. [This is part 2 of operation "remove the old permission message system".] TBRing trivial changes in ephemeral_app_launcher.cc and extension_install_dialog_view_browsertest.cc. TBR=benwells@chromium.org BUG=398257 Review URL: https://codereview.chromium.org/1218373003 Cr-Commit-Position: refs/heads/master@{#341734} --- chrome/browser/apps/ephemeral_app_launcher.cc | 2 +- .../extensions/api/permissions/permissions_api.cc | 10 ++- chrome/browser/extensions/extension_disabled_ui.cc | 16 ++-- .../browser/extensions/extension_install_prompt.cc | 24 +++--- .../browser/extensions/extension_install_prompt.h | 7 +- chrome/browser/extensions/extension_service.cc | 2 - .../extensions/permission_messages_unittest.cc | 9 ++- .../extension_install_view_controller_unittest.mm | 37 +++++---- .../app_info_dialog/app_info_permissions_panel.cc | 10 +-- .../app_info_dialog/app_info_permissions_panel.h | 5 +- .../extension_install_dialog_view_browsertest.cc | 24 +++--- .../chrome_permission_message_provider.cc | 5 +- .../chrome_permission_message_provider.h | 2 +- .../chrome_permission_message_provider_unittest.cc | 77 ++++++++++-------- .../permissions/chrome_permission_message_rules.cc | 36 +++++---- .../permissions/chrome_permission_message_rules.h | 8 +- .../permissions/permission_set_unittest.cc | 30 ++++--- .../browser/api/management/management_api.cc | 7 +- .../permissions/coalesced_permission_message.h | 5 +- extensions/common/permissions/permission_message.h | 1 - .../permissions/permission_message_provider.cc | 72 +++++------------ .../permissions/permission_message_provider.h | 29 +------ .../permissions/permission_message_test_util.cc | 93 +++++++++++----------- extensions/common/permissions/permissions_data.cc | 12 +-- extensions/common/permissions/permissions_data.h | 8 +- extensions/shell/common/shell_extensions_client.cc | 2 +- .../test/test_permission_message_provider.cc | 2 +- extensions/test/test_permission_message_provider.h | 2 +- 28 files changed, 244 insertions(+), 293 deletions(-) rewrite extensions/common/permissions/permission_message_provider.cc (68%) diff --git a/chrome/browser/apps/ephemeral_app_launcher.cc b/chrome/browser/apps/ephemeral_app_launcher.cc index b480345ef25a..b9192fbfc53c 100644 --- a/chrome/browser/apps/ephemeral_app_launcher.cc +++ b/chrome/browser/apps/ephemeral_app_launcher.cc @@ -393,7 +393,7 @@ EphemeralAppLauncher::CreateInstallPrompt() const { // Skip the prompt by returning null if the app does not need to display // permission warnings. - if (extension->permissions_data()->GetPermissionMessageStrings().empty()) + if (extension->permissions_data()->GetPermissionMessages().empty()) return NULL; return make_scoped_refptr(new ExtensionInstallPrompt::Prompt( diff --git a/chrome/browser/extensions/api/permissions/permissions_api.cc b/chrome/browser/extensions/api/permissions/permissions_api.cc index 3592432980d0..6a0ee31799d1 100644 --- a/chrome/browser/extensions/api/permissions/permissions_api.cc +++ b/chrome/browser/extensions/api/permissions/permissions_api.cc @@ -218,11 +218,13 @@ bool PermissionsRequestFunction::RunAsync() { // We don't need to show the prompt if there are no new warnings, or if // we're skipping the confirmation UI. All extension types but INTERNAL // are allowed to silently increase their permission level. + const PermissionMessageProvider* message_provider = + PermissionMessageProvider::Get(); bool has_no_warnings = - PermissionMessageProvider::Get() - ->GetPermissionMessageStrings(requested_permissions_.get(), - extension()->GetType()) - .empty(); + message_provider->GetPermissionMessages( + message_provider->GetAllPermissionIDs( + requested_permissions_.get(), + extension()->GetType())).empty(); if (auto_confirm_for_tests == PROCEED || has_no_warnings || extension_->location() == Manifest::COMPONENT) { InstallUIProceed(); diff --git a/chrome/browser/extensions/extension_disabled_ui.cc b/chrome/browser/extensions/extension_disabled_ui.cc index 0595d25baeb5..99eaff724519 100644 --- a/chrome/browser/extensions/extension_disabled_ui.cc +++ b/chrome/browser/extensions/extension_disabled_ui.cc @@ -43,13 +43,15 @@ #include "extensions/common/extension.h" #include "extensions/common/extension_icon_set.h" #include "extensions/common/manifest_handlers/icons_handler.h" -#include "extensions/common/permissions/permission_message_provider.h" +#include "extensions/common/permissions/coalesced_permission_message.h" #include "extensions/common/permissions/permissions_data.h" #include "ui/base/l10n/l10n_util.h" #include "ui/gfx/geometry/size.h" #include "ui/gfx/image/image.h" #include "ui/gfx/image/image_skia_operations.h" +using extensions::CoalescedPermissionMessage; +using extensions::CoalescedPermissionMessages; using extensions::Extension; namespace { @@ -292,10 +294,8 @@ base::string16 ExtensionDisabledGlobalError::GetBubbleViewTitle() { std::vector ExtensionDisabledGlobalError::GetBubbleViewMessages() { std::vector messages; - extensions::PermissionMessageStrings permission_warnings = - extensions::PermissionMessageProvider::Get()->GetPermissionMessageStrings( - extension_->permissions_data()->active_permissions().get(), - extension_->GetType()); + CoalescedPermissionMessages permission_warnings = + extension_->permissions_data()->GetPermissionMessages(); if (is_remote_install_) { messages.push_back(l10n_util::GetStringFUTF16( extension_->is_app() @@ -315,9 +315,9 @@ ExtensionDisabledGlobalError::GetBubbleViewMessages() { messages.push_back(l10n_util::GetStringUTF16( IDS_EXTENSION_PROMPT_WILL_NOW_HAVE_ACCESS_TO)); } - for (const extensions::PermissionMessageString& str : permission_warnings) { - messages.push_back(l10n_util::GetStringFUTF16( - IDS_EXTENSION_PERMISSION_LINE, str.message)); + for (const CoalescedPermissionMessage& msg : permission_warnings) { + messages.push_back(l10n_util::GetStringFUTF16(IDS_EXTENSION_PERMISSION_LINE, + msg.message())); } return messages; } diff --git a/chrome/browser/extensions/extension_install_prompt.cc b/chrome/browser/extensions/extension_install_prompt.cc index af92c1d5bba5..41a8e45cc725 100644 --- a/chrome/browser/extensions/extension_install_prompt.cc +++ b/chrome/browser/extensions/extension_install_prompt.cc @@ -52,8 +52,8 @@ using extensions::BundleInstaller; using extensions::Extension; using extensions::Manifest; -using extensions::PermissionMessageString; -using extensions::PermissionMessageStrings; +using extensions::CoalescedPermissionMessage; +using extensions::CoalescedPermissionMessages; using extensions::PermissionSet; namespace { @@ -246,7 +246,7 @@ ExtensionInstallPrompt::Prompt::~Prompt() { } void ExtensionInstallPrompt::Prompt::SetPermissions( - const PermissionMessageStrings& permissions, + const CoalescedPermissionMessages& permissions, PermissionsType permissions_type) { InstallPromptPermissions& install_permissions = GetPermissionsForType(permissions_type); @@ -255,13 +255,13 @@ void ExtensionInstallPrompt::Prompt::SetPermissions( install_permissions.details.clear(); install_permissions.is_showing_details.clear(); - for (const PermissionMessageString& str : permissions) { - install_permissions.permissions.push_back(str.message); + for (const CoalescedPermissionMessage& msg : permissions) { + install_permissions.permissions.push_back(msg.message()); // Add a dash to the front of each permission detail. base::string16 details; - if (!str.submessages.empty()) { + if (!msg.submessages().empty()) { std::vector detail_lines_with_bullets; - for (const auto& detail_line : str.submessages) { + for (const auto& detail_line : msg.submessages()) { detail_lines_with_bullets.push_back(base::ASCIIToUTF16("- ") + detail_line); } @@ -884,8 +884,9 @@ void ExtensionInstallPrompt::ShowConfirmation() { const extensions::PermissionMessageProvider* message_provider = extensions::PermissionMessageProvider::Get(); - prompt_->SetPermissions(message_provider->GetPermissionMessageStrings( - permissions_to_display.get(), type), + prompt_->SetPermissions(message_provider->GetPermissionMessages( + message_provider->GetAllPermissionIDs( + permissions_to_display.get(), type)), REGULAR_PERMISSIONS); scoped_refptr withheld = @@ -893,8 +894,9 @@ void ExtensionInstallPrompt::ShowConfirmation() { : nullptr; if (withheld && !withheld->IsEmpty()) { prompt_->SetPermissions( - message_provider->GetPermissionMessageStrings(withheld.get(), type), - PermissionsType::WITHHELD_PERMISSIONS); + message_provider->GetPermissionMessages( + message_provider->GetAllPermissionIDs(withheld.get(), type)), + WITHHELD_PERMISSIONS); } } diff --git a/chrome/browser/extensions/extension_install_prompt.h b/chrome/browser/extensions/extension_install_prompt.h index a931e5f7673f..0084e7f4ee73 100644 --- a/chrome/browser/extensions/extension_install_prompt.h +++ b/chrome/browser/extensions/extension_install_prompt.h @@ -15,7 +15,7 @@ #include "base/memory/scoped_ptr.h" #include "base/memory/weak_ptr.h" #include "base/strings/string16.h" -#include "extensions/common/permissions/permission_message_provider.h" +#include "extensions/common/permissions/coalesced_permission_message.h" #include "extensions/common/url_pattern.h" #include "third_party/skia/include/core/SkBitmap.h" #include "ui/gfx/image/image.h" @@ -102,8 +102,9 @@ class ExtensionInstallPrompt public: explicit Prompt(PromptType type); - void SetPermissions(const extensions::PermissionMessageStrings& permissions, - PermissionsType permissions_type); + void SetPermissions( + const extensions::CoalescedPermissionMessages& permissions, + PermissionsType permissions_type); void SetIsShowingDetails(DetailsType type, size_t index, bool is_showing_details); diff --git a/chrome/browser/extensions/extension_service.cc b/chrome/browser/extensions/extension_service.cc index 4b18d3ff57cd..1b1ca1f2debb 100644 --- a/chrome/browser/extensions/extension_service.cc +++ b/chrome/browser/extensions/extension_service.cc @@ -116,8 +116,6 @@ using extensions::ManagementPolicy; using extensions::Manifest; using extensions::PermissionID; using extensions::PermissionIDSet; -using extensions::PermissionMessage; -using extensions::PermissionMessageIDs; using extensions::PermissionSet; using extensions::SharedModuleInfo; using extensions::SharedModuleService; diff --git a/chrome/browser/extensions/permission_messages_unittest.cc b/chrome/browser/extensions/permission_messages_unittest.cc index 7ca9f3338e0d..0ddbec52ac01 100644 --- a/chrome/browser/extensions/permission_messages_unittest.cc +++ b/chrome/browser/extensions/permission_messages_unittest.cc @@ -112,10 +112,11 @@ class PermissionMessagesUnittest : public testing::Test { std::vector GetMessages( scoped_refptr permissions) { std::vector messages; - for (const PermissionMessageString& str : - message_provider_->GetPermissionMessageStrings(permissions.get(), - app_->GetType())) { - messages.push_back(str.message); + for (const CoalescedPermissionMessage& msg : + message_provider_->GetPermissionMessages( + message_provider_->GetAllPermissionIDs(permissions.get(), + app_->GetType()))) { + messages.push_back(msg.message()); } return messages; } diff --git a/chrome/browser/ui/cocoa/extensions/extension_install_view_controller_unittest.mm b/chrome/browser/ui/cocoa/extensions/extension_install_view_controller_unittest.mm index 456349487a97..3a2da5f4b17f 100644 --- a/chrome/browser/ui/cocoa/extensions/extension_install_view_controller_unittest.mm +++ b/chrome/browser/ui/cocoa/extensions/extension_install_view_controller_unittest.mm @@ -17,9 +17,10 @@ #include "testing/gtest/include/gtest/gtest.h" #import "testing/gtest_mac.h" +using extensions::CoalescedPermissionMessage; +using extensions::CoalescedPermissionMessages; using extensions::Extension; -using extensions::PermissionMessageString; -using extensions::PermissionMessageStrings; +using extensions::PermissionIDSet; // Base class for our tests. class ExtensionInstallViewControllerTest : public CocoaProfileTest { @@ -42,9 +43,10 @@ TEST_F(ExtensionInstallViewControllerTest, BasicsNormalCancel) { ExtensionInstallPrompt::PermissionsType type = ExtensionInstallPrompt::PermissionsType::REGULAR_PERMISSIONS; - PermissionMessageStrings permissions; + CoalescedPermissionMessages permissions; permissions.push_back( - PermissionMessageString(base::UTF8ToUTF16("warning 1"))); + CoalescedPermissionMessage(base::UTF8ToUTF16("warning 1"), + PermissionIDSet())); prompt->SetPermissions(permissions, type); base::scoped_nsobject controller( @@ -98,9 +100,10 @@ TEST_F(ExtensionInstallViewControllerTest, BasicsNormalOK) { ExtensionInstallPrompt::PermissionsType type = ExtensionInstallPrompt::PermissionsType::REGULAR_PERMISSIONS; - PermissionMessageStrings permissions; + CoalescedPermissionMessages permissions; permissions.push_back( - PermissionMessageString(base::UTF8ToUTF16("warning 1"))); + CoalescedPermissionMessage(base::UTF8ToUTF16("warning 1"), + PermissionIDSet())); prompt->SetPermissions(permissions, type); base::scoped_nsobject controller( @@ -127,15 +130,17 @@ TEST_F(ExtensionInstallViewControllerTest, MultipleWarnings) { ExtensionInstallPrompt::PermissionsType type = ExtensionInstallPrompt::PermissionsType::REGULAR_PERMISSIONS; - PermissionMessageStrings permissions; + CoalescedPermissionMessages permissions; permissions.push_back( - PermissionMessageString(base::UTF8ToUTF16("warning 1"))); + CoalescedPermissionMessage(base::UTF8ToUTF16("warning 1"), + PermissionIDSet())); one_warning_prompt->SetPermissions(permissions, type); scoped_refptr two_warnings_prompt = chrome::BuildExtensionInstallPrompt(extension_.get()); permissions.push_back( - PermissionMessageString(base::UTF8ToUTF16("warning 2"))); + CoalescedPermissionMessage(base::UTF8ToUTF16("warning 2"), + PermissionIDSet())); two_warnings_prompt->SetPermissions(permissions, type); base::scoped_nsobject controller1( @@ -277,9 +282,10 @@ TEST_F(ExtensionInstallViewControllerTest, PostInstallPermissionsPrompt) { ExtensionInstallPrompt::PermissionsType type = ExtensionInstallPrompt::PermissionsType::REGULAR_PERMISSIONS; - PermissionMessageStrings permissions; + CoalescedPermissionMessages permissions; permissions.push_back( - PermissionMessageString(base::UTF8ToUTF16("warning 1"))); + CoalescedPermissionMessage(base::UTF8ToUTF16("warning 1"), + PermissionIDSet())); prompt->SetPermissions(permissions, type); base::scoped_nsobject controller( @@ -306,10 +312,11 @@ TEST_F(ExtensionInstallViewControllerTest, PermissionsDetails) { ExtensionInstallPrompt::PermissionsType type = ExtensionInstallPrompt::PermissionsType::REGULAR_PERMISSIONS; - PermissionMessageStrings permissions; - permissions.push_back( - PermissionMessageString(base::UTF8ToUTF16("warning 1"), - base::UTF8ToUTF16("Detail 1"))); + CoalescedPermissionMessages permissions; + permissions.push_back(CoalescedPermissionMessage( + base::UTF8ToUTF16("warning 1"), + PermissionIDSet(), + std::vector(1, base::UTF8ToUTF16("Detail 1")))); prompt->SetPermissions(permissions, type); prompt->SetIsShowingDetails( ExtensionInstallPrompt::PERMISSIONS_DETAILS, 0, true); diff --git a/chrome/browser/ui/views/apps/app_info_dialog/app_info_permissions_panel.cc b/chrome/browser/ui/views/apps/app_info_dialog/app_info_permissions_panel.cc index f4b06df3ad1b..97862d6c85fc 100644 --- a/chrome/browser/ui/views/apps/app_info_dialog/app_info_permissions_panel.cc +++ b/chrome/browser/ui/views/apps/app_info_dialog/app_info_permissions_panel.cc @@ -229,9 +229,9 @@ void AppInfoPermissionsPanel::CreatePermissionsList() { // Add regular and host permission messages. for (const auto& message : GetActivePermissionMessages()) { - permissions_list->AddPermissionBullets( - message.message, message.submessages, gfx::ELIDE_MIDDLE, - base::Closure()); + permissions_list->AddPermissionBullets(message.message(), + message.submessages(), + gfx::ELIDE_MIDDLE, base::Closure()); } // Add USB devices, if the app has any. @@ -261,9 +261,9 @@ bool AppInfoPermissionsPanel::HasActivePermissionMessages() const { return !GetActivePermissionMessages().empty(); } -extensions::PermissionMessageStrings +extensions::CoalescedPermissionMessages AppInfoPermissionsPanel::GetActivePermissionMessages() const { - return app_->permissions_data()->GetPermissionMessageStrings(); + return app_->permissions_data()->GetPermissionMessages(); } int AppInfoPermissionsPanel::GetRetainedFileCount() const { diff --git a/chrome/browser/ui/views/apps/app_info_dialog/app_info_permissions_panel.h b/chrome/browser/ui/views/apps/app_info_dialog/app_info_permissions_panel.h index 50293af7b28c..bf8c80bcb0a0 100644 --- a/chrome/browser/ui/views/apps/app_info_dialog/app_info_permissions_panel.h +++ b/chrome/browser/ui/views/apps/app_info_dialog/app_info_permissions_panel.h @@ -50,10 +50,7 @@ class AppInfoPermissionsPanel : public AppInfoPanel { void LayoutPermissionsList(); bool HasActivePermissionMessages() const; - // Returns a list of active permission messages. The first entry is the title - // of the permission; the second is any sub-messages (such as host - // permissions) to be listed underneath that permission. - extensions::PermissionMessageStrings GetActivePermissionMessages() const; + extensions::CoalescedPermissionMessages GetActivePermissionMessages() const; int GetRetainedFileCount() const; base::string16 GetRetainedFileHeading() const; diff --git a/chrome/browser/ui/views/extensions/extension_install_dialog_view_browsertest.cc b/chrome/browser/ui/views/extensions/extension_install_dialog_view_browsertest.cc index 191f96f324e9..d4f1450436af 100644 --- a/chrome/browser/ui/views/extensions/extension_install_dialog_view_browsertest.cc +++ b/chrome/browser/ui/views/extensions/extension_install_dialog_view_browsertest.cc @@ -23,8 +23,9 @@ #include "ui/views/view.h" #include "ui/views/widget/widget.h" -using extensions::PermissionMessageString; -using extensions::PermissionMessageStrings; +using extensions::CoalescedPermissionMessage; +using extensions::CoalescedPermissionMessages; +using extensions::PermissionIDSet; // A simple delegate implementation that counts the number of times // |InstallUIProceed| and |InstallUIAbort| are called. @@ -85,7 +86,7 @@ class ExtensionInstallDialogViewTestBase : public ExtensionBrowserTest { content::WebContents* web_contents() { return web_contents_; } MockExtensionInstallPromptDelegate* delegate() { return &delegate_; } - void SetPromptPermissions(const PermissionMessageStrings& permissions); + void SetPromptPermissions(const CoalescedPermissionMessages& permissions); void SetPromptRetainedFiles(std::vector files); private: @@ -123,12 +124,12 @@ void ExtensionInstallDialogViewTestBase::SetUpOnMainThread() { gfx::Image icon = gfx::Image::CreateFrom1xBitmap(icon_bitmap); prompt_->set_icon(icon); - this->SetPromptPermissions(PermissionMessageStrings()); + this->SetPromptPermissions(CoalescedPermissionMessages()); this->SetPromptRetainedFiles(std::vector()); } void ExtensionInstallDialogViewTestBase::SetPromptPermissions( - const PermissionMessageStrings& permissions) { + const CoalescedPermissionMessages& permissions) { prompt_->SetPermissions(permissions, ExtensionInstallPrompt::REGULAR_PERMISSIONS); } @@ -175,9 +176,11 @@ bool ScrollbarTest::IsScrollbarVisible() { // install prompt. IN_PROC_BROWSER_TEST_F(ScrollbarTest, LongPromptScrollbar) { base::string16 permission_string(base::ASCIIToUTF16("Test")); - PermissionMessageStrings permissions; - for (int i = 0; i < 20; i++) - permissions.push_back(PermissionMessageString(permission_string)); + CoalescedPermissionMessages permissions; + for (int i = 0; i < 20; i++) { + permissions.push_back(CoalescedPermissionMessage(permission_string, + PermissionIDSet())); + } this->SetPromptPermissions(permissions); ASSERT_TRUE(IsScrollbarVisible()) << "Scrollbar is not visible"; } @@ -187,8 +190,9 @@ IN_PROC_BROWSER_TEST_F(ScrollbarTest, LongPromptScrollbar) { IN_PROC_BROWSER_TEST_F(ScrollbarTest, ScrollbarRegression) { base::string16 permission_string(base::ASCIIToUTF16( "Read and modify your data on *.facebook.com")); - PermissionMessageStrings permissions; - permissions.push_back(PermissionMessageString(permission_string)); + CoalescedPermissionMessages permissions; + permissions.push_back(CoalescedPermissionMessage(permission_string, + PermissionIDSet())); this->SetPromptPermissions(permissions); ASSERT_FALSE(IsScrollbarVisible()) << "Scrollbar is visible"; } diff --git a/chrome/common/extensions/permissions/chrome_permission_message_provider.cc b/chrome/common/extensions/permissions/chrome_permission_message_provider.cc index 4b8fceb80e70..21aeb1f78011 100644 --- a/chrome/common/extensions/permissions/chrome_permission_message_provider.cc +++ b/chrome/common/extensions/permissions/chrome_permission_message_provider.cc @@ -32,7 +32,7 @@ ChromePermissionMessageProvider::~ChromePermissionMessageProvider() { } CoalescedPermissionMessages -ChromePermissionMessageProvider::GetCoalescedPermissionMessages( +ChromePermissionMessageProvider::GetPermissionMessages( const PermissionIDSet& permissions) const { std::vector rules = ChromePermissionMessageRule::GetAllRules(); @@ -50,8 +50,7 @@ ChromePermissionMessageProvider::GetCoalescedPermissionMessages( PermissionIDSet used_permissions = remaining_permissions.GetAllPermissionsWithIDs( rule.all_permissions()); - messages.push_back( - rule.formatter()->GetPermissionMessage(used_permissions)); + messages.push_back(rule.GetPermissionMessage(used_permissions)); remaining_permissions = PermissionIDSet::Difference(remaining_permissions, used_permissions); diff --git a/chrome/common/extensions/permissions/chrome_permission_message_provider.h b/chrome/common/extensions/permissions/chrome_permission_message_provider.h index 1208b5e84c13..e2b1661d1c3b 100644 --- a/chrome/common/extensions/permissions/chrome_permission_message_provider.h +++ b/chrome/common/extensions/permissions/chrome_permission_message_provider.h @@ -26,7 +26,7 @@ class ChromePermissionMessageProvider : public PermissionMessageProvider { ~ChromePermissionMessageProvider() override; // PermissionMessageProvider implementation. - CoalescedPermissionMessages GetCoalescedPermissionMessages( + CoalescedPermissionMessages GetPermissionMessages( const PermissionIDSet& permissions) const override; bool IsPrivilegeIncrease(const PermissionSet* old_permissions, const PermissionSet* new_permissions, diff --git a/chrome/common/extensions/permissions/chrome_permission_message_provider_unittest.cc b/chrome/common/extensions/permissions/chrome_permission_message_provider_unittest.cc index eca17058cde8..37e4be9166b6 100644 --- a/chrome/common/extensions/permissions/chrome_permission_message_provider_unittest.cc +++ b/chrome/common/extensions/permissions/chrome_permission_message_provider_unittest.cc @@ -32,12 +32,12 @@ class ChromePermissionMessageProviderUnittest : public testing::Test { ~ChromePermissionMessageProviderUnittest() override {} protected: - PermissionMessageStrings GetMessages(const APIPermissionSet& permissions, - Manifest::Type type) { + CoalescedPermissionMessages GetMessages(const APIPermissionSet& permissions, + Manifest::Type type) { scoped_refptr permission_set = new PermissionSet( permissions, ManifestPermissionSet(), URLPatternSet(), URLPatternSet()); - return message_provider_->GetPermissionMessageStrings(permission_set.get(), - type); + return message_provider_->GetPermissionMessages( + message_provider_->GetAllPermissionIDs(permission_set.get(), type)); } private: @@ -50,32 +50,36 @@ class ChromePermissionMessageProviderUnittest : public testing::Test { // superset permission message is displayed if they are both present. TEST_F(ChromePermissionMessageProviderUnittest, SupersetOverridesSubsetPermission) { - APIPermissionSet permissions; - PermissionMessageStrings messages; - - permissions.clear(); - permissions.insert(APIPermission::kTab); - messages = GetMessages(permissions, Manifest::TYPE_PLATFORM_APP); - ASSERT_EQ(1U, messages.size()); - EXPECT_EQ( - l10n_util::GetStringUTF16(IDS_EXTENSION_PROMPT_WARNING_HISTORY_READ), - messages[0].message); - - permissions.clear(); - permissions.insert(APIPermission::kTopSites); - messages = GetMessages(permissions, Manifest::TYPE_PLATFORM_APP); - ASSERT_EQ(1U, messages.size()); - EXPECT_EQ(l10n_util::GetStringUTF16(IDS_EXTENSION_PROMPT_WARNING_TOPSITES), - messages[0].message); - - permissions.clear(); - permissions.insert(APIPermission::kTab); - permissions.insert(APIPermission::kTopSites); - messages = GetMessages(permissions, Manifest::TYPE_PLATFORM_APP); - ASSERT_EQ(1U, messages.size()); - EXPECT_EQ( - l10n_util::GetStringUTF16(IDS_EXTENSION_PROMPT_WARNING_HISTORY_READ), - messages[0].message); + { + APIPermissionSet permissions; + permissions.insert(APIPermission::kTab); + CoalescedPermissionMessages messages = + GetMessages(permissions, Manifest::TYPE_PLATFORM_APP); + ASSERT_EQ(1U, messages.size()); + EXPECT_EQ( + l10n_util::GetStringUTF16(IDS_EXTENSION_PROMPT_WARNING_HISTORY_READ), + messages.front().message()); + } + { + APIPermissionSet permissions; + permissions.insert(APIPermission::kTopSites); + CoalescedPermissionMessages messages = + GetMessages(permissions, Manifest::TYPE_PLATFORM_APP); + ASSERT_EQ(1U, messages.size()); + EXPECT_EQ(l10n_util::GetStringUTF16(IDS_EXTENSION_PROMPT_WARNING_TOPSITES), + messages.front().message()); + } + { + APIPermissionSet permissions; + permissions.insert(APIPermission::kTab); + permissions.insert(APIPermission::kTopSites); + CoalescedPermissionMessages messages = + GetMessages(permissions, Manifest::TYPE_PLATFORM_APP); + ASSERT_EQ(1U, messages.size()); + EXPECT_EQ( + l10n_util::GetStringUTF16(IDS_EXTENSION_PROMPT_WARNING_HISTORY_READ), + messages.front().message()); + } } // Checks that when permissions are merged into a single message, their details @@ -97,18 +101,21 @@ TEST_F(ChromePermissionMessageProviderUnittest, ASSERT_TRUE(usb->FromValue(devices_list.get(), nullptr, nullptr)); permissions.insert(usb.release()); - PermissionMessageStrings messages = + CoalescedPermissionMessages messages = GetMessages(permissions, Manifest::TYPE_EXTENSION); ASSERT_EQ(2U, messages.size()); + auto it = messages.begin(); + const CoalescedPermissionMessage& message0 = *it++; EXPECT_EQ( l10n_util::GetStringUTF16(IDS_EXTENSION_PROMPT_WARNING_USB_DEVICE_LIST), - messages[0].message); - EXPECT_FALSE(messages[0].submessages.empty()); + message0.message()); + EXPECT_FALSE(message0.submessages().empty()); + const CoalescedPermissionMessage& message1 = *it++; EXPECT_EQ( l10n_util::GetStringUTF16(IDS_EXTENSION_PROMPT_WARNING_HISTORY_READ), - messages[1].message); - EXPECT_TRUE(messages[1].submessages.empty()); + message1.message()); + EXPECT_TRUE(message1.submessages().empty()); } } // namespace extensions diff --git a/chrome/common/extensions/permissions/chrome_permission_message_rules.cc b/chrome/common/extensions/permissions/chrome_permission_message_rules.cc index 1083cff21f2b..41215d6bf2a3 100644 --- a/chrome/common/extensions/permissions/chrome_permission_message_rules.cc +++ b/chrome/common/extensions/permissions/chrome_permission_message_rules.cc @@ -27,7 +27,7 @@ class DefaultPermissionMessageFormatter ~DefaultPermissionMessageFormatter() override {} CoalescedPermissionMessage GetPermissionMessage( - PermissionIDSet permissions) const override { + const PermissionIDSet& permissions) const override { return CoalescedPermissionMessage(l10n_util::GetStringUTF16(message_id_), permissions); } @@ -35,7 +35,7 @@ class DefaultPermissionMessageFormatter private: int message_id_; - // DISALLOW_COPY_AND_ASSIGN(DefaultPermissionMessageFormatter); + DISALLOW_COPY_AND_ASSIGN(DefaultPermissionMessageFormatter); }; // A formatter that substitutes the parameter into the message using string @@ -47,7 +47,7 @@ class SingleParameterFormatter : public ChromePermissionMessageFormatter { ~SingleParameterFormatter() override {} CoalescedPermissionMessage GetPermissionMessage( - PermissionIDSet permissions) const override { + const PermissionIDSet& permissions) const override { DCHECK(permissions.size() > 0); std::vector parameters = permissions.GetAllPermissionParameters(); @@ -59,6 +59,8 @@ class SingleParameterFormatter : public ChromePermissionMessageFormatter { private: int message_id_; + + DISALLOW_COPY_AND_ASSIGN(SingleParameterFormatter); }; // Adds each parameter to a growing list, with the given |root_message_id| as @@ -70,7 +72,7 @@ class SimpleListFormatter : public ChromePermissionMessageFormatter { ~SimpleListFormatter() override {} CoalescedPermissionMessage GetPermissionMessage( - PermissionIDSet permissions) const override { + const PermissionIDSet& permissions) const override { DCHECK(permissions.size() > 0); return CoalescedPermissionMessage( l10n_util::GetStringUTF16(root_message_id_), permissions, @@ -79,6 +81,8 @@ class SimpleListFormatter : public ChromePermissionMessageFormatter { private: int root_message_id_; + + DISALLOW_COPY_AND_ASSIGN(SimpleListFormatter); }; // Creates a space-separated list of permissions with the given PermissionID. @@ -95,7 +99,7 @@ class SpaceSeparatedListFormatter : public ChromePermissionMessageFormatter { ~SpaceSeparatedListFormatter() override {} CoalescedPermissionMessage GetPermissionMessage( - PermissionIDSet permissions) const override { + const PermissionIDSet& permissions) const override { DCHECK(permissions.size() > 0); std::vector hostnames = permissions.GetAllPermissionParameters(); @@ -112,6 +116,8 @@ class SpaceSeparatedListFormatter : public ChromePermissionMessageFormatter { private: int message_id_for_one_host_; int message_id_for_multiple_hosts_; + + DISALLOW_COPY_AND_ASSIGN(SpaceSeparatedListFormatter); }; // Creates a comma-separated list of permissions with the given PermissionID. @@ -133,7 +139,7 @@ class CommaSeparatedListFormatter : public ChromePermissionMessageFormatter { ~CommaSeparatedListFormatter() override {} CoalescedPermissionMessage GetPermissionMessage( - PermissionIDSet permissions) const override { + const PermissionIDSet& permissions) const override { DCHECK(permissions.size() > 0); std::vector hostnames = permissions.GetAllPermissionParameters(); @@ -168,6 +174,8 @@ class CommaSeparatedListFormatter : public ChromePermissionMessageFormatter { int message_id_for_two_hosts_; int message_id_for_three_hosts_; int message_id_for_many_hosts_; + + DISALLOW_COPY_AND_ASSIGN(CommaSeparatedListFormatter); }; } // namespace @@ -201,10 +209,6 @@ std::set ChromePermissionMessageRule::optional_permissions() const { return optional_permissions_; } -ChromePermissionMessageFormatter* ChromePermissionMessageRule::formatter() - const { - return formatter_.get(); -} std::set ChromePermissionMessageRule::all_permissions() const { @@ -212,6 +216,11 @@ std::set ChromePermissionMessageRule::all_permissions() optional_permissions()); } +CoalescedPermissionMessage ChromePermissionMessageRule::GetPermissionMessage( + const PermissionIDSet& permissions) const { + return formatter_->GetPermissionMessage(permissions); +} + // static std::vector ChromePermissionMessageRule::GetAllRules() { @@ -609,11 +618,8 @@ ChromePermissionMessageRule::GetAllRules() { {IDS_EXTENSION_PROMPT_WARNING_FAVICON, {APIPermission::kFavicon}, {}}, }; - std::vector rules; - for (size_t i = 0; i < arraysize(rules_arr); i++) { - rules.push_back(rules_arr[i]); - } - return rules; + return std::vector( + rules_arr, rules_arr + arraysize(rules_arr)); } ChromePermissionMessageRule::PermissionIDSetInitializer:: diff --git a/chrome/common/extensions/permissions/chrome_permission_message_rules.h b/chrome/common/extensions/permissions/chrome_permission_message_rules.h index 689acd8508b3..534322a575ce 100644 --- a/chrome/common/extensions/permissions/chrome_permission_message_rules.h +++ b/chrome/common/extensions/permissions/chrome_permission_message_rules.h @@ -31,10 +31,10 @@ class ChromePermissionMessageFormatter { // |permissions| is guaranteed to have the IDs specified by the // required/optional permissions for the rule. The set will never be empty. virtual CoalescedPermissionMessage GetPermissionMessage( - PermissionIDSet permissions) const = 0; + const PermissionIDSet& permissions) const = 0; private: - // DISALLOW_COPY_AND_ASSIGN(ChromePermissionMessageFormatter); + DISALLOW_COPY_AND_ASSIGN(ChromePermissionMessageFormatter); }; // A simple rule to generate a coalesced permission message that stores the @@ -100,7 +100,9 @@ class ChromePermissionMessageRule { std::set required_permissions() const; std::set optional_permissions() const; std::set all_permissions() const; - ChromePermissionMessageFormatter* formatter() const; + + CoalescedPermissionMessage GetPermissionMessage( + const PermissionIDSet& permissions) const; private: std::set required_permissions_; diff --git a/chrome/common/extensions/permissions/permission_set_unittest.cc b/chrome/common/extensions/permissions/permission_set_unittest.cc index c7ee9d16a628..75002cb7bd9c 100644 --- a/chrome/common/extensions/permissions/permission_set_unittest.cc +++ b/chrome/common/extensions/permissions/permission_set_unittest.cc @@ -41,11 +41,13 @@ static void AddPattern(URLPatternSet* extent, const std::string& pattern) { extent->AddPattern(URLPattern(schemes, pattern)); } -size_t IndexOf(const PermissionMessageStrings& warnings, +size_t IndexOf(const CoalescedPermissionMessages& warnings, const std::string& warning) { - for (size_t i = 0; i < warnings.size(); ++i) { - if (warnings[i].message == base::ASCIIToUTF16(warning)) + size_t i = 0; + for (const CoalescedPermissionMessage& msg : warnings) { + if (msg.message() == base::ASCIIToUTF16(warning)) return i; + ++i; } return warnings.size(); @@ -89,7 +91,7 @@ testing::AssertionResult PermissionSetProducesMessage( const PermissionIDSet& expected_ids) { const PermissionMessageProvider* provider = PermissionMessageProvider::Get(); - CoalescedPermissionMessages msgs = provider->GetCoalescedPermissionMessages( + CoalescedPermissionMessages msgs = provider->GetPermissionMessages( provider->GetAllPermissionIDs(permissions, extension_type)); if (msgs.size() != 1) { return testing::AssertionFailure() @@ -1133,8 +1135,8 @@ TEST(PermissionsTest, GetWarningMessages_AudioVideo) { EXPECT_FALSE(VerifyHasPermissionMessage(set, extension->GetType(), kAudio)); EXPECT_FALSE(VerifyHasPermissionMessage(set, extension->GetType(), kVideo)); EXPECT_TRUE(VerifyHasPermissionMessage(set, extension->GetType(), kBoth)); - PermissionMessageStrings warnings = - provider->GetPermissionMessageStrings(set, extension->GetType()); + CoalescedPermissionMessages warnings = provider->GetPermissionMessages( + provider->GetAllPermissionIDs(set, extension->GetType())); size_t combined_index = IndexOf(warnings, kBoth); size_t combined_size = warnings.size(); @@ -1143,9 +1145,10 @@ TEST(PermissionsTest, GetWarningMessages_AudioVideo) { EXPECT_TRUE(VerifyHasPermissionMessage(set, extension->GetType(), kAudio)); EXPECT_FALSE(VerifyHasPermissionMessage(set, extension->GetType(), kVideo)); EXPECT_FALSE(VerifyHasPermissionMessage(set, extension->GetType(), kBoth)); - warnings = provider->GetPermissionMessageStrings(set, extension->GetType()); - EXPECT_EQ(combined_size, warnings.size()); - EXPECT_EQ(combined_index, IndexOf(warnings, kAudio)); + CoalescedPermissionMessages warnings2 = provider->GetPermissionMessages( + provider->GetAllPermissionIDs(set, extension->GetType())); + EXPECT_EQ(combined_size, warnings2.size()); + EXPECT_EQ(combined_index, IndexOf(warnings2, kAudio)); // Just video present. set->apis_.erase(APIPermission::kAudioCapture); @@ -1153,9 +1156,10 @@ TEST(PermissionsTest, GetWarningMessages_AudioVideo) { EXPECT_FALSE(VerifyHasPermissionMessage(set, extension->GetType(), kAudio)); EXPECT_TRUE(VerifyHasPermissionMessage(set, extension->GetType(), kVideo)); EXPECT_FALSE(VerifyHasPermissionMessage(set, extension->GetType(), kBoth)); - warnings = provider->GetPermissionMessageStrings(set, extension->GetType()); - EXPECT_EQ(combined_size, warnings.size()); - EXPECT_EQ(combined_index, IndexOf(warnings, kVideo)); + CoalescedPermissionMessages warnings3 = provider->GetPermissionMessages( + provider->GetAllPermissionIDs(set, extension->GetType())); + EXPECT_EQ(combined_size, warnings3.size()); + EXPECT_EQ(combined_index, IndexOf(warnings3, kVideo)); } TEST(PermissionsTest, GetWarningMessages_CombinedSessions) { @@ -1761,7 +1765,7 @@ TEST(PermissionsTest, ChromeURLs) { scoped_refptr permissions( new PermissionSet(APIPermissionSet(), ManifestPermissionSet(), allowed_hosts, URLPatternSet())); - PermissionMessageProvider::Get()->GetCoalescedPermissionMessages( + PermissionMessageProvider::Get()->GetPermissionMessages( PermissionMessageProvider::Get()->GetAllPermissionIDs( permissions.get(), Manifest::TYPE_EXTENSION)); } diff --git a/extensions/browser/api/management/management_api.cc b/extensions/browser/api/management/management_api.cc index 9cc75c1387f4..3536c7e7ecf7 100644 --- a/extensions/browser/api/management/management_api.cc +++ b/extensions/browser/api/management/management_api.cc @@ -37,6 +37,7 @@ #include "extensions/common/manifest_handlers/offline_enabled_info.h" #include "extensions/common/manifest_handlers/options_page_info.h" #include "extensions/common/manifest_url_handlers.h" +#include "extensions/common/permissions/coalesced_permission_message.h" #include "extensions/common/permissions/permission_set.h" #include "extensions/common/permissions/permissions_data.h" #include "extensions/common/url_pattern.h" @@ -61,9 +62,9 @@ AutoConfirmForTest auto_confirm_for_test = DO_NOT_SKIP; std::vector CreateWarningsList(const Extension* extension) { std::vector warnings_list; - for (const extensions::PermissionMessageString& str : - extension->permissions_data()->GetPermissionMessageStrings()) { - warnings_list.push_back(base::UTF16ToUTF8(str.message)); + for (const CoalescedPermissionMessage& msg : + extension->permissions_data()->GetPermissionMessages()) { + warnings_list.push_back(base::UTF16ToUTF8(msg.message())); } return warnings_list; diff --git a/extensions/common/permissions/coalesced_permission_message.h b/extensions/common/permissions/coalesced_permission_message.h index c1d3caf7ea55..2848091e36a0 100644 --- a/extensions/common/permissions/coalesced_permission_message.h +++ b/extensions/common/permissions/coalesced_permission_message.h @@ -7,6 +7,7 @@ #include #include +#include #include "extensions/common/permissions/api_permission_set.h" @@ -59,8 +60,8 @@ class CoalescedPermissionMessage { const std::vector submessages_; }; -// Use a linked list to store our list of messages, since we will commonly be -// iterating/removing elements but should never be accessing by index. +// TODO(treib): Make this an std::vector when we have C++11 library support on +// all platforms. (In C++03, std::vector's elements must be copy-assignable...) typedef std::list CoalescedPermissionMessages; } // namespace extensions diff --git a/extensions/common/permissions/permission_message.h b/extensions/common/permissions/permission_message.h index d95af255c73b..e4df834943d5 100644 --- a/extensions/common/permissions/permission_message.h +++ b/extensions/common/permissions/permission_message.h @@ -151,7 +151,6 @@ class PermissionMessage { }; typedef std::vector PermissionMessages; -typedef std::vector PermissionMessageIDs; } // namespace extensions diff --git a/extensions/common/permissions/permission_message_provider.cc b/extensions/common/permissions/permission_message_provider.cc dissimilarity index 68% index 206236d20bce..6f63133a7a5c 100644 --- a/extensions/common/permissions/permission_message_provider.cc +++ b/extensions/common/permissions/permission_message_provider.cc @@ -1,54 +1,18 @@ -// Copyright 2013 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 "extensions/common/permissions/permission_message_provider.h" - -#include "base/metrics/field_trial.h" -#include "base/strings/string_split.h" -#include "extensions/common/extensions_client.h" - -namespace extensions { - -PermissionMessageString::PermissionMessageString( - const CoalescedPermissionMessage& message) - : message(message.message()), submessages(message.submessages()) { -} - -PermissionMessageString::PermissionMessageString(const base::string16& message) - : message(message) { -} - -PermissionMessageString::PermissionMessageString( - const base::string16& message, - const std::vector& submessages) - : message(message), submessages(submessages) { -} - -PermissionMessageString::PermissionMessageString(const base::string16& message, - const base::string16& details) - : message(message) { - base::SplitString(details, base::char16('\n'), &submessages); -} - -PermissionMessageString::~PermissionMessageString() { -} - -// static -const PermissionMessageProvider* PermissionMessageProvider::Get() { - return &(ExtensionsClient::Get()->GetPermissionMessageProvider()); -} - -PermissionMessageStrings -PermissionMessageProvider::GetPermissionMessageStrings( - const PermissionSet* permissions, - Manifest::Type extension_type) const { - CoalescedPermissionMessages messages = GetCoalescedPermissionMessages( - GetAllPermissionIDs(permissions, extension_type)); - PermissionMessageStrings strings; - for (const CoalescedPermissionMessage& msg : messages) - strings.push_back(PermissionMessageString(msg)); - return strings; -} - -} // namespace extensions +// Copyright 2013 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 "extensions/common/permissions/permission_message_provider.h" + +#include "base/metrics/field_trial.h" +#include "base/strings/string_split.h" +#include "extensions/common/extensions_client.h" + +namespace extensions { + +// static +const PermissionMessageProvider* PermissionMessageProvider::Get() { + return &(ExtensionsClient::Get()->GetPermissionMessageProvider()); +} + +} // namespace extensions diff --git a/extensions/common/permissions/permission_message_provider.h b/extensions/common/permissions/permission_message_provider.h index 71a914db64c8..0a6b7e05734d 100644 --- a/extensions/common/permissions/permission_message_provider.h +++ b/extensions/common/permissions/permission_message_provider.h @@ -9,30 +9,12 @@ #include "extensions/common/manifest.h" #include "extensions/common/permissions/coalesced_permission_message.h" -#include "extensions/common/permissions/permission_message.h" namespace extensions { class PermissionIDSet; class PermissionSet; -// Temporary type to help the transition between old and new system. -// Essentially a CoalescedPermissionMessage minus the IDs. -// TODO(treib): Remove this once we've switched to the new system. -struct PermissionMessageString { - PermissionMessageString(const CoalescedPermissionMessage& message); - PermissionMessageString(const base::string16& message); - PermissionMessageString(const base::string16& message, - const std::vector& submessages); - PermissionMessageString(const base::string16& message, - const base::string16& details); - ~PermissionMessageString(); - - base::string16 message; - std::vector submessages; -}; -typedef std::vector PermissionMessageStrings; - // The PermissionMessageProvider interprets permissions, translating them // into warning messages to show to the user. It also determines whether // a new set of permissions entails showing new warning messages. @@ -45,19 +27,10 @@ class PermissionMessageProvider { static const PermissionMessageProvider* Get(); // Calculates and returns the full list of permission messages for the given - // |permissions|. This forwards to GetCoalescedPermissionMessages. - // TODO(treib): Remove this and update all callers to use - // GetCoalescedPermissionMessages directly. - PermissionMessageStrings GetPermissionMessageStrings( - const PermissionSet* permissions, - Manifest::Type extension_type) const; - - // Calculates and returns the full list of permission messages for the given // |permissions|. This involves converting the given PermissionIDs into // localized messages, as well as coalescing and parameterizing any messages // that require the permission ID's argument in their message. - // TODO(sashab): Rename this to GetPermissionMessages(). - virtual CoalescedPermissionMessages GetCoalescedPermissionMessages( + virtual CoalescedPermissionMessages GetPermissionMessages( const PermissionIDSet& permissions) const = 0; // Returns true if |new_permissions| has a greater privilege level than diff --git a/extensions/common/permissions/permission_message_test_util.cc b/extensions/common/permissions/permission_message_test_util.cc index 055596a04aef..77ee5d6a38db 100644 --- a/extensions/common/permissions/permission_message_test_util.cc +++ b/extensions/common/permissions/permission_message_test_util.cc @@ -17,15 +17,11 @@ namespace extensions { namespace { -PermissionMessageStrings GetNewMessages( - const PermissionsData* permissions_data) { - return permissions_data->GetPermissionMessageStrings(); -} - -PermissionMessageStrings GetNewMessages(const PermissionSet* permissions, +CoalescedPermissionMessages GetMessages(const PermissionSet* permissions, Manifest::Type extension_type) { - return PermissionMessageProvider::Get()->GetPermissionMessageStrings( - permissions, extension_type); + const PermissionMessageProvider* provider = PermissionMessageProvider::Get(); + return provider->GetPermissionMessages( + provider->GetAllPermissionIDs(permissions, extension_type)); } std::vector MakeVectorString16(const base::string16& str) { @@ -77,21 +73,20 @@ base::string16 MessagesVectorToString( base::ASCIIToUTF16("\"\n"); } -base::string16 MessagesToString(const PermissionMessageStrings& messages) { +base::string16 MessagesToString(const CoalescedPermissionMessages& messages) { std::vector messages_vec; - for (const PermissionMessageString& msg : messages) - messages_vec.push_back(msg.message); + for (const CoalescedPermissionMessage& msg : messages) + messages_vec.push_back(msg.message()); return MessagesVectorToString(messages_vec); } bool CheckThatSubmessagesMatch( - const PermissionMessageString& expected_message, + const base::string16& message, + const std::vector& expected_submessages, const std::vector& actual_submessages) { bool result = true; - std::vector expected_sorted; - for (const base::string16& submessage : expected_message.submessages) - expected_sorted.push_back(submessage); + std::vector expected_sorted(expected_submessages); std::sort(expected_sorted.begin(), expected_sorted.end()); std::vector actual_sorted(actual_submessages); @@ -100,9 +95,9 @@ bool CheckThatSubmessagesMatch( // This is always a failure, even within an EXPECT_FALSE. // Message: Expected submessages for "Message" to be { "Foo" }, but got // { "Bar", "Baz" } - ADD_FAILURE() << "Expected submessages for \"" << expected_message.message - << "\" to be:\n" << MessagesVectorToString(expected_sorted) - << "But got:\n" << MessagesVectorToString(actual_sorted); + ADD_FAILURE() << "Expected submessages for \"" << message << "\" to be:\n" + << MessagesVectorToString(expected_sorted) << "But got:\n" + << MessagesVectorToString(actual_sorted); result = false; } @@ -110,65 +105,70 @@ bool CheckThatSubmessagesMatch( } testing::AssertionResult VerifyHasPermissionMessageImpl( - const PermissionMessageString& expected_message, - const PermissionMessageStrings& actual_messages) { + const base::string16& expected_message, + const std::vector& expected_submessages, + const CoalescedPermissionMessages& actual_messages) { auto message_it = std::find_if(actual_messages.begin(), actual_messages.end(), - [&expected_message](const PermissionMessageString& msg) { - return msg.message == expected_message.message; + [&expected_message](const CoalescedPermissionMessage& msg) { + return msg.message() == expected_message; }); bool found = message_it != actual_messages.end(); if (!found) { // Message: Expected messages to contain "Foo", but got { "Bar", "Baz" } return testing::AssertionFailure() << "Expected messages to contain \"" - << expected_message.message - << "\", but got " + << expected_message << "\", but got " << MessagesToString(actual_messages); } - if (!CheckThatSubmessagesMatch(expected_message, message_it->submessages)) { + if (!CheckThatSubmessagesMatch(expected_message, expected_submessages, + message_it->submessages())) { return testing::AssertionFailure(); } // Message: Expected messages NOT to contain "Foo", but got { "Bar", "Baz" } return testing::AssertionSuccess() << "Expected messages NOT to contain \"" - << expected_message.message - << "\", but got " + << expected_message << "\", but got " << MessagesToString(actual_messages); } testing::AssertionResult VerifyPermissionMessagesWithSubmessagesImpl( - const PermissionMessageStrings& expected_messages, - const PermissionMessageStrings& actual_messages, + const std::vector& expected_messages, + const std::vector>& expected_submessages, + const CoalescedPermissionMessages& actual_messages, bool check_order) { + CHECK_EQ(expected_messages.size(), expected_submessages.size()); if (expected_messages.size() != actual_messages.size()) { // Message: Expected 2 messages { "Bar", "Baz" }, but got 0 {} return testing::AssertionFailure() << "Expected " << expected_messages.size() << " messages:\n" - << MessagesToString(expected_messages) << "But got " + << MessagesVectorToString(expected_messages) << "But got " << actual_messages.size() << " messages:\n" << MessagesToString(actual_messages); } if (check_order) { - for (size_t i = 0; i < expected_messages.size(); i++) { - if (expected_messages[i].message != actual_messages[i].message) { + auto it = actual_messages.begin(); + for (size_t i = 0; i < expected_messages.size(); i++, ++it) { + const CoalescedPermissionMessage& actual_message = *it; + if (expected_messages[i] != actual_message.message()) { // Message: Expected messages to be { "Foo" }, but got { "Bar", "Baz" } return testing::AssertionFailure() << "Expected messages to be:\n" - << MessagesToString(expected_messages) << "But got:\n" + << MessagesVectorToString(expected_messages) << "But got:\n" << MessagesToString(actual_messages); } if (!CheckThatSubmessagesMatch(expected_messages[i], - actual_messages[i].submessages)) { + expected_submessages[i], + actual_message.submessages())) { return testing::AssertionFailure(); } } } else { for (size_t i = 0; i < expected_messages.size(); i++) { - testing::AssertionResult result = - VerifyHasPermissionMessageImpl(expected_messages[i], actual_messages); + testing::AssertionResult result = VerifyHasPermissionMessageImpl( + expected_messages[i], expected_submessages[i], actual_messages); if (!result) return result; } @@ -189,8 +189,8 @@ testing::AssertionResult VerifyHasPermissionMessage( const PermissionsData* permissions_data, const base::string16& expected_message) { return VerifyHasPermissionMessageImpl( - PermissionMessageString(expected_message), - GetNewMessages(permissions_data)); + expected_message, std::vector(), + permissions_data->GetPermissionMessages()); } testing::AssertionResult VerifyHasPermissionMessage( @@ -206,8 +206,8 @@ testing::AssertionResult VerifyHasPermissionMessage( Manifest::Type extension_type, const base::string16& expected_message) { return VerifyHasPermissionMessageImpl( - PermissionMessageString(expected_message), - GetNewMessages(permissions, extension_type)); + expected_message, std::vector(), + GetMessages(permissions, extension_type)); } testing::AssertionResult VerifyNoPermissionMessages( @@ -235,8 +235,9 @@ testing::AssertionResult VerifyOnePermissionMessage( Manifest::Type extension_type, const base::string16& expected_message) { return VerifyPermissionMessagesWithSubmessagesImpl( - PermissionMessageStrings(1, PermissionMessageString(expected_message)), - GetNewMessages(permissions, extension_type), true); + MakeVectorString16(expected_message), + std::vector>(1), + GetMessages(permissions, extension_type), true); } testing::AssertionResult VerifyOnePermissionMessageWithSubmessages( @@ -311,13 +312,9 @@ testing::AssertionResult VerifyPermissionMessagesWithSubmessages( const std::vector>& expected_submessages, bool check_order) { CHECK_EQ(expected_messages.size(), expected_submessages.size()); - PermissionMessageStrings expected; - for (size_t i = 0; i < expected_messages.size(); i++) { - expected.push_back(PermissionMessageString(expected_messages[i], - expected_submessages[i])); - } return VerifyPermissionMessagesWithSubmessagesImpl( - expected, GetNewMessages(permissions_data), check_order); + expected_messages, expected_submessages, + permissions_data->GetPermissionMessages(), check_order); } } // namespace extensions diff --git a/extensions/common/permissions/permissions_data.cc b/extensions/common/permissions/permissions_data.cc index abc4883c1048..694ccfcf030a 100644 --- a/extensions/common/permissions/permissions_data.cc +++ b/extensions/common/permissions/permissions_data.cc @@ -194,16 +194,8 @@ bool PermissionsData::HasEffectiveAccessToAllHosts() const { return active_permissions()->HasEffectiveAccessToAllHosts(); } -PermissionMessageStrings PermissionsData::GetPermissionMessageStrings() const { - if (ShouldSkipPermissionWarnings(extension_id_)) - return PermissionMessageStrings(); - return PermissionMessageProvider::Get()->GetPermissionMessageStrings( - active_permissions().get(), manifest_type_); -} - -CoalescedPermissionMessages PermissionsData::GetCoalescedPermissionMessages() - const { - return PermissionMessageProvider::Get()->GetCoalescedPermissionMessages( +CoalescedPermissionMessages PermissionsData::GetPermissionMessages() const { + return PermissionMessageProvider::Get()->GetPermissionMessages( PermissionMessageProvider::Get()->GetAllPermissionIDs( active_permissions().get(), manifest_type_)); } diff --git a/extensions/common/permissions/permissions_data.h b/extensions/common/permissions/permissions_data.h index 93b3bb7936ba..ed55c2bb2302 100644 --- a/extensions/common/permissions/permissions_data.h +++ b/extensions/common/permissions/permissions_data.h @@ -130,15 +130,9 @@ class PermissionsData { // network, etc.) bool HasEffectiveAccessToAllHosts() const; - // Returns the full list of permission messages that should display at install - // time, including their submessages, as strings. - // TODO(treib): Remove this and move callers over to - // GetCoalescedPermissionMessages once we've fully switched to the new system. - PermissionMessageStrings GetPermissionMessageStrings() const; - // Returns the full list of permission details for messages that should // display at install time, in a nested format ready for display. - CoalescedPermissionMessages GetCoalescedPermissionMessages() const; + CoalescedPermissionMessages GetPermissionMessages() const; // Returns true if the extension has requested all-hosts permissions (or // something close to it), but has had it withheld. diff --git a/extensions/shell/common/shell_extensions_client.cc b/extensions/shell/common/shell_extensions_client.cc index 5c8c99813f1b..9b85bf3b5591 100644 --- a/extensions/shell/common/shell_extensions_client.cc +++ b/extensions/shell/common/shell_extensions_client.cc @@ -42,7 +42,7 @@ class ShellPermissionMessageProvider : public PermissionMessageProvider { ~ShellPermissionMessageProvider() override {} // PermissionMessageProvider implementation. - CoalescedPermissionMessages GetCoalescedPermissionMessages( + CoalescedPermissionMessages GetPermissionMessages( const PermissionIDSet& permissions) const override { return CoalescedPermissionMessages(); } diff --git a/extensions/test/test_permission_message_provider.cc b/extensions/test/test_permission_message_provider.cc index 64174326ae69..d05b323b8e63 100644 --- a/extensions/test/test_permission_message_provider.cc +++ b/extensions/test/test_permission_message_provider.cc @@ -13,7 +13,7 @@ TestPermissionMessageProvider::~TestPermissionMessageProvider() { } CoalescedPermissionMessages -TestPermissionMessageProvider::GetCoalescedPermissionMessages( +TestPermissionMessageProvider::GetPermissionMessages( const PermissionIDSet& permissions) const { return CoalescedPermissionMessages(); } diff --git a/extensions/test/test_permission_message_provider.h b/extensions/test/test_permission_message_provider.h index 6fcdd8a48702..7a095a33bd1b 100644 --- a/extensions/test/test_permission_message_provider.h +++ b/extensions/test/test_permission_message_provider.h @@ -16,7 +16,7 @@ class TestPermissionMessageProvider : public PermissionMessageProvider { ~TestPermissionMessageProvider() override; private: - CoalescedPermissionMessages GetCoalescedPermissionMessages( + CoalescedPermissionMessages GetPermissionMessages( const PermissionIDSet& permissions) const override; bool IsPrivilegeIncrease(const PermissionSet* old_permissions, const PermissionSet* new_permissions, -- 2.11.4.GIT