New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 894852 link

Starred by 2 users

Issue metadata

Status: Fixed
Owner:
Closed: Oct 23
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Linux , Windows , Mac
Pri: 1
Type: Bug



Sign in to add a comment

Check failed in model_association_manager.cc

Project Member Reported by se...@chromium.org, Oct 12

Issue description

Here is the check that failed: FATAL:model_association_manager.cc(194)] Check failed: error.IsSet() || (dtc->state() != DataTypeController::NOT_RUNNING). 


To repro, switch between Sync transport and sync feature while having credit cards autofill disabled at chrome://settings/payments
Once you are in the other state, try to toggle CC autofill back on.
I can repro rougly 33% of the time.


Here's the stack trace:

Received signal 6
#0 0x7f7daab1864f base::debug::StackTrace::StackTrace()
#1 0x7f7daab18131 base::debug::(anonymous namespace)::StackDumpSignalHandler()
#2 0x7f7d9e8770c0 <unknown>
#3 0x7f7d9c70afcf gsignal
#4 0x7f7d9c70c3fa abort
#5 0x7f7daab16f15 base::debug::BreakDebugger()
#6 0x7f7daaa42338 logging::LogMessage::~LogMessage()
#7 0x55c936acca45 syncer::ModelAssociationManager::StopDatatypeImpl()
#8 0x55c936accb4e syncer::ModelAssociationManager::StopDatatype()
#9 0x55c936ac4885 syncer::DataTypeManagerImpl::ReadyForStartChanged()
#10 0x55c936af5c61 browser_sync::ProfileSyncService::ReadyForStartChanged()
#11 0x55c936ac30bf browser_sync::AutofillWalletModelTypeController::OnUserPrefChanged()
#12 0x7f7da5ad6dec PrefChangeRegistrar::InvokeUnnamedCallback()
#13 0x7f7da5ad70c2 PrefChangeRegistrar::OnPreferenceChanged()
#14 0x7f7da5ad9ef9 PrefNotifierImpl::FireObservers()
#15 0x7f7da5ae260a PrefValueStore::PrefStoreKeeper::OnPrefValueChanged()
#16 0x55c93585c7c4 SegregatedPrefStore::AggregatingObserver::OnPrefValueChanged()
#17 0x7f7da5ad1614 JsonPrefStore::ReportValueChanged()
#18 0x7f7da5ad01dd JsonPrefStore::SetValue()
#19 0x55c93585d363 SegregatedPrefStore::SetValue()
#20 0x7f7da5adf7a4 PrefService::SetUserPrefValue()
#21 0x7f7da5adf65d PrefService::Set()
#22 0x55c93676239e extensions::PrefsUtil::SetPref()
#23 0x55c9368a0bea extensions::SettingsPrivateSetPrefFunction::Run()
#24 0x55c9355fc9fc ExtensionFunction::RunWithValidation()
#25 0x55c9355fec17 extensions::ExtensionFunctionDispatcher::DispatchWithCallbackInternal()
#26 0x55c9355fe892 extensions::ExtensionFunctionDispatcher::Dispatch()
#27 0x55c935625cf7 extensions::ExtensionWebContentsObserver::OnRequest()
#28 0x55c935625b5f _ZN3IPC8MessageTI29ExtensionHostMsg_Request_MetaNSt3__15tupleIJ31ExtensionHostMsg_Request_ParamsEEEvE8DispatchIN10extensions28ExtensionWebContentsObserverES9_N7content15RenderFrameHostEMS9_FvPSB_RKS4_EEEbPKNS_7MessageEPT_PT0_PT1_T2_
#29 0x55c935625a7f extensions::ExtensionWebContentsObserver::OnMessageReceived()
#30 0x55c93677644e extensions::ChromeExtensionWebContentsObserver::OnMessageReceived()
#31 0x7f7da82c00b2 content::WebContentsImpl::OnMessageReceived()
#32 0x7f7da7f13e85 content::RenderFrameHostImpl::OnMessageReceived()
#33 0x7f7da814eccb content::RenderProcessHostImpl::OnMessageReceived()
#34 0x7f7da8fd93b1 IPC::ChannelProxy::Context::OnDispatchMessage()
#35 0x7f7da8fdc0c8 _ZN4base8internal7InvokerINS0_9BindStateIMN3IPC12ChannelProxy7ContextEFvRKNS3_7MessageEEJ13scoped_refptrIS5_ES6_EEEFvvEE3RunEPNS0_13BindStateBaseE
#36 0x7f7daaa22b22 base::debug::TaskAnnotator::RunTask()
#37 0x7f7daaa4ff2f base::MessageLoop::RunTask()
#38 0x7f7daaa503c2 base::MessageLoop::DoWork()
#39 0x7f7daaa5475f base::(anonymous namespace)::WorkSourceDispatch()
#40 0x7f7d9e006fc7 g_main_context_dispatch
#41 0x7f7d9e007200 <unknown>
#42 0x7f7d9e00728c g_main_context_iteration
#43 0x7f7daaa54482 base::MessagePumpGlib::Run()
#44 0x7f7daaa4fa01 base::MessageLoop::Run()
#45 0x7f7daaa83ac6 base::RunLoop::Run()
#46 0x55c935c3d88a ChromeBrowserMainParts::MainMessageLoopRun()
#47 0x7f7da7d522e7 content::BrowserMainLoop::RunMainMessageLoopParts()
#48 0x7f7da7d553d3 content::BrowserMainRunnerImpl::Run()
#49 0x7f7da7d4e5e9 content::BrowserMain()
#50 0x7f7da888bb10 content::ContentMainRunnerImpl::Run()
#51 0x7f7daad8eb19 service_manager::Main()
#52 0x7f7da8889be1 content::ContentMain()
#53 0x55c93544e1b3 ChromeMain
#54 0x7f7d9c6f82b1 __libc_start_main
#55 0x55c93544e02a _start
  r8: 0000000000000000  r9: 00007ffc52619140 r10: 0000000000000008 r11: 0000000000000246
 r12: 00007ffc52619c28 r13: 00007ffc52619c18 r14: 00007ffc52619c20 r15: 00007ffc526193d9
  di: 0000000000000002  si: 00007ffc52619140  bp: 00007ffc52619380  bx: 0000000000000006
  dx: 0000000000000000  ax: 0000000000000000  cx: 00007f7d9c70afcf  sp: 00007ffc526191b8
  ip: 00007f7d9c70afcf efl: 0000000000000246 cgf: 002b000000000033 erf: 0000000000000000
 trp: 0000000000000000 msk: 0000000000000000 cr2: 0000000000000000
