Suppress tabs permission warning if there is already a browsingHistory warning.

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"
 #include "extensions/common/permissions/permission_message_util.h"
 #include "extensions/common/permissions/permission_set.h"
 #include "extensions/common/url_pattern.h"
 #include "extensions/common/url_pattern_set.h"
 #include "grit/generated_resources.h"
 #include "net/base/registry_controlled_domains/registry_controlled_domain.h"
 #include "ui/base/l10n/l10n_util.h"
 #include "url/gurl.h"
 
 namespace extensions {
 
 namespace {
 
+typedef std::set<PermissionMessage> PermissionMsgSet;
+
 bool ShouldWarnAllHosts(const PermissionSet* permissions) {
   if (permissions->HasEffectiveAccessToAllHosts())
     return true;
 
   const URLPatternSet& effective_hosts = permissions->effective_hosts();
   for (URLPatternSet::const_iterator iter = effective_hosts.begin();
        iter != effective_hosts.end();
        ++iter) {
     // If this doesn't even match subdomains, it can't possibly imply all hosts.
     if (!iter->match_subdomains())
@@ skipping 21 matching lines @@
         net::registry_controlled_domains::EXCLUDE_PRIVATE_REGISTRIES);
     // If we recognized this TLD, then this is a pattern like *.com, and it
     // should imply all hosts.
     if (registry_length > 0)
       return true;
   }
 
   return false;
 }
 
-PermissionMessages::iterator FindMessageByID(PermissionMessages& messages,
-                                             int id) {
-  for (PermissionMessages::iterator it = messages.begin();
+template<typename T>
+typename T::iterator FindMessageByID(T& messages, int id) {

[Devlin, 2014/04/15 15:09:16]

Isn't this almost exactly the same as std::find()? Can we get rid of this
function (especially since we only call it in one place)?  (Sorry I didn't see
this before.)

[meacer, 2014/04/15 16:46:16]

Using std::find with id field would require PermissionMessage to overload the ==
operator which isn't the case, so I don't think that would work. It's called
twice in SuppressMessage so unfortunately I can't inline it either.

[Devlin, 2014/04/15 16:52:45]

Oh, shoot, missed that...  and std::find_if would be overkill here.  My bad -
should have noticed that.

+  for (typename T::iterator it = messages.begin();
        it != messages.end(); ++it) {
     if (it->id() == id)
       return it;
   }
+  return messages.end();
+}
 
-  return messages.end();
+template<typename T>
+void SuppressMessage(T& messages,
+                     int suppressing_message,
+                     int suppressed_message) {
+  typename T::iterator suppressed = FindMessageByID(messages,
+                                                    suppressed_message);
+  if (suppressed != messages.end() &&
+      FindMessageByID(messages, suppressing_message) != messages.end()) {
+    messages.erase(suppressed);
+  }
 }
 
 }  // namespace
 
 ChromePermissionMessageProvider::ChromePermissionMessageProvider() {
 }
 
 ChromePermissionMessageProvider::~ChromePermissionMessageProvider() {
 }
 
 // static
 PermissionMessages ChromePermissionMessageProvider::GetPermissionMessages(
     const PermissionSet* permissions,
     Manifest::Type extension_type) const {
   PermissionMessages messages;
 
   if (permissions->HasEffectiveFullAccess()) {
     messages.push_back(PermissionMessage(
         PermissionMessage::kFullAccess,
         l10n_util::GetStringUTF16(IDS_EXTENSION_PROMPT_WARNING_FULL_ACCESS)));
     return messages;
   }
 
-  std::set<PermissionMessage> host_msgs =
+  PermissionMsgSet host_msgs =
       GetHostPermissionMessages(permissions, extension_type);
-  std::set<PermissionMessage> api_msgs = GetAPIPermissionMessages(permissions);
-  std::set<PermissionMessage> manifest_permission_msgs =
+  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());
 
-  // Special hack: bookmarks permission message supersedes override bookmarks UI
-  // permission message if both permissions are specified.
-  PermissionMessages::iterator override_bookmarks_ui =
-      FindMessageByID(messages, PermissionMessage::kOverrideBookmarksUI);
-  if (override_bookmarks_ui != messages.end() &&
-      FindMessageByID(messages, PermissionMessage::kBookmarks) !=
-          messages.end()) {
-    messages.erase(override_bookmarks_ui);
-  }
-
+  // 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);
+  SuppressMessage(messages,
+                  PermissionMessage::kBrowsingHistory,
+                  PermissionMessage::kTabs);
   return messages;
 }
 
 // static
 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);
