Can not reproduce. It doesn't crash on my side. Will keep working on how to reproduce.

Did you put the "dcheck_always_on = true" to you gn arguments?

Yes.

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

commit 4a6cceb9e12930863da1dec08627823f88cb3518
Author: sebsg <sebsg@chromium.org>
Date: Wed Jun 21 21:11:38 2017

Revert "Experiment to add bank name in autofill ui."

This reverts commit b0045cb0dec61387b7cc05049c2878b406bc298b.

BUG= 735574 

TBR=mathp@chromium.org, rkaplow@chromium.org, pkasting@chromium.org

Review-Url: https://codereview.chromium.org/2948943003
Cr-Commit-Position: refs/heads/master@{#481302}

[modify] https://crrev.com/4a6cceb9e12930863da1dec08627823f88cb3518/chrome/browser/about_flags.cc
[modify] https://crrev.com/4a6cceb9e12930863da1dec08627823f88cb3518/chrome/browser/flag_descriptions.cc
[modify] https://crrev.com/4a6cceb9e12930863da1dec08627823f88cb3518/chrome/browser/flag_descriptions.h
[modify] https://crrev.com/4a6cceb9e12930863da1dec08627823f88cb3518/components/autofill/core/browser/autofill_experiments.cc
[modify] https://crrev.com/4a6cceb9e12930863da1dec08627823f88cb3518/components/autofill/core/browser/autofill_experiments.h
[modify] https://crrev.com/4a6cceb9e12930863da1dec08627823f88cb3518/components/autofill/core/browser/autofill_manager.cc
[modify] https://crrev.com/4a6cceb9e12930863da1dec08627823f88cb3518/components/autofill/core/browser/autofill_metrics.cc
[modify] https://crrev.com/4a6cceb9e12930863da1dec08627823f88cb3518/components/autofill/core/browser/autofill_metrics.h
[modify] https://crrev.com/4a6cceb9e12930863da1dec08627823f88cb3518/components/autofill/core/browser/autofill_metrics_unittest.cc
[modify] https://crrev.com/4a6cceb9e12930863da1dec08627823f88cb3518/components/autofill/core/browser/credit_card.cc
[modify] https://crrev.com/4a6cceb9e12930863da1dec08627823f88cb3518/components/autofill/core/browser/credit_card.h
[modify] https://crrev.com/4a6cceb9e12930863da1dec08627823f88cb3518/components/autofill/core/browser/credit_card_unittest.cc
[modify] https://crrev.com/4a6cceb9e12930863da1dec08627823f88cb3518/components/autofill/core/browser/personal_data_manager.cc
[modify] https://crrev.com/4a6cceb9e12930863da1dec08627823f88cb3518/components/autofill/core/browser/personal_data_manager_unittest.cc
[modify] https://crrev.com/4a6cceb9e12930863da1dec08627823f88cb3518/components/autofill/core/browser/webdata/autofill_table.cc
[modify] https://crrev.com/4a6cceb9e12930863da1dec08627823f88cb3518/components/autofill/core/browser/webdata/autofill_table.h
[modify] https://crrev.com/4a6cceb9e12930863da1dec08627823f88cb3518/components/autofill/core/browser/webdata/autofill_table_unittest.cc
[modify] https://crrev.com/4a6cceb9e12930863da1dec08627823f88cb3518/components/autofill/core/browser/webdata/autofill_wallet_syncable_service.cc
[delete] https://crrev.com/db20a4e2b78608669a6572bdb3eedc3853f4657e/components/test/data/web_database/version_72.sql
[modify] https://crrev.com/4a6cceb9e12930863da1dec08627823f88cb3518/components/webdata/common/BUILD.gn
[modify] https://crrev.com/4a6cceb9e12930863da1dec08627823f88cb3518/components/webdata/common/web_database.cc
[modify] https://crrev.com/4a6cceb9e12930863da1dec08627823f88cb3518/components/webdata/common/web_database_migration_unittest.cc
[modify] https://crrev.com/4a6cceb9e12930863da1dec08627823f88cb3518/tools/metrics/histograms/enums.xml
[modify] https://crrev.com/4a6cceb9e12930863da1dec08627823f88cb3518/tools/metrics/histograms/histograms.xml

Still cannot reproduce.
Tried this:
1) create a new profile and run chrome without my patch. A version 72 database will be created.
2) run chrome with my patch. And the database can be successfully migrated to version 73. And we can get server credit cards.
Some manually added logs: https://screenshot.googleplex.com/qhuhLQVsSkv

My gn setting:
target_os = "linux"
use_goma = true
dcheck_always_on = true

Command to build:
ninja -j 2000 -C out/Release chrome

Command to run:
./out/Release/chrome --user-data-dir=~/.config/chromium/Profile\ 3

To gather more information about the crash. Andrey, can you pick this https://codereview.chromium.org/2946373002 and run chrome with your Profile?

Sure thing -- here's the output, and as I just realized there was an error message I did not notice before (the one about bank_name column).

