[Sync] Try to fix race conditions in CookieJarMismatch.

chrome/browser/sync/test/integration/single_client_sessions_sync_test.cc

 // Copyright 2012 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 "base/macros.h"
+#include "base/run_loop.h"
 #include "base/test/histogram_tester.h"
 #include "chrome/browser/sessions/session_service.h"
 #include "chrome/browser/sync/test/integration/profile_sync_service_harness.h"
 #include "chrome/browser/sync/test/integration/session_hierarchy_match_checker.h"
 #include "chrome/browser/sync/test/integration/sessions_helper.h"
 #include "chrome/browser/sync/test/integration/sync_test.h"
 #include "chrome/browser/sync/test/integration/typed_urls_helper.h"
 #include "chrome/browser/sync/test/integration/updated_progress_marker_checker.h"
 #include "chrome/common/url_constants.h"
 #include "components/browser_sync/profile_sync_service.h"
@@ skipping 301 matching lines @@
       {{base_url.spec(), moved_tab_url.spec()}, {new_window_url.spec()}}));
 
   // Move tab 1 in browser 0 to browser 1.
   MoveTab(0, 1, 1);
 
   WaitForHierarchyOnServer(SessionsHierarchy(
       {{base_url.spec()}, {new_window_url.spec(), moved_tab_url.spec()}}));
 }
 
 // crbug.com/689662
-#if defined(OS_CHROMEOS) || defined(OS_WIN)
+#if defined(OS_CHROMEOS)
 #define MAYBE_CookieJarMismatch DISABLED_CookieJarMismatch
 #else
 #define MAYBE_CookieJarMismatch CookieJarMismatch
 #endif
 IN_PROC_BROWSER_TEST_F(SingleClientSessionsSyncTest, MAYBE_CookieJarMismatch) {
   ASSERT_TRUE(SetupSync()) << "SetupSync() failed.";
 
   ASSERT_TRUE(CheckInitialState(0));
 
   sync_pb::ClientToServerMessage message;
 
   // The HistogramTester objects are scoped to allow more precise verification.
   {
     HistogramTester histogram_tester;
 
     // Add a new session to client 0 and wait for it to sync.
     GURL url = GURL(kURL1);
     ASSERT_TRUE(OpenTab(0, url));
     WaitForURLOnServer(url);
 
     // The cookie jar mismatch value will be true by default due to
     // the way integration tests trigger signin (which does not involve a normal
     // web content signin flow).
     ASSERT_TRUE(GetFakeServer()->GetLastCommitMessage(&message));
     ASSERT_TRUE(message.commit().config_params().cookie_jar_mismatch());
 
-    // It is possible that multiple sync cycles occured during the call to
+    // It is possible that multiple sync cycles occurred during the call to
     // OpenTab, which would cause multiple identical samples.
     ExpectUniqueSampleGE(histogram_tester, "Sync.CookieJarMatchOnNavigation",
                          false, 1);
     ExpectUniqueSampleGE(histogram_tester, "Sync.CookieJarEmptyOnMismatch",
                          true, 1);
+  }
 
-    // Trigger a cookie jar change (user signing in to content area).
-    gaia::ListedAccount signed_in_account;
-    signed_in_account.id =
-        GetClient(0)->service()->signin()->GetAuthenticatedAccountId();
-    std::vector<gaia::ListedAccount> accounts;
-    std::vector<gaia::ListedAccount> signed_out_accounts;
-    accounts.push_back(signed_in_account);
-    GoogleServiceAuthError error(GoogleServiceAuthError::NONE);
-    GetClient(0)->service()->OnGaiaAccountsInCookieUpdated(
-        accounts, signed_out_accounts, error);
-  }
+  // Trigger a cookie jar change (user signing in to content area).
+  gaia::ListedAccount signed_in_account;
+  signed_in_account.id =
+      GetClient(0)->service()->signin()->GetAuthenticatedAccountId();
+  std::vector<gaia::ListedAccount> accounts;
+  std::vector<gaia::ListedAccount> signed_out_accounts;
+  accounts.push_back(signed_in_account);
+  GoogleServiceAuthError error(GoogleServiceAuthError::NONE);
+
+  // Updating the cookie jar has to travel to the sync engine. It is possible
+  // something is already running or scheduled to run on the sync thread. We
+  // want to block here and not create the HistogramTester below until we know
+  // the cookie jar stats have been updated.
+  base::RunLoop run_loop;
+  GetClient(0)->service()->OnGaiaAccountsInCookieUpdatedWithCallback(
+      accounts, signed_out_accounts, error, run_loop.QuitClosure());
+  run_loop.Run();

[pavely, 2017/03/20 04:32:02]

Waiting for values to arrive on sync thread doesn't guarantee they will get
recorded in histogram before HistogramTester takes snapshot. What if you called
OnGaiaAccountsInCookieUpdatedWithCallback inside the block, after
HistogramTester? This will allow you to avoid wiring callback to SBHC and back.
I think you would need less strict function to verify histogram sample presence.

 
   {
     HistogramTester histogram_tester;
 
     // Trigger a sync and wait for it.
     GURL url = GURL(kURL2);
     ASSERT_TRUE(NavigateTab(0, url));
     WaitForURLOnServer(url);
 
     // Verify the cookie jar mismatch bool is set to false.
     ASSERT_TRUE(GetFakeServer()->GetLastCommitMessage(&message));
     ASSERT_FALSE(message.commit().config_params().cookie_jar_mismatch());
 
     // Verify the histograms were recorded properly.
     ExpectUniqueSampleGE(histogram_tester, "Sync.CookieJarMatchOnNavigation",
                          true, 1);
     histogram_tester.ExpectTotalCount("Sync.CookieJarEmptyOnMismatch", 0);
   }
 }