OLD | NEW |
(Empty) | |
| 1 # Life of a Feature |
| 2 |
| 3 In the years since the Chromium browser was first open-sourced, the `//net` |
| 4 directory has expanded from being the basis of loading web content in the |
| 5 Chromium browser to accommodating a wide variety of networking needs, |
| 6 both in the Chromium browser and in other Google and third-party products |
| 7 and projects. |
| 8 |
| 9 This brings with it many new opportunities, such as the ability to |
| 10 introduce new protocols rapidly or push Web security forward, as well as |
| 11 new challenges, such as how to balance the needs of various `//net` |
| 12 consumers effectively. |
| 13 |
| 14 To make it easier to contribute new features or to change existing |
| 15 behaviors in `//net`, this document tries to capture the life of a |
| 16 feature in `//net`, from initial design to the eventual possibility of |
| 17 deprecation and removal. |
| 18 |
| 19 ## Supported Projects |
| 20 |
| 21 When considering the introduction of a new `//net` feature or changing |
| 22 a `//net` behavior, it's first necessary to understand where `//net` |
| 23 is used, how it is used, and what the various constraints and limits are. |
| 24 |
| 25 To understand a more comprehensive matrix of the supported platforms and |
| 26 constraints, see [Supported Projects](supported-projects.md). When |
| 27 examining a new feature request, or a change in behavior, it's necessary |
| 28 to consider dimensions such as: |
| 29 |
| 30 * Does this feature apply to all supported projects, or is this something |
| 31 like a Browser-only feature? |
| 32 * Does this feature apply to all supported platforms, or is this something |
| 33 specific to a particular subset? |
| 34 * Is the feature a basic networking library feature, or is it specific to |
| 35 something in the Web Platform? |
| 36 * Will some projects wish to strip the feature in order to meet targets |
| 37 such as memory usage (RAM) or binary size? |
| 38 * Does it depend on Google services or Google-specific behaviors or |
| 39 features? |
| 40 * How will this feature be tested / experimented with? For example, |
| 41 __Field Trials (Finch)__ and __User Metrics (UMA)__ may not be available |
| 42 on a number of platforms. |
| 43 * How risky is the feature towards compatibility/stability? How will it |
| 44 be undone if there is a bug? |
| 45 * Are the power/memory/CPU/bandwidth requirements appropriate for the |
| 46 targeted projects and/or platforms? |
| 47 |
| 48 ## Design and Layering |
| 49 |
| 50 Once the supported platforms and constraints are identified, it's necessary |
| 51 to determine how to actually design the feature to meet those constraints, |
| 52 in hopefully the easiest way possible both for implementation and consumption. |
| 53 |
| 54 ### Designing for multiple platforms |
| 55 |
| 56 In general, `//net` features try to support all platforms with a common |
| 57 interface, and generally eschew OS-specific interfaces from being exposed as |
| 58 part of `//net`. |
| 59 |
| 60 Cross-platform code is generally done via declaring an interface named |
| 61 `foo.h`, which is common for all platforms, and then using the build-system to |
| 62 do compile-time switching between implementations in `foo_win.cc`, `foo_mac.cc`, |
| 63 `foo_android.cc`, etc. |
| 64 |
| 65 The goal is to ensure that consumers generally don't have to think about |
| 66 OS-specific considerations, and can instead code to the interface. |
| 67 |
| 68 ### Designing for multiple products |
| 69 |
| 70 While premature abstraction can significantly harm readability, if it is |
| 71 anticipated that different products will have different implementation needs, |
| 72 or may wish to selectively disable the feature, it's often necessary to |
| 73 abstract that interface sufficiently in `//net` to allow for dependency |
| 74 injection. |
| 75 |
| 76 This is true whether discussing concrete classes and interfaces or something |
| 77 as simple a boolean configuration flag that different consumers wish to set |
| 78 differently. |
| 79 |
| 80 The two most common approaches in `//net` are injection and delegation. |
| 81 |
| 82 #### Injection |
| 83 |
| 84 Injection refers to the pattern of defining the interface or concrete |
| 85 configuration parameter (such as a boolean), along with the concrete |
| 86 implementation, but requiring the `//net` embedder to supply an instance |
| 87 of the interface or the configuration parameters (perhaps optionally). |
| 88 |
| 89 Examples of this pattern include things such as the `ProxyConfigService`, |
| 90 which has concrete implementations in `//net` for a variety of platforms' |
| 91 configuration of proxies, but which requires it be supplied as part of the |
| 92 `URLRequestContextGetter` building, if proxies are going to be supported. |
| 93 |
| 94 An example of injecting configuration flags can be seen in the |
| 95 `HttpNetworkSession::Params` structure, which is used to provide much of |
| 96 the initialization parameters for the HTTP layer. |
| 97 |
| 98 The ideal form of injection is to pass ownership of the injected object, |
| 99 such as via a `std::unique_ptr<Foo>`. While this is not consistently |
| 100 applied in `//net`, as there are a number of places in which ownership is |
| 101 either shared or left to the embedder, with the injected object passed |
| 102 around as a naked/raw pointer, this is generally seen as an anti-pattern |
| 103 and not to be mirrored for new features. |
| 104 |
| 105 #### Delegation |
| 106 |
| 107 Delegation refers to forcing the `//net` embedder to respond to specific |
| 108 delegated calls via a Delegate interface that it implements. In general, |
| 109 when using the delegate pattern, ownership of the delegate should be |
| 110 transferred, so that the lifetime and threading semantics are clear and |
| 111 unambiguous. |
| 112 |
| 113 That is, for a given class `Foo`, which has a `Foo::Delegate` interface |
| 114 defined to allow embedders to alter behavior, prefer a constructor that |
| 115 is |
| 116 ``` |
| 117 explicit Foo(std::unique_ptr<Delegate> delegate); |
| 118 ``` |
| 119 so that it is clear that the lifetime of `delegate` is determined by |
| 120 `Foo`. |
| 121 |
| 122 While this may appear similar to Injection, the general difference |
| 123 between the two approaches is determining where the bulk of the |
| 124 implementation lies. With Injection, the interface describes a |
| 125 behavior contract that concrete implementations must adhere to; this |
| 126 allows for much more flexibility with behavior, but with the downside |
| 127 of significantly more work to implement or extend. Delegation attempts |
| 128 to keep the bulk of the implementation in `//net`, and the decision as |
| 129 to which implementation to use in `//net`, but allows `//net` to |
| 130 provide specific ways in which embedders can alter behaviors. |
| 131 |
| 132 The most notable example of the delegate pattern is `URLRequest::Delegate`, |
| 133 which keeps the vast majority of the loading logic within `URLRequest`, |
| 134 but allows the `URLRequest::Delegate` to participate during specific times |
| 135 in the request lifetime and alter specific behaviors as necessary. (Note: |
| 136 `URLRequest::Delegate`, like much of the original `//net` code, doesn't |
| 137 adhere to the recommended lifetime patterns of passing ownership of the |
| 138 Delegate. It is from the experience debugging and supporting these APIs |
| 139 that the `//net` team strongly encourages all new code pass explicit |
| 140 ownership, to reduce the complexity and risk of lifetime issues). |
| 141 |
| 142 While the use of a `base::Callback` can also be considered a form of |
| 143 delegation, the `//net` layer tries to eschew any callbacks that can be |
| 144 called more than once, and instead favors defining class interfaces |
| 145 with concrete behavioral requirements in order to ensure the correct |
| 146 lifetimes of objects and to adjust over time. When `//net` takes a |
| 147 callback (e.g. `net::CompletionCallback`), the intended pattern is to |
| 148 signal the asynchronous completion of a single method, invoking that |
| 149 callback at most once before deallocating it. For more discussion |
| 150 of these patterns, see [Code Patterns](code-patterns.md). |
| 151 |
| 152 ### Understanding the Layering |
| 153 |
| 154 A significant challenge many feature proposals face is understanding the |
| 155 layering in `//net` and what different portions of `//net` are allowed to |
| 156 know. |
| 157 |
| 158 #### Socket Pools |
| 159 |
| 160 The most common challenge feature proposals encounter is the awareness |
| 161 that the act of associating an actual request to make with a socket is |
| 162 done lazily, referred to as "late-binding". |
| 163 |
| 164 With late-bound sockets, a given `URLRequest` will not be assigned an actual |
| 165 transport connection until the request is ready to be sent. This allows for |
| 166 reprioritizing requests as they come in, to ensure that higher priority requests |
| 167 get preferential treatment, but it also means that features or data associated |
| 168 with a `URLRequest` generally don't participate in socket establishment or |
| 169 maintenance. |
| 170 |
| 171 For example, a feature that wants to associate the low-level network socket |
| 172 with a `URLRequest` during connection establishment is not something that the |
| 173 `//net` design supports, since the `URLRequest` is kept unaware of how sockets |
| 174 are established by virtue of the socket pools and late binding. This allows for |
| 175 more flexibility when working to improve performance, such as the ability to |
| 176 coalesce multiple logical 'sockets' over a single HTTP/2 or QUIC stream, which |
| 177 may only have a single physical network socket involved. |
| 178 |
| 179 #### Making Additional Requests |
| 180 |
| 181 From time to time, `//net` feature proposals will involve needing to load |
| 182 a secondary resource as part of processing. For example, SDCH involves loading |
| 183 additional dictionaries that are advertised in a header, and other feature |
| 184 proposals have involved fetching a `/.well-known/` URI or reporting errors to |
| 185 a particular URL. |
| 186 |
| 187 This is particularly challenging, because often, these features are implemented |
| 188 deeper in the network stack, such as [`//net/cert`](../cert), [`//net/http`](../
http), |
| 189 or [`//net/filter`](../filter), which [`//net/url_request`](../url_request) depe
nds |
| 190 on. Because `//net/url_request` depends on these low-level directories, it would |
| 191 be a circular dependency to have these directories depend on `//net/url_request`
, |
| 192 and circular dependencies are forbidden. |
| 193 |
| 194 The recommended solution to address this is to adopt the delegation or injection |
| 195 patterns. The lower-level directory will define some interface that represents t
he |
| 196 "I need this URL" request, and then elsewhere, in a directory allowed to depend |
| 197 on `//net/url_request`, an implementation of that interface/delegate that uses |
| 198 `//net/url_request` is implemented. |
| 199 |
| 200 ### Understanding the Lifetimes |
| 201 |
| 202 Understanding the object lifetime and dependency graph can be one of the largest |
| 203 challenges to contributing and maintaining `//net`. As a consequence, features |
| 204 which require introducing more complexity to the lifetimes of objects generally |
| 205 have a greater challenge to acceptance. |
| 206 |
| 207 The `//net` stack is designed heavily around a sync-or-async pattern, as |
| 208 documented in [Code Patterns](code-patterns.md), while also having a strong |
| 209 requirement that it be possible to cleanly shutdown the network stack. As a |
| 210 consequence, features should have precise, well-defined lifetime semantics |
| 211 and support graceful cleanup. Further, because much of the network stack can |
| 212 have web-observable side-effects, it is often required for tasks to have |
| 213 defined sequencing that cannot be reordered. To be ensure these requirements |
| 214 are met, features should attempt to model object lifetimes as a hierarchical |
| 215 DAG, using explicit ownership and avoiding the use of reference counting or |
| 216 weak pointers as part of any of the exposed API contracts (even for features |
| 217 only consumed in `//net`). Features that pay close attention to the lifetime |
| 218 semantics are more likely to be reviewed and accepted than those that leave |
| 219 it ambiguous. |
| 220 |
| 221 In addition to preferring explicit lifetimes, such as through judicious use of |
| 222 `std::unique_ptr<>` to indicate ownership transfer of dependencies, many |
| 223 features in `//net` also expect that if a `base::Callback` is involved (which |
| 224 includes `net::CompletionCallback`), then it's possible that invoking the |
| 225 callback may result in the deletion of the current (calling) object. As |
| 226 further expanded upon in [Code Patterns](code-patterns.md), features and |
| 227 changes should be designed such that any callback invocation is the last |
| 228 bit of code executed, and that the callback is accessed via the stack (such |
| 229 as through the use of either `base::ResetAndReturn(callback_).Run()` or |
| 230 `std::move(callback_).Run()`. |
| 231 |
| 232 ### Specs: What Are They Good For |
| 233 |
| 234 As `//net` is used as the basis for a number of browsers, it's an important part |
| 235 of the design philosophy to ensure behaviors are well-specified, and that the |
| 236 implementation conforms to those specifications. This may be seen as burdensome |
| 237 when it's unclear whether or not a feature will 'take off,' but it's equally |
| 238 critical to ensure that the Chromium projects do not fork the Web Platform. |
| 239 |
| 240 #### Incubation Is Required |
| 241 |
| 242 `//net` respects Chromium's overall position of [incubation first](https://group
s.google.com/a/chromium.org/d/msg/blink-dev/PJ_E04kcFb8/baiLN3DTBgAJ) standards
development. |
| 243 |
| 244 With an incubation first approach, before introducing any new features that |
| 245 might be exposed over the wire to servers, whether they are explicit behaviors, |
| 246 such as adding new headers, or implicit behaviors such as |
| 247 [Happy Eyeballs](https://tools.ietf.org/html/rfc6555), should have some form |
| 248 of specification written. That specification should at least be on an |
| 249 incubation track, and the expectation is that the specification should have a |
| 250 direct path to an appropriate standards track. Features which don't adhere to |
| 251 this pattern, or which are not making progress towards a standards track, will |
| 252 require high-level approvals, to ensure that the Platform doesn't fragment. |
| 253 |
| 254 #### Introducing New Headers |
| 255 |
| 256 A common form of feature request is the introduction of new headers, either via |
| 257 the `//net` implementation directly, or through consuming `//net` interfaces |
| 258 and modifying headers on the fly. |
| 259 |
| 260 The introduction of any additional headers SHOULD have an incubated spec attache
d, |
| 261 ideally with cross-vendor interest. Particularly, headers which only apply to |
| 262 Google or Google services are very likely to be rejected outright. |
| 263 |
| 264 #### Making Additional Requests |
| 265 |
| 266 While it's necessary to provide abstraction around `//net/url_request` for |
| 267 any lower-level components that may need to make additional requests, for most |
| 268 features, that's not all that is necessary. Because `//net/url_request` only |
| 269 provides a basic HTTP fetching mechanism, it's insufficient for any Web Platform |
| 270 feature, because it doesn't consider the broader platform concerns such as |
| 271 interactions with CORS, Service Workers, cookie and authentication policies, or |
| 272 even basic interactions with optional features like Extensions or SafeBrowsing. |
| 273 |
| 274 To account for all of these things, any resource fetching that is to support |
| 275 a feature of the Web Platform, whether because the resource will be directly |
| 276 exposed to web content (for example, an image load or prefetch) or because it |
| 277 is in response to invoking a Web Platform API (for example, invoking the |
| 278 credential management API), the feature's resource fetching should be |
| 279 explainable in terms of the [Fetch Living Standard](https://fetch.spec.whatwg.o
rg/). |
| 280 The Fetch standard defines a JavaScript API for fetching resources, but more |
| 281 importantly, defines a common set of infrastructure and terminology that |
| 282 tries to define how all resource loads in the Web Platform happen - whether |
| 283 it be through the JavaScript API, through `XMLHttpRequest`, or the `src` |
| 284 attribute in HTML tags, for example. |
| 285 |
| 286 This also includes any resource fetching that wishes to use the same socket |
| 287 pools or caches as the Web Platform, to ensure that every resource that is |
| 288 web exposed (directly or indirectly) is fetched in a consistent and |
| 289 well-documented way, thus minimizing platform fragmentation and security |
| 290 issues. |
| 291 |
| 292 There are exceptions to this, however, but they're generally few and far |
| 293 between. In general, any feature that needs to define an abstraction to |
| 294 allow it to "fetch resources," likely needs to also be "explained in terms |
| 295 of Fetch". |
| 296 |
| 297 ## Implementation |
| 298 |
| 299 In general, prior to implementing, try to get a review on net-dev@chromium.org |
| 300 for the general feedback and design review. |
| 301 |
| 302 In addition to the net-dev@chromium.org early review, `//net` requires that any |
| 303 browser-exposed behavior should also adhere to the |
| 304 [Blink Process](https://www.chromium.org/blink#new-features), which includes an |
| 305 "Intent to Implement" message to blink-dev@chromium.org |
| 306 |
| 307 For features that are unclear about their future, such as experiments or trials, |
| 308 it's also expected that the design planning will also account for how features |
| 309 will be removed cleanly. For features that radically affect the architecture of |
| 310 `//net`, expect a high bar of justification, since reversing those changes if |
| 311 they fail to pan out can cause significant disruption to productivity and |
| 312 stability. |
| 313 |
| 314 ## Deprecation |
| 315 |
| 316 Plan for obsolence, hope for success. Similar to implementation, features that |
| 317 are to be removed should also go through the |
| 318 [Blink Process](https://www.chromium.org/blink#TOC-Web-Platform-Changes:-Process
) |
| 319 for removing features. |
| 320 |
| 321 Note that due to the diversity of [Supported Projects](supported-projects.md), |
| 322 removing a feature while minimizing disruption can be just as complex as adding |
| 323 a feature. For example, relying solely on __User Metrics (UMA)__ to signal the |
| 324 safety of removing a feature may not consider all projects, and relying on |
| 325 __Field Trials (Finch)__ to assess risk or restore the 'legacy' behavior may not |
| 326 work on all projects either. |
| 327 |
| 328 It's precisely because of these challenges that there's such a high bar for |
| 329 adding features, because they may represent multi-year commitments to support, |
| 330 even when the feature itself is deprecated or targeted for removal. |
OLD | NEW |