Permission messages: Add a bunch of missing combination/suppression rules.

chrome/common/extensions/permissions/chrome_permission_message_rules.cc

 // Copyright 2014 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_rules.h"
 
 #include "base/stl_util.h"
 #include "base/strings/string_util.h"
 #include "base/strings/utf_string_conversions.h"
 #include "chrome/grit/generated_resources.h"
@@ skipping 293 matching lines @@
                              APIPermission::ID b,
                              APIPermission::ID c,
                              APIPermission::ID d,
                              APIPermission::ID e,
                              APIPermission::ID f,
                              APIPermission::ID g)
       : PermissionIDSetInitializer(a, b, c, d, e, f) {
     insert(g);
   }
 
+  PermissionIDSetInitializer(APIPermission::ID a,
+                             APIPermission::ID b,
+                             APIPermission::ID c,
+                             APIPermission::ID d,
+                             APIPermission::ID e,
+                             APIPermission::ID f,
+                             APIPermission::ID g,
+                             APIPermission::ID h)
+      : PermissionIDSetInitializer(a, b, c, d, e, f, g) {
+    insert(h);
+  }
+
+  PermissionIDSetInitializer(APIPermission::ID a,

[Devlin, 2015/09/01 16:08:51]

This is getting a little nuts here.  Looking briefly at our style guide, I don't
see anything banning variadic functions (did I miss it?).  Using one variadic
function would make this muuuuuch cleaner...

[Marc Treib, 2015/09/01 16:25:50]

Hm, seems you're right, I also can't find anything about them in the style
guide. OTOH, I haven't seen a variadic function used in Chrome (or anywhere
else, really) except for printf-style functions. I also don't think I've ever
written one.
I actually think the current mass of ctors is easier to parse than a variadic
function (if only because those are so uncommon). I guess the "correct" solution
would be an std::initializer_list, but those aren't allowed yet. A variadic
template might also work, but will also have readability issues.

Would it be acceptable for you to just put in something like
// TODO(treib): Replace all these ctors by a single initializer_list version
when we have those.
for now?

[Devlin, 2015/09/01 16:33:43]

Eh, variadic templates aren't *that* hard to parse, especially when the argument
list is all one type.  And another approach, which would be just as good, is to
pass in a std::set or std::vector of ids which can be brace-initialized, so that
you can still have one constructor, and do PermissionIdSetInitializer({id1, id2,
...idn});, but I think that vector brace initialization is still verboten as
part of uniform initialization (do you know for sure?).

But, in any case, this is mostly just an eyesore, not a safety or performance
concern, so as long as I stop looking at it, we'll be okay. :)

[Marc Treib, 2015/09/02 10:43:55]

Yup, the uniform initialization of set/vector is based on std::initializer_list
which we don't have yet.

But, indeed, variadic templates are less weird-looking than I would've thought.
So I'm now the proud author of my very first variadic template :)