caseq@caseq:~/src$ ./out/Release/chrome --enable-logging=stderr 
[37056:37080:0621/173639.203776:WARNING:web_database.cc(79)] 3fengLog: in init
[37056:37080:0621/173639.206472:WARNING:web_database.cc(154)] 3fengLog: current_version:73
[37056:37080:0621/173639.206484:WARNING:web_database.cc(177)] 3fengLog: latest version:73
[37056:37080:0621/173639.207943:WARNING:autofill_table.cc(1213)] 3fengLog: get server credit cards start.
[37056:37080:0621/173639.208052:FATAL:connection.cc(1516)] SQL compile error no such column: bank_name
#0 0x5640e2c4b537 base::debug::StackTrace::StackTrace()
#1 0x5640e2c658ad logging::LogMessage::~LogMessage()
#2 0x5640e42021b5 sql::Connection::GetStatementImpl()
#3 0x5640e41fe422 sql::Connection::GetUniqueStatement()
#4 0x5640e491d02f autofill::AutofillTable::GetServerCreditCards()
#5 0x5640e493ead6 autofill::AutofillWebDataBackendImpl::GetServerCreditCards()
#6 0x5640e44a1542 _ZN4base8internal7InvokerINS0_9BindStateIM21KeywordWebDataServiceFSt10unique_ptrI13WDTypedResultSt14default_deleteIS5_EEP11WebDatabaseEJ13scoped_refptrIS3_EEEEFS8_SA_EE3RunEPNS0_13BindStateBaseEOSA_
#7 0x5640e41fb109 WebDatabaseBackend::DBReadTaskWrapper()
#8 0x5640e41f9d2f _ZN4base8internal7InvokerINS0_9BindStateIM18WebDatabaseBackendFvRKNS_8CallbackIFN11WebDatabase5StateEPS5_ELNS0_8CopyModeE1ELNS0_10RepeatModeE1EEESt10unique_ptrI14WebDataRequestSt14default_deleteISF_EEEJ13scoped_refptrIS3_ESB_NS0_13PassedWrapperISI_EEEEEFvvEE3RunEPNS0_13BindStateBaseE
#9 0x5640e0fd2e31 _ZNO4base8CallbackIFvvELNS_8internal8CopyModeE1ELNS2_10RepeatModeE1EE3RunEv
#10 0x5640e2cfcaa3 base::debug::TaskAnnotator::RunTask()
#11 0x5640e2cb3746 base::internal::TaskTracker::PerformRunTask()
#12 0x5640e2cb3de0 base::internal::TaskTrackerPosix::PerformRunTask()
#13 0x5640e2cb2f4c base::internal::TaskTracker::RunNextTask()
#14 0x5640e2d082fe base::internal::SchedulerWorker::Thread::ThreadMain()
#15 0x5640e2cbbb2c base::(anonymous namespace)::ThreadFunc()
#16 0x7f86ce4d7184 start_thread
#17 0x7f86c81eabed clone

Hmm, so it's in version 73 and still complaining about the lack of the bank_name column...

OK. I see the problem now. Somehow the profile is promoted to version 73 before. Therefore migration function is not run at all. That's why there's no bank_name column.

In other words, the database of the profile was in a dirty state.

> Hmm, so it's in version 73 and still complaining about the lack of the bank_name column...

Which I guess means that it that there was some other CL that used this version number that was subsequently reverted. And here it is: https://chromium.googlesource.com/chromium/src/+/1aea7837a31d25fb5e7422c93a2efbd1a4b8c388%5E%21/#F76

+Rouslan, you and Shanfeng conflicted on version 73 of the database.

Thanks caseq@ for looking into this with us!

No problem, sorry for initially missing the error message that would perhaps make it simpler :-)

Oh. yes. finally! I should have add rouslan@ as a reviewer.
Spending whole day debugging this. Glad that we finally figure it out. Thanks Andrey, Mathieu, and Seb for your help.

In this case:
If user launched Chromium from 06/20 08:56 to 06/20 11:53, their database is migrated to rouslan's version_73.
If user launched Chromium from 06/20 18:15 to 06/21 14:10, their database is migrated to my version_73.
Do we need to clean up?

rouslan@ we need to coordinate about who should submit the code first. FYI, I need to submit it before feature freeze for M61, which is this Friday.

Shanfeng we wrongly reverted your change, apologies then. Please create another identical change (reverting the revert, essentially). 

How about using version 74 when relanding? This would automatically take care of my profile, and perhaps some other users like me.

My version 73 code has been reverted in http://crrev.com/c/540793. I'm currently working on relanding it in http://crrev.com/c/541476 with version 74.

If your bank_name change has been reverted, please land it as version 73 again. I've already done the work for using version 74 in my patch.

> Do we need to clean up?

You can do it manually on your own developer machine and also maybe send out and PSA to chromium-dev@ to let other devs know.

> I need to submit it before feature freeze for M61, which is this Friday.

I'm not in a hurry, so go ahead with landing your change first.

Issue 735612 has been merged into this issue.

A couple of thoughts on the issue:
- if profile database structure is inconsistent, it's an error condition, but not a DCHECK(). CHECK()/DCHECK() is for code invariants, and some assertions about the environment your code runs in (state of the disk, network etc.) are not invariants;
- this particular DCHECK() does not seem to be directly produced by unexpected database schema, it's rather from the SQL connection being used on the wrong thread, which is apparently somehow induced by wrong schema  -- so the root issue here is the way we handle SQL errors and not just the fact that we had a version collision;

Ah, had a closer look, never mind the part about the "wrong thread", the relevant DCHECK() is from DLOG(FATAL) << "SQL compile error", and this is what needs to be fixed I guess.

Can this bug be updated with a link to the cleanup instructions?

I believe the cleanup is to completely remove the user profile. That would be:

$ rm -rf ~/.config/chromium

Don't you feel that forcing your users and colleagues to remove their entire profiles is a bit... rough?

I was under the impression that most people use Google Chrome for day to day work. That profile is stored in ~/.config/google-chrome*. ~/.config/chromium is only for Chromium built from source. Do people use that for actual work? Anywho, the more surgical operation is the following:

$ rm ~/.config/chromium/Default/Web\ Data

Well, people happen to have some specific extensions, flags & experiments, DevTools workspaces, snippets and history on their dev browsers. Why not just bump the database version number? Or nuke the DCHECK() that is against the code style anyway.

Going to programmatically fix this in http://crrev.com/c/549255.