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 ordered. To be ensure these requirements | |
Randy Smith (Not in Mondays)
2017/02/21 17:01:04
nit: I don't understand what "defined sequencing t
| |
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 |