Brett Wilson | a5a898f1 | 2024-01-25 17:03:38 | [diff] [blame] | 1 | # The architecture of Browser/ChromeOS code |
| 2 | |
| 3 | ## Overview |
| 4 | |
| 5 | We want to have clean code, good architecture, and clear ownership to ensure |
| 6 | everybody can efficiently deliver high quality products. Toward this goal, this |
| 7 | document discusses how code in the Chromium git repository is and should be |
| 8 | architected between the browser and ChromeOS code. |
| 9 | |
| 10 | ## Background |
| 11 | |
| 12 | Originally, ChromeOS was just the Linux Chrome browser with a few extra |
| 13 | additions for UI system management. As such, and to keep the system requirements |
Kyle Horimoto | 1b722865 | 2024-09-17 02:58:10 | [diff] [blame] | 14 | very low, the entire ChromeOS UI was built into the Chrome "browser" process. |
Brett Wilson | a5a898f1 | 2024-01-25 17:03:38 | [diff] [blame] | 15 | Over time, ChromeOS has gotten substantially more sophisticated and capable. |
| 16 | Many important services run in separate processes, services, or VMs, but most of |
| 17 | the UI still runs in the main browser process. |
| 18 | |
Kyle Horimoto | 1b722865 | 2024-09-17 02:58:10 | [diff] [blame] | 19 | The Lacros project aimed to separate the Linux processes and the software |
| 20 | releases between the browser and the OS shell by shipping the Chrome web browser |
| 21 | as a standalone app which could be updated independently of ChromeOS. Lacros |
| 22 | communicated between the Chrome browser and ChromeOS via an IPC interface called |
| 23 | crosapi. However, the Lacros project has been deprioritized and relevant code is |
| 24 | in the process of being deprecated and deleted. |
Brett Wilson | a5a898f1 | 2024-01-25 17:03:38 | [diff] [blame] | 25 | |
| 26 | ### Definitions |
| 27 | |
| 28 | - **Browser:** General term referring to a process with web browsing capabilities. |
| 29 | |
| 30 | - **Ash:** The ChromeOS system UI. In this document, this term is used broadly |
| 31 | to include most of the non-browser UI features including the app launcher, the |
| 32 | system tray and notifications, the window manager, the system compositor, and |
| 33 | the login UI. |
| 34 | |
Kyle Horimoto | 1b722865 | 2024-09-17 02:58:10 | [diff] [blame] | 35 | - **Lacros (deprecate, being deleted):** The ChromeOS-specific browser that does |
| 36 | not include Ash. This is similar to the Linux browser but with ChromeOS- |
| 37 | specific features and integrations. |
Brett Wilson | a5a898f1 | 2024-01-25 17:03:38 | [diff] [blame] | 38 | |
Kyle Horimoto | 1b722865 | 2024-09-17 02:58:10 | [diff] [blame] | 39 | - **Ash Browser:** The "classic" (non-Lacros) ChromeOS software that includes |
Brett Wilson | a5a898f1 | 2024-01-25 17:03:38 | [diff] [blame] | 40 | Ash and the browser in one process. |
| 41 | |
| 42 | - **Browser code:** Code required to build a browser. This includes |
| 43 | platform-specific integrations with the host OS rather than just the |
| 44 | cross-platform parts. For ChromeOS, this includes many important ChromeOS |
Kyle Horimoto | 1b722865 | 2024-09-17 02:58:10 | [diff] [blame] | 45 | browser features but does not include anything considered "Ash." |
Brett Wilson | a5a898f1 | 2024-01-25 17:03:38 | [diff] [blame] | 46 | |
Kyle Horimoto | 1b722865 | 2024-09-17 02:58:10 | [diff] [blame] | 47 | - **OS code:** Any ChromeOS-specific code that isn’t "browser code." This is |
Brett Wilson | a5a898f1 | 2024-01-25 17:03:38 | [diff] [blame] | 48 | mostly Ash when referring to code in the Chromium repository. |
| 49 | |
dljames | 5167ecad | 2024-11-27 01:00:28 | [diff] [blame] | 50 | - **Shared code:** Code used in both browser and OS code including `//base`, |
| 51 | `//mojo`, `//ui`, and some components. |
Brett Wilson | a5a898f1 | 2024-01-25 17:03:38 | [diff] [blame] | 52 | |
| 53 | ## Desired state |
| 54 | |
| 55 | _This section describes the long-term architectural goal rather than the current |
| 56 | state or the current requirements. See below for what to do for current work._ |
| 57 | |
Kyle Horimoto | 1b722865 | 2024-09-17 02:58:10 | [diff] [blame] | 58 | The desired end-state is that "browser code" (including ChromeOS-specific |
| 59 | browser features) and "OS code" have a clear separation. Communication between |
Brett Wilson | a5a898f1 | 2024-01-25 17:03:38 | [diff] [blame] | 60 | these layers should be done using well-defined APIs. Function calls in the code |
Kyle Horimoto | 1b722865 | 2024-09-17 02:58:10 | [diff] [blame] | 61 | happen "down" the stack from the browser to the OS, and any calls "up" from the |
Brett Wilson | a5a898f1 | 2024-01-25 17:03:38 | [diff] [blame] | 62 | OS to the browser happen via events, observers, and callbacks configured by the |
| 63 | browser layers. |
| 64 | |
dljames | 5167ecad | 2024-11-27 01:00:28 | [diff] [blame] | 65 | Shared code like `//views` may have ChromeOS-specific parts and take |
| 66 | contributions from anyone, but the Browser and OS teams should agree that the |
| 67 | code is appropriate for such sharing. |
Brett Wilson | a5a898f1 | 2024-01-25 17:03:38 | [diff] [blame] | 68 | |
| 69 | In this desired state: |
| 70 | |
dljames | 5167ecad | 2024-11-27 01:00:28 | [diff] [blame] | 71 | - The `//chrome` directory is for the implementation of the Chrome browser. It |
| 72 | should not have any OS code in it (for example, `//chrome/browser/ash` is |
| 73 | undesirable) and OS code should not call directly into `//chrome` code outside |
Kyle Horimoto | 1b722865 | 2024-09-17 02:58:10 | [diff] [blame] | 74 | of the above-mentioned callbacks. |
Brett Wilson | a5a898f1 | 2024-01-25 17:03:38 | [diff] [blame] | 75 | |
dljames | 5167ecad | 2024-11-27 01:00:28 | [diff] [blame] | 76 | - The `//content` directory is the API for building a web browser. Even though |
| 77 | Ash does use web technology for rendering many things, it is not itself a web |
Brett Wilson | a5a898f1 | 2024-01-25 17:03:38 | [diff] [blame] | 78 | browser and there should be no OS code in this directory or calling directly |
| 79 | into it. |
| 80 | |
Kyle Horimoto | 1b722865 | 2024-09-17 02:58:10 | [diff] [blame] | 81 | - Browser code should only call into OS code through well-defined APIs (e.g., |
| 82 | extension APIs). This provides a conceptual separation between browser and OS |
| 83 | concerns. |
Brett Wilson | a5a898f1 | 2024-01-25 17:03:38 | [diff] [blame] | 84 | |
| 85 | Not all parts of the product fit neatly into the browser and OS layers, with |
| 86 | extensions and apps being big examples. How web page embedding should be done |
| 87 | from Ash is an area of active design and there is not currently good guidance |
| 88 | for this. In these less well-defined areas, work toward as clear a separation as |
| 89 | practical given the current state and the long-term requirements of that |
| 90 | component. |
| 91 | |
| 92 | ## Current policies |
| 93 | |
dljames | 5167ecad | 2024-11-27 01:00:28 | [diff] [blame] | 94 | New features should be designed to adhere to the "desired state" as closely as |
Kyle Horimoto | f9aea4e | 2024-05-13 16:51:41 | [diff] [blame] | 95 | practical. However, it is currently not possible to implement all functionality |
| 96 | in Ash according to that state: |
Brett Wilson | a5a898f1 | 2024-01-25 17:03:38 | [diff] [blame] | 97 | |
Darren Shen | 0e1f621 | 2024-12-16 22:12:03 | [diff] [blame] | 98 | - Some functionality (e.g., the `Profile` or `BrowserContext` classes) are only |
| 99 | available in `//chrome` or `//content`, and there are no clear alternatives |
| 100 | to use. |
Brett Wilson | a5a898f1 | 2024-01-25 17:03:38 | [diff] [blame] | 101 | |
dljames | 5167ecad | 2024-11-27 01:00:28 | [diff] [blame] | 102 | - Legacy code still has significant `//chrome` dependencies and has not been |
Kyle Horimoto | f9aea4e | 2024-05-13 16:51:41 | [diff] [blame] | 103 | migrated away from this state. |
Brett Wilson | a5a898f1 | 2024-01-25 17:03:38 | [diff] [blame] | 104 | |
Kyle Horimoto | f9aea4e | 2024-05-13 16:51:41 | [diff] [blame] | 105 | Thus, we must be pragmatic about implementing Ash features in the meantime, |
| 106 | using the following guidance: |
Brett Wilson | a5a898f1 | 2024-01-25 17:03:38 | [diff] [blame] | 107 | |
Kyle Horimoto | f9aea4e | 2024-05-13 16:51:41 | [diff] [blame] | 108 | - Any new Ash functionality should add its core functionality outside of |
dljames | 5167ecad | 2024-11-27 01:00:28 | [diff] [blame] | 109 | `//chrome`. |
Kyle Horimoto | f9aea4e | 2024-05-13 16:51:41 | [diff] [blame] | 110 | - In this context, "core" functionality includes the primary business logic of |
| 111 | a feature. |
| 112 | - Guidance on where this code should exist: |
dljames | 5167ecad | 2024-11-27 01:00:28 | [diff] [blame] | 113 | - **Ash-only code which is not system UI:** `//chromeos/ash/components` |
| 114 | - **Ash-only system UI code:** `//ash` |
Kyle Horimoto | f9aea4e | 2024-05-13 16:51:41 | [diff] [blame] | 115 | - **Shared by both Ash and Lacros:** |
dljames | 5167ecad | 2024-11-27 01:00:28 | [diff] [blame] | 116 | - *UI code:* `//chromeos/ui` |
| 117 | - *Non-UI code:* `//chromeos/components` |
Kyle Horimoto | 1b722865 | 2024-09-17 02:58:10 | [diff] [blame] | 118 | - **NOTE:** Lacros is in the process of being deprecated. Do not add any |
| 119 | new Lacros code. |
| 120 | - **Shared between ChromeOS (i.e., ash-chrome) and other platforms:** |
dljames | 5167ecad | 2024-11-27 01:00:28 | [diff] [blame] | 121 | `//components` |
Brett Wilson | a5a898f1 | 2024-01-25 17:03:38 | [diff] [blame] | 122 | |
dljames | 5167ecad | 2024-11-27 01:00:28 | [diff] [blame] | 123 | - For code which must depend on `//chrome`, push logic down lower in the |
Kyle Horimoto | f9aea4e | 2024-05-13 16:51:41 | [diff] [blame] | 124 | dependency graph as much as possible, and only implement a thin wrapper in |
dljames | 5167ecad | 2024-11-27 01:00:28 | [diff] [blame] | 125 | `//chrome`. With this pattern, the code in `//chrome` is mostly "glue" or |
Kyle Horimoto | f9aea4e | 2024-05-13 16:51:41 | [diff] [blame] | 126 | initialization code, which will minimize the effort required in the future to |
| 127 | break these dependencies completely. |
| 128 | - Example 1: Phone Hub's [`BrowserTabsModelProvider`](https://source.chromium.org/chromium/chromium/src/+/main:chromeos/ash/components/phonehub/browser_tabs_model_provider.h;drc=2a153c1bc9f24cae375eee3cc875903866997918) |
dljames | 5167ecad | 2024-11-27 01:00:28 | [diff] [blame] | 129 | is declared in `//chromeos/ash/components` alongside related code logic, but |
Kyle Horimoto | f9aea4e | 2024-05-13 16:51:41 | [diff] [blame] | 130 | [`BrowserTabsModelProviderImpl`](https://source.chromium.org/chromium/chromium/src/+/main:chrome/browser/ash/phonehub/browser_tabs_model_provider_impl.h;drc=fe132eeb21687c455d695d6af346f15454828d01) |
dljames | 5167ecad | 2024-11-27 01:00:28 | [diff] [blame] | 131 | (in `//chrome`) implements the interface using a `//chrome` dependency. |
Kyle Horimoto | f9aea4e | 2024-05-13 16:51:41 | [diff] [blame] | 132 | - Example 2: Phone Hub's [`PhoneHubManagerImpl`](https://source.chromium.org/chromium/chromium/src/+/main:chromeos/ash/components/phonehub/phone_hub_manager_impl.h;drc=6b2b6f5aa258a1616fab24634c4e9477cfef5daf) |
dljames | 5167ecad | 2024-11-27 01:00:28 | [diff] [blame] | 133 | is declared in `//chromeos/ash/components` and has dependencies outside of |
| 134 | `//chrome`, but the concrete implementations of some of these components are |
Kyle Horimoto | f9aea4e | 2024-05-13 16:51:41 | [diff] [blame] | 135 | [`KeyedService`](https://source.chromium.org/chromium/chromium/src/+/main:components/keyed_service/core/keyed_service.h;drc=d23075f3066f6aab6fd5f8446ea5dde3ebff1097)s |
dljames | 5167ecad | 2024-11-27 01:00:28 | [diff] [blame] | 136 | requiring `//chrome`. In this case, [`PhoneHubManagerFactory`](https://source.chromium.org/chromium/chromium/src/+/main:chrome/browser/ash/phonehub/phone_hub_manager_factory.h;drc=d23075f3066f6aab6fd5f8446ea5dde3ebff1097) |
Kyle Horimoto | f9aea4e | 2024-05-13 16:51:41 | [diff] [blame] | 137 | instantiates [`PhoneHubManagerImpl`](https://source.chromium.org/chromium/chromium/src/+/main:chromeos/ash/components/phonehub/phone_hub_manager_impl.h;drc=6b2b6f5aa258a1616fab24634c4e9477cfef5daf) |
dljames | 5167ecad | 2024-11-27 01:00:28 | [diff] [blame] | 138 | in `//chrome` (serving as a thin wrapper around the dependencies), but the |
Kyle Horimoto | f9aea4e | 2024-05-13 16:51:41 | [diff] [blame] | 139 | vast majority of logic is lower in the dependency graph. |
| 140 | |
dljames | 5167ecad | 2024-11-27 01:00:28 | [diff] [blame] | 141 | - A few common `//chrome` dependencies that may be able to be broken easily: |
Kyle Horimoto | f9aea4e | 2024-05-13 16:51:41 | [diff] [blame] | 142 | - Instead of using [`ProfileKeyedServiceFactory`](https://source.chromium.org/chromium/chromium/src/+/main:chrome/browser/profiles/profile_keyed_service_factory.h;drc=77a7a02b1822640e35cac72c0ddd7af7275eeb9b) |
dljames | 5167ecad | 2024-11-27 01:00:28 | [diff] [blame] | 143 | (in `//chrome`), consider using [`BrowserContextKeyedServiceFactory`](https://source.chromium.org/chromium/chromium/src/+/main:components/keyed_service/content/browser_context_keyed_service_factory.h;drc=371515598109bf869e1acbe5ea67813fc1a4cc3d) |
| 144 | (in `//components`) instead. |
Kyle Horimoto | f9aea4e | 2024-05-13 16:51:41 | [diff] [blame] | 145 | - Instead of using a [`Profile`](https://source.chromium.org/chromium/chromium/src/+/main:chrome/browser/profiles/profile.h;l=308-311;drc=3f4203f7dca2f7e804f30cfa783e24f90acd9059) |
dljames | 5167ecad | 2024-11-27 01:00:28 | [diff] [blame] | 146 | (in `//chrome`) to access user prefs, consider using |
Kyle Horimoto | f9aea4e | 2024-05-13 16:51:41 | [diff] [blame] | 147 | [`User::GetProfilePrefs()`](https://source.chromium.org/chromium/chromium/src/+/main:components/user_manager/user.h;l=127-131;drc=e49b1aec9585b0a527c24502dd4b0ee94b142c3c) |
dljames | 5167ecad | 2024-11-27 01:00:28 | [diff] [blame] | 148 | (in `//components`) instead. |
Kyle Horimoto | f9aea4e | 2024-05-13 16:51:41 | [diff] [blame] | 149 | |
dljames | 5167ecad | 2024-11-27 01:00:28 | [diff] [blame] | 150 | - For any new code added in `//chrome/browser/ash`, a `DEPS` file must be created |
| 151 | which explicitly declares `//chrome` dependencies. People residing in |
| 152 | `//chrome/OWNERS` can help suggest alternatives to these dependencies if |
| 153 | possible when reviewing the code which adds this new `DEPS` file. See |
Kyle Horimoto | f9aea4e | 2024-05-13 16:51:41 | [diff] [blame] | 154 | [b/332805865](http://b/332805865) for more details. |
| 155 | |
Kyle Horimoto | f9aea4e | 2024-05-13 16:51:41 | [diff] [blame] | 156 | If you need advice to help you make a decision regarding your design, please |
| 157 | reach out to ash-chrome-refactor@google.com for feedback. |
| 158 | |
| 159 | ## Path forward |
Brett Wilson | a5a898f1 | 2024-01-25 17:03:38 | [diff] [blame] | 160 | |
| 161 | The current policy aims to stop accumulating more undesirable OS/browser |
| 162 | dependencies while acknowledging there is a large amount of legacy code that |
Kyle Horimoto | f9aea4e | 2024-05-13 16:51:41 | [diff] [blame] | 163 | does not follow the guidelines. The team is moving toward the desired state |
Kyle Horimoto | 1b722865 | 2024-09-17 02:58:10 | [diff] [blame] | 164 | using the following approach outlined in go/ash-chrome-refactor: |
Brett Wilson | a5a898f1 | 2024-01-25 17:03:38 | [diff] [blame] | 165 | |
dljames | 5167ecad | 2024-11-27 01:00:28 | [diff] [blame] | 166 | - Ensure that all Ash code in `//chrome` is defined using granular BUILD.gn |
| 167 | files. Each directory should define its own build targets and list dependencies |
Kyle Horimoto | 1b722865 | 2024-09-17 02:58:10 | [diff] [blame] | 168 | explicitly. |
| 169 | - See https://crbug.com/335314438, https://crbug.com/351889236. |
| 170 | - ...more to come as the Ash //chrome Refactor makes progress. |