DCHECK hit when extensions triggers a sign-in flow via the chrome.identity API when browser sign-in is not allowed |
||||||||||||
Issue descriptionReceived signal 6 #0 0x55558872aebf base::debug::StackTrace::StackTrace() #1 0x55558872a9a1 base::debug::(anonymous namespace)::StackDumpSignalHandler() #2 0x7f7cbdc730c0 <unknown> #3 0x7f7cb7af5fcf gsignal #4 0x7f7cb7af73fa abort #5 0x555588729785 base::debug::BreakDebugger() #6 0x555588677edf logging::LogMessage::~LogMessage() #7 0x55558aff1cdf chrome::ShowBrowserSignin() #8 0x55558b0c541d LoginUIService::ShowLoginPopup() #9 0x55558a9eda87 extensions::IdentityGetAuthTokenFunction::StartSigninFlow() #10 0x55558a9ed833 extensions::IdentityGetAuthTokenFunction::OnReceivedExtensionAccountInfo() #11 0x55558a9ed469 extensions::IdentityGetAuthTokenFunction::OnReceivedPrimaryAccountInfo() #12 0x55558a9efd76 _ZN4base8internal7InvokerINS0_9BindStateIMN10extensions28IdentityGetAuthTokenFunctionEFvRKNSt3__13setINS5_12basic_stringIcNS5_11char_traitsIcEENS5_9allocatorIcEEEENS5_4lessISC_EENSA_ISC_EEEERKSC_RKNS_8OptionalI11AccountInfoEERKN8identity12AccountStateEEJ13scoped_refptrIS4_ESG_SC_EEEFvSP_ST_EE7RunOnceEPNS0_13BindStateBaseESP_ST_ #13 0x555586f44d88 identity::mojom::IdentityManager_GetPrimaryAccountInfo_ForwardToCallback::Accept() #14 0x555588795239 mojo::InterfaceEndpointClient::HandleValidatedMessage() #15 0x555588797926 mojo::FilterChain::Accept() #16 0x555588796602 mojo::InterfaceEndpointClient::HandleIncomingMessage() #17 0x55558879dd5c mojo::internal::MultiplexRouter::ProcessIncomingMessage() #18 0x55558879d120 mojo::internal::MultiplexRouter::Accept() #19 0x555588797926 mojo::FilterChain::Accept() #20 0x555588792b73 mojo::Connector::ReadSingleMessage() #21 0x555588793764 mojo::Connector::ReadAllAvailableMessages() #22 0x5555887935c6 mojo::Connector::OnHandleReadyInternal() #23 0x55558604d254 mojo::SimpleWatcher::DiscardReadyState() #24 0x5555887b4c13 mojo::SimpleWatcher::OnHandleReady() #25 0x5555862408dc _ZN4base8internal7InvokerINS0_9BindStateIMN3viz14GpuServiceImplEFvN3gfx21GenericSharedMemoryIdEiRKN3gpu9SyncTokenEEJNS_7WeakPtrIS4_EES6_iS8_EEEFvvEE7RunImplISC_NSt3__15tupleIJSE_S6_iS8_EEEJLm0ELm1ELm2ELm3EEEEvOT_OT0_NSJ_16integer_sequenceImJXspT1_EEEE #26 0x55558867ffc2 base::debug::TaskAnnotator::RunTask() #27 0x55558867ef0f base::MessageLoop::RunTask() #28 0x55558867f2e2 base::MessageLoop::DoWork() #29 0x555588682fc9 base::MessagePumpGlib::Run() #30 0x55558867e9e1 base::MessageLoop::Run() #31 0x5555886a9756 base::RunLoop::Run() #32 0x55558828470a ChromeBrowserMainParts::MainMessageLoopRun() #33 0x55558657c837 content::BrowserMainLoop::RunMainMessageLoopParts() #34 0x55558657f903 content::BrowserMainRunnerImpl::Run() #35 0x5555865789a9 content::BrowserMain() #36 0x5555882317a0 content::ContentMainRunnerImpl::Run() #37 0x555588266469 service_manager::Main() #38 0x55558822f871 content::ContentMain() #39 0x5555855ff1b3 ChromeMain #40 0x7f7cb7ae32b1 __libc_start_main #41 0x5555855ff02a _start r8: 0000000000000000 r9: 00007ffd02db8bf0 r10: 0000000000000008 r11: 0000000000000246 r12: 00007ffd02db9968 r13: 00007ffd02db9958 r14: 00007ffd02db9960 r15: 00007ffd02db8e89 di: 0000000000000002 si: 00007ffd02db8bf0 bp: 00007ffd02db8e30 bx: 0000000000000006 dx: 0000000000000000 ax: 0000000000000000 cx: 00007f7cb7af5fcf sp: 00007ffd02db8c68 ip: 00007f7cb7af5fcf efl: 0000000000000246 cgf: 002b000000000033 erf: 0000000000000000 trp: 0000000000000000 msk: 0000000000000000 cr2: 0000000000000000 [end of stack trace] Calling _exit(1). Core file will not be generated. [Finished in 2.2s with exit code 1] [cmd: ['out/Release/chrome --user-data-dir=/tmp/chromium_6fdsaffsf --enable-logging --vmodule=account_reconcilor=2,gaia_cookie_manager_service=1,dice_bubble_sync_promo_view=1']] [dir: /usr/local/google/work/chromium/src] [path: /usr/lib/google-golang/bin:/usr/local/buildtools/java/jdk/bin:/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin:/usr/local/google/home/msarda/Development/depot_tools:/usr/local/google/home/msarda/gsutil:]
,
Oct 10
,
Oct 10
I will just return a custom error to the extension. The extension can afterwards decide to present it to the user is needed.
,
Oct 10
CC+ Devlin Just to be explicit, I think allowing extensions to trigger UI owned by the browser is bad as it will appear without any context - we have had bad user experiences in the past (remember when Chrome presented a new sign-in screen every 10 seconds?).
,
Oct 10
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/fb977abb942751a4bfa938e212448899574da0ac commit fb977abb942751a4bfa938e212448899574da0ac Author: Mihai Sardarescu <msarda@google.com> Date: Wed Oct 10 20:49:38 2018 Do not start a sign-in flow when browser sign-in is disallowed. This CL returns errors to all extensions that attempt to start a sign-in flow via the chrome.identity API if browser sign-in is not allowed. Bug: 894071 Change-Id: I38d25e551a89ba9dfdd8c4e67f935611acc95a70 Reviewed-on: https://chromium-review.googlesource.com/c/1273645 Commit-Queue: Mihai Sardarescu <msarda@chromium.org> Reviewed-by: David Roger <droger@chromium.org> Cr-Commit-Position: refs/heads/master@{#598490} [modify] https://crrev.com/fb977abb942751a4bfa938e212448899574da0ac/chrome/browser/extensions/api/identity/identity_apitest.cc [modify] https://crrev.com/fb977abb942751a4bfa938e212448899574da0ac/chrome/browser/extensions/api/identity/identity_constants.cc [modify] https://crrev.com/fb977abb942751a4bfa938e212448899574da0ac/chrome/browser/extensions/api/identity/identity_constants.h [modify] https://crrev.com/fb977abb942751a4bfa938e212448899574da0ac/chrome/browser/extensions/api/identity/identity_get_auth_token_function.cc
,
Oct 10
Nice, yeah, I agree that's a more elegant solution. Thanks Mihai!
,
Oct 11
,
Oct 11
[Auto-generated comment by a script] We noticed that this issue is targeted for M-71; it appears the fix may have landed after branch point, meaning a merge might be required. Please confirm if a merge is required here - if so add Merge-Request-71 label, otherwise remove Merge-TBD label. Thanks.
,
Oct 12
This is already in M71 branch. M71 branch (3578), branched at chromium revision 599034.
,
Oct 12
Is this need a merge to M70? If yes, pls request ASAP.
,
Oct 14
This DCHECK occurs in M70 and it somehow breaks the flow for extensions that use the chrome.identity API. We should indeed merge it to M70.
,
Oct 14
This bug requires manual review: We are only 1 days from stable. Please contact the milestone owner if you have questions. Owners: benmason@(Android), kariahda@(iOS), geohsu@(ChromeOS), abdulsyed@(Desktop) For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
,
Oct 14
How safe is this merge overall? well tested?
,
Oct 15
I think this is safe to merge: * the bug has a browser test * in only affects users that use an extension that uses the chrome.identity API and that turn off "Sign in to Chrome allowed" preference in the settings
,
Oct 15
,
Oct 15
Tested in Canary now and the bug is fixed. Merging to M70 now.
,
Oct 15
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/11b6e1514b81e3ad1524262d5d56ef98b29f4527 commit 11b6e1514b81e3ad1524262d5d56ef98b29f4527 Author: Mihai Sardarescu <msarda@google.com> Date: Mon Oct 15 15:41:44 2018 Do not start a sign-in flow when browser sign-in is disallowed. This CL returns errors to all extensions that attempt to start a sign-in flow via the chrome.identity API if browser sign-in is not allowed. Bug: 894071 Change-Id: I38d25e551a89ba9dfdd8c4e67f935611acc95a70 Reviewed-on: https://chromium-review.googlesource.com/c/1273645 Commit-Queue: Mihai Sardarescu <msarda@chromium.org> Reviewed-by: David Roger <droger@chromium.org> Cr-Original-Commit-Position: refs/heads/master@{#598490}(cherry picked from commit fb977abb942751a4bfa938e212448899574da0ac) Reviewed-on: https://chromium-review.googlesource.com/c/1280432 Reviewed-by: Mihai Sardarescu <msarda@chromium.org> Cr-Commit-Position: refs/branch-heads/3538@{#999} Cr-Branched-From: 79f7c91a2b2a2932cd447fa6f865cb6662fa8fa6-refs/heads/master@{#587811} [modify] https://crrev.com/11b6e1514b81e3ad1524262d5d56ef98b29f4527/chrome/browser/extensions/api/identity/identity_apitest.cc [modify] https://crrev.com/11b6e1514b81e3ad1524262d5d56ef98b29f4527/chrome/browser/extensions/api/identity/identity_constants.cc [modify] https://crrev.com/11b6e1514b81e3ad1524262d5d56ef98b29f4527/chrome/browser/extensions/api/identity/identity_constants.h [modify] https://crrev.com/11b6e1514b81e3ad1524262d5d56ef98b29f4527/chrome/browser/extensions/api/identity/identity_get_auth_token_function.cc
,
Oct 15
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/11b6e1514b81e3ad1524262d5d56ef98b29f4527 Commit: 11b6e1514b81e3ad1524262d5d56ef98b29f4527 Author: msarda@google.com Commiter: msarda@chromium.org Date: 2018-10-15 15:41:44 +0000 UTC Do not start a sign-in flow when browser sign-in is disallowed. This CL returns errors to all extensions that attempt to start a sign-in flow via the chrome.identity API if browser sign-in is not allowed. Bug: 894071 Change-Id: I38d25e551a89ba9dfdd8c4e67f935611acc95a70 Reviewed-on: https://chromium-review.googlesource.com/c/1273645 Commit-Queue: Mihai Sardarescu <msarda@chromium.org> Reviewed-by: David Roger <droger@chromium.org> Cr-Original-Commit-Position: refs/heads/master@{#598490}(cherry picked from commit fb977abb942751a4bfa938e212448899574da0ac) Reviewed-on: https://chromium-review.googlesource.com/c/1280432 Reviewed-by: Mihai Sardarescu <msarda@chromium.org> Cr-Commit-Position: refs/branch-heads/3538@{#999} Cr-Branched-From: 79f7c91a2b2a2932cd447fa6f865cb6662fa8fa6-refs/heads/master@{#587811} |
||||||||||||
►
Sign in to add a comment |
||||||||||||
Comment 1 by ew...@chromium.org
, Oct 10