Update ExtensionInstallChecker to use PreloadCheck classes

extensions/browser/preload_check_test_util.cc

 // 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.
 
 #include "extensions/browser/preload_check_test_util.h"
 
 #include "base/bind.h"
 #include "base/bind_helpers.h"
 #include "base/logging.h"
 #include "base/memory/ptr_util.h"
 #include "base/run_loop.h"
+#include "base/single_thread_task_runner.h"
+#include "base/threading/thread_task_runner_handle.h"
+#include "extensions/common/extension.h"
 #include "testing/gtest/include/gtest/gtest.h"
 
 namespace extensions {
 
+// PreloadCheckRunner:
 PreloadCheckRunner::PreloadCheckRunner() : called_(false) {}
 PreloadCheckRunner::~PreloadCheckRunner() {}
 
 void PreloadCheckRunner::OnCheckComplete(PreloadCheck::Errors errors) {
   ASSERT_FALSE(called_);
   called_ = true;
   errors_ = errors;
   if (run_loop_)
     run_loop_->Quit();
 }
@@ skipping 14 matching lines @@
 PreloadCheck::ResultCallback PreloadCheckRunner::GetCallback() {
   return base::Bind(&PreloadCheckRunner::OnCheckComplete,
                     base::Unretained(this));
 }
 
 void PreloadCheckRunner::WaitForIdle() {
   run_loop_ = base::MakeUnique<base::RunLoop>();
   run_loop_->RunUntilIdle();
 }
 
+// PreloadCheckStub:
+PreloadCheckStub::PreloadCheckStub(bool is_async)
+    : PreloadCheck(nullptr), is_async_(is_async), weak_ptr_factory_(this) {}
+
+PreloadCheckStub::~PreloadCheckStub() {}
+
+void PreloadCheckStub::AddError(Error error) {
+  errors_.insert(error);
+}
+
+void PreloadCheckStub::Start(ResultCallback callback) {
+  DCHECK(!callback.is_null());
+  if (is_async_) {
+    base::ThreadTaskRunnerHandle::Get()->PostTask(
+        FROM_HERE,
+        base::Bind(&PreloadCheckStub::StartInternal,

[Devlin, 2017/03/22 15:08:46]

Another option would be to do
PostTask(FROM_HERE, base::Bind(callback, errors_));

Two potential concerns:
- Errors added after Start() don't get used.  That's probably fine (seems wrong
to do that).
- Calling the callback is no longer bound to the lifetime of this object.

If you don't care about those, I'd say go with the simpler approach and nix
StartInternal().  If you do, add a comment why.

[michaelpg, 2017/03/24 02:34:17]

Since I'm using a OnceCallback (new best practice), that doesn't compile. Added
comment.

+                   weak_ptr_factory_.GetWeakPtr(), base::Passed(&callback)));
+  } else {
+    StartInternal(std::move(callback));
+  }
+}
+
+void PreloadCheckStub::StartInternal(ResultCallback callback) {

[Devlin, 2017/03/22 15:08:46]

StartInternal is a funny name for something that deterministically ends the flow
:P  Maybe Finish()?

[michaelpg, 2017/03/24 02:34:16]

RunCallback sound good?

+  std::move(callback).Run(errors_);
+}
+
+base::string16 PreloadCheckStub::GetErrorMessage() const {
+  return message_;
+}
+
 }  // namespace extensions