+                             APIPermission::ID b,
+                             APIPermission::ID c,
+                             APIPermission::ID d,
+                             APIPermission::ID e,
+                             APIPermission::ID f,
+                             APIPermission::ID g,
+                             APIPermission::ID h,
+                             APIPermission::ID i)
+      : PermissionIDSetInitializer(a, b, c, d, e, f, g, h) {
+    insert(i);
+  }
+
   virtual ~PermissionIDSetInitializer() {}
 };
 
 ChromePermissionMessageRule::ChromePermissionMessageRule(
     int message_id,
     const PermissionIDSetInitializer& required,
     const PermissionIDSetInitializer& optional)
     : ChromePermissionMessageRule(
           new DefaultPermissionMessageFormatter(message_id),
           required,
@@ skipping 58 matching lines @@
   // permission suppresses kTab, be careful to also add kTopSites and kFavicon
   // to the kHistory absorb list. Ideally, the rules system should be simple
   // enough that rules like this should not occur; the visibility of the rules
   // system should allow us to design a system that is simple enough to explain
   // yet powerful enough to encapsulate all the messages we want to display.
   ChromePermissionMessageRule rules_arr[] = {
       // Full access permission messages.
       {IDS_EXTENSION_PROMPT_WARNING_DEBUGGER, {APIPermission::kDebugger}, {}},
       {IDS_EXTENSION_PROMPT_WARNING_FULL_ACCESS,
        {APIPermission::kPlugin},
-       // TODO(treib): Add the other IDs implied by kFullAccess/kHostsAll.
-       {APIPermission::kFullAccess, APIPermission::kHostsAll,
-        APIPermission::kHostsAllReadOnly, APIPermission::kDeclarativeWebRequest,
-        APIPermission::kTopSites, APIPermission::kTab}},
+       {APIPermission::kDeclarativeWebRequest, APIPermission::kFavicon,
+        APIPermission::kFullAccess, APIPermission::kHostsAll,
+        APIPermission::kHostsAllReadOnly, APIPermission::kProcesses,
+        APIPermission::kTab, APIPermission::kTopSites,
+        APIPermission::kWebNavigation}},
       {IDS_EXTENSION_PROMPT_WARNING_FULL_ACCESS,
        {APIPermission::kFullAccess},
-       // TODO(treib): Add the other IDs implied by kHostsAll.
-       {APIPermission::kHostsAll, APIPermission::kHostsAllReadOnly,
-        APIPermission::kDeclarativeWebRequest, APIPermission::kTopSites,
-        APIPermission::kTab}},
+       {APIPermission::kDeclarativeWebRequest, APIPermission::kFavicon,
+        APIPermission::kHostsAll, APIPermission::kHostsAllReadOnly,
+        APIPermission::kProcesses, APIPermission::kTab,
+        APIPermission::kTopSites, APIPermission::kWebNavigation}},
 
       // Hosts permission messages.
       // Full host access already allows DeclarativeWebRequest, reading the list
       // of most frequently visited sites, and tab access.
       // 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.
       {IDS_EXTENSION_PROMPT_WARNING_ALL_HOSTS,
        {APIPermission::kHostsAll},
-       // TODO(treib): Add kHostReadWrite and kHostReadOnly.
        {APIPermission::kDeclarativeWebRequest, APIPermission::kFavicon,
-        APIPermission::kHostsAllReadOnly, APIPermission::kProcesses,
+        APIPermission::kHostsAllReadOnly, APIPermission::kHostReadOnly,
+        APIPermission::kHostReadWrite, APIPermission::kProcesses,
         APIPermission::kTab, APIPermission::kTopSites,
         APIPermission::kWebNavigation}},
       {IDS_EXTENSION_PROMPT_WARNING_ALL_HOSTS_READ_ONLY,
        {APIPermission::kHostsAllReadOnly},
-       // TODO(treib): Add kHostReadOnly.
-       {APIPermission::kFavicon, APIPermission::kProcesses, APIPermission::kTab,
+       {APIPermission::kFavicon, APIPermission::kHostReadOnly,
+        APIPermission::kProcesses, APIPermission::kTab,
         APIPermission::kTopSites, APIPermission::kWebNavigation}},
 
+      {new CommaSeparatedListFormatter(IDS_EXTENSION_PROMPT_WARNING_1_HOST,
+                                       IDS_EXTENSION_PROMPT_WARNING_2_HOSTS,
+                                       IDS_EXTENSION_PROMPT_WARNING_3_HOSTS,
+                                       IDS_EXTENSION_PROMPT_WARNING_HOSTS_LIST),
+       {APIPermission::kHostReadWrite},
+       {}},
       {new CommaSeparatedListFormatter(
            IDS_EXTENSION_PROMPT_WARNING_1_HOST_READ_ONLY,
            IDS_EXTENSION_PROMPT_WARNING_2_HOSTS_READ_ONLY,
            IDS_EXTENSION_PROMPT_WARNING_3_HOSTS_READ_ONLY,
            IDS_EXTENSION_PROMPT_WARNING_HOSTS_LIST_READ_ONLY),
        {APIPermission::kHostReadOnly},
        {}},
-      {new CommaSeparatedListFormatter(IDS_EXTENSION_PROMPT_WARNING_1_HOST,
-                                       IDS_EXTENSION_PROMPT_WARNING_2_HOSTS,
-                                       IDS_EXTENSION_PROMPT_WARNING_3_HOSTS,
-                                       IDS_EXTENSION_PROMPT_WARNING_HOSTS_LIST),
-       {APIPermission::kHostReadWrite},
-       {}},
 
       // History-related permission messages.
       // History already allows reading favicons, tab access and accessing the
       // list of most frequently visited sites.
       {IDS_EXTENSION_PROMPT_WARNING_HISTORY_WRITE_AND_SESSIONS,
-       {APIPermission::kSessions, APIPermission::kHistory},
+       {APIPermission::kHistory, APIPermission::kSessions},
        {APIPermission::kFavicon, APIPermission::kProcesses, APIPermission::kTab,
         APIPermission::kTopSites, APIPermission::kWebNavigation}},
-      {IDS_EXTENSION_PROMPT_WARNING_HISTORY_READ_AND_SESSIONS,
-       {APIPermission::kSessions, APIPermission::kTab},
-       {APIPermission::kFavicon, APIPermission::kProcesses,
-        APIPermission::kTopSites, APIPermission::kWebNavigation}},
       {IDS_EXTENSION_PROMPT_WARNING_HISTORY_WRITE,
        {APIPermission::kHistory},
        {APIPermission::kFavicon, APIPermission::kProcesses, APIPermission::kTab,
         APIPermission::kTopSites, APIPermission::kWebNavigation}},
+      {IDS_EXTENSION_PROMPT_WARNING_HISTORY_READ_AND_SESSIONS,
+       {APIPermission::kTab, APIPermission::kSessions},
+       {APIPermission::kFavicon, APIPermission::kProcesses,
+        APIPermission::kTopSites, APIPermission::kWebNavigation}},
+      {IDS_EXTENSION_PROMPT_WARNING_HISTORY_READ_AND_SESSIONS,
+       {APIPermission::kProcesses, APIPermission::kSessions},

[Devlin, 2015/09/01 16:08:51]

See comment on the other CL - I think I was wrong about these.

[Devlin, 2015/09/01 16:09:53]

Aaaaand, nevermind. :)

+       {APIPermission::kFavicon, APIPermission::kTopSites,
+        APIPermission::kWebNavigation}},
+      {IDS_EXTENSION_PROMPT_WARNING_HISTORY_READ_AND_SESSIONS,
+       {APIPermission::kWebNavigation, APIPermission::kSessions},
+       {APIPermission::kFavicon, APIPermission::kTopSites}},
       {IDS_EXTENSION_PROMPT_WARNING_HISTORY_READ,
        {APIPermission::kTab},
        {APIPermission::kFavicon, APIPermission::kProcesses,
         APIPermission::kTopSites, APIPermission::kWebNavigation}},
