Continue attempts to reinstall corrupt policy extensions across restarts

chrome/browser/extensions/installed_loader.cc

 // Copyright (c) 2012 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/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"
 #include "base/metrics/sparse_histogram.h"
 #include "base/strings/stringprintf.h"
 #include "base/strings/utf_string_conversions.h"
 #include "base/threading/thread_restrictions.h"
 #include "base/trace_event/trace_event.h"
 #include "base/values.h"
 #include "chrome/browser/browser_process.h"
@@ skipping 180 matching lines @@
     extension = NULL;
   }
 
   // Check policy on every load in case an extension was blacklisted while
   // Chrome was not running.
   const ManagementPolicy* policy = extensions::ExtensionSystem::Get(
       extension_service_->profile())->management_policy();
   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);
   }
 
   if (!extension.get()) {
     ExtensionErrorReporter::GetInstance()->ReportLoadError(
         info.extension_path,
         error,
         extension_service_->profile(),
@@ skipping 378 matching lines @@
 int InstalledLoader::GetCreationFlags(const ExtensionInfo* info) {
   int flags = extension_prefs_->GetCreationFlags(info->extension_id);
   if (!Manifest::IsUnpackedLocation(info->extension_location))
     flags |= Extension::REQUIRE_KEY;
   if (extension_prefs_->AllowFileAccess(info->extension_id))
     flags |= Extension::ALLOW_FILE_ACCESS;
   return flags;
 }
 
 }  // namespace extensions