Issue 812554  has been merged into this issue.

Same issue for "ChromeBrowsingDataRemoverDelegateTest.RemoveSafeBrowsingCookieLastHour".

Flakes @ https://chromium-try-flakes.appspot.com/all_flake_occurrences?key=ahVzfmNocm9taXVtLXRyeS1mbGFrZXNyUQsSBUZsYWtlIkZDaHJvbWVCcm93c2luZ0RhdGFSZW1vdmVyRGVsZWdhdGVUZXN0LlJlbW92ZVNhZmVCcm93c2luZ0Nvb2tpZUxhc3RIb3VyDA

@pwnall who recently touched SQL all over the place in relevant regression ranges.


Find a sample test flake below.

In this case it seems like sql::Connection::Execute resulted in SQLITE_ERROR (1).

Speculation : the database is initialized asynchronously but the profile can be deleted (in the test through profile_.reset()) before this task runs?

SQLITE_ERROR also seems to be rising in the wild : https://uma.googleplex.com/p/chrome/timeline_v2/?sid=961b404f8a4c5e19d725e2f8c53101b0

66.0.3335.0 appears to be the first bad Canary (or around there, somewhat noisy), suspected regression range : https://chromium.googlesource.com/chromium/src/+log/66.0.3334.0..66.0.3335.0?pretty=fuller&n=10000

There are many CLs that change SQL in this range by pwnall@, PTaL. Not clear to me which CL is at fault.


