Change topSites API permission 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"
@@ skipping 68 matching lines @@
   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.
+  // Warning for history permission already covers warning for tabs and
+  // topSites permission.
   SuppressMessage(messages,
                   PermissionMessage::kBrowsingHistory,
                   PermissionMessage::kTabs);
+  SuppressMessage(messages,
+                  PermissionMessage::kBrowsingHistory,
+                  PermissionMessage::kTopSites);
+  // Warning for topSites permission already covers warning for tabs
+  // permission.
+  SuppressMessage(messages,
+                  PermissionMessage::kTopSites,
+                  PermissionMessage::kTabs);

[meacer, 2014/06/11 20:03:10]

See my comment below: if we just had "read browsing history" and "read and
modify browsing history" and get rid of "browsing activity", this would be
cleaner: kBrowsingHistoryWrite would suppress kBrowsingHistoryRead.

   // Warning for full access permission already covers warning for tabs
   // permission.
   SuppressMessage(messages,
                   PermissionMessage::kHostsAll,
                   PermissionMessage::kTabs);
 
   return messages;
 }
 
 std::vector<base::string16> ChromePermissionMessageProvider::GetWarningMessages(
@@ skipping 52 matching lines @@
             IDS_EXTENSION_PROMPT_WARNING_MEDIA_GALLERIES_READ_DELETE;
         message_strings.push_back(l10n_util::GetStringUTF16(m_id));
         continue;
       } else if (id == PermissionMessage::kMediaGalleriesAllGalleriesCopyTo ||
                  id == PermissionMessage::kMediaGalleriesAllGalleriesDelete) {
         // The combined message will be pushed above.
         continue;
       }
     }
     if (permissions->HasAPIPermission(APIPermission::kSessions) &&
-        id == PermissionMessage::kTabs) {
+        (id == PermissionMessage::kTabs ||
+         id == PermissionMessage::kTopSites)) {
       message_strings.push_back(l10n_util::GetStringUTF16(
           IDS_EXTENSION_PROMPT_WARNING_TABS_AND_SESSIONS));

[meacer, 2014/06/11 20:03:10]

This is getting a bit complicated. The warning for tabs is "browsing activity",
and for topSites it is "browsing history", but when combined, they both become
"browsing history on all devices".

Question to you and kalman: What do you think about getting rid of "browsing
activity" altogether and use the same warning for both topSites and tabs? ("read
browsing history")

Then there would be four different strings:
Read your browsing history
Read your browsing history on all devices
Read and modify your browsing history
Read and modify your browsing history on all devices

[wjywbs, 2014/06/11 23:43:18]

The "tabs" API does not have any access to the history. You can collect history
by getting a list of opened URLs and monitoring to tab activities, such as
onClosed, onUpdated, etc. My only concern is that "read browsing history" is not
very accurate.

[meacer, 2014/06/12 00:05:08]

I mean, it kind of does, depending on one's definition of history. If you have
been using the extension for a month, then it has access to your 30 day browsing
history. The distinction between "browsing activity" and "browsing history" is
too subtle that I'm not sure if anyone really understands it.

       continue;
     }
     if (permissions->HasAPIPermission(APIPermission::kSessions) &&
         id == PermissionMessage::kBrowsingHistory) {
       message_strings.push_back(l10n_util::GetStringUTF16(
           IDS_EXTENSION_PROMPT_WARNING_BROWSING_HISTORY_AND_SESSIONS));
       continue;
     }
 
     message_strings.push_back(i->message());
@@ skipping 189 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