Update extension install prompt to reflect withheld permissions

chrome/browser/extensions/extension_install_prompt.cc

Index: chrome/browser/extensions/extension_install_prompt.cc
diff --git a/chrome/browser/extensions/extension_install_prompt.cc b/chrome/browser/extensions/extension_install_prompt.cc
index dec1aa54f3b756b3d1ba147b74b8179e6c912cf0..27c5bb4ba9926618b60b015301355296e7af7a93 100644
--- a/chrome/browser/extensions/extension_install_prompt.cc
+++ b/chrome/browser/extensions/extension_install_prompt.cc
@@ -16,6 +16,7 @@
 #include "chrome/browser/extensions/bundle_installer.h"
 #include "chrome/browser/extensions/extension_install_ui.h"
 #include "chrome/browser/extensions/extension_util.h"
+#include "chrome/browser/extensions/permissions_updater.h"
 #include "chrome/browser/profiles/profile.h"
 #include "chrome/browser/ui/browser.h"
 #include "chrome/browser/ui/browser_window.h"
@@ -219,16 +220,41 @@ ExtensionInstallPrompt::Prompt::~Prompt() {
 }
 
 void ExtensionInstallPrompt::Prompt::SetPermissions(
-    const std::vector<base::string16>& permissions) {
-  permissions_ = permissions;
+    const std::vector<base::string16>& permissions,
+    PermissionsType permissions_type) {
+  switch (permissions_type) {
+    case REGULAR_PERMISSIONS:
+      permissions_ = permissions;
+      break;
+    case WITHHELD_PERMISSIONS:
+      withheld_permissions_ = permissions;
+      break;
+    case ALL_PERMISSIONS:
+      NOTREACHED();
+      break;
+  }
 }
 
 void ExtensionInstallPrompt::Prompt::SetPermissionsDetails(
-    const std::vector<base::string16>& details) {
-  details_ = details;
-  is_showing_details_for_permissions_.clear();
-  for (size_t i = 0; i < details.size(); ++i)
-    is_showing_details_for_permissions_.push_back(false);
+    const std::vector<base::string16>& details,
+    PermissionsType permissions_type) {
+  switch (permissions_type) {
+    case REGULAR_PERMISSIONS:
+      details_ = details;
+      is_showing_details_for_permissions_.clear();
+      for (size_t i = 0; i < details.size(); ++i)
+        is_showing_details_for_permissions_.push_back(false);
+      break;
+    case WITHHELD_PERMISSIONS:
+      withheld_details_ = details;
+      is_showing_details_for_withheld_permissions_.clear();
+      for (size_t i = 0; i < details.size(); ++i)
+        is_showing_details_for_withheld_permissions_.push_back(false);
+      break;
+    case ALL_PERMISSIONS:
+      NOTREACHED();
+      break;
+  }
 }
 
 void ExtensionInstallPrompt::Prompt::SetIsShowingDetails(
@@ -239,6 +265,9 @@ void ExtensionInstallPrompt::Prompt::SetIsShowingDetails(
     case PERMISSIONS_DETAILS:
       is_showing_details_for_permissions_[index] = is_showing_details;
       break;
+    case WITHHELD_PERMISSIONS_DETAILS:
+      is_showing_details_for_withheld_permissions_[index] = is_showing_details;
+      break;
     case RETAINED_FILES_DETAILS:
       is_showing_details_for_retained_files_ = is_showing_details;
       break;
@@ -354,8 +383,17 @@ base::string16 ExtensionInstallPrompt::Prompt::GetAbortButtonLabel() const {
   return l10n_util::GetStringUTF16(kAbortButtonIds[type_]);
 }
 
-base::string16 ExtensionInstallPrompt::Prompt::GetPermissionsHeading() const {
-  return l10n_util::GetStringUTF16(kPermissionsHeaderIds[type_]);
+base::string16 ExtensionInstallPrompt::Prompt::GetPermissionsHeading(
+    PermissionsType permissions_type) const {
+  switch (permissions_type) {
+    case REGULAR_PERMISSIONS:
+      return l10n_util::GetStringUTF16(kPermissionsHeaderIds[type_]);
+    case WITHHELD_PERMISSIONS:
+      return l10n_util::GetStringUTF16(IDS_EXTENSION_PROMPT_WITHHELD);
+    case ALL_PERMISSIONS:
+      NOTREACHED();
+      return base::string16();
+  }
 }
 
 base::string16 ExtensionInstallPrompt::Prompt::GetRetainedFilesHeading() const {
@@ -375,7 +413,9 @@ base::string16 ExtensionInstallPrompt::Prompt::GetRetainedFilesHeading() const {
 }
 
 bool ExtensionInstallPrompt::Prompt::ShouldShowPermissions() const {
-  return GetPermissionCount() > 0 || type_ == POST_INSTALL_PERMISSIONS_PROMPT;
+  return GetPermissionCount(REGULAR_PERMISSIONS) > 0 ||
+         GetPermissionCount(WITHHELD_PERMISSIONS) > 0 ||
+         type_ == POST_INSTALL_PERMISSIONS_PROMPT;
 }
 
 void ExtensionInstallPrompt::Prompt::AppendRatingStars(
@@ -423,24 +463,60 @@ base::string16 ExtensionInstallPrompt::Prompt::GetUserCount() const {
   return base::string16();
 }
 
-size_t ExtensionInstallPrompt::Prompt::GetPermissionCount() const {
-  return permissions_.size();
+size_t ExtensionInstallPrompt::Prompt::GetPermissionCount(
+    PermissionsType permissions_type) const {
+  switch (permissions_type) {
+    case REGULAR_PERMISSIONS:
+      return permissions_.size();
+    case WITHHELD_PERMISSIONS:
+      return withheld_permissions_.size();
+    case ALL_PERMISSIONS:
+      return permissions_.size() + withheld_permissions_.size();
+  }
 }
 
-size_t ExtensionInstallPrompt::Prompt::GetPermissionsDetailsCount() const {
-  return details_.size();
+size_t ExtensionInstallPrompt::Prompt::GetPermissionsDetailsCount(
+    PermissionsType permissions_type) const {
+  switch (permissions_type) {
+    case REGULAR_PERMISSIONS:
+      return details_.size();
+    case WITHHELD_PERMISSIONS:
+      return withheld_details_.size();
+    case ALL_PERMISSIONS:
+      return details_.size() + withheld_details_.size();
+  }
 }
 
-base::string16 ExtensionInstallPrompt::Prompt::GetPermission(size_t index)
-    const {
-  CHECK_LT(index, permissions_.size());
-  return permissions_[index];
+base::string16 ExtensionInstallPrompt::Prompt::GetPermission(
+    size_t index,
+    PermissionsType permissions_type) const {
+  switch (permissions_type) {
+    case REGULAR_PERMISSIONS:
+      CHECK_LT(index, permissions_.size());
+      return permissions_[index];
+    case WITHHELD_PERMISSIONS:
+      CHECK_LT(index, withheld_permissions_.size());
+      return withheld_permissions_[index];
+    case ALL_PERMISSIONS:
+      NOTREACHED();
+      return base::string16();
+  }
 }
 
 base::string16 ExtensionInstallPrompt::Prompt::GetPermissionsDetails(
-    size_t index) const {
-  CHECK_LT(index, details_.size());
-  return details_[index];
+    size_t index,
+    PermissionsType permissions_type) const {
+  switch (permissions_type) {
+    case REGULAR_PERMISSIONS:
+      CHECK_LT(index, details_.size());
+      return details_[index];
+    case WITHHELD_PERMISSIONS:
+      CHECK_LT(index, withheld_details_.size());
+      return withheld_details_[index];
+    case ALL_PERMISSIONS:
+      NOTREACHED();
+      return base::string16();
+  }
 }
 
 bool ExtensionInstallPrompt::Prompt::GetIsShowingDetails(
@@ -449,6 +525,9 @@ bool ExtensionInstallPrompt::Prompt::GetIsShowingDetails(
     case PERMISSIONS_DETAILS:
       CHECK_LT(index, is_showing_details_for_permissions_.size());
       return is_showing_details_for_permissions_[index];
+    case WITHHELD_PERMISSIONS_DETAILS:
+      CHECK_LT(index, is_showing_details_for_withheld_permissions_.size());
+      return is_showing_details_for_withheld_permissions_[index];
     case RETAINED_FILES_DETAILS:
       return is_showing_details_for_retained_files_;
   }
@@ -747,10 +826,32 @@ void ExtensionInstallPrompt::ShowConfirmation() {
         extension_ ? extension_->GetType() : Manifest::TYPE_UNKNOWN;
     const extensions::PermissionMessageProvider* message_provider =
         extensions::PermissionMessageProvider::Get();
+
+    // Initialize permissions so that withheld permissions don't end up in the
+    // install prompt.
+    if (extension_) {
+      extensions::PermissionsUpdater(install_ui_->profile())
+          .InitializePermissions(
+              extension_, extensions::PermissionsUpdater::INIT_FLAG_TRANSIENT);
+      permissions_ = extension_->permissions_data()->active_permissions();

[Devlin, 2014/09/02 21:07:51]

This will fantastically break optional permissions' use of the install prompt.

OptionalPermissions creates an install prompt and sets its extension and
permissions, but the permissions will be set to the *desired* permissions, not
to the existing permissions. [1]  As such, we need to (in some cases) respect
the |permissions_| which were set earlier.

[gpdavis, 2014/09/04 19:40:04]

Okay, I think I figured this out.  I've removed the permissions_ setters that
just use active permissions, and instead, I added logic that will initialize
permissions and set permissions_ if permissions_ hasn't already been set.  This
way, special cases can manually set permissions_ before ShowConfirmation is
called, and they won't get overridden.

Can you think of any issues that might arise from this implementation?

+    }
     prompt_->SetPermissions(
-        message_provider->GetWarningMessages(permissions_.get(), type));
+        message_provider->GetWarningMessages(permissions_.get(), type),
+        REGULAR_PERMISSIONS);
     prompt_->SetPermissionsDetails(
-        message_provider->GetWarningMessagesDetails(permissions_.get(), type));
+        message_provider->GetWarningMessagesDetails(permissions_.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),
+          PermissionsType::WITHHELD_PERMISSIONS);
+      prompt_->SetPermissionsDetails(
+          message_provider->GetWarningMessagesDetails(withheld.get(), type),
+          PermissionsType::WITHHELD_PERMISSIONS);
+    }
   }
 
   switch (prompt_->type()) {