Cleaned up the code for suppressed messages

chrome/common/extensions/permissions/chrome_permission_message_provider.cc

 // 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 "chrome/common/extensions/permissions/chrome_permission_message_provider.h"
 
 #include "base/stl_util.h"
 #include "base/strings/stringprintf.h"
 #include "extensions/common/extensions_client.h"
 #include "extensions/common/permissions/permission_message.h"
@@ skipping 62 matching lines @@
 
 ChromePermissionMessageProvider::ChromePermissionMessageProvider() {
 }
 
 ChromePermissionMessageProvider::~ChromePermissionMessageProvider() {
 }
 
 PermissionMessages ChromePermissionMessageProvider::GetPermissionMessages(
     const PermissionSet* permissions,
     Manifest::Type extension_type) const {
+  // Some warnings are more generic and/or powerful and superseed other
+  // warnings. In that case, the first message suppresses the second one.
+  const std::multimap<PermissionMessage::ID,
+                      PermissionMessage::ID> kSuppressList = {
+      {PermissionMessage::kBluetooth, PermissionMessage::kBluetoothDevices},
+      {PermissionMessage::kBookmarks, PermissionMessage::kOverrideBookmarksUI},
+      // History already allows reading favicons.
+      {PermissionMessage::kBrowsingHistory, PermissionMessage::kFavicon},
+      // History already allows tabs access.
+      {PermissionMessage::kBrowsingHistory, PermissionMessage::kTabs},
+      // A special hack: If kFileSystemWriteDirectory would be displayed, hide
+      // kFileSystemDirectory as the write directory message implies it.
+      // TODO(sammc): Remove this. See http://crbug.com/284849.

[meacer, 2014/07/18 22:19:49]

Would it make sense to make this comment similar to the others and close this
bug now?

[mhm, 2014/07/18 22:40:41]

I am not sure what is needed for that bug.

+      {PermissionMessage::kFileSystemWriteDirectory,
+       PermissionMessage::kFileSystemDirectory},
+      // Full access already allows DeclarativeWebRequest.
+      {PermissionMessage::kHostsAll, PermissionMessage::kDeclarativeWebRequest},
+      // Full access already covers tabs access.
+      {PermissionMessage::kHostsAll, PermissionMessage::kTabs},
+      // Tabs already allows reading favicons.
+      {PermissionMessage::kTabs, PermissionMessage::kFavicon}};
+
   PermissionMessages messages;
 
   if (permissions->HasEffectiveFullAccess()) {
     messages.push_back(PermissionMessage(
         PermissionMessage::kFullAccess,
         l10n_util::GetStringUTF16(IDS_EXTENSION_PROMPT_WARNING_FULL_ACCESS)));
     return messages;
   }
 
   PermissionMsgSet host_msgs =
       GetHostPermissionMessages(permissions, extension_type);
   PermissionMsgSet api_msgs = GetAPIPermissionMessages(permissions);
   PermissionMsgSet manifest_permission_msgs =
       GetManifestPermissionMessages(permissions);
   messages.insert(messages.end(), host_msgs.begin(), host_msgs.end());
   messages.insert(messages.end(), api_msgs.begin(), api_msgs.end());
   messages.insert(messages.end(), manifest_permission_msgs.begin(),
                   manifest_permission_msgs.end());
 
-  // Some warnings are more generic and/or powerful and superseed other
-  // warnings. In that case, suppress the superseeded warning.
-  SuppressMessage(messages,
-                  PermissionMessage::kBookmarks,
-                  PermissionMessage::kOverrideBookmarksUI);
-  // Both tabs and history already allow reading favicons.
-  SuppressMessage(messages,
-                  PermissionMessage::kTabs,
-                  PermissionMessage::kFavicon);
-  SuppressMessage(messages,
-                  PermissionMessage::kBrowsingHistory,
-                  PermissionMessage::kFavicon);
-  // Warning for history permission already covers warning for tabs permission.
-  SuppressMessage(messages,
-                  PermissionMessage::kBrowsingHistory,
-                  PermissionMessage::kTabs);
-  // Warning for full access permission already covers warning for tabs
-  // permission.
-  SuppressMessage(messages,
-                  PermissionMessage::kHostsAll,
-                  PermissionMessage::kTabs);
-  // Warning for full access already covers warning for DeclarativeWebRequest
-  // permission.
-  SuppressMessage(messages,
-                  PermissionMessage::kHostsAll,
-                  PermissionMessage::kDeclarativeWebRequest);
+  for (std::multimap<PermissionMessage::ID,
+                     PermissionMessage::ID>::const_iterator it =
+           kSuppressList.begin();
+       it != kSuppressList.end();
+       it++) {

[meacer, 2014/07/18 22:19:49]

nit: ++it instead of it++

[mhm, 2014/07/18 22:40:41]

Done.

+    SuppressMessage(messages, it->first, it->second);
+  }
 
   return messages;
 }
 
 std::vector<base::string16> ChromePermissionMessageProvider::GetWarningMessages(
     const PermissionSet* permissions,
     Manifest::Type extension_type) const {
   std::vector<base::string16> message_strings;
   PermissionMessages messages =
       GetPermissionMessages(permissions, extension_type);
 
-  SuppressMessage(messages,
-                  PermissionMessage::kBluetooth,
-                  PermissionMessage::kBluetoothDevices);
-
   for (PermissionMessages::const_iterator i = messages.begin();
        i != messages.end(); ++i) {
     int id = i->id();
     // Access to users' devices should provide a single warning message
     // specifying the transport method used; USB, serial and/or Bluetooth.
     if (id == PermissionMessage::kBluetooth ||
         id == PermissionMessage::kSerial ||
         id == PermissionMessage::kUsb) {
       if (ContainsMessages(messages,
                            PermissionMessage::kBluetooth,
@@ skipping 156 matching lines @@
   PermissionMsgSet messages;
   for (APIPermissionSet::const_iterator permission_it =
            permissions->apis().begin();
        permission_it != permissions->apis().end(); ++permission_it) {
     if (permission_it->HasMessages()) {
       PermissionMessages new_messages = permission_it->GetMessages();
       messages.insert(new_messages.begin(), new_messages.end());
     }
   }
 
-  // A special hack: If kFileSystemWriteDirectory would be displayed, hide
-  // kFileSystemDirectory as the write directory message implies it.
-  // TODO(sammc): Remove this. See http://crbug.com/284849.
-  SuppressMessage(messages,
-                  PermissionMessage::kFileSystemWriteDirectory,
-                  PermissionMessage::kFileSystemDirectory);
   // A special hack: The warning message for declarativeWebRequest
   // permissions speaks about blocking parts of pages, which is a
   // subset of what the "<all_urls>" access allows. Therefore we
   // display only the "<all_urls>" warning message if both permissions
   // are required.
   if (permissions->ShouldWarnAllHosts()) {
     messages.erase(
         PermissionMessage(
             PermissionMessage::kDeclarativeWebRequest, base::string16()));
   }
@@ skipping 116 matching lines @@
   std::set<std::string> old_hosts_set(
       permission_message_util::GetDistinctHosts(old_list, false, false));
   std::set<std::string> new_hosts_only =
       base::STLSetDifference<std::set<std::string> >(new_hosts_set,
                                                      old_hosts_set);
 
   return !new_hosts_only.empty();
 }
 
 }  // namespace extensions