Replace ExtensionInstallChecker with generic PreloadCheckGroup

extensions/browser/preload_check_group.h

+// Copyright 2017 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.
+
+#ifndef EXTENSIONS_BROWSER_PRELOAD_CHECK_GROUP_H_
+#define EXTENSIONS_BROWSER_PRELOAD_CHECK_GROUP_H_
+
+#include <vector>
+
+#include "base/macros.h"
+#include "base/memory/weak_ptr.h"
+#include "base/threading/thread_checker.h"
+#include "extensions/browser/preload_check.h"
+
+namespace extensions {
+
+// PreloadCheckGroup runs a collection of other PreloadChecks and reports their
+// collective status once they have all finished. To stop the remaining checks
+// upon hitting the first error, use set_stop_on_first_error().
+class PreloadCheckGroup : public PreloadCheck {
+ public:
+  PreloadCheckGroup();
+  ~PreloadCheckGroup() override;
+
+  // Adds a check to run. Not owned. Must be called before Start().
+  void AddCheck(PreloadCheck* check);
+
+  // PreloadCheck:
+  void Start(ResultCallback callback) override;
+
+  void set_stop_on_first_error(bool value) { stop_on_first_error_ = value; }
+
+ private:
+  // Saves any errors and may invoke the callback.
+  virtual void OnCheckComplete(Errors errors);
+
+  // Invokes the callback if the checks are considered finished.
+  void MaybeInvokeCallback();
+
+  base::ThreadChecker thread_checker_;
+
+  // If true, the callback is invoked early when the first check fails,
+  // stopping the remaining checks.
+  bool stop_on_first_error_ = false;
+
+  // Checks to run. Not owned.

[Devlin, 2017/04/04 01:37:26]

Any particular reason to not own these?

[michaelpg, 2017/04/04 21:41:29]

TL;DR: unless you have a preference one way or another, I'll leave as-is and
maybe change it later depending on what use cases come up.

Users of PreloadCheckGroup may want to retain pointers to their individual
checks (e.g., CrxInstaller using its policy_check_ to get the policy-specific
error message). To own the checks here, PreloadCheckGroup would have to
explicitly guarantee that smart pointers moved into AddCheck() are not destroyed
until this class is destroyed. Users would have to ensure raw pointers to
PreloadChecks are not used after the PreloadCheckGroup is destroyed. Just seems
to require too much knowledge of the PreloadCheckGroup implementation.

OTOH, retaining a raw pointer after a function call ends is a bit sketchy. But
the owner of this class probably also owns the object pointed to, so it doesn't
seem like an anti-pattern to me. Style guide doesn't explicitly disallow it:
But I guess passing ownership to the Group would make things easier for users
who want to just create a check and pass it in, and not have to hold on to it.

+  std::vector<PreloadCheck*> checks_;
+
+  ResultCallback callback_;
+  int running_checks_ = 0;
+  Errors errors_;
+
+  base::WeakPtrFactory<PreloadCheckGroup> weak_ptr_factory_;
+
+  DISALLOW_COPY_AND_ASSIGN(PreloadCheckGroup);
+};
+
+}  // namespace extensions
+
+#endif  // EXTENSIONS_BROWSER_PRELOAD_CHECK_GROUP_H_