d2c9d7c6d6
This reverts commit e7f8c2b338.
Reason for revert: Will consider putting this documentation elsewhere
Original change's description:
> Document WebView flag requirement in flag_guarding_guidelines.md
>
> Bug: None
> Change-Id: I19afe75cb0b2500ca3fb096041549b09d851543c
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4387340
> Commit-Queue: Andrew Williams <awillia@chromium.org>
> Reviewed-by: Richard Coles <torne@chromium.org>
> Cr-Commit-Position: refs/heads/main@{#1124759}
Bug: None
Change-Id: I3b9d6a82fa2645615e8b3ae7da63188e5f8dd6b7
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4391302
Bot-Commit: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com>
Commit-Queue: Andrew Williams <awillia@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1124980}
77 lines
4.2 KiB
Markdown
77 lines
4.2 KiB
Markdown
# Chromium Flag Guarding Guidelines
|
||
|
||
This document describes using [`base::Feature`](/base/feature_list.h) flags which
|
||
can be remotely set via a server. This applies to both A/B experiments
|
||
([internal link](http://go/finch101)) (disabled by default) and to kill switches
|
||
([internal link](http://go/finch-killswitch)) (enabled by default).
|
||
|
||
Google maintains its own server which you'll see referenced by its internal name
|
||
Finch. Other embedders can and do run their own server for their products.
|
||
|
||
[TOC]
|
||
|
||
## Goals
|
||
* Prevent large scale outages and reduce the response time latency of outages of
|
||
Chromium and Android WebView
|
||
* Reduce the need for a binary respin to address problems in the field
|
||
* Catch regressions in core product vitals
|
||
|
||
## Non-Goals
|
||
* Require a flag per CL/bug without consideration. This is not scalable. See the
|
||
next section for guidance of when flags should be used.
|
||
* Flag-guarding of ChromeOS-specific features.
|
||
* Add a lot of long-lived server-configurable flags across the code base. New
|
||
flags generated by this proposal should be removed 1-2 milestones after launch.
|
||
* Mandate that all changes be rolled out via server side configurations.
|
||
|
||
## When is a flag required?
|
||
* Every project/feature launch shall use a flag unless it’s not [feasible](#feasible)
|
||
* Every feature going through Launch Process (Note, you do not need a launch bug
|
||
to use a flag)
|
||
* Every feature using origin trials, per existing [guidelines](https://www.chromium.org/blink/origin-trials/running-an-origin-trial/#is-your-feature-ready-to-be-an-origin-trial:~:text=Have%20a%20way%20to%20remotely%20disable%20the%20feature)
|
||
* Every deprecation/addition of web platform APIs
|
||
* Very large structural changes that have very different paths (e.g. navigation
|
||
rewrite in PlzNavigate, networking rewrite in Network Service, Out-of-process
|
||
Rasterization etc.).
|
||
* Refactorings in code paths that have historically been risky or prone to
|
||
accidental breakages should also be treated the same as a new feature.
|
||
* Regardless of whether it is a new feature, refactoring, or bug fix, there is
|
||
no minimum size that dictates whether a flag is required (either for isolated
|
||
CLs or for many CLs that form a project/feature). A one line change with
|
||
potential to impact stability, performance, usability is just as required to use
|
||
a flag as a multi-thousand line feature.
|
||
* {#feasible}If, as a CL author, you are uncertain whether a flag can/should be
|
||
used, talk to the relevant TL/Uber-TL and if still unsure, just use a flag.
|
||
|
||
## When is a flag not required?
|
||
* Targeted/micro performance optimizations: projects like V8/Skia/decoders etc.
|
||
that have their own large correctness and performance test suites to not have to
|
||
use server rollouts since they have large confidence based on their tests
|
||
* Changes to core data structures where it would almost be impossible to
|
||
maintain both worlds (e.g. V8 pointer compression where adding a branch in each
|
||
dereference would not be practical).
|
||
* Features shipped via component updater: if we ship a bad component we can
|
||
update to a fixed one
|
||
* Chrome A/B binary experiments: we can use Omaha/AppStore/Play to update users
|
||
from bad builds
|
||
* Non-chromium-repo binary drops: e.g. SwiftShader
|
||
* Rolling/Updating third party dependencies (e.g. libvpx, libwebp etc.)
|
||
* Mechanical/automated refactorings
|
||
* Changes to internal API naming
|
||
* Simple parameter changes (adding params, changing the type etc.)
|
||
* Isolated refactorings where test coverage with high test coverage / confidence
|
||
|
||
|
||
## What type of flag rollout to use?
|
||
* If a change has the potential to affect performance or memory
|
||
([internal link](http://go/chrome-browser-guiding-metrics)), or you want to
|
||
analyze the impact of the launch on feature-specific metrics, use a
|
||
disabled-by-default base::Feature flag and run an A/B experiment.
|
||
Non-Googler committers will need to work with owners of the code that work at
|
||
Google to launch and monitor the experiment.
|
||
* Otherwise it should be guarded minimally by an enabled-by-default
|
||
base::Feature flag, which can be remotely disabled by a server configuration.
|
||
* For code in blink, this can be as simple as using a
|
||
[Runtime Enabled Feature](/third_party/blink/renderer/platform/RuntimeEnabledFeatures.md),
|
||
which has long been common-practice for new or changed APIs.
|