-      // TODO(treib): Should we have _AND_SESSIONS versions of these 2 as well?
       {IDS_EXTENSION_PROMPT_WARNING_HISTORY_READ,
        {APIPermission::kProcesses},
-       {}},
+       {APIPermission::kFavicon, APIPermission::kTopSites,
+        APIPermission::kWebNavigation}},
       {IDS_EXTENSION_PROMPT_WARNING_HISTORY_READ,
        {APIPermission::kWebNavigation},
-       {}},
+       {APIPermission::kFavicon, APIPermission::kTopSites}},
       {IDS_EXTENSION_PROMPT_WARNING_FAVICON, {APIPermission::kFavicon}, {}},
       {IDS_EXTENSION_PROMPT_WARNING_TOPSITES, {APIPermission::kTopSites}, {}},
+
       {IDS_EXTENSION_PROMPT_WARNING_DECLARATIVE_WEB_REQUEST,
        {APIPermission::kDeclarativeWebRequest},
        {}},
 
       // Messages generated by the sockets permission.
       {IDS_EXTENSION_PROMPT_WARNING_SOCKET_ANY_HOST,
        {APIPermission::kSocketAnyHost},
-       // TODO(treib): Add kSocketDomainHosts and kSocketSpecificHosts.
-       {}},
+       {APIPermission::kSocketDomainHosts,
+        APIPermission::kSocketSpecificHosts}},
       {new SpaceSeparatedListFormatter(
            IDS_EXTENSION_PROMPT_WARNING_SOCKET_HOSTS_IN_DOMAIN,
            IDS_EXTENSION_PROMPT_WARNING_SOCKET_HOSTS_IN_DOMAINS),
        {APIPermission::kSocketDomainHosts},
        {}},
       {new SpaceSeparatedListFormatter(
            IDS_EXTENSION_PROMPT_WARNING_SOCKET_SPECIFIC_HOST,
            IDS_EXTENSION_PROMPT_WARNING_SOCKET_SPECIFIC_HOSTS),
        {APIPermission::kSocketSpecificHosts},
        {}},
