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

Side by Side Diff: net/docs/life-of-a-feature.md

Issue 2700673002: [net] Add documentation for feature process & supported platforms (Closed)
Patch Set: Created 3 years, 10 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View unified diff | Download patch
« no previous file with comments | « no previous file | net/docs/supported-projects.md » ('j') | net/docs/supported-projects.md » ('J')
Toggle Intra-line Diffs ('i') | Expand Comments ('e') | Collapse Comments ('c') | Show Comments Hide Comments ('s')
OLDNEW
(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 accomodating 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 behaviours 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` behaviour, 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 behaviour, 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 behaviours 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 targetted 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 it (perhaps
87 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 While the ideal form of injection is to pass ownership of the injected
99 object, such as via a `std::unique_ptr<Foo>`, there are a number of places
100 in `//net` in which ownership is shared / left to the embedder, and the
101 injected object is passed around as a naked/raw pointer.
102
103 #### Delegation
104
105 Delegation refers to forcing the `//net` embedder to respond to specific
106 delegated calls via a Delegate interface that it implements. In general,
107 when using the delegated pattern, ownership of the delegate should be
108 transferred to the thing consuming the delegate, so that the lifetime and
Charlie Harrison 2017/02/16 18:33:11 Can you be more specific about the "thing consumin
109 threading semantics are always clear.
110
111 The most notable example of the delegate pattern is `URLRequest::Delegate`.
112
113 While the use of a `base::Callback` can also be considered a form of
114 delegation, the `//net` layer tries to eschew any callbacks that can be
Charlie Harrison 2017/02/16 18:33:11 might want to mention e.g. base::OnceCallback if y
Charlie Harrison 2017/02/16 18:33:11 might want to mention e.g. base::OnceCallback if y
115 called more than once, and instead favors defining class interfaces
116 with concrete behavioural requirements in order to ensure the correct
117 lifetimes of objects and to adjust over time.
118
119 ### Understanding the Layering
120
121 A significant challenge many feature proposals face is understanding the
122 layering in `//net` and what different portions of `//net` are allowed to
123 know.
124
125 #### Socket Pools
126
127 The most common challenge feature proposals encounter is the awareness
128 that the act of associating an actual request to make with a socket is
129 done lazily, referred to as "late-binding".
130
131 With late-bound sockets, a given `URLRequest` will not be assigned an actual
132 transport connection until the request is ready to be sent. This allows for
133 reprioritizing requests as they come in, to ensure that higher priority requests
134 get preferential treatment, but it also means that features or data associated
135 with a `URLRequest` generally don't participate in socket establishment or
136 maintenance.
137
138 For example, a feature that wanted to associate the low-level network socket
139 with a `URLRequest` during connection establishment is not something that the
140 `//net` design supports, since the `URLRequest` is kept unaware of how sockets
141 are established by virtue of the socket pools and late binding. This allows for
142 more flexibility when working to improve performance, such as the ability to
143 coalesce multiple logical 'sockets' over a single HTTP/2 or QUIC stream, which
144 may only have a single physical network socket involved.
145
146 #### Making Additional Requests
147
148 From time to time, `//net` feature proposals will involve needing to load
149 a secondary resource as part of processing. For example, SDCH involves loading
150 additional dictionaries that are advertised in a header, and other feature
151 proposals have involved fetching a `/.well-known/` URI or reporting errors to
152 a particular URL.
153
154 This is particularly challenging, because often, these features are implemented
155 deeper in the network stack, such as [`//net/cert`](../cert), [`//net/http`](../ http),
156 or [`//net/filter`](../filter), which [`//net/url_request`](../url_request) depe nds
157 on. Because `//net/url_request` depends on these low-level directories, it would
158 be a circular dependency to have these directories depend on `//net/url_request` ,
159 and circular dependencies are forbidden.
160
161 The recommended solution to address this is to adopt the delegation or injection
162 patterns. The lower-level directory will define some interface that represents t he
163 "I need this URL" request, and then elsewhere, in a directory allowed to depend
164 on `//net/url_request`, an implementation of that interface/delegate that uses
165 `//net/url_request` is implemented.
166
167 ### Specs: What Are They Good For
168
169 As `//net` is used as the basis for a number of browsers, it's an important part
170 of the design philosophy to ensure behaviours are well-specified, and that the
171 implementation conforms to those specifications. This may be seen as burdensome
172 when it's unclear whether or not a feature will 'take off,' but it's equally
173 critical to ensure that the Chromium projects do not fork the Web Platform.
174
175 #### Incubation Is Required
176
177 `//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.
178
179 With an incubation first approach, before introducing any new features that
180 might be exposed over the wire to servers, whether they be explicit behaviours
181 such as adding new headers to implicit behaviours such as
182 [Happy Eyeballs](https://tools.ietf.org/html/rfc6555), should have some form
183 of specification written. That specification should at least be incubation
Charlie Harrison 2017/02/16 18:33:11 This sentence is a bit of a run-on. Suggest slight
184 track, although the expectation is that the specification SHOULD have a direct
185 path towards an appropriate standards track, and moving towards that. Features
186 which don't adhere to this pattern require a number of high-level approvals, to
187 ensure that the Platform doesn't fragment.
188
189 #### Introducing New Headers
190
191 A common form of feature request is the introduction of new headers, either via
192 the `//net` implementation directly, or through consuming `//net` interfaces
193 and modifying headers on the fly.
194
195 The introduction of any additional headers SHOULD have an incubated spec attache d,
196 ideally with cross-vendor interest. Particularly, headers which only apply to
197 Google or Google services are very likely to be rejected outright.
198
199 #### Making Additional Requests
200
201 While it's necessary to provide abstraction around `//net/url_request` for
202 any lower-level components that may need to make additional requests, for most
203 features, that's not all that is necessary. Because `//net/url_request` only
204 provides a basic HTTP fetching mechanism, it's insufficient for any Web Platform
205 feature, because it doesn't consider the broader platform concerns such as
206 interactions with CORS, Service Workers, cookie and authentication policies, or
207 even basic interactions with optional features like Extensions or SafeBrowsing.
208
209 To account for all of these things, any resource fetching that is to support a
210 Web Platform feature (i.e. be exposed in any of the Browser projects), the
Charlie Harrison 2017/02/16 18:33:11 The "i.e." confuses me, and I think it is due to t
211 feature should be explainable in terms of the
212 [Fetch Living Standard](https://fetch.spec.whatwg.org/). The Fetch standard defi nes
213 a JavaScript API for fetching resources, but more importantly, defines a common
214 set of infrastructure and terminology that tries to define how all resource load s
215 in the Web Platform happen - whether it be through the JavaScript API, through
216 `XMLHttpRequest`, or the `src` attribute in HTML tags, for example.
217
218 Thus, in addition to defining a class abstraction to "fetch resources," any feat ure
219 proposal that needs to do so, and will be part of any of the browser projects,
220 should be "explained in terms of Fetch".
221
222 ## Implementation
223
224 In general, prior to implementing, try to get a review on net-dev@chromium.org
225 for the general feedback and design review.
226
227 In addition to the net-dev@chromium.org early review, `//net` requires that any
228 browser-exposed behaviour should also adhere to the
Charlie Harrison 2017/02/16 18:33:11 ditto, I think this should probably be "Web Platfo
229 [Blink Process](https://www.chromium.org/blink#new-features), which includes an
230 "Intent to Implement" message to blink-dev@chromium.org
231
232 For features that are unclear about their future, such as experiments or trials,
233 it's also expected that the design planning will also account for how features
234 will be removed cleanly. For features that radically affect the architecture of
235 `//net`, expect a high bar of justification, since reversing those changes if
236 it fails to pan out can cause significant disruption to productivity and
237 stability.
238
239 ## Deprecation
240
241 Plan for obsolence, hope for success. Similar to implementation, features that
242 are to be removed should also go through the
243 [Blink Process](https://www.chromium.org/blink#TOC-Web-Platform-Changes:-Process )
244 for removing features.
245
246 Note that due to the diversity of [Supported Projects](supported-projects.md),
247 removing a feature while minimizing disruption can be just as complex as adding
248 a feature. For example, relying solely on __User Metrics (UMA)__ to signal the
249 safety of removing a feature may not consider all projects, and relying on
250 __Field Trials (Finch)__ to assess risk or restore the 'legacy' behaviour may no t
251 work on all projects either.
252
253 It's precisely because of these challenges that there's such a high bar for
254 adding features, because they may represent multi-year committments to support,
255 even when the feature itself is deprecated or targetted for removal.
OLDNEW
« no previous file with comments | « no previous file | net/docs/supported-projects.md » ('j') | net/docs/supported-projects.md » ('J')

Powered by Google App Engine
This is Rietveld 408576698