From af090156af6f58c827fc2f57e48dd2a28b304c44 Mon Sep 17 00:00:00 2001 From: David Roger Date: Wed, 7 Mar 2018 12:46:34 +0000 Subject: [PATCH] [signin] Dont delete DiceTurnSyncOnHelper from its constructor This seems to be undefined behavior and is caught by the ASAN bot. TBR=droger@chromium.org (cherry picked from commit 62a290638e4fa602a1bed5331e13613353510ee0) Bug: 816372 Change-Id: I16435111c860115bf288798b0c88ea9e37daf147 Reviewed-on: https://chromium-review.googlesource.com/948843 Reviewed-by: Mihai Sardarescu Commit-Queue: David Roger Cr-Original-Commit-Position: refs/heads/master@{#541061} Reviewed-on: https://chromium-review.googlesource.com/952965 Reviewed-by: David Roger Cr-Commit-Position: refs/branch-heads/3359@{#55} Cr-Branched-From: 66afc5e5d10127546cc4b98b9117aff588b5e66b-refs/heads/master@{#540276} --- chrome/browser/ui/webui/signin/dice_turn_sync_on_helper.cc | 7 ++++++- .../ui/webui/signin/dice_turn_sync_on_helper_unittest.cc | 2 ++ 2 files changed, 8 insertions(+), 1 deletion(-) diff --git a/chrome/browser/ui/webui/signin/dice_turn_sync_on_helper.cc b/chrome/browser/ui/webui/signin/dice_turn_sync_on_helper.cc index bfcdab00c84c8..b664c08893bab 100644 --- a/chrome/browser/ui/webui/signin/dice_turn_sync_on_helper.cc +++ b/chrome/browser/ui/webui/signin/dice_turn_sync_on_helper.cc @@ -5,11 +5,13 @@ #include "chrome/browser/ui/webui/signin/dice_turn_sync_on_helper.h" #include "base/bind.h" +#include "base/location.h" #include "base/logging.h" #include "base/metrics/histogram_macros.h" #include "base/metrics/user_metrics.h" #include "base/metrics/user_metrics_action.h" #include "base/strings/utf_string_conversions.h" +#include "base/threading/sequenced_task_runner_handle.h" #include "chrome/browser/browser_process.h" #include "chrome/browser/policy/cloud/user_policy_signin_service.h" #include "chrome/browser/policy/cloud/user_policy_signin_service_factory.h" @@ -71,7 +73,10 @@ DiceTurnSyncOnHelper::DiceTurnSyncOnHelper( DCHECK(!signin_util::IsForceSigninEnabled()); if (HasCanOfferSigninError()) { - AbortAndDelete(); + // Do not self-destruct synchronously in the constructor. + base::SequencedTaskRunnerHandle::Get()->PostTask( + FROM_HERE, base::BindOnce(&DiceTurnSyncOnHelper::AbortAndDelete, + base::Unretained(this))); return; } diff --git a/chrome/browser/ui/webui/signin/dice_turn_sync_on_helper_unittest.cc b/chrome/browser/ui/webui/signin/dice_turn_sync_on_helper_unittest.cc index 5e964de9b4753..d3925d28f8602 100644 --- a/chrome/browser/ui/webui/signin/dice_turn_sync_on_helper_unittest.cc +++ b/chrome/browser/ui/webui/signin/dice_turn_sync_on_helper_unittest.cc @@ -428,6 +428,7 @@ TEST_F(DiceTurnSyncOnHelperTest, CanOfferSigninErrorKeepAccount) { // Signin flow. CreateDiceTurnOnSyncHelper( DiceTurnSyncOnHelper::SigninAbortedMode::KEEP_ACCOUNT); + base::RunLoop().RunUntilIdle(); // Check expectations. EXPECT_FALSE(signin_manager()->IsAuthenticated()); EXPECT_TRUE(token_service()->RefreshTokenIsAvailable(account_id())); @@ -443,6 +444,7 @@ TEST_F(DiceTurnSyncOnHelperTest, CanOfferSigninErrorRemoveAccount) { // Signin flow. CreateDiceTurnOnSyncHelper( DiceTurnSyncOnHelper::SigninAbortedMode::REMOVE_ACCOUNT); + base::RunLoop().RunUntilIdle(); // Check expectations. EXPECT_FALSE(signin_manager()->IsAuthenticated()); EXPECT_FALSE(token_service()->RefreshTokenIsAvailable(account_id()));