developerPrivate.repair: Skip not-corrupted and policy extensions

chrome/browser/extensions/api/developer_private/developer_private_api_unittest.cc

 // Copyright 2015 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 <memory>
 #include <utility>
 
 #include "base/files/file_util.h"
 #include "base/macros.h"
 #include "base/memory/ptr_util.h"
 #include "base/strings/utf_string_conversions.h"
+#include "chrome/browser/chrome_notification_types.h"
 #include "chrome/browser/extensions/error_console/error_console.h"
 #include "chrome/browser/extensions/extension_function_test_utils.h"
 #include "chrome/browser/extensions/extension_service.h"
-#include "chrome/browser/extensions/extension_service_test_base.h"
+#include "chrome/browser/extensions/extension_service_test_with_install.h"
 #include "chrome/browser/extensions/extension_util.h"
 #include "chrome/browser/extensions/scripting_permissions_modifier.h"
 #include "chrome/browser/extensions/test_extension_dir.h"
 #include "chrome/browser/extensions/test_extension_system.h"
 #include "chrome/browser/extensions/unpacked_installer.h"
 #include "chrome/browser/ui/browser.h"
 #include "chrome/common/extensions/api/developer_private.h"
 #include "chrome/common/pref_names.h"
 #include "chrome/test/base/test_browser_window.h"
 #include "components/crx_file/id_util.h"
 #include "components/policy/core/browser/browser_policy_connector.h"
 #include "components/policy/core/common/mock_configuration_policy_provider.h"
 #include "components/policy/core/common/policy_map.h"
 #include "components/policy/core/common/policy_service_impl.h"
 #include "components/policy/core/common/policy_types.h"
 #include "components/policy/policy_constants.h"
 #include "components/sync_preferences/testing_pref_service_syncable.h"
+#include "content/public/browser/notification_service.h"
 #include "content/public/test/web_contents_tester.h"
 #include "extensions/browser/api_test_utils.h"
 #include "extensions/browser/event_router_factory.h"
 #include "extensions/browser/extension_error_test_util.h"
 #include "extensions/browser/extension_prefs.h"
 #include "extensions/browser/extension_registry.h"
 #include "extensions/browser/extension_system.h"
 #include "extensions/browser/extension_util.h"
