commit 897b5e592cf64e3c04ed1151d19ce5cd51a90592
author Gilad Arnold <garnold@chromium.org> Wed May 21 09:37:18 2014 -0700
committer chrome-internal-fetch <chrome-internal-fetch@google.com> Sat May 31 01:47:05 2014 +0000
tree 0afb03927a90ac153f2a6d2e60cfcb341bff5426
parent 3d21b12092a0108f77c3332b85eaf81524fe7c32

PM: Blocking policy requests cannot be called synchronously.

Policy requests that may return EvalStatus::kAskMeAgainLater are
considered blocking and should only be called asynchronously. It is
therefore an error to call a policy returning this value via
UpdateManager::PolicyRequest(), and so we kill the program with an
assertion (DCHECK) if this happens; for release builds, a warning log is
emitted.

Note: the associated death test builds and runs iff DCHECK is enabled.

BUG=None
TEST=Unit tests.

Change-Id: I75e2f5e4498f85857aff41778df0300d7c8898e7
Reviewed-on: https://chromium-review.googlesource.com/200760
Reviewed-by: Gilad Arnold <garnold@chromium.org>
Commit-Queue: Gilad Arnold <garnold@chromium.org>
Tested-by: Gilad Arnold <garnold@chromium.org>

update_manager/update_manager-inl.h

diff --git a/update_manager/update_manager-inl.h b/update_manager/update_manager-inl.h
index 1b1dc49..544a371 100644
--- a/update_manager/update_manager-inl.h
+++ b/update_manager/update_manager-inl.h
@@ -28,13 +28,13 @@
                                                       result, args...);
 
   if (status == EvalStatus::kFailed) {
-    LOG(WARNING) << "PolicyRequest() failed with error: " << error;
+    LOG(WARNING) << "Policy request failed: " << error;
     error.clear();
     status = (default_policy_.*policy_method)(ec, state_.get(), &error,
                                               result, args...);
 
     if (status == EvalStatus::kFailed) {
-      LOG(WARNING) << "Request to DefaultPolicy also failed, passing error.";
+      LOG(WARNING) << "Request to default policy also failed: " << error;
     }
   }
   // TODO(deymo): Log the actual state used from the EvaluationContext.
@@ -86,7 +86,13 @@
   // IMPORTANT: To ensure that ActualArgs can be converted to ExpectedArgs, we
   // explicitly instantiate EvaluatePolicy with the latter in lieu of the
   // former.
-  return EvaluatePolicy<R, ExpectedArgs...>(ec, policy_method, result, args...);
+  EvalStatus ret = EvaluatePolicy<R, ExpectedArgs...>(ec, policy_method, result,
+                                                      args...);
+  // Sync policy requests must not block, if they do then this is an error.
+  DCHECK(EvalStatus::kAskMeAgainLater != ret);
+  LOG_IF(WARNING, EvalStatus::kAskMeAgainLater == ret)
+      << "Sync request used with an async policy";
+  return ret;
 }
 
 template<typename R, typename... ActualArgs, typename... ExpectedArgs>