Retry reinstallation of corrupted policy extensions.

chrome/browser/extensions/content_verifier_browsertest.cc

 // Copyright 2014 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 <list>
 #include <set>
 #include <string>
 
 #include "base/bind_helpers.h"
 #include "base/callback_helpers.h"
 #include "base/macros.h"
 #include "base/memory/ptr_util.h"
 #include "base/run_loop.h"
 #include "base/scoped_observer.h"
 #include "base/strings/string_split.h"
 #include "base/threading/thread_task_runner_handle.h"
 #include "chrome/browser/extensions/browsertest_util.h"
 #include "chrome/browser/extensions/chrome_content_verifier_delegate.h"
 #include "chrome/browser/extensions/extension_browsertest.h"
 #include "chrome/browser/extensions/extension_management_test_util.h"
 #include "chrome/browser/extensions/extension_service.h"
+#include "chrome/browser/extensions/policy_extension_reinstaller.h"
 #include "chrome/common/chrome_switches.h"
 #include "components/policy/core/browser/browser_policy_connector.h"
 #include "components/policy/core/common/mock_configuration_policy_provider.h"
 #include "content/public/common/browser_side_navigation_policy.h"
 #include "content/public/test/test_utils.h"
 #include "extensions/browser/content_verifier.h"
 #include "extensions/browser/content_verify_job.h"
 #include "extensions/browser/crx_file_info.h"
 #include "extensions/browser/extension_prefs.h"
 #include "extensions/browser/extension_registry.h"
@@ skipping 741 matching lines @@
   EXPECT_TRUE(registry->enabled_extensions().Contains(id_));
 }
 
 namespace {
 
 // A helper for intercepting the normal action that
 // ChromeContentVerifierDelegate would take on discovering corruption, letting
 // us track the delay for each consecutive reinstall.
 class DelayTracker {
  public:
-  DelayTracker() {}
+  DelayTracker()
+      : action_(base::Bind(&DelayTracker::ReinstallAction,
+                           base::Unretained(this))) {
+    PolicyExtensionReinstaller::set_policy_reinstall_action_for_test(&action_);
+  }
+
+  ~DelayTracker() {
+    PolicyExtensionReinstaller::set_policy_reinstall_action_for_test(nullptr);
+  }
 
   const std::vector<base::TimeDelta>& calls() { return calls_; }
 
-  void ReinstallAction(base::TimeDelta delay) { calls_.push_back(delay); }
+  void ReinstallAction(const base::Closure& callback, base::TimeDelta delay) {
+    saved_callback_ = callback;
+    calls_.push_back(delay);
+  }
+
+  void Proceed() {
+    DCHECK(saved_callback_);

[Devlin, 2017/04/01 03:00:53]

nit: ASSERTs?

[lazyboy, 2017/04/03 18:13:04]

Done.

+    DCHECK(!saved_callback_->is_null());
+    // Run() will set |saved_callback_| again, so use a temporary: |callback|.
+    base::Closure callback = saved_callback_.value();
+    saved_callback_.reset();
+    callback.Run();
+  }
 
  private:
   std::vector<base::TimeDelta> calls_;
+  base::Optional<base::Closure> saved_callback_;
+  base::Callback<void(const base::Closure& callback, base::TimeDelta)> action_;
 
   DISALLOW_COPY_AND_ASSIGN(DelayTracker);
 };
 
 }  // namespace
 
 IN_PROC_BROWSER_TEST_F(ContentVerifierPolicyTest, Backoff) {
   ExtensionRegistry* registry = ExtensionRegistry::Get(profile());
   ExtensionSystem* system = ExtensionSystem::Get(profile());
-  ExtensionService* service = system->extension_service();
   ContentVerifier* verifier = system->content_verifier();
 
   // Wait for the extension to be installed by the policy we set up in
   // SetUpInProcessBrowserTestFixture.
   if (!registry->GetInstalledExtension(id_)) {
     RegistryObserver registry_observer(registry);
     EXPECT_TRUE(registry_observer.WaitForInstall(id_));
   }
 
   // Setup to intercept reinstall action, so we can see what the delay would
   // have been for the real action.
   DelayTracker delay_tracker;
-  base::Callback<void(base::TimeDelta)> action = base::Bind(
-      &DelayTracker::ReinstallAction, base::Unretained(&delay_tracker));
-  ChromeContentVerifierDelegate::set_policy_reinstall_action_for_test(&action);
 
   // Do 4 iterations of disabling followed by reinstall.
   const size_t iterations = 4;
   for (size_t i = 0; i < iterations; i++) {
+    // Setup to intercept reinstall action, so we can see what the delay would

[lazyboy, 2017/04/03 18:13:04]

note: Incorrect copy-paste.

+    // have been for the real action.
     RegistryObserver registry_observer(registry);
     verifier->VerifyFailed(id_, ContentVerifyJob::HASH_MISMATCH);
     EXPECT_TRUE(registry_observer.WaitForUnload(id_));
     // Trigger reinstall manually (since we overrode default reinstall action).
-    service->CheckForExternalUpdates();
+    delay_tracker.Proceed();
     EXPECT_TRUE(registry_observer.WaitForInstall(id_));
   }
   const std::vector<base::TimeDelta>& calls = delay_tracker.calls();
 
+  // We will get one more call than |iterations| because of how DelayTracker is

[Devlin, 2017/04/01 03:00:53]

Can you elaborate on where this "one more" comes from?  Also, if it's exactly
one more, could we assert_eq(iterations, calls.size() - 1)?

[lazyboy, 2017/04/03 18:13:04]

Added, also changed to iterations == calls.size()-1

+  // implemented.
+  ASSERT_LE(iterations, calls.size());
   // Assert that the first reinstall action happened with a delay of 0, and
   // then kept growing each additional time.
-  ASSERT_EQ(iterations, calls.size());
   EXPECT_EQ(base::TimeDelta(), delay_tracker.calls()[0]);
   for (size_t i = 1; i < delay_tracker.calls().size(); i++) {
     EXPECT_LT(calls[i - 1], calls[i]);
   }
 }
 
+// Tests that if CheckForExternalUpdates() fails, then we retry reinstalling

[Devlin, 2017/04/01 03:00:53]

Elaborate on when this can happen

[lazyboy, 2017/04/03 18:13:04]

Done.

+// corrupted policy extensions.
+IN_PROC_BROWSER_TEST_F(ContentVerifierPolicyTest, FailedUpdateRetries) {
+  ExtensionRegistry* registry = ExtensionRegistry::Get(profile());
+  ExtensionSystem* system = ExtensionSystem::Get(profile());
+  ExtensionService* service = system->extension_service();
+  ContentVerifier* verifier = system->content_verifier();
+
+  // Wait for the extension to be installed by the policy we set up in
+  // SetUpInProcessBrowserTestFixture.
+  if (!registry->GetInstalledExtension(id_)) {
+    RegistryObserver registry_observer(registry);
+    EXPECT_TRUE(registry_observer.WaitForInstall(id_));
+  }
+
+  DelayTracker delay_tracker;
+  base::Callback<void(const base::Closure& callback, base::TimeDelta)> action =
+      base::Bind(&DelayTracker::ReinstallAction,
+                 base::Unretained(&delay_tracker));
+  PolicyExtensionReinstaller::set_policy_reinstall_action_for_test(&action);

[Devlin, 2017/04/01 03:00:53]

Doesn't DelayTracker's ctor do this?

[lazyboy, 2017/04/03 18:13:04]

Right, thanks.
Done.

+
+  service->set_external_updates_disabled_for_test(true);
+  RegistryObserver registry_observer(registry);
+  verifier->VerifyFailed(id_, ContentVerifyJob::HASH_MISMATCH);
+  EXPECT_TRUE(registry_observer.WaitForUnload(id_));
+
+  const std::vector<base::TimeDelta>& calls = delay_tracker.calls();
+  ASSERT_EQ(1u, calls.size());
+  EXPECT_EQ(base::TimeDelta(), delay_tracker.calls()[0]);
+
+  delay_tracker.Proceed();
+
+  // Remove the override and set ExtensionService to update again. The extension
+  // should be now installed.
+  PolicyExtensionReinstaller::set_policy_reinstall_action_for_test(nullptr);
+  service->set_external_updates_disabled_for_test(false);
+  delay_tracker.Proceed();
+
+  EXPECT_TRUE(registry_observer.WaitForInstall(id_));
+}
+
 }  // namespace extensions