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

commit 1eb3f04e9c385598f3efd08fbdcaa927c5f6d238
Author: dalecurtis <dalecurtis@chromium.org>
Date: Fri Oct 28 20:51:39 2016

Disable ProfileManagerBrowserTest.DeletePasswords on Linux.

Suddenly started failing, no responsible CLs are in range though;
verified through manually reverting locally.

BUG=660488
TEST=none
TBR=mlerman

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

[modify] https://crrev.com/1eb3f04e9c385598f3efd08fbdcaa927c5f6d238/chrome/browser/profiles/profile_manager_browsertest.cc

Thanks for the report. Linking this to our flaky tests umbrella bug just for reference, although if this is 100% reproducible, it might be a prod issue instead. Will try to have a look.

Also, attaching the failing output from https://build.chromium.org/p/chromium.linux/builders/Linux%20Tests%20%28dbg%29%281%29%2832%29/builds/34972 and the blamelist: https://chromium.googlesource.com/chromium/src/+log/ed9d17790ce6f192dd1eb06737fa111432e5d0a1..ad9e85d3ae876b9531b7850d120a3f363ea0ad34?pretty=fuller&n=10000

Deleted: Log File contents.html 32.0 KB

Log File contents.html 32.0 KB View Download

Also Cc-ing vasilii@, who created this test some years ago.

I removed all the password code from the test and got the same error. ProfileManagerBrowserTest.DeletePasswords is the only test on Linux that called ScheduleProfileForDeletion. I'll try to understand what's going on. A production issue is likely.

Here what happens:
- ScheduleProfileForDeletion creates a new profile asynchronously and cleans the old one. The clean up includes ClearLocalStorageOnUIThread() which calls DOMStorageContextWrapper::GetLocalStorageUsage(). The latter posts a task on a background thread.
- After that run_loop.Run() in the test exits. The main thread is starting a clean-up.
- The background posts a reply back to the UI thread.
- The main thread destroys the pending tasks. It causes DOMStorageContextImpl::~DOMStorageContextImpl() which posts a BLOCK_SHUTDOWN task. But it's too late because shutdown has completed.

Looks like the issue may happen in the production too. 
Stefan, how does production code ensure that profiles::RemoveBrowsingDataForProfile really completed on all the background threads?

My knowledge here is rusty at best (3 years away from it). I asked around and Michael Lerm (on fiber now) suggested that there are a few CLs, from profile_manager's revision history, that have played with Profile Deletion "recently"

https://chromium.googlesource.com/chromium/src/+/09ec753c800ef11bce69c67c79ca1374675ede78
https://chromium.googlesource.com/chromium/src/+/9f27972d0fbb00d451c9ae3f741d1e22ddd25891
https://chromium.googlesource.com/chromium/src/+/c684d2e6774693c6b975e0ee7e7f3a134d5f31ea
https://chromium.googlesource.com/chromium/src/+/4552c5134b38e43f1169e10810820bd10e459aa4

If the problem is new he suggested trying locally patching reverts of each of those and seeing if it fixes the problem (I am currently working on NYC ARC++ rebasing and have my hands more then full) one "easy" hunch would be that we've introduced something that relies on the MessageLoop and so there's a posted task that needs to be run?

I tried the first CLs and it's not relevant. The others are too old and cause conflicts.
There is a real bug in RemoveBrowsingDataForProfile() because it uses BrowsingDataRemover and nothing waits til it finishes. There is no privacy issue in the production code due to ProfileManager::NukeDeletedProfilesFromDisk(). It's called on exit and removes the directories.
The DCHECK can still happen in production pure theoretically. How do owners feel? Should we fix the test only by running the loop longer?

So in production code this should not be a problem.

If the user would clear the data and immediately log out he *might* hit this - but since we have only 3s's to log out/shut down, it would then probably also  get stopped before finishing anyways.

As such I am in favor of fixing the test. 

Anthony?

We are also experiencing flakiness. I would like to pick that issue if you don't mind. Do you think that proper fix of RemoveBrowsingDataForProfile call chain is too complicated? Maybe some tips on how to approach that?

The proper fix is to pass a callback to ScheduleProfileForDeletion which is called when browsing data for profile is removed. The quick fix of the test would be just to synchronize with the culprit thread.
Feel free to pick it.

This issue has been Available for over a year. If it's no longer important or seems unlikely to be fixed, please consider closing it out. If it is important, please re-triage the issue.

Sorry for the inconvenience if the bug really should have been left as Available. If you change it back, also remove the "Hotlist-Recharge-Cold" label.

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

This issue has been Available for over a year. If it's no longer important or seems unlikely to be fixed, please consider closing it out. If it is important, please re-triage the issue.

Sorry for the inconvenience if the bug really should have been left as Available.

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