Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(166)

Issue 2847423002: Revert of Reland "[Chromecast] Use base::FeatureList to control features." (Closed)

Created:
3 years, 7 months ago by slan
Modified:
3 years, 7 months ago
Reviewers:
halliwell, maclellant
CC:
chromium-reviews, alokp+watch_chromium.org, lcwu+watch_chromium.org, halliwell+watch_chromium.org
Target Ref:
refs/heads/master
Project:
chromium
Visibility:
Public.

Description

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} Committed: https://chromium.googlesource.com/chromium/src/+/efddbe79f23b2ecbaf250302bd70c4fd82c002dd

Patch Set 1 #

Unified diffs Side-by-side diffs Delta from patch set Stats (+5 lines, -876 lines) Patch
M chromecast/base/BUILD.gn View 2 chunks +0 lines, -3 lines 0 comments Download
D chromecast/base/cast_features.h View 1 chunk +0 lines, -52 lines 0 comments Download
D chromecast/base/cast_features.cc View 1 chunk +0 lines, -197 lines 0 comments Download
D chromecast/base/cast_features_unittest.cc View 1 chunk +0 lines, -285 lines 0 comments Download
M chromecast/base/pref_names.h View 1 chunk +0 lines, -2 lines 0 comments Download
M chromecast/base/pref_names.cc View 1 chunk +0 lines, -6 lines 0 comments Download
M chromecast/browser/BUILD.gn View 2 chunks +0 lines, -3 lines 0 comments Download
M chromecast/browser/cast_browser_main_parts.h View 2 chunks +0 lines, -2 lines 0 comments Download
M chromecast/browser/cast_browser_main_parts.cc View 4 chunks +5 lines, -23 lines 0 comments Download
M chromecast/browser/pref_service_helper.cc View 1 chunk +0 lines, -2 lines 0 comments Download
D chromecast/browser/test/cast_features_browsertest.cc View 1 chunk +0 lines, -301 lines 0 comments Download

Messages

Total messages: 10 (5 generated)
slan
Created Revert of Reland "[Chromecast] Use base::FeatureList to control features."
3 years, 7 months ago (2017-04-29 20:02:38 UTC) #2
commit-bot: I haz the power
CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.org/2847423002/1
3 years, 7 months ago (2017-04-29 20:02:45 UTC) #3
commit-bot: I haz the power
Try jobs failed on following builders: linux_android_rel_ng on master.tryserver.chromium.android (JOB_FAILED, https://build.chromium.org/p/tryserver.chromium.android/builders/linux_android_rel_ng/builds/282863)
3 years, 7 months ago (2017-04-29 20:51:26 UTC) #5
commit-bot: I haz the power
CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.org/2847423002/1
3 years, 7 months ago (2017-04-29 20:52:56 UTC) #7
commit-bot: I haz the power
3 years, 7 months ago (2017-04-29 22:10:37 UTC) #10
Message was sent while issue was closed.
Committed patchset #1 (id:1) as
https://chromium.googlesource.com/chromium/src/+/efddbe79f23b2ecbaf250302bd70...

Powered by Google App Engine
This is Rietveld 408576698