developerPrivate.repair: Skip not-corrupted and policy extensions

chrome/browser/extensions/api/developer_private/developer_private_api.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/api/developer_private/developer_private_api.h"
 
 #include <stddef.h>
 #include <utility>
 
 #include "base/bind.h"
@@ skipping 57 matching lines @@
 #include "extensions/browser/file_highlighter.h"
 #include "extensions/browser/management_policy.h"
 #include "extensions/browser/notification_types.h"
 #include "extensions/browser/warning_service.h"
 #include "extensions/common/constants.h"
 #include "extensions/common/extension_set.h"
 #include "extensions/common/feature_switch.h"
 #include "extensions/common/install_warning.h"
 #include "extensions/common/manifest.h"
 #include "extensions/common/manifest_handlers/options_page_info.h"
+#include "extensions/common/manifest_url_handlers.h"
 #include "extensions/common/permissions/permissions_data.h"
 #include "extensions/grit/extensions_browser_resources.h"
 #include "storage/browser/fileapi/external_mount_points.h"
 #include "storage/browser/fileapi/file_system_context.h"
 #include "storage/browser/fileapi/file_system_operation.h"
 #include "storage/browser/fileapi/file_system_operation_runner.h"
 #include "storage/browser/fileapi/isolated_context.h"
 #include "ui/base/l10n/l10n_util.h"
 
 namespace extensions {
@@ skipping 14 matching lines @@
 const char kNoSuchRendererError[] = "No such renderer.";
 const char kInvalidPathError[] = "Invalid path.";
 const char kManifestKeyIsRequiredError[] =
     "The 'manifestKey' argument is required for manifest files.";
 const char kCouldNotFindWebContentsError[] =
     "Could not find a valid web contents.";
 const char kCannotUpdateSupervisedProfileSettingsError[] =
     "Cannot change settings for a supervised profile.";
 const char kNoOptionsPageForExtensionError[] =
     "Extension does not have an options page.";
+const char kCannotRepairNotCorruptedExtension[] =
+    "Cannot repair a not corrupted extension.";
 
 const char kUnpackedAppsFolder[] = "apps_target";
 const char kManifestFile[] = "manifest.json";
 
 ExtensionService* GetExtensionService(content::BrowserContext* context) {
   return ExtensionSystem::Get(context)->extension_service();
 }
 
 std::string ReadFileToString(const base::FilePath& path) {
   std::string data;
@@ skipping 1332 matching lines @@
 
 ExtensionFunction::ResponseAction
 DeveloperPrivateRepairExtensionFunction::Run() {
   std::unique_ptr<developer::RepairExtension::Params> params(
       developer::RepairExtension::Params::Create(*args_));
   EXTENSION_FUNCTION_VALIDATE(params);
   const Extension* extension = GetExtensionById(params->extension_id);
   if (!extension)
     return RespondNow(Error(kNoSuchExtensionError));
 
+  ExtensionSystem* extension_system = ExtensionSystem::Get(browser_context());
+  if (extension_system->management_policy()->MustRemainEnabled(extension,
+                                                               nullptr)) {
+    int disable_reason = ExtensionPrefs::Get(browser_context())
+                             ->GetDisableReasons(extension->id());
+    // Do not let a policy extension that is not corrupted to be
+    // reinstalled/repaired. If we let |reinstaller| continue with the repair,
+    // this would have uninstalled the extension but then we would have failed
+    // to reinstall it for policy check.
+    if ((disable_reason & Extension::DISABLE_CORRUPTED) == 0)

[Devlin, 2017/04/06 15:12:31]

To elaborate on a previous comment, I don't think we should be allowing repair()
for uncorrupted extensions at all.

What I'd see this looking like is:
if (!ExtensionPrefs::Get()->HasDisableReason(id, DISABLE_CORRUPTED)) {
  return RespondNow(Error(kCannot...));
}

// Depending on the answer to my other comment
if (management_policy()->MustRemainEnabled/Installed())
  return RespondNow(Error(kCannotRepairPolicyExtension));

There's no reason we should be trying to repair an uncorrupted extension, so we
may as well disallow it in all cases (even if it's not as much of a concern).

[lazyboy, 2017/04/06 18:06:05]

Done.

+      return RespondNow(Error(kCannotRepairNotCorruptedExtension));
+  }
+
   content::WebContents* web_contents = GetSenderWebContents();
   if (!web_contents)
     return RespondNow(Error(kCouldNotFindWebContentsError));
 
   scoped_refptr<WebstoreReinstaller> reinstaller(new WebstoreReinstaller(
       web_contents,
       params->extension_id,
       base::Bind(&DeveloperPrivateRepairExtensionFunction::OnReinstallComplete,
                  this)));
   reinstaller->BeginReinstall();
@@ skipping 83 matching lines @@
         update.extension_id, update.command_name, *update.keybinding);
   }
 
   return RespondNow(NoArguments());
 }
 
 
 }  // namespace api
 
 }  // namespace extensions