Use FakeServer-based invalidations for Sync tests (try #2)

sync/test/fake_server/fake_server_http_post_provider.cc

Index: sync/test/fake_server/fake_server_http_post_provider.cc
diff --git a/sync/test/fake_server/fake_server_http_post_provider.cc b/sync/test/fake_server/fake_server_http_post_provider.cc
index e0b34368fa349a1241a3ab0b228ed2634b889119..5dd45c28c164a3a9e9bcb0c0a060519c45efe39b 100644
--- a/sync/test/fake_server/fake_server_http_post_provider.cc
+++ b/sync/test/fake_server/fake_server_http_post_provider.cc
@@ -6,6 +6,11 @@
 
 #include <string>
 
+#include "base/bind.h"
+#include "base/location.h"
+#include "base/memory/ref_counted.h"
+#include "base/sequenced_task_runner.h"
+#include "base/synchronization/waitable_event.h"
 #include "sync/test/fake_server/fake_server.h"
 
 using syncer::HttpPostProviderInterface;
@@ -13,7 +18,9 @@ using syncer::HttpPostProviderInterface;
 namespace fake_server {
 
 FakeServerHttpPostProviderFactory::FakeServerHttpPostProviderFactory(
-    FakeServer* fake_server) : fake_server_(fake_server) { }
+    FakeServer* fake_server,
+    scoped_refptr<base::SequencedTaskRunner> task_runner)
+        : fake_server_(fake_server), task_runner_(task_runner) { }
 
 FakeServerHttpPostProviderFactory::~FakeServerHttpPostProviderFactory() { }
 
@@ -21,7 +28,7 @@ void FakeServerHttpPostProviderFactory::Init(const std::string& user_agent) { }
 
 HttpPostProviderInterface* FakeServerHttpPostProviderFactory::Create() {
   FakeServerHttpPostProvider* http =
-      new FakeServerHttpPostProvider(fake_server_);
+      new FakeServerHttpPostProvider(fake_server_, task_runner_);
   http->AddRef();
   return http;
 }
@@ -32,7 +39,11 @@ void FakeServerHttpPostProviderFactory::Destroy(
 }
 
 FakeServerHttpPostProvider::FakeServerHttpPostProvider(
-    FakeServer* fake_server) : fake_server_(fake_server) { }
+    FakeServer* fake_server,
+    scoped_refptr<base::SequencedTaskRunner> task_runner)
+        : fake_server_(fake_server),
+          task_runner_(task_runner),
+          post_complete_(false, false) { }
 
 FakeServerHttpPostProvider::~FakeServerHttpPostProvider() { }
 
@@ -54,13 +65,35 @@ void FakeServerHttpPostProvider::SetPostPayload(const char* content_type,
   request_content_.assign(content, content_length);
 }
 
+void FakeServerHttpPostProvider::OnPostComplete(int error_code,
+                                                int response_code,
+                                                const std::string& response) {
+  post_error_code_ = error_code;
+  post_response_code_ = response_code;
+  response_ = response;
+  post_complete_.Signal();
+}
+
 bool FakeServerHttpPostProvider::MakeSynchronousPost(int* error_code,
                                                      int* response_code) {
-  // This assumes that a POST is being made to /command.
-  *error_code = fake_server_->HandleCommand(request_content_,
-                                            response_code,
-                                            &response_);
-  return (*error_code == 0);
+  // It is assumed that a POST is being made to /command.
+  FakeServer::HandleCommandCallback callback = base::Bind(
+      &FakeServerHttpPostProvider::OnPostComplete, base::Unretained(this));
+  task_runner_->PostNonNestableTask(FROM_HERE,
+                                    base::Bind(&FakeServer::HandleCommand,
+                                               base::Unretained(fake_server_),
+                                               base::ConstRef(request_content_),
+                                               base::ConstRef(callback)));
+  const int kTimeoutSecs = 5;

[rlarocque, 2014/05/02 20:51:21]

Is a timeout really necessary?

If there's a bug that causes us to hang, then the test framework should kill the
test.  I don't think it's this part of the code's responsibility to work around
serious logic errors.

[pval...(no longer on Chromium), 2014/05/05 18:33:36]

I agree; I can't justify the added complexity.

FYI I also found a bug here (checking *error_code before it was set) that it now
resolved.

+  bool signaled = post_complete_.TimedWait(
+      base::TimeDelta::FromSeconds(kTimeoutSecs));
+  if (!signaled || *error_code != 0) {
+    return false;
+  }
+
+  *error_code = post_error_code_;
+  *response_code = post_response_code_;
+  return true;
 }
 
 int FakeServerHttpPostProvider::GetResponseContentLength() const {