Retry: Clean up TestProfileSyncService tests

This is a retry of r238348, which was reverted in r238368.  I believe
the build failure that led to the revert, though related to my change,
was a flake caused by stale object files on the build bot.

This CL refactors many of the tests in profile_sync_service_unittest.cc.
It continues the refactoring work begun in r233533, r235661, and
r235854.

The JsController tests have been deleted. There is not much point in
testing the JsController here; it can be more easily tested on its own
in sync_js_controller_uniittest.cc. The SyncJsController unit tests have
been updated so they now cover the few cases that were formerly only
exercised in the ProfileSyncService unit tests.

It converts all remaining uncoverted tests from relying on the
TestProfileSyncService to using a real ProfileSyncService with an
injected backend. The injected backend makes it easier to create the
scenarios we want to test. We can inject a specially crafted SBH rather
than fiddling with "synchronous init" and "fail initial download" flags.

Since the TestProfileSyncService no longer needs to support the wide
variety of test scenarios required by the tests in
profile_sync_service_unittest.cc, we can greatly simplify its
implementation. Many of its parameters and associated code have been
removed.

BUG=140354,312994

Review URL: https://codereview.chromium.org/102513002

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@239615 0039d316-1c4b-4281-b951-d872f2087c98

1 files changed, 38 insertions, 17 deletions

diff --git a/sync/js/sync_js_controller_unittest.cc b/sync/js/sync_js_controller_unittest.cc
index ac46935..eca617c 100644
--- a/sync/js/sync_js_controller_unittest.cc
+++ b/sync/js/sync_js_controller_unittest.cc
@@ -30,10 +30,15 @@ class SyncJsControllerTest : public testing::Test {
   base::MessageLoop message_loop_;
 };
 
+ACTION_P(ReplyToMessage, reply_name) {
+  arg2.Call(FROM_HERE, &JsReplyHandler::HandleJsReply, reply_name, JsArgList());
+}
+
 TEST_F(SyncJsControllerTest, Messages) {
   InSequence dummy;
   // |mock_backend| needs to outlive |sync_js_controller|.
   StrictMock<MockJsBackend> mock_backend;
+  StrictMock<MockJsReplyHandler> mock_reply_handler;
   SyncJsController sync_js_controller;
 
   base::ListValue arg_list1, arg_list2;
@@ -41,17 +46,23 @@ TEST_F(SyncJsControllerTest, Messages) {
   arg_list2.Append(new base::FundamentalValue(5));
   JsArgList args1(&arg_list1), args2(&arg_list2);
 
-  // TODO(akalin): Write matchers for WeakHandle and use them here
-  // instead of _.
   EXPECT_CALL(mock_backend, SetJsEventHandler(_));
-  EXPECT_CALL(mock_backend, ProcessJsMessage("test1", HasArgs(args2), _));
-  EXPECT_CALL(mock_backend, ProcessJsMessage("test2", HasArgs(args1), _));
+  EXPECT_CALL(mock_backend, ProcessJsMessage("test1", HasArgs(args2), _))
+      .WillOnce(ReplyToMessage("test1_reply"));
+  EXPECT_CALL(mock_backend, ProcessJsMessage("test2", HasArgs(args1), _))
+      .WillOnce(ReplyToMessage("test2_reply"));
 
   sync_js_controller.AttachJsBackend(mock_backend.AsWeakHandle());
-  sync_js_controller.ProcessJsMessage("test1", args2,
-                                      WeakHandle<JsReplyHandler>());
-  sync_js_controller.ProcessJsMessage("test2", args1,
-                                      WeakHandle<JsReplyHandler>());
+  sync_js_controller.ProcessJsMessage("test1",
+                                      args2,
+                                      mock_reply_handler.AsWeakHandle());
+  sync_js_controller.ProcessJsMessage("test2",
+                                      args1,
+                                      mock_reply_handler.AsWeakHandle());
+
+  // The replies should be waiting on our message loop.
+  EXPECT_CALL(mock_reply_handler, HandleJsReply("test1_reply", _));
+  EXPECT_CALL(mock_reply_handler, HandleJsReply("test2_reply", _));
   PumpLoop();
 
   // Let destructor of |sync_js_controller| call RemoveBackend().
@@ -60,6 +71,7 @@ TEST_F(SyncJsControllerTest, Messages) {
 TEST_F(SyncJsControllerTest, QueuedMessages) {
   // |mock_backend| needs to outlive |sync_js_controller|.
   StrictMock<MockJsBackend> mock_backend;
+  StrictMock<MockJsReplyHandler> mock_reply_handler;
   SyncJsController sync_js_controller;
 
   base::ListValue arg_list1, arg_list2;
@@ -68,20 +80,29 @@ TEST_F(SyncJsControllerTest, QueuedMessages) {
   JsArgList args1(&arg_list1), args2(&arg_list2);
 
   // Should queue messages.
-  sync_js_controller.ProcessJsMessage("test1", args2,
-                                      WeakHandle<JsReplyHandler>());
-  sync_js_controller.ProcessJsMessage("test2", args1,
-                                      WeakHandle<JsReplyHandler>());
+  sync_js_controller.ProcessJsMessage(
+      "test1",
+      args2,
+      mock_reply_handler.AsWeakHandle());
+  sync_js_controller.ProcessJsMessage(
+      "test2",
+      args1,
+      mock_reply_handler.AsWeakHandle());
 
+  // Should do nothing.
+  PumpLoop();
   Mock::VerifyAndClearExpectations(&mock_backend);
 
-  // TODO(akalin): Write matchers for WeakHandle and use them here
-  // instead of _.
-  EXPECT_CALL(mock_backend, SetJsEventHandler(_));
-  EXPECT_CALL(mock_backend, ProcessJsMessage("test1", HasArgs(args2), _));
-  EXPECT_CALL(mock_backend, ProcessJsMessage("test2", HasArgs(args1), _));
 
   // Should call the queued messages.
+  EXPECT_CALL(mock_backend, SetJsEventHandler(_));
+  EXPECT_CALL(mock_backend, ProcessJsMessage("test1", HasArgs(args2), _))
+      .WillOnce(ReplyToMessage("test1_reply"));
+  EXPECT_CALL(mock_backend, ProcessJsMessage("test2", HasArgs(args1), _))
+      .WillOnce(ReplyToMessage("test2_reply"));
+  EXPECT_CALL(mock_reply_handler, HandleJsReply("test1_reply", _));
+  EXPECT_CALL(mock_reply_handler, HandleJsReply("test2_reply", _));
+
   sync_js_controller.AttachJsBackend(mock_backend.AsWeakHandle());
   PumpLoop();