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

commit be290776c2d278b87a9aa707305c9367d75b6f95
Author: Xida Chen <xidachen@chromium.org>
Date: Tue Apr 24 13:46:10 2018

Disable NoFeature_NoMessages on win

TBR=clamy@chromium.org
NOTRY=true

Bug:  836215 
Change-Id: I43d5372cead82d285da6089dc5adbfaaa2857d80
Reviewed-on: https://chromium-review.googlesource.com/1025324
Commit-Queue: Xida Chen <xidachen@chromium.org>
Reviewed-by: Xida Chen <xidachen@chromium.org>
Cr-Commit-Position: refs/heads/master@{#553075}
[modify] https://crrev.com/be290776c2d278b87a9aa707305c9367d75b6f95/chrome/browser/ui/blocked_content/safe_browsing_triggered_popup_blocker_browsertest.cc

[1228:3864:0424/002258.718:FATAL:settings_reset_prompt_config.cc(54)] Check failed: IsPromptEnabled(). 
Backtrace:
	base::debug::StackTrace::StackTrace [0x70A844E6+102]
	base::debug::StackTrace::StackTrace [0x70A831DB+27]
	logging::LogMessage::~LogMessage [0x70AF8DA4+148]
	safe_browsing::SettingsResetPromptConfig::UrlToResetDomainId [0x095994AA+202]
	safe_browsing::SettingsResetPromptModel::InitDefaultSearchData [0x095A06D4+340]
	safe_browsing::SettingsResetPromptModel::SettingsResetPromptModel [0x095A00C4+1588]
	std::make_unique<safe_browsing::SettingsResetPromptModel,Profile * &,std::unique_ptr<safe_browsing::SettingsResetPromptConfig,std::default_delete<safe_browsing::SettingsResetPromptConfig> >,std::unique_ptr<ProfileResetter,std::default_delete<ProfileResett [0x095972AD+189]
	base::BindOnce<void (__cdecl&)(std::unique_ptr<safe_browsing::SettingsResetPromptConfig,std::default_delete<safe_browsing::SettingsResetPromptConfig> >),base::internal::PassedWrapper<std::unique_ptr<safe_browsing::SettingsResetPromptConfig,std::default_de [0x09596299+585]
	base::internal::FunctorTraits<void (__cdecl*)(std::unique_ptr<safe_browsing::SettingsResetPromptConfig,std::default_delete<safe_browsing::SettingsResetPromptConfig> >),void>::Invoke<std::unique_ptr<safe_browsing::SettingsResetPromptConfig,std::default_del [0x095968A2+82]
	base::internal::InvokeHelper<0,void>::MakeItSo<void (__cdecl*)(std::unique_ptr<safe_browsing::SettingsResetPromptConfig,std::default_delete<safe_browsing::SettingsResetPromptConfig> >),std::unique_ptr<safe_browsing::SettingsResetPromptConfig,std::default_ [0x0959676C+60]
	base::internal::Invoker<base::internal::BindState<void (__cdecl*)(std::unique_ptr<safe_browsing::SettingsResetPromptConfig,std::default_delete<safe_browsing::SettingsResetPromptConfig> >),base::internal::PassedWrapper<std::unique_ptr<safe_browsing::Settin [0x095966B9+89]
	base::internal::Invoker<base::internal::BindState<void (__cdecl*)(std::unique_ptr<safe_browsing::SettingsResetPromptConfig,std::default_delete<safe_browsing::SettingsResetPromptConfig> >),base::internal::PassedWrapper<std::unique_ptr<safe_browsing::Settin [0x09596534+84]
	base::OnceCallback<void __cdecl(void)>::Run [0x70A14110+80]
	base::debug::TaskAnnotator::RunTask [0x70A89953+1075]
	base::internal::IncomingTaskQueue::RunTask [0x70B33458+232]
	base::MessageLoop::RunTask [0x70B3E18B+875]
	base::MessageLoop::DeferOrRunPendingTask [0x70B3EBF9+73]
	base::MessageLoop::DoDelayedWork [0x70B3F1F7+519]
	base::MessagePumpForUI::DoRunLoop [0x70B4D3D0+128]
	base::MessagePumpWin::Run [0x70B4C5E9+185]
	base::MessageLoop::Run [0x70B3D946+486]
	base::RunLoop::Run [0x70C2FEE8+488]
	content::RunThisRunLoop [0x06F2200C+44]
	content::MessageLoopRunner::Run [0x06F23982+226]
	content::WindowedNotificationObserver::Wait [0x06F242DB+123]
	content::WaitForLoadStopWithoutSuccessCheck [0x06F345F4+132]
	content::WaitForLoadStop [0x06F3468D+77]
	SafeBrowsingTriggeredPopupBlockerBrowserTest_NoFeature_NoMessages_Test::RunTestOnMainThread [0x0269A13F+255]
	content::BrowserTestBase::ProxyRunTestOnMainThreadLoop [0x06F29ED4+804]
	??$Invoke@PAVBrowserTestBase@content@@$$V@?$FunctorTraits@P8BrowserTestBase@content@@AEXXZX@internal@base@@SAXP8BrowserTestBase@content@@AEXXZ$$QAPAV34@@Z [0x06F2C36C+28]
	base::internal::InvokeHelper<0,void>::MakeItSo<void (__thiscall content::BrowserTestBase::*const &)(void),content::BrowserTestBase *> [0x06F2C28F+79]
	base::internal::Invoker<base::internal::BindState<void (__thiscall content::BrowserTestBase::*)(void),base::internal::UnretainedWrapper<content::BrowserTestBase> >,void __cdecl(void)>::RunImpl<void (__thiscall content::BrowserTestBase::*const &)(void),std [0x06F2C215+85]
	base::internal::Invoker<base::internal::BindState<void (__thiscall content::BrowserTestBase::*)(void),base::internal::UnretainedWrapper<content::BrowserTestBase> >,void __cdecl(void)>::Run [0x06F2C0CF+63]
	base::RepeatingCallback<void __cdecl(void)>::Run [0x019643B2+50]
	ChromeBrowserMainParts::PreMainMessageLoopRunImpl [0x0769E238+7112]
	ChromeBrowserMainParts::PreMainMessageLoopRun [0x0769C56E+398]
	content::BrowserMainLoop::PreMainMessageLoopRun [0x5DFBCFC6+342]
	??$Invoke@PAVBrowserMainLoop@content@@$$V@?$FunctorTraits@P8BrowserMainLoop@content@@AEHXZX@internal@base@@SAHP8BrowserMainLoop@content@@AEHXZ$$QAPAV34@@Z [0x5DFC923C+28]
	base::internal::InvokeHelper<0,int>::MakeItSo<int (__thiscall content::BrowserMainLoop::*const &)(void),content::BrowserMainLoop *> [0x5DFC915F+79]
	base::internal::Invoker<base::internal::BindState<int (__thiscall content::BrowserMainLoop::*)(void),base::internal::UnretainedWrapper<content::BrowserMainLoop> >,int __cdecl(void)>::RunImpl<int (__thiscall content::BrowserMainLoop::*const &)(void),std::t [0x5DFC90D5+85]
	base::internal::Invoker<base::internal::BindState<int (__thiscall content::BrowserMainLoop::*)(void),base::internal::UnretainedWrapper<content::BrowserMainLoop> >,int __cdecl(void)>::Run [0x5DFC8F3F+63]
	base::RepeatingCallback<int __cdecl(void)>::Run [0x5F518352+50]
	content::StartupTaskRunner::RunAllTasksNow [0x5F518099+137]
	content::BrowserMainLoop::CreateStartupTasks [0x5DFBA555+1493]
	content::BrowserMainRunnerImpl::Initialize [0x5DFCF9DD+1901]
	content::BrowserMain [0x5DFB2ABD+173]
	content::RunNamedProcessTypeMain [0x60FA3E70+208]
	content::ContentMainRunnerImpl::Run [0x60FA4B29+569]
	content::ContentServiceManagerMainDelegate::RunEmbedderProcess [0x60FA0561+33]
	service_manager::Main [0x5A206203+1363]
	content::ContentMain [0x60FA3CDC+92]

Other bugs: issue 780300, issue 767862,  issue 729579 .

Looks like the problem is altering the features during the test. My hunch is that "InitFromCommandLine" is the culprit, but I'll do a general fix to ensure features are finalized during test setup, not execution.

+asvitkine FYI, to check me. Here's my understanding:
1. Changing arbitrary features during test execution is _not_ okay
2. Scoping feature changes to features you know will be safe _is__ okay
3. Using InitFromCommandLine clobbers everything

Are those points correct? If so, it seems reasonable to completely disallow InitFromCommandLine in the test body, but careful use of (2) should still be allowed (often this improves test ergonomics significantly). 

Let me know if there is a bug on file about improving (e.g. DCHECKing) some of these constraints.

I have https://chromium-review.googlesource.com/1026235 which tries to enforce some DCHECK, let's see what the bots think.

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

commit e77a96869ef33a1cb05219297344efbbea2749c6
Author: Charlie Harrison <csharrison@chromium.org>
Date: Wed Apr 25 13:24:43 2018

Deflake SafeBrowsing triggered popup test

In general, features should not be changed in the test body. My guess
is that InitFromCommandLine is clobbering all features in a non-safe
way.

This CL stops all feature setting in all test bodies in favor of just
using test subclasses.

Bug:  836215 
Change-Id: I4ffb6f9d16f87359ed1d9df9966faa345f4a72a3
Reviewed-on: https://chromium-review.googlesource.com/1025640
Reviewed-by: Josh Karlin <jkarlin@chromium.org>
Commit-Queue: Charlie Harrison <csharrison@chromium.org>
Cr-Commit-Position: refs/heads/master@{#553529}
[modify] https://crrev.com/e77a96869ef33a1cb05219297344efbbea2749c6/chrome/browser/ui/blocked_content/safe_browsing_triggered_popup_blocker_browsertest.cc

"+asvitkine FYI, to check me. Here's my understanding:
1. Changing arbitrary features during test execution is _not_ okay
-> Correct.

2. Scoping feature changes to features you know will be safe _is__ okay
-> Not sure what you mean by this.

3. Using InitFromCommandLine clobbers everything
-> That's what the warning on it says.

Are those points correct? If so, it seems reasonable to completely disallow InitFromCommandLine in the test body, but careful use of (2) should still be allowed (often this improves test ergonomics significantly). 

Let me know if there is a bug on file about improving (e.g. DCHECKing) some of these constraints."

Agree that InitFromCommandLine() should be discouraged.

In terms of related bugs, there's some historical bugs about specific issues that people encountered - but nothing about adding dchecks specifically. Feel free to file one.

Sorry for (2) I mean it should be allowed to use some ScopedFeatureList API in the test body as long as it is not a method with a big warning, as long as the developer is careful to ensure they are doing it correctly.

For methods without the warning, the changes are scoped to individual feature overrides so there is less spooky action at a distance.