• Elly Fong-Jones's avatar
    flags: honor configured values of unexpired flags · f8a4aa42
    Elly Fong-Jones authored
    Currently, feature initialization looks like this:
    
    1. The pref store is initialized (which contains saved flag states)
    2. A FlagsState instance is initialized from that prefs store
      a. FlagsState computes which switches and features should be in what
         states based on flags
      b. FlagsState excludes switches and features from flags that are
         currently expired
    3. A FeatureList is initialized from that FlagsState
    
    Unfortunately, deciding whether a flag is expired or not requires
    knowing the state of the various TemporaryUnexpireFlagsM$M features,
    which aren't initialized until step 3, so step 2b doesn't actually work
    properly. During step 2b, every unexpiry feature is treated as if it was
    in its default state (i.e. disabled), which means that every flag whose
    expiration has passed is treated as expired, regardless of the state of
    the unexpiry flag. As a result of *that*, when the FeatureList is
    initialized at step 3, any such flag's configured value gets ignored.
    
    The fix for this is deeply unfortunate: at step 2b, add a special case
    in checking whether a flag should be considered expired or not.
    Specifically, consider not just the state of the unexpiry feature (which
    may not have been initialized yet) but also the state of the backing
    unexpiry *flag*. To enable this, pass a FlagsStorage into the
    FlagsState::Delegate.
    
    Change-Id: Ia2b6423357d24318baaeb3ca34208d1a2f1c36aa
    Bug: 1101828
    Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2302925Reviewed-by: default avatarNico Weber <thakis@chromium.org>
    Reviewed-by: default avatarAlexei Svitkine <asvitkine@chromium.org>
    Commit-Queue: Elly Fong-Jones <ellyjones@chromium.org>
    Cr-Commit-Position: refs/heads/master@{#794557}
    f8a4aa42
unexpire_flags.cc 4.78 KB