[end of stack trace]

 
Cc: mamir@chromium.org treib@chromium.org jkrcal@chromium.org mastiz@chromium.org
 Issue 895467  has been merged into this issue.
Repro steps from  Issue 895467 :

Repro steps:
- Fresh profile.
- Sign in.
- In the Sync confirmation dialog, click "Settings".
- Turn off "Use sync and all services" (so that the data type toggles become available).
- Disable "Payment methods ...". (Also happens when disabling "Autofill" since that includes payments.)
From having a very quick look at this, the regular ModelAssociationManager::Stop method checks that the data type state is not STOPPING or NOT_RUNNING, but we don't check this in StopDatatype. I think it would make sense to add the same check there, because we don't actually know if the data type is running when that's called (and obviously it isn't always).

Owner: feuunk@chromium.org
Status: Started (was: Assigned)
In progress: https://chromium-review.googlesource.com/c/chromium/src/+/1290149
Project Member

Comment 5 by bugdroid1@chromium.org, Oct 22

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/37b83cf92779c09cf759bd280e78e6087efb5231

commit 37b83cf92779c09cf759bd280e78e6087efb5231
Author: Florian Uunk <feuunk@chromium.org>
Date: Mon Oct 22 10:08:55 2018

Only stop datatypes that are actually running

We have this check in ModelAssociationManager::Stop, so it makes sense
to add it to ModelAssociationManager::StopDatatype too.

