Extensions: Prepare switch to new permission message system

PermissionMessageProvider:
- add GetPermissionMessageStrings which calls either the old or the new system, based on a field trial
- rename GetWarningMessages/GetWarningMessagesDetails to *Legacy*
- replace GetPermissionMessages by GetLegacyPermissionMessageIDs -> remove the old PermissionMessages type from the interface

Callers have *not* been updated to use the new GetPermissionMessageStrings yet.

TBRing trivial change in ephemeral_app_launcher.cc
TBR=asargent@chromium.org

BUG=398257

Review URL: https://codereview.chromium.org/1006453002

Cr-Commit-Position: refs/heads/master@{#320702}

chrome/browser/apps/ephemeral_app_launcher.cc

@@ -393,9 +393,7 @@ EphemeralAppLauncher::CreateInstallPrompt() const {
 
   // Skip the prompt by returning null if the app does not need to display
   // permission warnings.
-  extensions::PermissionMessages permissions =
-      extension->permissions_data()->GetPermissionMessages();
-  if (permissions.empty())
+  if (extension->permissions_data()->GetPermissionMessageStrings().empty())
     return NULL;
 
   return make_scoped_refptr(new ExtensionInstallPrompt::Prompt(

chrome/browser/extensions/api/permissions/permissions_api.cc

@@ -218,10 +218,11 @@ 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.
-  bool has_no_warnings = PermissionMessageProvider::Get()
-                             ->GetWarningMessages(requested_permissions_.get(),
-                                                  extension()->GetType())
-                             .empty();
+  bool has_no_warnings =
+      PermissionMessageProvider::Get()
+          ->GetLegacyWarningMessages(requested_permissions_.get(),
+                                     extension()->GetType())
+          .empty();
   if (auto_confirm_for_tests == PROCEED || has_no_warnings ||
       extension_->location() == Manifest::COMPONENT) {
     InstallUIProceed();

chrome/browser/extensions/extension_disabled_ui.cc

@@ -293,7 +293,7 @@ std::vector<base::string16>
 ExtensionDisabledGlobalError::GetBubbleViewMessages() {
   std::vector<base::string16> messages;
   std::vector<base::string16> permission_warnings =
-      extensions::PermissionMessageProvider::Get()->GetWarningMessages(
+      extensions::PermissionMessageProvider::Get()->GetLegacyWarningMessages(
           extension_->permissions_data()->active_permissions().get(),
           extension_->GetType());
   if (is_remote_install_) {

chrome/browser/extensions/extension_install_prompt.cc

@@ -928,21 +928,23 @@ void ExtensionInstallPrompt::ShowConfirmation() {
         extension_ ? extension_->GetType() : Manifest::TYPE_UNKNOWN;
     const extensions::PermissionMessageProvider* message_provider =
         extensions::PermissionMessageProvider::Get();
-    prompt_->SetPermissions(message_provider->GetWarningMessages(
+    prompt_->SetPermissions(message_provider->GetLegacyWarningMessages(
                                 permissions_to_display.get(), type),
                             REGULAR_PERMISSIONS);
-    prompt_->SetPermissionsDetails(message_provider->GetWarningMessagesDetails(
-                                       permissions_to_display.get(), type),
-                                   REGULAR_PERMISSIONS);
+    prompt_->SetPermissionsDetails(
+        message_provider->GetLegacyWarningMessagesDetails(
+            permissions_to_display.get(), type),
+        REGULAR_PERMISSIONS);
 
     scoped_refptr<const extensions::PermissionSet> withheld =
         extension_->permissions_data()->withheld_permissions();
     if (!withheld->IsEmpty()) {
       prompt_->SetPermissions(
-          message_provider->GetWarningMessages(withheld.get(), type),
+          message_provider->GetLegacyWarningMessages(withheld.get(), type),
           PermissionsType::WITHHELD_PERMISSIONS);
       prompt_->SetPermissionsDetails(
-          message_provider->GetWarningMessagesDetails(withheld.get(), type),
+          message_provider->GetLegacyWarningMessagesDetails(withheld.get(),
+                                                            type),
           PermissionsType::WITHHELD_PERMISSIONS);
     }
   }

chrome/browser/extensions/extension_service.cc

@@ -109,7 +109,7 @@ using extensions::InstallVerifier;
 using extensions::ManagementPolicy;
 using extensions::Manifest;
 using extensions::PermissionMessage;
-using extensions::PermissionMessages;
+using extensions::PermissionMessageIDs;
 using extensions::PermissionSet;
 using extensions::SharedModuleInfo;
 using extensions::SharedModuleService;
@@ -1017,14 +1017,13 @@ void ExtensionService::RecordPermissionMessagesHistogram(
       PermissionMessage::kEnumBoundary + 1,
       base::HistogramBase::kUmaTargetedHistogramFlag);
 
-  PermissionMessages permissions =
-      extension->permissions_data()->GetPermissionMessages();
+  PermissionMessageIDs permissions =
+      extension->permissions_data()->GetLegacyPermissionMessageIDs();
   if (permissions.empty()) {
     counter->Add(PermissionMessage::kNone);
   } else {
-    for (PermissionMessages::iterator it = permissions.begin();
-         it != permissions.end(); ++it)
-      counter->Add(it->id());
+    for (PermissionMessage::ID id : permissions)
+      counter->Add(id);
   }
 }
 

chrome/browser/extensions/permission_messages_unittest.cc

@@ -113,8 +113,8 @@ class PermissionMessagesUnittest : public testing::Test {
  private:
   std::vector<base::string16> GetMessages(
       scoped_refptr<const PermissionSet> permissions) {
-    return message_provider_->GetWarningMessages(permissions.get(),
-                                                 app_->GetType());
+    return message_provider_->GetLegacyWarningMessages(permissions.get(),
+                                                       app_->GetType());
   }
 
   extensions::TestExtensionEnvironment env_;

chrome/common/extensions/permissions/chrome_permission_message_provider.cc

@@ -5,6 +5,7 @@
 #include "chrome/common/extensions/permissions/chrome_permission_message_provider.h"
 
 #include "base/memory/scoped_vector.h"
+#include "base/metrics/field_trial.h"
 #include "base/stl_util.h"
 #include "base/strings/string_util.h"
 #include "base/strings/stringprintf.h"
@@ -24,6 +25,12 @@ namespace extensions {
 
 namespace {
 
+bool IsNewPermissionMessageSystemEnabled() {
+  const std::string group_name =
+      base::FieldTrialList::FindFullName("PermissionMessageSystem");
+  return group_name == "NewSystem";
+}
+
 typedef std::set<PermissionMessage> PermissionMsgSet;
 
 template<typename T>
@@ -155,6 +162,40 @@ PermissionMessages ChromePermissionMessageProvider::GetPermissionMessages(
   return messages;
 }
 
+PermissionMessageStrings
+ChromePermissionMessageProvider::GetPermissionMessageStrings(
+    const PermissionSet* permissions,
+    Manifest::Type extension_type) const {
+  PermissionMessageStrings strings;
+  if (IsNewPermissionMessageSystemEnabled()) {
+    CoalescedPermissionMessages messages = GetCoalescedPermissionMessages(
+        GetAllPermissionIDs(permissions, extension_type));
+    for (const CoalescedPermissionMessage& msg : messages)
+      strings.push_back(PermissionMessageString(msg));
+  } else {
+    std::vector<base::string16> messages =
+        GetLegacyWarningMessages(permissions, extension_type);
+    std::vector<base::string16> details =
+        GetLegacyWarningMessagesDetails(permissions, extension_type);
+    DCHECK_EQ(messages.size(), details.size());
+    for (size_t i = 0; i < messages.size(); i++)
+      strings.push_back(PermissionMessageString(messages[i], details[i]));
+  }
+  return strings;
+}
+
+PermissionMessageIDs
+ChromePermissionMessageProvider::GetLegacyPermissionMessageIDs(
+    const PermissionSet* permissions,
+    Manifest::Type extension_type) const {
+  PermissionMessageIDs ids;
+  for (const PermissionMessage& msg :
+       GetPermissionMessages(permissions, extension_type)) {
+    ids.push_back(msg.id());
+  }
+  return ids;
+}
+
 CoalescedPermissionMessages
 ChromePermissionMessageProvider::GetCoalescedPermissionMessages(
     const PermissionIDSet& permissions) const {
@@ -185,7 +226,8 @@ ChromePermissionMessageProvider::GetCoalescedPermissionMessages(
   return messages;
 }
 
-std::vector<base::string16> ChromePermissionMessageProvider::GetWarningMessages(
+std::vector<base::string16>
+ChromePermissionMessageProvider::GetLegacyWarningMessages(
     const PermissionSet* permissions,
     Manifest::Type extension_type) const {
   std::vector<base::string16> message_strings;
@@ -198,7 +240,7 @@ std::vector<base::string16> ChromePermissionMessageProvider::GetWarningMessages(
 }
 
 std::vector<base::string16>
-ChromePermissionMessageProvider::GetWarningMessagesDetails(
+ChromePermissionMessageProvider::GetLegacyWarningMessagesDetails(
     const PermissionSet* permissions,
     Manifest::Type extension_type) const {
   std::vector<base::string16> message_strings;

chrome/common/extensions/permissions/chrome_permission_message_provider.h

@@ -26,15 +26,21 @@ class ChromePermissionMessageProvider : public PermissionMessageProvider {
   ~ChromePermissionMessageProvider() override;
 
   // PermissionMessageProvider implementation.
-  PermissionMessages GetPermissionMessages(
+  // See comments in permission_message_provider.h. TL;DR: You want to use only
+  // GetPermissionMessageStrings to get messages, not the *Legacy* or
+  // *Coalesced* methods.
+  PermissionMessageStrings GetPermissionMessageStrings(
+      const PermissionSet* permissions,
+      Manifest::Type extension_type) const override;
+  PermissionMessageIDs GetLegacyPermissionMessageIDs(
       const PermissionSet* permissions,
       Manifest::Type extension_type) const override;
   CoalescedPermissionMessages GetCoalescedPermissionMessages(
       const PermissionIDSet& permissions) const override;
-  std::vector<base::string16> GetWarningMessages(
+  std::vector<base::string16> GetLegacyWarningMessages(
       const PermissionSet* permissions,
       Manifest::Type extension_type) const override;
-  std::vector<base::string16> GetWarningMessagesDetails(
+  std::vector<base::string16> GetLegacyWarningMessagesDetails(
       const PermissionSet* permissions,
       Manifest::Type extension_type) const override;
   bool IsPrivilegeIncrease(const PermissionSet* old_permissions,
@@ -45,6 +51,10 @@ class ChromePermissionMessageProvider : public PermissionMessageProvider {
       Manifest::Type extension_type) const override;
 
  private:
+  // TODO(treib): Remove this once we've switched to the new system.
+  PermissionMessages GetPermissionMessages(const PermissionSet* permissions,
+                                           Manifest::Type extension_type) const;
+
   // Gets the permission messages for the API permissions. Also adds any
   // permission IDs from API Permissions to |permission_ids|.
   // TODO(sashab): Deprecate the |permissions| argument, and rename this to

chrome/common/extensions/permissions/chrome_permission_message_provider_unittest.cc

@@ -36,15 +36,16 @@ class ChromePermissionMessageProviderUnittest : public testing::Test {
                                           Manifest::Type type) {
     scoped_refptr<const PermissionSet> permission_set = new PermissionSet(
         permissions, ManifestPermissionSet(), URLPatternSet(), URLPatternSet());
-    return message_provider_->GetWarningMessages(permission_set.get(), type);
+    return message_provider_->GetLegacyWarningMessages(permission_set.get(),
+                                                       type);
   }
 
   std::vector<base::string16> GetDetails(const APIPermissionSet& permissions,
                                          Manifest::Type type) {
     scoped_refptr<const PermissionSet> permission_set = new PermissionSet(
         permissions, ManifestPermissionSet(), URLPatternSet(), URLPatternSet());
-    return message_provider_->GetWarningMessagesDetails(permission_set.get(),
-                                                        type);
+    return message_provider_->GetLegacyWarningMessagesDetails(
+        permission_set.get(), type);
   }
 
  private:

extensions/browser/api/management/management_api.cc

@@ -61,11 +61,9 @@ AutoConfirmForTest auto_confirm_for_test = DO_NOT_SKIP;
 
 std::vector<std::string> CreateWarningsList(const Extension* extension) {
   std::vector<std::string> warnings_list;
-  PermissionMessages warnings =
-      extension->permissions_data()->GetPermissionMessages();
-  for (PermissionMessages::const_iterator iter = warnings.begin();
-       iter != warnings.end(); ++iter) {
-    warnings_list.push_back(base::UTF16ToUTF8(iter->message()));
+  for (const base::string16& warning :
+       extension->permissions_data()->GetPermissionMessageStrings()) {
+    warnings_list.push_back(base::UTF16ToUTF8(warning));
   }
 
   return warnings_list;

extensions/common/permissions/permission_message.h

@@ -145,6 +145,7 @@ class PermissionMessage {
 };
 
 typedef std::vector<PermissionMessage> PermissionMessages;
+typedef std::vector<PermissionMessage::ID> PermissionMessageIDs;
 
 }  // namespace extensions
 

extensions/common/permissions/permission_message_provider.cc

@@ -4,10 +4,25 @@
 
 #include "extensions/common/permissions/permission_message_provider.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,
+                                                 const base::string16& details)
+    : message(message) {
+  base::SplitString(details, base::char16('\n'), &submessages);
+}
+
+PermissionMessageString::~PermissionMessageString() {
+}
+
 // static
 const PermissionMessageProvider* PermissionMessageProvider::Get() {
   return &(ExtensionsClient::Get()->GetPermissionMessageProvider());

extensions/common/permissions/permission_message_provider.h

@@ -16,6 +16,20 @@ 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,
+                          const base::string16& details);
+  ~PermissionMessageString();
+
+  base::string16 message;
+  std::vector<base::string16> submessages;
+};
+typedef std::vector<PermissionMessageString> 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.
@@ -27,11 +41,20 @@ class PermissionMessageProvider {
   // Return the global permission message provider.
   static const PermissionMessageProvider* Get();
 
-  // Gets the localized permission messages that represent this set.
-  // The set of permission messages shown varies by extension type.
-  // TODO(sashab): Deprecate this method in favor of
-  // GetCoalescedPermissionMessages() instead.
-  virtual PermissionMessages GetPermissionMessages(
+  // Calculates and returns the full list of permission messages for the given
+  // |permissions|. This forwards to the new GetCoalescedPermissionMessages or
+  // to the old GetWarningMessages/GetWarningMessagesDetails, depending on a
+  // cmdline flag.
+  // TODO(treib): Remove this once we've switched to the new system, and update
+  // all callers to use GetCoalescedPermissionMessages directly.
+  virtual PermissionMessageStrings GetPermissionMessageStrings(
+      const PermissionSet* permissions,
+      Manifest::Type extension_type) const = 0;
+
+  // Gets the legacy permission message IDs that represent this set.
+  // Deprecated. You DO NOT want to call this!
+  // TODO(treib): Remove this once we've switched to the new system.
+  virtual PermissionMessageIDs GetLegacyPermissionMessageIDs(
       const PermissionSet* permissions,
       Manifest::Type extension_type) const = 0;
 
@@ -48,18 +71,18 @@ class PermissionMessageProvider {
   // as strings). The set of permission messages shown varies by extension type.
   // Any permissions added by API, host or manifest permissions need to be added
   // to |permissions| before this function is called.
-  // TODO(sashab): Deprecate this method in favor of
-  // GetCoalescedPermissionMessages().
-  virtual std::vector<base::string16> GetWarningMessages(
+  // Deprecated; use GetPermissionMessageStrings instead.
+  // TODO(treib): Remove this once we've switched to the new system.
+  virtual std::vector<base::string16> GetLegacyWarningMessages(
       const PermissionSet* permissions,
       Manifest::Type extension_type) const = 0;
 
   // Gets the localized permission details for messages that represent this set
   // (represented as strings). The set of permission messages shown varies by
   // extension type.
-  // TODO(sashab): Deprecate this method in favor of
-  // GetCoalescedPermissionMessages().
-  virtual std::vector<base::string16> GetWarningMessagesDetails(
+  // Deprecated; use GetPermissionMessageStrings instead.
+  // TODO(treib): Remove this once we've switched to the new system.
+  virtual std::vector<base::string16> GetLegacyWarningMessagesDetails(
       const PermissionSet* permissions,
       Manifest::Type extension_type) const = 0;
 

extensions/common/permissions/permissions_data.cc

@@ -196,11 +196,11 @@ bool PermissionsData::HasEffectiveAccessToAllHosts() const {
   return active_permissions()->HasEffectiveAccessToAllHosts();
 }
 
-PermissionMessages PermissionsData::GetPermissionMessages() const {
+PermissionMessageIDs PermissionsData::GetLegacyPermissionMessageIDs() const {
   if (ShouldSkipPermissionWarnings(extension_id_)) {
-    return PermissionMessages();
+    return PermissionMessageIDs();
   } else {
-    return PermissionMessageProvider::Get()->GetPermissionMessages(
+    return PermissionMessageProvider::Get()->GetLegacyPermissionMessageIDs(
         active_permissions().get(), manifest_type_);
   }
 }
@@ -209,7 +209,7 @@ std::vector<base::string16> PermissionsData::GetPermissionMessageStrings()
     const {
   if (ShouldSkipPermissionWarnings(extension_id_))
     return std::vector<base::string16>();
-  return PermissionMessageProvider::Get()->GetWarningMessages(
+  return PermissionMessageProvider::Get()->GetLegacyWarningMessages(
       active_permissions().get(), manifest_type_);
 }
 
@@ -217,7 +217,7 @@ std::vector<base::string16>
 PermissionsData::GetPermissionMessageDetailsStrings() const {
   if (ShouldSkipPermissionWarnings(extension_id_))
     return std::vector<base::string16>();
-  return PermissionMessageProvider::Get()->GetWarningMessagesDetails(
+  return PermissionMessageProvider::Get()->GetLegacyWarningMessagesDetails(
       active_permissions().get(), manifest_type_);
 }
 

extensions/common/permissions/permissions_data.h

@@ -131,10 +131,10 @@ class PermissionsData {
   // network, etc.)
   bool HasEffectiveAccessToAllHosts() const;
 
-  // Returns the full list of permission messages that should display at
-  // install time.
-  // TODO(sashab): Deprecate this in favor of GetCoalescedPermissionMessages().
-  PermissionMessages GetPermissionMessages() const;
+  // Returns the full list of legacy permission message IDs.
+  // Deprecated. You DO NOT want to call this!
+  // TODO(treib): Remove once we've switched to the new system.
+  PermissionMessageIDs GetLegacyPermissionMessageIDs() const;
 
   // Returns the full list of permission messages that should display at install
   // time as strings.

extensions/shell/common/shell_extensions_client.cc

@@ -42,10 +42,16 @@ class ShellPermissionMessageProvider : public PermissionMessageProvider {
   ~ShellPermissionMessageProvider() override {}
 
   // PermissionMessageProvider implementation.
-  PermissionMessages GetPermissionMessages(
+  PermissionMessageStrings GetPermissionMessageStrings(
       const PermissionSet* permissions,
       Manifest::Type extension_type) const override {
-    return PermissionMessages();
+    return PermissionMessageStrings();
+  }
+
+  PermissionMessageIDs GetLegacyPermissionMessageIDs(
+      const PermissionSet* permissions,
+      Manifest::Type extension_type) const override {
+    return PermissionMessageIDs();
   }
 
   CoalescedPermissionMessages GetCoalescedPermissionMessages(
@@ -53,13 +59,13 @@ class ShellPermissionMessageProvider : public PermissionMessageProvider {
     return CoalescedPermissionMessages();
   }
 
-  std::vector<base::string16> GetWarningMessages(
+  std::vector<base::string16> GetLegacyWarningMessages(
       const PermissionSet* permissions,
       Manifest::Type extension_type) const override {
     return std::vector<base::string16>();
   }
 
-  std::vector<base::string16> GetWarningMessagesDetails(
+  std::vector<base::string16> GetLegacyWarningMessagesDetails(
       const PermissionSet* permissions,
       Manifest::Type extension_type) const override {
     return std::vector<base::string16>();

extensions/test/test_permission_message_provider.cc

@@ -12,10 +12,18 @@ TestPermissionMessageProvider::TestPermissionMessageProvider() {
 TestPermissionMessageProvider::~TestPermissionMessageProvider() {
 }
 
-PermissionMessages TestPermissionMessageProvider::GetPermissionMessages(
+PermissionMessageStrings
+TestPermissionMessageProvider::GetPermissionMessageStrings(
     const PermissionSet* permissions,
     Manifest::Type extension_type) const {
-  return PermissionMessages();
+  return PermissionMessageStrings();
+}
+
+PermissionMessageIDs
+TestPermissionMessageProvider::GetLegacyPermissionMessageIDs(
+    const PermissionSet* permissions,
+    Manifest::Type extension_type) const {
+  return PermissionMessageIDs();
 }
 
 CoalescedPermissionMessages
@@ -24,14 +32,15 @@ TestPermissionMessageProvider::GetCoalescedPermissionMessages(
   return CoalescedPermissionMessages();
 }
 
-std::vector<base::string16> TestPermissionMessageProvider::GetWarningMessages(
+std::vector<base::string16>
+TestPermissionMessageProvider::GetLegacyWarningMessages(
     const PermissionSet* permissions,
     Manifest::Type extension_type) const {
   return std::vector<base::string16>();
 }
 
 std::vector<base::string16>
-TestPermissionMessageProvider::GetWarningMessagesDetails(
+TestPermissionMessageProvider::GetLegacyWarningMessagesDetails(
     const PermissionSet* permissions,
     Manifest::Type extension_type) const {
   return std::vector<base::string16>();

extensions/test/test_permission_message_provider.h

@@ -16,15 +16,18 @@ class TestPermissionMessageProvider : public PermissionMessageProvider {
   ~TestPermissionMessageProvider() override;
 
  private:
-  PermissionMessages GetPermissionMessages(
+  PermissionMessageStrings GetPermissionMessageStrings(
+      const PermissionSet* permissions,
+      Manifest::Type extension_type) const override;
+  PermissionMessageIDs GetLegacyPermissionMessageIDs(
       const PermissionSet* permissions,
       Manifest::Type extension_type) const override;
   CoalescedPermissionMessages GetCoalescedPermissionMessages(
       const PermissionIDSet& permissions) const override;
-  std::vector<base::string16> GetWarningMessages(
+  std::vector<base::string16> GetLegacyWarningMessages(
       const PermissionSet* permissions,
       Manifest::Type extension_type) const override;
-  std::vector<base::string16> GetWarningMessagesDetails(
+  std::vector<base::string16> GetLegacyWarningMessagesDetails(
       const PermissionSet* permissions,
       Manifest::Type extension_type) const override;
   bool IsPrivilegeIncrease(const PermissionSet* old_permissions,