@@ skipping 97 matching lines @@
 
   if (IsManifestPermissionPrivilegeIncrease(old_permissions, new_permissions))
     return true;
 
   return false;
 }
 
 std::set<PermissionMessage>
 ChromePermissionMessageProvider::GetAPIPermissionMessages(
     const PermissionSet* permissions) const {
-  std::set<PermissionMessage> messages;
+  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 and and kFileSystemWrite as the write directory
   // message implies the other two.
   // TODO(sammc): Remove this. See http://crbug.com/284849.
-  std::set<PermissionMessage>::iterator write_directory_message =
-      messages.find(PermissionMessage(
-          PermissionMessage::kFileSystemWriteDirectory, base::string16()));
-  if (write_directory_message != messages.end()) {
-    messages.erase(
-        PermissionMessage(PermissionMessage::kFileSystemWrite,
-                          base::string16()));
-    messages.erase(
-        PermissionMessage(PermissionMessage::kFileSystemDirectory,
-                          base::string16()));
-  }
-
+  SuppressMessage(messages,
+                  PermissionMessage::kFileSystemWriteDirectory,
+                  PermissionMessage::kFileSystemWrite);
+  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 (ShouldWarnAllHosts(permissions)) {
     messages.erase(
         PermissionMessage(
             PermissionMessage::kDeclarativeWebRequest, base::string16()));
   }
-
   return messages;
 }
 
 std::set<PermissionMessage>
 ChromePermissionMessageProvider::GetManifestPermissionMessages(
     const PermissionSet* permissions) const {
-  std::set<PermissionMessage> messages;
+  PermissionMsgSet messages;
   for (ManifestPermissionSet::const_iterator permission_it =
            permissions->manifest_permissions().begin();
       permission_it != permissions->manifest_permissions().end();
       ++permission_it) {
     if (permission_it->HasMessages()) {
       PermissionMessages new_messages = permission_it->GetMessages();
       messages.insert(new_messages.begin(), new_messages.end());
     }
   }
   return messages;
 }
 
 std::set<PermissionMessage>
 ChromePermissionMessageProvider::GetHostPermissionMessages(
     const PermissionSet* permissions,
     Manifest::Type extension_type) const {
-  std::set<PermissionMessage> messages;
+  PermissionMsgSet messages;
   // Since platform apps always use isolated storage, they can't (silently)
   // access user data on other domains, so there's no need to prompt.
   // Note: this must remain consistent with IsHostPrivilegeIncrease.
   // See crbug.com/255229.
   if (extension_type == Manifest::TYPE_PLATFORM_APP)
     return messages;
 
   if (ShouldWarnAllHosts(permissions)) {
     messages.insert(PermissionMessage(
         PermissionMessage::kHostsAll,
@@ skipping 10 matching lines @@
   }
   return messages;
 }
 
 bool ChromePermissionMessageProvider::IsAPIPrivilegeIncrease(
     const PermissionSet* old_permissions,
     const PermissionSet* new_permissions) const {
   if (new_permissions == NULL)
     return false;
 
-  typedef std::set<PermissionMessage> PermissionMsgSet;
   PermissionMsgSet old_warnings = GetAPIPermissionMessages(old_permissions);
   PermissionMsgSet new_warnings = GetAPIPermissionMessages(new_permissions);
   PermissionMsgSet delta_warnings =
       base::STLSetDifference<PermissionMsgSet>(new_warnings, old_warnings);
 
   // A special hack: kFileSystemWriteDirectory implies kFileSystemDirectory and
   // kFileSystemWrite.
   // TODO(sammc): Remove this. See http://crbug.com/284849.
   if (old_warnings.find(PermissionMessage(
           PermissionMessage::kFileSystemWriteDirectory, base::string16())) !=
       old_warnings.end()) {
     delta_warnings.erase(
         PermissionMessage(PermissionMessage::kFileSystemDirectory,
                           base::string16()));
     delta_warnings.erase(
         PermissionMessage(PermissionMessage::kFileSystemWrite,
                           base::string16()));
   }
 
   // It is a privilege increase if there are additional warnings present.
   return !delta_warnings.empty();
 }
 
 bool ChromePermissionMessageProvider::IsManifestPermissionPrivilegeIncrease(
     const PermissionSet* old_permissions,
     const PermissionSet* new_permissions) const {
   if (new_permissions == NULL)
     return false;
 
-  typedef std::set<PermissionMessage> PermissionMsgSet;
   PermissionMsgSet old_warnings =
       GetManifestPermissionMessages(old_permissions);
   PermissionMsgSet new_warnings =
       GetManifestPermissionMessages(new_permissions);
   PermissionMsgSet delta_warnings =
       base::STLSetDifference<PermissionMsgSet>(new_warnings, old_warnings);
 
   // It is a privilege increase if there are additional warnings present.
   return !delta_warnings.empty();
 }
@@ skipping 27 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