Bug:  894852 
Change-Id: If5f472ea74cba7bbe60f796b1255c8e54d65a069
Reviewed-on: https://chromium-review.googlesource.com/c/1290149
Commit-Queue: Florian Uunk <feuunk@chromium.org>
Reviewed-by: Marc Treib <treib@chromium.org>
Cr-Commit-Position: refs/heads/master@{#601514}
[modify] https://crrev.com/37b83cf92779c09cf759bd280e78e6087efb5231/components/sync/driver/model_association_manager.cc
[modify] https://crrev.com/37b83cf92779c09cf759bd280e78e6087efb5231/components/sync/driver/model_association_manager_unittest.cc

Labels: Merge-Request-71
Requesting a merge for this crashfix. I will wait to verify on Canary before doing the actual merge.
Project Member

Comment 7 by sheriffbot@chromium.org, Oct 23

Labels: -Merge-Request-71 Hotlist-Merge-Approved Merge-Approved-71
Your change meets the bar and is auto-approved for M71. Please go ahead and merge the CL to branch 3578 manually. Please contact milestone owner if you have questions.
Owners: benmason@(Android), kariahda@(iOS), kbleicher@(ChromeOS), govind@(Desktop)

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

Comment 8 by bugdroid1@chromium.org, Oct 23

Labels: -merge-approved-71 merge-merged-3578
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/5d601a980b5914d30195c9f8eae7f74fdd5d45d2

commit 5d601a980b5914d30195c9f8eae7f74fdd5d45d2
Author: Florian Uunk <feuunk@chromium.org>
Date: Tue Oct 23 11:04:40 2018

Only stop datatypes that are actually running

We have this check in ModelAssociationManager::Stop, so it makes sense
to add it to ModelAssociationManager::StopDatatype too.

Bug:  894852 
Change-Id: If5f472ea74cba7bbe60f796b1255c8e54d65a069
Reviewed-on: https://chromium-review.googlesource.com/c/1290149
Commit-Queue: Florian Uunk <feuunk@chromium.org>
Reviewed-by: Marc Treib <treib@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#601514}(cherry picked from commit 37b83cf92779c09cf759bd280e78e6087efb5231)
Reviewed-on: https://chromium-review.googlesource.com/c/1296529
Reviewed-by: Florian Uunk <feuunk@chromium.org>
Cr-Commit-Position: refs/branch-heads/3578@{#259}
Cr-Branched-From: 4226ddf99103e493d7afb23a4c7902ee496108b6-refs/heads/master@{#599034}
[modify] https://crrev.com/5d601a980b5914d30195c9f8eae7f74fdd5d45d2/components/sync/driver/model_association_manager.cc
[modify] https://crrev.com/5d601a980b5914d30195c9f8eae7f74fdd5d45d2/components/sync/driver/model_association_manager_unittest.cc

Status: Fixed (was: Started)
Labels: Merge-Merged-71-3578
The following revision refers to this bug: 
https://chromium.googlesource.com/chromium/src.git/+/5d601a980b5914d30195c9f8eae7f74fdd5d45d2

Commit: 5d601a980b5914d30195c9f8eae7f74fdd5d45d2
Author: feuunk@chromium.org
Commiter: feuunk@chromium.org
Date: 2018-10-23 11:04:40 +0000 UTC

Only stop datatypes that are actually running

We have this check in ModelAssociationManager::Stop, so it makes sense
to add it to ModelAssociationManager::StopDatatype too.

Bug:  894852 
Change-Id: If5f472ea74cba7bbe60f796b1255c8e54d65a069
Reviewed-on: https://chromium-review.googlesource.com/c/1290149
Commit-Queue: Florian Uunk <feuunk@chromium.org>
Reviewed-by: Marc Treib <treib@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#601514}(cherry picked from commit 37b83cf92779c09cf759bd280e78e6087efb5231)
Reviewed-on: https://chromium-review.googlesource.com/c/1296529
Reviewed-by: Florian Uunk <feuunk@chromium.org>
Cr-Commit-Position: refs/branch-heads/3578@{#259}
Cr-Branched-From: 4226ddf99103e493d7afb23a4c7902ee496108b6-refs/heads/master@{#599034}

Sign in to add a comment