Chromium Code Reviews| OLD | NEW |
|---|---|
| (Empty) | |
| 1 # Servicification Strategies | |
| 2 | |
| 3 This document captures strategies, hints, and best practices for solving typical | |
| 4 challenges enountered when converting existing Chromium | |
| 5 code to services. It is assumed that you have already read the high-level | |
| 6 documentation on [what a service is](/services). | |
| 7 | |
| 8 If you're looking for Mojo documentation, please see the [general | |
| 9 Mojo documentation](/mojo) and/or the [documentation on converting Chrome IPC to | |
| 10 Mojo](/ipc). | |
| 11 | |
| 12 Note that throughout the below document we link to CLs to illustrate the | |
| 13 strategies being made. Over the course of time code tends to shift, so it is | |
| 14 likely that the code on trunk does not exactly match what it was at the time of | |
| 15 the CLs. When necessary, use the CLs as a starting point for examining the | |
| 16 current state of the codebase with respect to these issues (e.g., exactly where | |
| 17 a service is embedded within the content layer). | |
| 18 | |
| 19 [TOC] | |
| 20 | |
| 21 ## Questions to Answer When Getting Started | |
| 22 | |
| 23 For the basic nuts and bolts of how to create a new service, see [the | |
| 24 documentation on adding a new service](/services#Adding-a-new-service). This | |
| 25 section gives questions that you should answer in order to shape the design of | |
| 26 your service, as well as hints as to which answers make sense given your | |
| 27 situation. | |
| 28 | |
| 29 ### Is your service global or per-BrowserContext? | |
| 30 The Service Manager can either: | |
| 31 | |
| 32 - field all connection requests for a given service via the same instance or | |
| 33 - create one service instance per user ID. | |
| 34 | |
| 35 Which of these policies the Service Manager employs is determined by the | |
| 36 contents of your service manifest: the former is the default, while the latter | |
|
Sam McNally
2017/05/01 00:51:03
Did the policy order get swapped?
blundell
2017/05/02 09:00:26
Oops, yup! Thanks for noticing!
| |
| 37 is selected by informing the Service Manager that your service requires the | |
| 38 service_manager:all_users capability([example](https://cs.chromium.org/chromium/ src/services/device/manifest.json)). | |
| 39 | |
| 40 In practice, there is one user ID per-BrowserContext, so the question becomes: | |
| 41 Is your Service a global or keyed by BrowserContext? In considering this | |
| 42 question, there is one obvious hint: If you are converting per-Profile classes | |
| 43 (e.g., KeyedServices), then your service is almost certainly going to be | |
| 44 per-user. More generally, if you envision needing to use *any* state related to | |
| 45 the user (e.g., you need to store files in the user's home directory), then your | |
| 46 service should be per-user. | |
| 47 | |
| 48 Conversely, your service could be a good fit for being global if it is a utility | |
| 49 that is unconcerned with the identity of the requesting client (e.g., the [data | |
| 50 decoder service](/services/data_decoder), which simply decodes untrusted data in | |
| 51 a separate process. | |
| 52 | |
| 53 ### Will you embed your service in //content, //chrome, or neither? | |
| 54 | |
| 55 At the start (and potentially even long-term), your service will likely not | |
| 56 actually run in its own process but will rather be embedded in the browser | |
| 57 process. This is especially true in the common case where you are converting | |
| 58 existing browser-process code. | |
| 59 | |
| 60 You then have a question: Where should it be embedded? | |
| 61 | |
| 62 There are two common choices, //content and //chrome: | |
| 63 | |
| 64 - //content is the default choice. This is where your service should go if | |
|
jam
2017/05/01 15:32:25
nit: shouldn't this be based on where the code was
blundell
2017/05/02 09:00:27
Done.
| |
| 65 the code that you are converting has no //chrome dependencies. Global services | |
| 66 are embedded by [content::ServiceManagerContext](https://cs.chromium.org/chrom ium/src/content/browser/service_manager/service_manager_context.cc?type=cs&q=Cre ateDeviceService), | |
| 67 while per-user services are naturally embedded by [content::BrowserContext](ht tps://cs.chromium.org/chromium/src/content/browser/browser_context.cc?type=cs&q= CreateFileService). | |
| 68 | |
| 69 - If your service is converting existing //chrome code, then you will need | |
| 70 to embed your service in //chrome rather than //content. Global services | |
| 71 are embedded by [ChromeContentBrowserClient](https://cs.chromium.org/chromium/ src/chrome/browser/chrome_content_browser_client.cc?type=cs&q=CreateMediaService ), | |
| 72 while per-user services are embedded by [ProfileImpl](https://cs.chromium.org/ chromium/src/chrome/browser/profiles/profile_impl.cc?type=cs&q=CreateIdentitySer vice). | |
| 73 | |
|
Ken Rockot(use gerrit already)
2017/04/28 18:07:34
PDF compositor[1] demonstrates another option, whe
blundell
2017/05/02 09:00:26
Done.
| |
| 74 ### What is your service's threading model? | |
| 75 | |
| 76 By default, your service will run on the IO thread. You can change that by | |
|
Ken Rockot(use gerrit already)
2017/04/28 18:07:34
This is only true specifically of services which a
blundell
2017/05/02 09:00:26
Thanks, specified.
| |
| 77 specifying a task runner as part of the information for constructing your | |
| 78 service. In particular, if the code that you are converting is UI-thread code, | |
| 79 then you likely want your service running on the UI thread. Look at the changes | |
| 80 to profile_impl.cc in [this CL](https://codereview.chromium.org/2753753007) to | |
| 81 see an example of setting the task runner that a service should be run on as | |
| 82 part of the factory for creating the service. | |
| 83 | |
| 84 ### What is your approach for incremental conversion? | |
| 85 | |
| 86 In creating your service, you likely have two goals: | |
| 87 | |
| 88 - Making the service available to other services | |
| 89 - Making the service self-contained | |
| 90 | |
| 91 Those two goals are not the same, and to some extent are at tension: | |
| 92 | |
| 93 - To satisfy the first, you need to build out the API surface of the service to | |
| 94 a sufficient degree for the anticipated use cases. | |
| 95 | |
| 96 - To satisfy the second, you need to convert all clients of the code that you | |
| 97 are servicifying to instead use the service, and then fold that code into the | |
| 98 internal implementation of the service. | |
| 99 | |
| 100 Whatever your goals, you will need to proceed incrementally if your project is | |
| 101 at all non-trivial (as they basically all are given the nature of the effort). | |
| 102 You should explicitly decide what your approach to incremental bringup and | |
| 103 conversion will be. Here some approaches that have been taken for various | |
| 104 services: | |
| 105 | |
| 106 - Build out your service depending directly on existing code, | |
| 107 convert the clients of that code 1-by-1, and fold the existing code into the | |
| 108 service implementation when complete ([Identity Service](https://docs.google.c om/document/d/1EPLEJTZewjiShBemNP5Zyk3b_9sgdbrZlXn7j1fubW0/edit)). | |
| 109 - Build out the service with new code and make the existing code | |
| 110 into a client library of the service. In that fashion, all consumers of the | |
| 111 existing code get converted transparently ([Preferences Service](https://docs. google.com/document/d/1JU8QUWxMEXWMqgkvFUumKSxr7Z-nfq0YvreSJTkMVmU/edit#heading= h.19gc5b5u3e3x)). | |
| 112 - Build out the new service piece-by-piece by picking a given | |
| 113 bite-size piece of functionality and entirely servicifying that functionality | |
| 114 ([Device Service](https://docs.google.com/document/d/1_1Vt4ShJCiM3fin-leaZx00- FoIPisOr8kwAKsg-Des/edit#heading=h.c3qzrjr1sqn7)). | |
| 115 | |
| 116 These all have tradeoffs: | |
| 117 | |
| 118 - The first lets you incrementally validate your API and implementation, but | |
| 119 leaves the service depending on external code for a long period of time. | |
| 120 - The second can create a self-contained service more quickly, but leaves | |
| 121 all the existing clients in place as potential cleanup work. | |
| 122 - The third ensures that you're being honest as you go, but delays having | |
| 123 the breadth of the service API up and going. | |
| 124 | |
| 125 Which makes sense depends both on the nature of the existing code and on | |
| 126 the priorities for doing the servicification. The first two enable making the | |
| 127 service available for new use cases sooner at the cost of leaving legacy code in | |
| 128 place longer, while the last is most suitable when you want to be very exacting | |
| 129 about doing the servicification cleanly as you go. | |
| 130 | |
| 131 ## Platform-Specific Issues | |
| 132 | |
| 133 ### Android | |
| 134 As you servicify code running on Android, you might find that you need to port | |
| 135 interfaces that are served in Java. Here is an [example CL](https://codereview.c hromium.org/2643713002) that gives a basic | |
| 136 pattern to follow in doing this. | |
| 137 | |
| 138 You also might need to register JNI in your service. That is simple to set | |
| 139 up, as illustrated in [this CL](https://codereview.chromium.org/2690963002). | |
| 140 (Note that that CL is doing more than *just* enabling the Device Service to | |
| 141 register JNI; you should take the register_jni.cc file added there as your | |
| 142 starting point to examine the pattern to follow). | |
| 143 | |
| 144 Finally, it is possible that your feature will have coupling to UI process state | |
| 145 (e.g., the Activity) via Android system APIs. To handle this challenging | |
| 146 issue, see the section on [Coupling to UI](#Coupling-to-UI). | |
| 147 | |
| 148 ### iOS | |
| 149 The high-level [servicification design doc](https://docs.google.com/document/d/1 5I7sQyQo6zsqXVNAlVd520tdGaS8FCicZHrN0yRu-oU/edit) gives the motivations and | |
| 150 vision for supporting services on iOS (in particular, search for the mentions | |
| 151 of iOS within the doc). | |
| 152 | |
| 153 Services are not *yet* supported, but this support is being actively being | |
| 154 worked on; see [this bug](crbug.com/705982) for the current status. If you have | |
| 155 a use case or need for services on iOS, contact blundell@chromium.org. | |
| 156 | |
| 157 ## Client-Specific Issues | |
| 158 | |
| 159 ### Services and Blink | |
| 160 Connecting to services directly from Blink is fully supported. [This | |
| 161 CL](https://codereview.chromium.org/2698083007) gives a basic example of | |
| 162 connecting to an arbitrary service by name from Blink (look at the change to | |
| 163 SensorProviderProxy.cpp as a starting point). | |
| 164 | |
| 165 Below, we go through strategies for some common challenges encountered when | |
| 166 servicifying features that have Blink as a client. | |
| 167 | |
| 168 #### Mocking Interface Impls in JS | |
| 169 It is a common pattern in Blink's layout tests to mock a remote Mojo interface | |
| 170 in JS. [This CL](https://codereview.chromium.org/2643713002) illustrates the | |
| 171 basic pattern for porting such mocking of an interface hosted by | |
| 172 //content/browser to an interface hosted by an arbitrary service (see the | |
| 173 changes to mock-battery-monitor.js). | |
| 174 | |
| 175 #### Feature Impls That Depend on Blink Headers | |
| 176 In the course of servicifying a feature that has Blink as a client, you might | |
| 177 encounter cases where the feature implementation has dependencies on Blink | |
| 178 public headers (e.g., defining POD structs that are used both by the client and | |
| 179 by the feature implementation). These dependencies pose a challenge: | |
| 180 | |
| 181 - Services should not depend on Blink, as this is a dependency inversion (Blink | |
| 182 is a client of services). | |
| 183 - However, Blink is very careful about accepting dependencies from Chromium. | |
| 184 | |
| 185 To meet this challenge, you have two options: | |
| 186 | |
| 187 1. Move the code in question from C++ to mojom (e.g., if it is simple structs). | |
| 188 2. Move the code into the service's C++ client library, being very explicit | |
| 189 about its usage by Blink. See [this CL](https://codereview.chromium.org/24150 83002) for a basic pattern to follow. | |
| 190 | |
| 191 #### Frame-Scoped Connections | |
| 192 You must think carefully about the scoping of the connection being made | |
| 193 from Blink. In particular, some feature requests are necessarily scoped to a | |
| 194 frame in the context of Blink (e.g., geolocation, where permission to access the | |
| 195 interface is origin-scoped). Servicifying these features is then challenging, as | |
| 196 Blink has no frame-scoped connection to arbitrary services (by design, as | |
| 197 arbitrary services have no knowledge of frames or even a notion of what a frame | |
| 198 is). | |
| 199 | |
| 200 After a [long | |
| 201 discussion](https://groups.google.com/a/chromium.org/forum/#!topic/services-dev/ CSnDUjthAuw), | |
| 202 the policy that we have adopted for this challenge is the following: | |
| 203 | |
| 204 CURRENT | |
| 205 | |
| 206 - The renderer makes a request through its frame-scoped connection to the | |
| 207 browser. | |
| 208 - The browser obtains the necessary permissions before directly servicing the | |
| 209 request. | |
| 210 | |
| 211 AFTER SERVICIFYING THE FEATURE IN QUESTION | |
| 212 | |
| 213 - The renderer makes a request through its frame-scoped connection to the | |
| 214 browser. | |
| 215 - The browser obtains the necessary permissions before forwarding the | |
| 216 request on to the underlying service that hosts the feature. | |
| 217 | |
| 218 Notably, from the renderer's POV essentially nothing changes here. | |
| 219 | |
| 220 In the longer term, this will still be the basic model, only with "the browser" | |
| 221 replaced by "the Navigation Service" or "the web permissions broker". | |
| 222 | |
| 223 ## Strategies for Challenges to Decoupling from //content | |
| 224 | |
| 225 ### Coupling to UI | |
| 226 | |
| 227 Some feature implementations have hard constraints on coupling to UI on various | |
| 228 platforms. An example is NFC on Android, which requires the Activity of the view | |
| 229 in which the requesting client is hosted in order to access the NFC platform | |
| 230 APIs. This coupling is at odds with the vision of servicification, which is to | |
| 231 make the service physically isolatable. However, when it occurs, we need to | |
| 232 accommodate it. | |
| 233 | |
| 234 The high-level decision that we have reached is to scope the coupling to the | |
| 235 feature *and* platform in question (rather than e.g. introducing a | |
| 236 general-purpose FooServiceDelegate), in order to make it completely explicit | |
| 237 what requires the coupling and to avoid the coupling creeping in scope. | |
| 238 | |
| 239 The basic strategy to support this coupling while still servicifying the feature | |
| 240 in question is to inject a mechanism of mapping from an opaque "context ID" to | |
| 241 the required context. The embedder (e.g., //content) maintains this map, and the | |
| 242 service makes use of it. The embedder also serves as an intermediary: It | |
| 243 provides a connection that is appropriately context-scoped to clients. When | |
| 244 clients request the feature in question, the embedder forwards the request on | |
| 245 along with the appropriate context ID. The service impl can then map that | |
| 246 context ID back to the needed context on-demand using the mapping functionality | |
| 247 injected into the service impl. | |
| 248 | |
| 249 To make this more concrete, see [this CL](https://codereview.chromium.org/273494 3003). | |
| 250 | |
| 251 ### Shutdown of singletons | |
| 252 | |
| 253 You might find that your feature includes singletons that are shut down as part | |
| 254 of //content's shutdown process. As part of decoupling the feature | |
| 255 implementation entirely from //content, the shutdown of these singletons must be | |
| 256 either ported into your service or eliminated: | |
| 257 | |
| 258 - In general, as Chromium is moving away from graceful shutdown, the first | |
| 259 question to analyze is: Do the singletons actually need to be shut down at | |
| 260 all? | |
| 261 - If you need to preserve shutdown of the singleton, the naive approach is to | |
| 262 move the shutdown of the singleton to the destructor of your service | |
| 263 - However, you should carefully examine when your service is destroyed compared | |
| 264 to when the previous code was executing, and ensure that any differences | |
| 265 introduced do not impact correctness. | |
| 266 | |
| 267 See [this thread](https://groups.google.com/a/chromium.org/forum/#!topic/service s-dev/Y9FKZf9n1ls) for more discussion of this issue. | |
| 268 | |
| 269 ### Tests that muck with service internals | |
| 270 It is often the case that browsertests reach directly into what will become part | |
| 271 of the internal service implementation to either inject mock/fake state or to | |
| 272 monitor private state. | |
| 273 | |
| 274 This poses a challenge: As part of servicification, *no* code outside the | |
| 275 service impl should depend on the service impl. Thus, these dependencies need to | |
| 276 be removed. The question is how to do so while preserving testing coverage. | |
| 277 | |
| 278 To answer this question, there are several different strategies. These | |
| 279 strategies are not mutually-exclusive; they can and should be combined to | |
| 280 preserve the full breadth of coverage. | |
| 281 | |
| 282 - Blink client-side behavior can be tested via [layout tests](https://codereview .chromium.org/2731953003) | |
| 283 - To test service impl behavior, create [service tests](https://codereview.chrom ium.org/2774783003). | |
| 284 - To preserve tests of end-to-end behavior (e.g., that when Blink makes a | |
| 285 request via a Web API in JS, the relevant feature impl receives a connection | |
| 286 request), we are planning on introducing the ability to register mock | |
| 287 implementations with the Service Manager. | |
| 288 | |
| 289 To emphasize one very important point: it is in general necessary to leave | |
| 290 *some* test of end-to-end functionality, as otherwise it is too easy for bustage | |
| 291 to slip in via e.g. changes to how services are registered. See [this thread](ht tps://groups.google.com/a/chromium.org/forum/#!topic/services-dev/lJCKAElWz-E) | |
| 292 for further discussion of this point. | |
| OLD | NEW |