Commit 6aad9a04 authored by Paul Miller's avatar Paul Miller Committed by Commit Bot

WebView: Fix "previously registered pref" crash in variations setup

Commit 37dac0ef fixed the DCHECK crash
where variations_safe_compressed_seed was not registered, by calling
VariationsService::RegisterPrefs to register it. However, that
conflicted with some other prefs (variations_compressed_seed being the
first to fail) which were already being individually registered by
AwFieldTrialCreator. Registering them again caused a 2nd DCHECK crash.

To fix the 2nd crash, don't register any of those prefs individually;
they're all covered by VariationsService::RegisterPrefs.

Fixing the 2nd crash reveals yet a 3rd DCHECK crash, where FeatureList
is created twice: first in BrowserMainLoop::EarlyInitialization and then
in AwFieldTrialCreator. The second fails in FeatureList::SetInstance,
which requires that there be only one global FeatureList. This bug is
unrelated to the 1st and 2nd crashes; it just didn't appear because the
previous crashes prevented us from getting that far.

Since variations is always enabled in Chrome, Chrome always creates the
FeatureList during field trial setup, and always skips creating it in
BrowserMainLoop::EarlyInitialization. In WebView, variations may or may
not be enabled, so we must create FeatureList in exactly one of those
two places, depending on kEnableWebViewVariations.

BUG=842934

Change-Id: Ia592db3710a1f996729aa318853e7339bb3dff4f
Reviewed-on: https://chromium-review.googlesource.com/1069770
Commit-Queue: Paul Miller <paulmiller@chromium.org>
Reviewed-by: default avatarBo <boliu@chromium.org>
Cr-Commit-Position: refs/heads/master@{#560850}
parent 00921cf0
......@@ -57,6 +57,13 @@ AwBrowserMainParts::AwBrowserMainParts(AwContentBrowserClient* browser_client)
AwBrowserMainParts::~AwBrowserMainParts() {
}
bool AwBrowserMainParts::ShouldContentCreateFeatureList() {
// If variations is enabled, the FeatureList will be created in
// AwFieldTrialCreator.
base::CommandLine* cmd = base::CommandLine::ForCurrentProcess();
return !cmd->HasSwitch(switches::kEnableWebViewVariations);
}
int AwBrowserMainParts::PreEarlyInitialization() {
// Network change notifier factory must be singleton, only set factory
// instance while it is not been created.
......
......@@ -26,6 +26,7 @@ class AwBrowserMainParts : public content::BrowserMainParts {
~AwBrowserMainParts() override;
// Overriding methods from content::BrowserMainParts.
bool ShouldContentCreateFeatureList() override;
int PreEarlyInitialization() override;
int PreCreateThreads() override;
void PreMainMessageLoopRun() override;
......
......@@ -61,22 +61,6 @@ AwFieldTrialCreator::~AwFieldTrialCreator() {}
std::unique_ptr<PrefService> AwFieldTrialCreator::CreateLocalState() {
scoped_refptr<PrefRegistrySimple> pref_registry =
base::MakeRefCounted<PrefRegistrySimple>();
// Register the variations prefs with default values that must be overridden.
pref_registry->RegisterTimePref(variations::prefs::kVariationsSeedDate,
base::Time());
pref_registry->RegisterTimePref(variations::prefs::kVariationsLastFetchTime,
base::Time());
pref_registry->RegisterStringPref(variations::prefs::kVariationsCountry,
std::string());
pref_registry->RegisterStringPref(
variations::prefs::kVariationsCompressedSeed, std::string());
pref_registry->RegisterStringPref(variations::prefs::kVariationsSeedSignature,
std::string());
pref_registry->RegisterListPref(
variations::prefs::kVariationsPermanentConsistencyCountry,
std::make_unique<base::ListValue>());
variations::VariationsService::RegisterPrefs(pref_registry.get());
pref_service_factory_.set_user_prefs(
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment