ProfileSyncService: Add missing NotifyObservers() on unrecoverable error

...plus fix and reenable
SingleClientDirectorySyncTest.DeleteDirectoryWhenCorrupted.
This test waits until it sees an unrecoverable error through the
SyncServiceObserver. Since we weren't actually notifying in this case,
it relied on a random different notification. Interestingly, adding the
missing NotifyObservers() made the test hang consistently.
My theory is that there was a possible deadlock between the sync thread
and the main thread. It seems to be fixed by explicitly waiting for the
main thread tasks to run before waiting on the sync thread.

Bug: 850980
Change-Id: I2f636ddfa457b6d977f909e0d0a17a26aca5c166
Reviewed-on: https://chromium-review.googlesource.com/1097479Reviewed-by: Mikel Astiz <mastiz@chromium.org>
Commit-Queue: Marc Treib <treib@chromium.org>
Cr-Commit-Position: refs/heads/master@{#566801}

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

@@ -101,18 +101,8 @@ IN_PROC_BROWSER_TEST_F(SingleClientDirectorySyncTest,
 
 // Verify that when the sync directory's backing store becomes corrupted, we
 // trigger an unrecoverable error and delete the database.
-//
-// TODO(treib): Update the below comment; it refers to something that does
-// not exist.
-// If this test fails, see the definition of kNumEntriesRequiredForCorruption
-// for one possible cause.
-#if defined(OS_MACOSX) || defined(OS_WIN)
-#define MAYBE_DeleteDirectoryWhenCorrupted DISABLED_DeleteDirectoryWhenCorrupted
-#else
-#define MAYBE_DeleteDirectoryWhenCorrupted DeleteDirectoryWhenCorrupted
-#endif
 IN_PROC_BROWSER_TEST_F(SingleClientDirectorySyncTest,
-                       MAYBE_DeleteDirectoryWhenCorrupted) {
+                       DeleteDirectoryWhenCorrupted) {
   ASSERT_TRUE(SetupClients()) << "SetupClients() failed.";
   // Sync and wait for syncing to complete.
   ASSERT_TRUE(SetupSync()) << "SetupSync() failed.";
@@ -147,6 +137,14 @@ IN_PROC_BROWSER_TEST_F(SingleClientDirectorySyncTest,
   ASSERT_TRUE(SyncUnrecoverableErrorChecker(sync_service).Wait());
   ASSERT_TRUE(sync_service->HasUnrecoverableError());
 
+  // An unrecoverable error causes ProfileSyncService to post a shutdown task to
+  // its task runner. Make sure that task gets run before we wait for the sync
+  // thread.
+  base::RunLoop run_loop;
+  base::ThreadTaskRunnerHandle::Get()->PostTask(FROM_HERE,
+                                                run_loop.QuitClosure());
+  run_loop.Run();
+
   // Wait until the sync loop has processed any existing tasks and see that the
   // directory no longer exists.
   ASSERT_TRUE(WaitForExistingTasksOnLoop(sync_loop));

components/browser_sync/profile_sync_service.cc

@@ -828,6 +828,8 @@ void ProfileSyncService::OnUnrecoverableErrorImpl(
   LOG(ERROR) << "Unrecoverable error detected at " << from_here.ToString()
              << " -- ProfileSyncService unusable: " << message;
 
+  NotifyObservers();
+
   // Shut all data types down.
   base::SequencedTaskRunnerHandle::Get()->PostTask(
       FROM_HERE, base::BindOnce(&ProfileSyncService::ShutdownImpl,