New issue
Advanced search Search tips

Issue 884827 link

Starred by 0 users

Issue metadata

Status: Verified
Owner:
Closed: Sep 20
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Android
Pri: 2
Type: Bug

Blocking:
issue 866722



Sign in to add a comment

WebView applies experiments both from seed and test config

Project Member Reported by paulmiller@chromium.org, Sep 17

Issue description

Chrome uses IsFetchingEnabled() in variations_service.cc to prevent fetching when the test config (testing/variations/fieldtrial_testing_config.json) is in use, but that doesn't help WebView because WebView doesn't use VariationsService. This causes certain experiments to get applied twice, hitting a DCHECK in base::FeatureList::RegisterFieldTrialOverride(). (Currently this only happens with WebSocketHandshakeReuseConnectionStable.)
 
Project Member

Comment 1 by bugdroid1@chromium.org, Sep 20

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

commit cea27bb08bf7ea3de7b90cd573418142c689ac6b
Author: Paul Miller <paulmiller@google.com>
Date: Thu Sep 20 20:40:41 2018

Variations: Ignore seed when using test config

Chrome uses IsFetchingEnabled() to enable getting the seed under any of
these conditions:
  • is_chrome_branded
  • --variations-server-url
  • g_should_fetch_for_testing

Chrome also enables the test config if none of these conditions are met:
  • fieldtrial_testing_like_official_build
  • is_chrome_branded
  • --disable-field-trial-config
  • --force-fieldtrials
  • --variations-server-url
(See VariationsFieldTrialCreator::SetupFieldTrials().)

So by default (is_chrome_branded=false), the seed will be disabled and
the test config will be enabled. In the released builds
(is_chrome_branded=true), the seed will be enabled and the test config
will be disabled. But depending on flags and build args, it's possible
to enable both. The same experiment in both places will then be added
twice, hitting a DCHECK in FeatureList::RegisterFieldTrialOverride().

WebView shares the same conditions for enabling the test config, but
before this change, would always use the seed as well, since WebView
doesn't call IsFetchingEnabled().

BUG= 884827 

Change-Id: I3b8281a3a44ebbae894e744fc0a84d6d1f94a0d4
Reviewed-on: https://chromium-review.googlesource.com/1228913
Commit-Queue: Paul Miller <paulmiller@chromium.org>
Reviewed-by: Alexei Svitkine <asvitkine@chromium.org>
Reviewed-by: Steven Holte <holte@chromium.org>
Reviewed-by: Bo <boliu@chromium.org>
Cr-Commit-Position: refs/heads/master@{#592934}
[modify] https://crrev.com/cea27bb08bf7ea3de7b90cd573418142c689ac6b/components/variations/service/variations_field_trial_creator.cc
[modify] https://crrev.com/cea27bb08bf7ea3de7b90cd573418142c689ac6b/components/variations/service/variations_field_trial_creator_unittest.cc

Status: Fixed (was: Assigned)
Please add any manual verification steps if applicable.Thanks in advance
Status: Verified (was: Fixed)
I guess that's difficult for QA to do until I implement experiment logging. I verified it myself for https://crbug.com/881553#c2.
Thanks Paul for the verification .

Sign in to add a comment