Skip to content

Commit

Permalink
[signin] Dont delete DiceTurnSyncOnHelper from its constructor
Browse files Browse the repository at this point in the history
This seems to be undefined behavior and is caught by the ASAN bot.

TBR=droger@chromium.org

(cherry picked from commit 62a2906)

Bug: 816372
Change-Id: I16435111c860115bf288798b0c88ea9e37daf147
Reviewed-on: https://chromium-review.googlesource.com/948843
Reviewed-by: Mihai Sardarescu <msarda@chromium.org>
Commit-Queue: David Roger <droger@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#541061}
Reviewed-on: https://chromium-review.googlesource.com/952965
Reviewed-by: David Roger <droger@chromium.org>
Cr-Commit-Position: refs/branch-heads/3359@{#55}
Cr-Branched-From: 66afc5e-refs/heads/master@{#540276}
  • Loading branch information
David Roger committed Mar 7, 2018
1 parent 8c35ac9 commit af09015
Show file tree
Hide file tree
Showing 2 changed files with 8 additions and 1 deletion.
7 changes: 6 additions & 1 deletion chrome/browser/ui/webui/signin/dice_turn_sync_on_helper.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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()));
Expand All @@ -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()));
Expand Down

0 comments on commit af09015

Please sign in to comment.