Register granting of previously withheld permissions as a permissions

chrome/browser/extensions/crx_installer.cc

Index: chrome/browser/extensions/crx_installer.cc
diff --git a/chrome/browser/extensions/crx_installer.cc b/chrome/browser/extensions/crx_installer.cc
index 8a370a03f3bee350911abfcbe7003d3f0afab23f..bc49ed2a00d32d848d88be62d800ae50b8004c52 100644
--- a/chrome/browser/extensions/crx_installer.cc
+++ b/chrome/browser/extensions/crx_installer.cc
@@ -459,6 +459,26 @@ void CrxInstaller::OnUnpackSuccess(
   // the temp dir.
   unpacked_extension_root_ = extension_dir;
 
+  if (!BrowserThread::PostTask(
+           BrowserThread::UI,
+           FROM_HERE,
+           base::Bind(&CrxInstaller::InitializeExtensionPermissions, this)))

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

Ew.  We already bounce around a ton in extension installation, but I'd love to
avoid any extra.  Can we instead move the permissions update to
OnInstallChecksComplete()?  InstallChecker doesn't check anything with
permissions, and, if you're worried, we can put in a comment mentioning that the
permissions in the extension in installchecker might not reflect the total
permissions.

[gpdavis, 2014/09/09 17:48:53]

Well, the goal here is to call InitializePermissions before IsPrivilegeIncrease
is called in AllowInstall, because that's what's catching the permissions
elevation.  This is why I was having threading trouble; the timing is quite
sensitive.  If we move it to OnInstallChecksComplete, we'd have to make another
call to IsPrivilegeIncrease (or move the call) to catch it with the appropriate
permissions, which is certainly possible, but I'm not sure if it would have any
unintended consequences.

I certainly agree that the thread jumping is ugly, though.  It just seemed like
the least impactful change.  What do you think?

[Devlin, 2014/09/09 17:57:44]

Hmm... that makes me wonder if this is actually the right place for this.

The goal is to have this trigger a permissions increase via
ExtensionService::CheckPermissionsIncrease().  This actually looks like all it
will do is not install the extension (and throw an error saying the manifest is
invalid).  That's not quite right, right?

+    NOTREACHED();
+}
+
+void CrxInstaller::InitializeExtensionPermissions() {
+  DCHECK_CURRENTLY_ON(BrowserThread::UI);
+  PermissionsUpdater(profile()).InitializePermissions(
+      install_checker_.extension().get());
+  if (!installer_task_runner_->PostTask(
+           FROM_HERE,
+           base::Bind(&CrxInstaller::ContinueUnpackSuccess, this)))
+    NOTREACHED();
+}
+
+void CrxInstaller::ContinueUnpackSuccess() {
+  DCHECK(installer_task_runner_->RunsTasksOnCurrentThread());
+  const Extension* extension = install_checker_.extension().get();
   CrxInstallerError error = AllowInstall(extension);
   if (error.type() != CrxInstallerError::ERROR_NONE) {
     ReportFailureFromFileThread(error);