[ RUN      ] ChromeBrowsingDataRemoverDelegateTest.RemoveSafeBrowsingCookieForeverWithPredicate
[2192:3904:0215/040623.908:139948:ERROR:connection.cc(1888)] Cookie sqlite error 1, errno 0: no such column: persistent, sql: DELETE FROM cookies WHERE persistent != 1
[2192:3904:0215/040623.908:139948:FATAL:connection.cc(1383)] Check failed: error != 1 (1 vs. 1)SQL Error in DELETE FROM cookies WHERE persistent != 1, no such column: persistent
Backtrace:
	base::debug::StackTrace::StackTrace [0x03FFFC60+32]
	base::debug::StackTrace::StackTrace [0x03FDCADD+13]
	logging::LogMessage::~LogMessage [0x03F6D0E0+80]
	sql::Connection::Execute [0x04AC7AFC+296]
	net::SQLitePersistentCookieStore::Backend::DeleteSessionCookiesOnStartup [0x04C7B63C+122]
	net::SQLitePersistentCookieStore::Backend::InitializeDatabase [0x04C79787+1499]
	net::SQLitePersistentCookieStore::Backend::LoadAndNotifyInBackground [0x04C78D6B+267]
	base::internal::Invoker<base::internal::BindState<void (__thiscall net::SQLitePersistentCookieStore::Backend::*)(base::RepeatingCallback<void __cdecl(std::vector<std::unique_ptr<net::CanonicalCookie,std::default_delete<net::CanonicalCookie> >,std::allocat [0x04C7CC70+32]
	base::debug::TaskAnnotator::RunTask [0x03FFD037+231]
	base::internal::TaskTracker::RunOrSkipTask [0x0400190B+635]
	base::test::ScopedTaskEnvironment::TestTaskTracker::RunOrSkipTask [0x038E88C0+160]
	base::internal::TaskTracker::RunNextTask [0x04000FD1+385]
	base::internal::SchedulerWorker::Thread::ThreadMain [0x0403CF9F+1023]
	base::PlatformThread::SetCurrentThreadPriority [0x03F7F905+533]
	BaseThreadInitThunk [0x76E3336A+18]
	RtlInitializeExceptionChain [0x779A9902+99]
	RtlInitializeExceptionChain [0x779A98D5+54]

Also, those tests usage of MessageLoopRunner is old school and deprecated, please update to RunLoop if touching them. And lastly, TaskScheduler::FlushForTesting() shouldn't be necessary (the scheduler is always running and waiting on condition to occur and quit main loop should be sufficient).

Removing from Sheriff queue for now. Will considering disabling tests if new batch of flakes comes in.

Detected 3 new flakes for test/step "ChromeBrowsingDataRemoverDelegateTest.RemoveSafeBrowsingCookieForever". To see the actual flakes, please visit https://chromium-try-flakes.appspot.com/all_flake_occurrences?key=ahVzfmNocm9taXVtLXRyeS1mbGFrZXNyUAsSBUZsYWtlIkVDaHJvbWVCcm93c2luZ0RhdGFSZW1vdmVyRGVsZWdhdGVUZXN0LlJlbW92ZVNhZmVCcm93c2luZ0Nvb2tpZUZvcmV2ZXIM. This message was posted automatically by the chromium-try-flakes app. Since flakiness is ongoing, the issue was moved back into Sheriff Bug Queue (unless already there).

I'll look at this today. Sorry for the slow response!

Detected 4 new flakes for test/step "ChromeBrowsingDataRemoverDelegateTest.RemoveSafeBrowsingCookieLastHour". To see the actual flakes, please visit https://chromium-try-flakes.appspot.com/all_flake_occurrences?key=ahVzfmNocm9taXVtLXRyeS1mbGFrZXNyUQsSBUZsYWtlIkZDaHJvbWVCcm93c2luZ0RhdGFSZW1vdmVyRGVsZWdhdGVUZXN0LlJlbW92ZVNhZmVCcm93c2luZ0Nvb2tpZUxhc3RIb3VyDA. This message was posted automatically by the chromium-try-flakes app. Since flakiness is ongoing, the issue was moved back into Sheriff Bug Queue (unless already there).

pwnall@, WDYT about disabling the test while looking into it (and enable it when it's ready).

dschuyler@: Sent chttps://crrev.com/c/924761 your way. Can you please sanity-check?

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

commit 7a07bc981207c6a392766b782dc438419940765b
Author: Victor Costan <pwnall@chromium.org>
Date: Sat Feb 17 01:30:52 2018

Disable flaky tests in ChromeBrowsingDataRemoverDelegateTest.

ChromeBrowsingDataRemoverDelegateTest.RemoveSafeBrowsingCookie* are
flaking due to errors in the cookie store. It seems like the cookie
store is initialized incorrectly. Disabling until we can investigate
whether the problem is in the test setup or in the cookie store
initialization sequence.

TBR=msramek@chromium.org

Bug: 812589
Change-Id: I174790c0ed3518b88eca3e45304015113d5a5ba8
Reviewed-on: https://chromium-review.googlesource.com/924761
Reviewed-by: Victor Costan <pwnall@chromium.org>
Reviewed-by: Dave Schuyler <dschuyler@chromium.org>
Commit-Queue: Victor Costan <pwnall@chromium.org>
Cr-Commit-Position: refs/heads/master@{#537498}
[modify] https://crrev.com/7a07bc981207c6a392766b782dc438419940765b/chrome/browser/browsing_data/chrome_browsing_data_remover_delegate_unittest.cc

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

commit b482d796009843373dbc38abb2da6134489a668f
Author: Victor Costan <pwnall@chromium.org>
Date: Tue Feb 20 23:37:26 2018

sqlite: Use SQLITE_ENABLE_API_ARMOR when DCHECK is on.

SQLITE_ENABLE_API_ARMOR [1] adds checks against SQLite API misuse. This
flag is equivalent to DCHECKs that check preconditions at the begining
of an API call, so it should be enabled in builds where DCHECKs are
enabled.

[1] https://sqlite.org/compile.html#enable_api_armor

Bug:  807093 , 812589
Change-Id: I97721723334bd7bc4166971e05f6f032d7376abc
Cq-Include-Trybots: master.tryserver.chromium.win:win_chrome_official;master.tryserver.chromium.linux:linux_chromium_msan_rel_ng;master.tryserver.chromium.linux:linux_chromium_tsan_rel_ng;master.tryserver.chromium.mac:mac_chromium_asan_rel_ng
Reviewed-on: https://chromium-review.googlesource.com/924565
Reviewed-by: Chris Mumford <cmumford@chromium.org>
Commit-Queue: Victor Costan <pwnall@chromium.org>
Cr-Commit-Position: refs/heads/master@{#537934}
[modify] https://crrev.com/b482d796009843373dbc38abb2da6134489a668f/third_party/sqlite/BUILD.gn
[modify] https://crrev.com/b482d796009843373dbc38abb2da6134489a668f/third_party/sqlite/amalgamation/sqlite3.c
[add] https://crrev.com/b482d796009843373dbc38abb2da6134489a668f/third_party/sqlite/patches/0011-Fix-thread-sanitizer-TSAN-error-when-SQLITE_ENABLE_A.patch
[modify] https://crrev.com/b482d796009843373dbc38abb2da6134489a668f/third_party/sqlite/src/src/mutex_unix.c

Hi,
what is the status of these sqlite errors? Could we try to reenable the tests?

I haven't found anything wrong with the tests. IIRC, a few days later, some bots were showing I/O errors due to full disks. We could re-enable and see if that's the problem.

Thanks, I tried to enable the test but it looks like a recent network service change broke it:

[1301:1301:0608/095348.297465:64014611008:FATAL:safe_browsing_url_request_context_getter.cc(33)] Check failed: system_context_getter_. 
#0 0x7f9c4cff104c base::debug::StackTrace::StackTrace()
#1 0x7f9c4cf3a6db logging::LogMessage::~LogMessage()
#2 0x000002f5b400 safe_browsing::SafeBrowsingURLRequestContextGetter::SafeBrowsingURLRequestContextGetter()
#3 0x0000040db09b safe_browsing::SafeBrowsingService::Initialize()
#4 0x00000124db39 (anonymous namespace)::RemoveSafeBrowsingCookieTester::RemoveSafeBrowsingCookieTester()
#5 0x00000124e0ef ChromeBrowsingDataRemoverDelegateTest_RemoveSafeBrowsingCookieLastHour_Test::TestBody()

There was a post on chromium-dev that mentions a problem that sounds very similar to the flakiness of the test, should we wait until it is fixed?
https://groups.google.com/a/chromium.org/forum/?utm_medium=email&utm_source=footer#!msg/chromium-dev/-oH0n_O9Tjc/BEZ6t0jqAwAJ

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

commit 0aa1d243f6df50e26930493492164a1550ff63d1
Author: Reilly Grant <reillyg@google.com>
Date: Wed Oct 03 19:41:11 2018

Use network::mojom::CookieManager in RemoveSafeBrowsingCookieTester

This change updates RemoveSafeBrowsingCookieTester and its base class
RemoveCookieTester to use the CookieManager Mojo interface instead of
the net::CookieStore interface directly.

These tests now pass however I am leaving them marked as disabled until
issue 812589 is investigated more thoroughly.

Bug:  721395 ,812589
Change-Id: Iaadf98b31969bd159dfda5f5ff0802b8c8dd3239
Reviewed-on: https://chromium-review.googlesource.com/c/1257785
Commit-Queue: Reilly Grant <reillyg@chromium.org>
Reviewed-by: Joshua Bell <jsbell@chromium.org>
Cr-Commit-Position: refs/heads/master@{#596328}
[modify] https://crrev.com/0aa1d243f6df50e26930493492164a1550ff63d1/chrome/browser/browsing_data/chrome_browsing_data_remover_delegate_unittest.cc