+#include "extensions/browser/external_install_info.h"
 #include "extensions/browser/test_extension_registry_observer.h"
 #include "extensions/common/extension.h"
 #include "extensions/common/extension_builder.h"
 #include "extensions/common/extension_set.h"
 #include "extensions/common/feature_switch.h"
 #include "extensions/common/manifest_constants.h"
 #include "extensions/common/test_util.h"
 #include "extensions/common/value_builder.h"
 
 using testing::Return;
 using testing::_;
 
 namespace extensions {
 
 namespace {
 
+const char good_crx[] = "ldnnhddmnhbkjipkidpdiheffobcpfmf";
+
 std::unique_ptr<KeyedService> BuildAPI(content::BrowserContext* context) {
   return base::MakeUnique<DeveloperPrivateAPI>(context);
 }
 
 std::unique_ptr<KeyedService> BuildEventRouter(
     content::BrowserContext* profile) {
   return base::MakeUnique<EventRouter>(profile, ExtensionPrefs::Get(profile));
 }
 
 bool HasAllUrlsPermission(const Extension* extension,
                           content::BrowserContext* context) {
   return ScriptingPermissionsModifier(context, extension).IsAllowedOnAllUrls();
 }
 
 bool HasPrefsPermission(bool (*has_pref)(const std::string&,
                                          content::BrowserContext*),
                         content::BrowserContext* context,
                         const std::string& id) {
   return has_pref(id, context);
 }
 
+// This class is almost entirely copied from extension_service_unittest.cc.
+// TODO(lazyboy): ) Extract to a separate file.
+class MockExtensionProvider : public extensions::ExternalProviderInterface {
+ public:
+  MockExtensionProvider(VisitorInterface* visitor, Manifest::Location location)
+      : location_(location), visitor_(visitor) {}
+
+  ~MockExtensionProvider() override {}
+
+  void AddExtension(const std::string& id,
+                    const std::string& version,
+                    const base::FilePath& path) {
+    extension_map_[id] = std::make_pair(version, path);
+  }
+
+  void RemoveExtension(const std::string& id) { extension_map_.erase(id); }
+
+  // ExternalProvider implementation:
+  void VisitRegisteredExtension() override {
+    for (DataMap::const_iterator i = extension_map_.begin();
+         i != extension_map_.end(); ++i) {
+      std::unique_ptr<base::Version> version(
+          new base::Version(i->second.first));
+
+      std::unique_ptr<ExternalInstallInfoFile> info(new ExternalInstallInfoFile(
+          i->first, std::move(version), i->second.second, location_,
+          Extension::NO_FLAGS, false, false));
+      visitor_->OnExternalExtensionFileFound(*info);
+    }
+    visitor_->OnExternalProviderReady(this);
+  }
+
+  bool HasExtension(const std::string& id) const override {
+    return extension_map_.find(id) != extension_map_.end();
+  }
+
+  bool GetExtensionDetails(
+      const std::string& id,
+      Manifest::Location* location,
+      std::unique_ptr<base::Version>* version) const override {
+    DataMap::const_iterator it = extension_map_.find(id);
+    if (it == extension_map_.end())
+      return false;
+
+    if (version)
+      version->reset(new base::Version(it->second.first));
+
+    if (location)
+      *location = location_;
+
+    return true;
+  }
+
+  bool IsReady() const override { return true; }
+
+  void ServiceShutdown() override {}
+
+ private:
+  using DataMap = std::map<std::string, std::pair<std::string, base::FilePath>>;
+  DataMap extension_map_;
+  Manifest::Location location_;
+  VisitorInterface* visitor_;
+
+  DISALLOW_COPY_AND_ASSIGN(MockExtensionProvider);
+};
+
 }  // namespace
 
-class DeveloperPrivateApiUnitTest : public ExtensionServiceTestBase {
+class DeveloperPrivateApiUnitTest : public ExtensionServiceTestWithInstall {
  protected:
   DeveloperPrivateApiUnitTest() {}
   ~DeveloperPrivateApiUnitTest() override {}
 
+  void AddMockExternalProvider(
+      std::unique_ptr<ExternalProviderInterface> provider) {
+    service()->AddProviderForTesting(std::move(provider));
+  }
+
   // A wrapper around extension_function_test_utils::RunFunction that runs with
   // the associated browser, no flags, and can take stack-allocated arguments.
   bool RunFunction(const scoped_refptr<UIThreadExtensionFunction>& function,
                    const base::ListValue& args);
 
   // Loads an unpacked extension that is backed by a real directory, allowing
   // it to be reloaded.
   const Extension* LoadUnpackedExtension();
 
   // Loads an extension with no real directory; this is faster, but means the
@@ skipping 750 matching lines @@
       ListBuilder()
           .Append(
               DictionaryBuilder().Set("extensionId", extension->id()).Build())
           .Build();
   function = new api::DeveloperPrivateDeleteExtensionErrorsFunction();
   EXPECT_TRUE(RunFunction(function, *args)) << function->GetError();
   // No more errors!
   EXPECT_TRUE(error_console->GetErrorsForExtension(extension->id()).empty());
 }
 
+// Tests that developerPrivate.repair does not succeed for a non-corrupted
+// extension and any policy extension.
+// Regression test for https://crbug.com/577959.
+TEST_F(DeveloperPrivateApiUnitTest, RepairNotBrokenExtension) {
+  std::string extension_id(good_crx);
+
+  // Set up a mock provider with a policy extension.
+  std::unique_ptr<MockExtensionProvider> mock_provider =
+      base::MakeUnique<MockExtensionProvider>(
+          service(), Manifest::EXTERNAL_POLICY_DOWNLOAD);
+  MockExtensionProvider* mock_provider_ptr = mock_provider.get();
+  AddMockExternalProvider(std::move(mock_provider));
+  mock_provider_ptr->AddExtension(extension_id, "1.0.0.0",
+                                  data_dir().AppendASCII("good.crx"));
+  // Reloading extensions should find our externally registered extension
+  // and install it.
+  content::WindowedNotificationObserver observer(
+      extensions::NOTIFICATION_CRX_INSTALLER_DONE,
+      content::NotificationService::AllSources());
+  service()->CheckForExternalUpdates();
+  observer.Wait();
+
+  // Attempt to repair the good extension, expect failure.
+  std::unique_ptr<base::ListValue> args =
+      ListBuilder().Append(extension_id).Build();
+  scoped_refptr<UIThreadExtensionFunction> function =
+      new api::DeveloperPrivateRepairExtensionFunction();
+  EXPECT_FALSE(RunFunction(function, *args));
+  EXPECT_EQ("Cannot repair a not corrupted extension.", function->GetError());
+
+  // Corrupt the extension , still expect repair failure because this is a
+  // policy extension.
+  service()->DisableExtension(extension_id, Extension::DISABLE_CORRUPTED);
+  args = ListBuilder().Append(extension_id).Build();
+  function = new api::DeveloperPrivateRepairExtensionFunction();
+  EXPECT_FALSE(RunFunction(function, *args));
+  EXPECT_EQ("Cannot repair a policy extension.", function->GetError());

[Devlin, 2017/04/06 21:01:32]

I'd slightly prefer two separate tests - one for uncorrupted, and one for policy
- so that we know we don't allow repair of uncorrupted extensions independent of
policy status.

[lazyboy, 2017/04/06 22:32:56]

Done.

+}
+
 // Test developerPrivate.updateProfileConfiguration: Try to turn on devMode
 // when DeveloperToolsDisabled policy is active.
 TEST_F(DeveloperPrivateApiUnitTest, DeveloperPrivateDevModeDisabledPolicy) {
   testing_pref_service()->SetManagedPref(prefs::kExtensionsUIDeveloperMode,
                                          base::MakeUnique<base::Value>(false));
 
   UpdateProfileConfigurationDevMode(true);
 
   EXPECT_FALSE(
       profile()->GetPrefs()->GetBoolean(prefs::kExtensionsUIDeveloperMode));
@@ skipping 15 matching lines @@
   EXPECT_TRUE(
       profile()->GetPrefs()->GetBoolean(prefs::kExtensionsUIDeveloperMode));
 
   std::unique_ptr<api::developer_private::ProfileInfo> profile_info;
   ASSERT_NO_FATAL_FAILURE(GetProfileConfiguration(&profile_info));
   EXPECT_TRUE(profile_info->in_developer_mode);
   EXPECT_FALSE(profile_info->is_developer_mode_controlled_by_policy);
 }
 
 }  // namespace extensions