Using the per-revision bisect providing the bisect results,
Good Build : 56.0.2913.0 (Revision:430459)
Bad Build : 56.0.2914.0 (Revision:430837)

You are probably looking for a change made after 430708 (known good), but no later than 430709 (first known bad).

CHANGELOG URL:

The script might not always return single CL as suspect as some perf builds might get missing due to failure.

  https://chromium.googlesource.com/chromium/src/+log/03ca49d47984d3370cb8fe7c13512b34b4367d5c..2267565831e4e2fb9d5931bce7cb701231a93513

From the CL above, assigning the issue to the concern owner 

@ellyjones - Could you please check whether this is caused with respect to your change, if not please help us in assigning it to the right owner.

Review-Url: https://codereview.chromium.org/2483753003

Thanks !

Adding release block label, please undo if not the case.

That CL is not related; it's removing some unused code from views_examples, which is a debugging tool that developers can build. None of that code is ever shipped or involved in chrome builds in any way. I'm doubtful of that bisect range.

-> hdodda@

Retried the per-vision bisect ,

Good Build : 56.0.2913.0 (Revision:430459)
Bad Build : 56.0.2914.0 (Revision:430837)

You are probably looking for a change made after 430748 (known good), but no later than 430749 (first known bad).

CHANGELOG URL:

The script might not always return single CL as suspect as some perf builds might get missing due to failure.
  https://chromium.googlesource.com/chromium/src/+log/49fb258aa991899f72e7ce5daa489e67c1175920..fddb451107a7468066fd17996e1920d1ee3289a4

From the CL above, assigning the issue to the concern owner 

@rockot- Could you please check whether this is caused with respect to your change, if not please help us in assigning it to the right owner.

Review-Url: https://codereview.chromium.org/2487803002

Thanks !

Definitely also not that CL, sorry.

Please find the bisect information as below

Good Build : 56.0.2913.0 (build revision 430459)
Bad Build : 56.0.2915.0 (build revision 431137)

Change Log::
https://chromium.googlesource.com/chromium/src/+log/9f08c082a00b1c49d11520c6399c05cb9a8a563b..5db87096a32b73bd2aaca04710152de495ba50ea

Possible suspect::
https://chromium.googlesource.com/chromium/src/+/5db87096a32b73bd2aaca04710152de495ba50ea

zmin@ Could you please look into this issue if it is related to your change,else please help us in finding the appropriate owner for this issue.

Thanks,

cc anthonyvd@ for finding the owner.

There're actually two issues here. 
1) The sign in reason is determined by the sign in URL. Since the url here is get from the navigation history, we'll treat the second sign in as a fresh sign in. Which will trigger the DCHECK failure in SigninManager::StartSignInWithRefreshToken()

2) Regardless the DCHECK, this dialog will be created by SigninViewController::ShowModalSyncConfirmationDialog() and displayed by SigninViewController::SetModalSigninHeight(). However, SigninViewController::CloseModalSignin() is called for the second sign in via InlineLoginHandler::HandleDialogClose() right after the dialog creation. This is triggered by the javascript event 'dialogClose'.


Theoretically, I don't think my CL is able to trigger this issue. But I don't know what triggers this javascript event either. Ideally, since the second fresh sign in is treated as an illegal operation due to the DCHECK, we shall block it in the early stage instead of going through the whole process.

Adding the new identity team folks. Not sure if that's an edge case we want/know how to fix.

Let me clarify a few things:

This only occurs when a user manually navigates (either via the back button or via typing a URL in) to chrome://chrome-signin while *already* signed-in to Chrome. The user then has to enter the credentials for the same account with which they're already signed in. If the user enters credentials for a different accounts, an error bubble pops up.

The expected behavior shouldn't be to pop up the sync confirmation dialogue again. We should actually probably just navigate directly to the new tab page, without doing anything else, after the user signs in again with their account. The user is already signed in to Chrome - there's no reason for us to allow them to sign in again. We should probably also refresh the cookie jar with the user's account (if it's not already in there), but we definitely don't need to pop up the sync confirmation dialogue again.

I'm updating the title of the issue to reflect the bug. Given that this only happens when a user manually navigates to chrome://chrome-signin, this is low-impact. We're also in the process of deprecating that page, so it'll be even lower-impact pretty soon.

However, I'm not sure exactly when that page will go away entirely. It may make sense to still fix this edge case in the interim. Mihai, assigning to you for now to look into. It's low priority, though.

For the second issue(page go away entirely), I have found the reason for that and will upload the fix soon.

Sorry, which second issue are you referring to? I'm just saying that we're removing chrome://chrome-signin with the new first run experience, so that page (i.e. chrome://chrome-signin) will at some point go away completely.

I think the only bug here is that we just shouldn't be showing the sync confirmation dialogue if you try to sign in while you're already signed in to Chrome. We should sign you into the cookie jar (if you aren't already), and then just navigate to the NTP.

The 2) I mentioned in #7 which is the sync confirm window close before display issue. I'll track this issue in  crbug.com/667227 .

Thanks Owen! Owen and I briefly chatted offline. Just for posterity's sake, here's what we discussed.

There are technically two "bugs" here:
(1) We shouldn't be treating a sign-in through chrome://chrome-signin as a "fresh sign-in" when the user is already signed in. Instead, we should just refresh the cookie jar, and navigate to the new tab page. That is what the focus of this bug will continue to be, and what Mihai should look into fixing.

(2) Assuming that (1) doesn't work as expected (namely, assuming we *do* treat sign-ins through chrome://chrome-signin as "fresh sign-ins," even if the user is already signed in), we are also not displaying the sync confirmation dialogue on subsequent sign-ins correctly. This is the same root case as  Issue 667227 . This is what Owen will address in his fix (which will fix the issue described in  Issue 667227 ).

If anyone has any questions, let me know. Mihai, does all of that make sense to you?

Fixing (1) is still low priority, since chrome://chrome-signin is being deprecated.

--Chrome Identity automated triaging--

This bug is Assigned and has gone one month without any activity, so it is being moved to Available to indicate that it is not actively being worked on. If you are working on this bug, please mark yourself as the owner and move back to Assigned. Please see https://goo.gl/78kbny for more details. Please remove the Services>SignIn or UI>Browser>Profiles components if this bug isn't related to Chrome Identity.

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot

--Chrome Identity automated triaging--

This bug is Assigned and has gone one month without any activity, so it is being moved to Available to indicate that it is not actively being worked on. If you are working on this bug, please mark yourself as the owner and move back to Assigned. Please see https://goo.gl/78kbny for more details. Please remove the Services>SignIn or UI>Browser>Profiles components if this bug isn't related to Chrome Identity.

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot

--Chrome Identity automated triaging--

This bug is Available and has gone one year without any activity. If another month passes without any activity, this bug will be closed out. Please provide an update with the latest status for this bug. Please see https://goo.gl/78kbny for more details. Please remove the Services>SignIn or UI>Browser>Profiles components if this bug isn't related to Chrome Identity.

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot

--Chrome Identity automated triaging--

This available, signin or profiles-related bug has gone at least 30 days since the last automated post without any further update. This bug will be closed out due to inactivity. Please re-open the bug and provide an update if it is still a valid or reproducible bug. Please see https://goo.gl/78kbny for more details. Please remove the Services>SignIn or UI>Browser>Profiles components if this bug isn't related to Chrome Identity.

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot