PreloadCheck class for extension pre-install checks

chrome/browser/extensions/blacklist_check.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 "chrome/browser/extensions/blacklist_check.h"
+
+#include "base/bind.h"
+#include "chrome/browser/extensions/blacklist.h"
+#include "extensions/common/constants.h"

[Devlin, 2017/03/08 03:01:44]

Needed?

[michaelpg, 2017/03/09 01:53:29]

Removed. (Originally had the error codes in there, but they seem fine in
preload_check.h.)

+
+namespace extensions {
+
+BlacklistCheck::BlacklistCheck(Blacklist* blacklist, const Extension* extension)
+    : PreloadCheck(extension), blacklist_(blacklist), weak_ptr_factory_(this) {}
+
+BlacklistCheck::~BlacklistCheck() {}
+
+void BlacklistCheck::Start(ResultCallback callback) {
+  callback_ = std::move(callback);
+
+  blacklist_->IsBlacklisted(extension()->id(),
+                            base::Bind(&BlacklistCheck::OnGetIsBlacklisted,
+                                       weak_ptr_factory_.GetWeakPtr()));
+}
+
+void BlacklistCheck::OnGetIsBlacklisted(BlacklistState blacklist_state) {

[Devlin, 2017/03/08 03:01:44]

OnGetIsBlacklisted sounds strange - maybe OnRetrievedBlacklistState or
something?

[michaelpg, 2017/03/09 01:53:29]

Done.

+  Errors errors;
+  if (blacklist_state == BlacklistState::BLACKLISTED_MALWARE)
+    errors.insert(PreloadCheck::BLACKLISTED_ID);
+  else if (blacklist_state == BlacklistState::BLACKLISTED_UNKNOWN)
+    errors.insert(PreloadCheck::BLACKLISTED_UNKNOWN);
+  std::move(callback_).Run(errors);

[Devlin, 2017/03/08 03:01:44]

clever - but std::move() is designed to leave things in an undefined state (to
allow for whatever optimization is fastest).  As a OnceCallback, is the internal
state not cleaned up after calling Run()?

[michaelpg, 2017/03/09 01:53:29]

Those considerations don't apply to OnceCallback. Making this work and
understanding it was a big pain point. Here's my explanation:

It isn't optional; OnceCallback::Run *without* the move doesn't compile:
https://cs.chromium.org/chromium/src/base/callback.h?type=cs&q=callback.h&sq=...

callback_ being an lvalue, callback_.Run() would select OnceCallback:Run()&.
std::move(callback_) makes it usable as rvalue for Run()&&.

docs/callback.md has examples (also attempting to fix some of the documentation:
https://codereview.chromium.org/2735903002/)

Since Run()&& is selected and that calls operator=(OnceCallback&&), callback_'s
members are fully moved out of it, so it's not left in an unspecified state.

+}
+
+}  // namespace extensions