[Chromecast] Use base::FeatureList to control remotely-enabled experiments. |
|
Issue descriptionThis is a public tracking bug for this internal bug: b/35424335. Use base::FeatureList to control features enabled from Chromecast's Device Configuration Server. This class was designed with Finch in mind, but can be leveraged for our design as well. We get the benefits of many of the features for free, including use across different processes, and command-line overrides. See the internal design doc here: https://docs.google.com/document/d/1tbRrHUpZj3VCyKnlnDOsEdyzgDLZkPcgoxIfo-Tud2U/edit
,
Apr 25 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/46cfdeaf9d8e4c78dee471195ad0c1b3a6dcf9e3 commit 46cfdeaf9d8e4c78dee471195ad0c1b3a6dcf9e3 Author: slan <slan@chromium.org> Date: Tue Apr 25 17:55:19 2017 Revert of [Chromecast] Use base::FeatureList to control features. (patchset #7 id:120001 of https://codereview.chromium.org/2825873002/ ) Reason for revert: cast_features_browsertest.cc is flaky on our internal CQ bot - it's unclear why this did not manifest on the Chromium trybots. I assume that state persisting in the PrefService between tests is causing the flakiness. Reverting this patch until I understand the root cause. Original issue's description: > [Chromecast] Use base::FeatureList to control features. > > In Chromium, Finch-enabled features are controlled through base::FeatureList, > a class which abstracts the experiment framework and developer overrides > from client code. Though Chromecast's experiment framework is fundamentally > different (in that it is server-driven) Cast builds can still make use of > this class. Introduce some utilities to help. > > At boot-up, the pref store will be queried for experiment configs, which > were cached to disk on the most recent config fetch from the last boot > cycle. If a developer overrides these features from the command line, > that value takes precedence. These features will be used to initialize > base::FeatureList, which can then be statically queried from any client > code that depends on //base. > > This patch does not actually introduce or convert any existing features > to use this framework. > > BUG=714291 > BUG= internal b/35424335 > > Review-Url: https://codereview.chromium.org/2825873002 > Cr-Commit-Position: refs/heads/master@{#466507} > Committed: https://chromium.googlesource.com/chromium/src/+/ef03d2b9a67352094516f4e866373378fb961ee8 TBR=halliwell@chromium.org,maclellant@chromium.org,asvitkine@chromium.org # Not skipping CQ checks because original CL landed more than 1 days ago. BUG=714291 Review-Url: https://codereview.chromium.org/2838813003 Cr-Commit-Position: refs/heads/master@{#467041} [modify] https://crrev.com/46cfdeaf9d8e4c78dee471195ad0c1b3a6dcf9e3/chromecast/base/BUILD.gn [delete] https://crrev.com/6444ffe806aa0c99eaee4ebe64c31e9962d140da/chromecast/base/cast_features.cc [delete] https://crrev.com/6444ffe806aa0c99eaee4ebe64c31e9962d140da/chromecast/base/cast_features.h [delete] https://crrev.com/6444ffe806aa0c99eaee4ebe64c31e9962d140da/chromecast/base/cast_features_unittest.cc [modify] https://crrev.com/46cfdeaf9d8e4c78dee471195ad0c1b3a6dcf9e3/chromecast/base/pref_names.cc [modify] https://crrev.com/46cfdeaf9d8e4c78dee471195ad0c1b3a6dcf9e3/chromecast/base/pref_names.h [modify] https://crrev.com/46cfdeaf9d8e4c78dee471195ad0c1b3a6dcf9e3/chromecast/browser/BUILD.gn [modify] https://crrev.com/46cfdeaf9d8e4c78dee471195ad0c1b3a6dcf9e3/chromecast/browser/cast_browser_main_parts.cc [modify] https://crrev.com/46cfdeaf9d8e4c78dee471195ad0c1b3a6dcf9e3/chromecast/browser/cast_browser_main_parts.h [modify] https://crrev.com/46cfdeaf9d8e4c78dee471195ad0c1b3a6dcf9e3/chromecast/browser/pref_service_helper.cc [delete] https://crrev.com/6444ffe806aa0c99eaee4ebe64c31e9962d140da/chromecast/browser/test/cast_features_browsertest.cc
,
Apr 28 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/aab6bf24d32d931359b2d0c25fb39b3ed51fbc86 commit aab6bf24d32d931359b2d0c25fb39b3ed51fbc86 Author: slan <slan@chromium.org> Date: Fri Apr 28 15:09:38 2017 Reland "[Chromecast] Use base::FeatureList to control features." This feature was reverted due to the new browsertest being flaky on internal Cast infrastructure: crrev.com/2838813003 === Original Commit Message === In Chromium, Finch-enabled features are controlled through base::FeatureList, a class which abstracts the experiment framework and developer overrides from client code. Though Chromecast's experiment framework is fundamentally different (in that it is server-driven) Cast builds can still make use of this class. Introduce some utilities to help. At boot-up, the pref store will be queried for experiment configs, which were cached to disk on the most recent config fetch from the last boot cycle. If a developer overrides these features from the command line, that value takes precedence. These features will be used to initialize base::FeatureList, which can then be statically queried from any client code that depends on //base. This patch does not actually introduce or convert any existing features to use this framework. BUG=714291 BUG= internal b/35424335 Review-Url: https://codereview.chromium.org/2836263003 Cr-Commit-Position: refs/heads/master@{#467998} [modify] https://crrev.com/aab6bf24d32d931359b2d0c25fb39b3ed51fbc86/chromecast/base/BUILD.gn [add] https://crrev.com/aab6bf24d32d931359b2d0c25fb39b3ed51fbc86/chromecast/base/cast_features.cc [add] https://crrev.com/aab6bf24d32d931359b2d0c25fb39b3ed51fbc86/chromecast/base/cast_features.h [add] https://crrev.com/aab6bf24d32d931359b2d0c25fb39b3ed51fbc86/chromecast/base/cast_features_unittest.cc [modify] https://crrev.com/aab6bf24d32d931359b2d0c25fb39b3ed51fbc86/chromecast/base/pref_names.cc [modify] https://crrev.com/aab6bf24d32d931359b2d0c25fb39b3ed51fbc86/chromecast/base/pref_names.h [modify] https://crrev.com/aab6bf24d32d931359b2d0c25fb39b3ed51fbc86/chromecast/browser/BUILD.gn [modify] https://crrev.com/aab6bf24d32d931359b2d0c25fb39b3ed51fbc86/chromecast/browser/cast_browser_main_parts.cc [modify] https://crrev.com/aab6bf24d32d931359b2d0c25fb39b3ed51fbc86/chromecast/browser/cast_browser_main_parts.h [modify] https://crrev.com/aab6bf24d32d931359b2d0c25fb39b3ed51fbc86/chromecast/browser/pref_service_helper.cc [add] https://crrev.com/aab6bf24d32d931359b2d0c25fb39b3ed51fbc86/chromecast/browser/test/cast_features_browsertest.cc
,
Apr 29 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/efddbe79f23b2ecbaf250302bd70c4fd82c002dd commit efddbe79f23b2ecbaf250302bd70c4fd82c002dd Author: slan <slan@chromium.org> Date: Sat Apr 29 22:10:16 2017 Revert of Reland "[Chromecast] Use base::FeatureList to control features." (patchset #4 id:60001 of https://codereview.chromium.org/2836263003/ ) Reason for revert: Breaks ATV at runtime. Original issue's description: > Reland "[Chromecast] Use base::FeatureList to control features." > > This feature was reverted due to the new browsertest being flaky on > internal Cast infrastructure: crrev.com/2838813003 > > === Original Commit Message === > In Chromium, Finch-enabled features are controlled through base::FeatureList, > a class which abstracts the experiment framework and developer overrides > from client code. Though Chromecast's experiment framework is fundamentally > different (in that it is server-driven) Cast builds can still make use of > this class. Introduce some utilities to help. > > At boot-up, the pref store will be queried for experiment configs, which > were cached to disk on the most recent config fetch from the last boot > cycle. If a developer overrides these features from the command line, > that value takes precedence. These features will be used to initialize > base::FeatureList, which can then be statically queried from any client > code that depends on //base. > > This patch does not actually introduce or convert any existing features > to use this framework. > > BUG=714291 > BUG= internal b/35424335 > > Review-Url: https://codereview.chromium.org/2836263003 > Cr-Commit-Position: refs/heads/master@{#467998} > Committed: https://chromium.googlesource.com/chromium/src/+/aab6bf24d32d931359b2d0c25fb39b3ed51fbc86 TBR=maclellant@chromium.org,halliwell@chromium.org # Not skipping CQ checks because original CL landed more than 1 days ago. BUG=714291 Review-Url: https://codereview.chromium.org/2847423002 Cr-Commit-Position: refs/heads/master@{#468249} [modify] https://crrev.com/efddbe79f23b2ecbaf250302bd70c4fd82c002dd/chromecast/base/BUILD.gn [delete] https://crrev.com/934e8926ee432fe5f5e6b2b9a1f196455ede77df/chromecast/base/cast_features.cc [delete] https://crrev.com/934e8926ee432fe5f5e6b2b9a1f196455ede77df/chromecast/base/cast_features.h [delete] https://crrev.com/934e8926ee432fe5f5e6b2b9a1f196455ede77df/chromecast/base/cast_features_unittest.cc [modify] https://crrev.com/efddbe79f23b2ecbaf250302bd70c4fd82c002dd/chromecast/base/pref_names.cc [modify] https://crrev.com/efddbe79f23b2ecbaf250302bd70c4fd82c002dd/chromecast/base/pref_names.h [modify] https://crrev.com/efddbe79f23b2ecbaf250302bd70c4fd82c002dd/chromecast/browser/BUILD.gn [modify] https://crrev.com/efddbe79f23b2ecbaf250302bd70c4fd82c002dd/chromecast/browser/cast_browser_main_parts.cc [modify] https://crrev.com/efddbe79f23b2ecbaf250302bd70c4fd82c002dd/chromecast/browser/cast_browser_main_parts.h [modify] https://crrev.com/efddbe79f23b2ecbaf250302bd70c4fd82c002dd/chromecast/browser/pref_service_helper.cc [delete] https://crrev.com/934e8926ee432fe5f5e6b2b9a1f196455ede77df/chromecast/browser/test/cast_features_browsertest.cc
,
Jun 13 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/0a02ecd5e26cd16d87a195f627885abddeeed89b commit 0a02ecd5e26cd16d87a195f627885abddeeed89b Author: Stephen Lanham <slan@google.com> Date: Tue Jun 13 16:08:37 2017 Reland "[Chromecast] Use base::FeatureList to control features." This change was reverted because it broke ATV. That is fixed now. === Original Commit Message === In Chromium, Finch-enabled features are controlled through base::FeatureList, a class which abstracts the experiment framework and developer overrides from client code. Though Chromecast's experiment framework is fundamentally different (in that it is server-driven) Cast builds can still make use of this class. Introduce some utilities to help. At boot-up, the pref store will be queried for experiment configs, which were cached to disk on the most recent config fetch from the last boot cycle. If a developer overrides these features from the command line, that value takes precedence. These features will be used to initialize base::FeatureList, which can then be statically queried from any client code that depends on //base. This patch does not actually introduce or convert any existing features to use this framework. BUG=714291 BUG= internal b/35424335 Change-Id: I1ec7bed07c3e1bec1035f8709fea302e509acb5c Review-Url: https://codereview.chromium.org/2836263003 Cr-Original-Commit-Position: refs/heads/master@{#467998} Reviewed-on: https://chromium-review.googlesource.com/531672 Reviewed-by: Alok Priyadarshi <alokp@chromium.org> Commit-Queue: Stephen Lanham <slan@chromium.org> Cr-Commit-Position: refs/heads/master@{#479036} [modify] https://crrev.com/0a02ecd5e26cd16d87a195f627885abddeeed89b/chromecast/base/BUILD.gn [add] https://crrev.com/0a02ecd5e26cd16d87a195f627885abddeeed89b/chromecast/base/cast_features.cc [add] https://crrev.com/0a02ecd5e26cd16d87a195f627885abddeeed89b/chromecast/base/cast_features.h [add] https://crrev.com/0a02ecd5e26cd16d87a195f627885abddeeed89b/chromecast/base/cast_features_unittest.cc [modify] https://crrev.com/0a02ecd5e26cd16d87a195f627885abddeeed89b/chromecast/base/pref_names.cc [modify] https://crrev.com/0a02ecd5e26cd16d87a195f627885abddeeed89b/chromecast/base/pref_names.h [modify] https://crrev.com/0a02ecd5e26cd16d87a195f627885abddeeed89b/chromecast/browser/BUILD.gn [modify] https://crrev.com/0a02ecd5e26cd16d87a195f627885abddeeed89b/chromecast/browser/cast_browser_main_parts.cc [modify] https://crrev.com/0a02ecd5e26cd16d87a195f627885abddeeed89b/chromecast/browser/cast_browser_main_parts.h [modify] https://crrev.com/0a02ecd5e26cd16d87a195f627885abddeeed89b/chromecast/browser/pref_service_helper.cc [add] https://crrev.com/0a02ecd5e26cd16d87a195f627885abddeeed89b/chromecast/browser/test/cast_features_browsertest.cc |
|
►
Sign in to add a comment |
|
Comment 1 by bugdroid1@chromium.org
, Apr 22 2017