Continue attempts to reinstall corrupt policy extensions across restarts

chrome/browser/extensions/installed_loader.cc

Index: chrome/browser/extensions/installed_loader.cc
diff --git a/chrome/browser/extensions/installed_loader.cc b/chrome/browser/extensions/installed_loader.cc
index 197410b5c9b9e188108ab09c1c440e43e1b9a9df..221b765ffcd000afff003bfd1c421f36325127aa 100644
--- a/chrome/browser/extensions/installed_loader.cc
+++ b/chrome/browser/extensions/installed_loader.cc
@@ -5,6 +5,9 @@
 #include "chrome/browser/extensions/installed_loader.h"
 
 #include <stddef.h>
+#include <memory>

[lazyboy, 2016/11/16 01:44:53]

Do you need these includes? I don't see any new code using them.

[asargent_no_longer_on_chrome, 2016/11/17 21:48:01]

I'm just fixing lint errors since I'm modifying the file anyway (I try to be in
the habit of running 'git cl lint' on my CLs and fixing any problems that show
whether related to my changes or not)

[lazyboy, 2016/11/17 21:59:09]

Good to know, thanks.

+#include <string>
+#include <vector>
 
 #include "base/files/file_path.h"
 #include "base/metrics/histogram_macros.h"
@@ -205,16 +208,23 @@ void InstalledLoader::Load(const ExtensionInfo& info, bool write_to_prefs) {
   if (extension.get()) {
     Extension::DisableReason disable_reason = Extension::DISABLE_NONE;
     bool force_disabled = false;
-    if (!policy->UserMayLoad(extension.get(), NULL)) {
+    if (!policy->UserMayLoad(extension.get(), nullptr)) {
       // The error message from UserMayInstall() often contains the extension ID
       // and is therefore not well suited to this UI.
       error = errors::kDisabledByPolicy;
       extension = NULL;
     } else if (!extension_prefs_->IsExtensionDisabled(extension->id()) &&
-               policy->MustRemainDisabled(
-                   extension.get(), &disable_reason, NULL)) {
+               policy->MustRemainDisabled(extension.get(), &disable_reason,
+                                          nullptr)) {
       extension_prefs_->SetExtensionDisabled(extension->id(), disable_reason);
       force_disabled = true;
+    } else if (extension_prefs_->IsExtensionDisabled(extension->id()) &&
+               policy->MustRemainEnabled(extension.get(), nullptr) &&
+               extension_prefs_->HasDisableReason(
+                   extension->id(), Extension::DISABLE_CORRUPTED)) {
+      PendingExtensionManager* pending_manager =

[lazyboy, 2016/11/16 01:44:53]

I'd add a note saying sth that explains this, as it's harder to figure out
what's going on:
If we shut down last time while corrupted policy force extension were being
reinstalled and but the reinstallation didn't succeed/finish, retry.

I also wonder if some clients can get into a state where this keeps happening on
every startup, maybe because the policy has changed to not include |extension|
anymore. Can that even happen? Just curious.

[asargent_no_longer_on_chrome, 2016/11/17 21:48:01]

Good feedback - I've added a comment. 
Good question - I think we are safe in this case. If the policy changes to not
include the extension anymore, it will get uninstalled by
ExtensionService::OnAllExternalProvidersReady (it iterates over all extensions,
and any with an "external" location that aren't provided by any of the
ExternalProvider's gets uninstalled). 

[lazyboy, 2016/11/17 21:59:09]

Acknowledged.

+          extension_service_->pending_extension_manager();
+      pending_manager->ExpectPolicyReinstallForCorruption(extension->id());
     }
     UMA_HISTOGRAM_BOOLEAN("ExtensionInstalledLoader.ForceDisabled",
                           force_disabled);