@@ skipping 35 matching lines @@
        {APIPermission::kAccessibilityFeaturesModify,
         APIPermission::kAccessibilityFeaturesRead},
        {}},
       {IDS_EXTENSION_PROMPT_WARNING_ACCESSIBILITY_FEATURES_MODIFY,
        {APIPermission::kAccessibilityFeaturesModify},
        {}},
       {IDS_EXTENSION_PROMPT_WARNING_ACCESSIBILITY_FEATURES_READ,
        {APIPermission::kAccessibilityFeaturesRead},
        {}},
 
-      // TODO(sashab): Add the missing combinations of media galleries
-      // permissions so a valid permission is generated for all combinations.
+      // Media galleries permissions. We don't have strings for every possible
+      // combination, e.g. we don't bother with a special string for "write, but
+      // not read" - just show the "read and write" string instead, etc.
       {IDS_EXTENSION_PROMPT_WARNING_MEDIA_GALLERIES_READ_WRITE_DELETE,
        {APIPermission::kMediaGalleriesAllGalleriesCopyTo,
-        APIPermission::kMediaGalleriesAllGalleriesDelete,
-        APIPermission::kMediaGalleriesAllGalleriesRead},
-       {}},
+        APIPermission::kMediaGalleriesAllGalleriesDelete},
+       {APIPermission::kMediaGalleriesAllGalleriesRead}},
       {IDS_EXTENSION_PROMPT_WARNING_MEDIA_GALLERIES_READ_WRITE,
-       {APIPermission::kMediaGalleriesAllGalleriesCopyTo,
-        APIPermission::kMediaGalleriesAllGalleriesRead},
-       {}},
+       {APIPermission::kMediaGalleriesAllGalleriesCopyTo},
+       {APIPermission::kMediaGalleriesAllGalleriesRead}},
       {IDS_EXTENSION_PROMPT_WARNING_MEDIA_GALLERIES_READ_DELETE,
-       {APIPermission::kMediaGalleriesAllGalleriesDelete,
-        APIPermission::kMediaGalleriesAllGalleriesRead},
-       {}},
+       {APIPermission::kMediaGalleriesAllGalleriesDelete},
+       {APIPermission::kMediaGalleriesAllGalleriesRead}},
       {IDS_EXTENSION_PROMPT_WARNING_MEDIA_GALLERIES_READ,
        {APIPermission::kMediaGalleriesAllGalleriesRead},
        {}},
 
       // The permission string for "fileSystem" is only shown when
       // "write" or "directory" is present. Read-only access is only
       // granted after the user has been shown a file or directory
       // chooser dialog and selected a file or directory. Selecting
       // the file or directory is considered consent to read it.
       {IDS_EXTENSION_PROMPT_WARNING_FILE_SYSTEM_WRITE_DIRECTORY,
@@ skipping 68 matching lines @@
        {APIPermission::kContentSettings},
        {}},
       {IDS_EXTENSION_PROMPT_WARNING_COPRESENCE,
        {APIPermission::kCopresence},
        {}},
       {IDS_EXTENSION_PROMPT_WARNING_DOCUMENT_SCAN,
        {APIPermission::kDocumentScan},
        {}},
       {IDS_EXTENSION_PROMPT_WARNING_INTERCEPT_ALL_KEYS,
        {APIPermission::kInterceptAllKeys},
-       {}},  // TODO(treib): This should probably suppress kInput.
+       {APIPermission::kInput}},
       {IDS_EXTENSION_PROMPT_WARNING_INPUT, {APIPermission::kInput}, {}},
       {IDS_EXTENSION_PROMPT_WARNING_MANAGEMENT,
        {APIPermission::kManagement},
        {}},
       {IDS_EXTENSION_PROMPT_WARNING_MDNS, {APIPermission::kMDns}, {}},
       {IDS_EXTENSION_PROMPT_WARNING_NATIVE_MESSAGING,
        {APIPermission::kNativeMessaging},
        {}},
       {IDS_EXTENSION_PROMPT_WARNING_PRIVACY, {APIPermission::kPrivacy}, {}},
       {IDS_EXTENSION_PROMPT_WARNING_SIGNED_IN_DEVICES,
@@ skipping 37 matching lines @@
       {IDS_EXTENSION_PROMPT_WARNING_USERS_PRIVATE,
        {APIPermission::kUsersPrivate},
        {}},
   };
 
   return std::vector<ChromePermissionMessageRule>(
       rules_arr, rules_arr + arraysize(rules_arr));
 }
 
 }  // namespace extensions