Daniel Cheng | 7ecc1d6 | 2017-06-12 20:24:30 | [diff] [blame] | 1 | # Blink C++ Style Guide |
| 2 | |
Peter Kasting | 777e130 | 2020-06-02 21:44:40 | [diff] [blame] | 3 | This document is a list of differences from the overall |
| 4 | [Chromium Style Guide](c++.md), which is in turn a set of differences from the |
| 5 | [Google C++ Style Guide](https://google.github.io/styleguide/cppguide.html). The |
Daniel Cheng | 7ecc1d6 | 2017-06-12 20:24:30 | [diff] [blame] | 6 | long-term goal is to make both Chromium and Blink style more similar to Google |
| 7 | style over time, so this document aims to highlight areas where Blink style |
Peter Kasting | 777e130 | 2020-06-02 21:44:40 | [diff] [blame] | 8 | differs from Chromium style. |
Daniel Cheng | 7ecc1d6 | 2017-06-12 20:24:30 | [diff] [blame] | 9 | |
| 10 | [TOC] |
| 11 | |
Dmitry Gozman | ca984d7 | 2018-06-21 22:47:25 | [diff] [blame] | 12 | ## Prefer WTF types over STL and base types |
Daniel Cheng | 01b0ec51 | 2018-03-13 20:50:42 | [diff] [blame] | 13 | |
Dmitry Gozman | ca984d7 | 2018-06-21 22:47:25 | [diff] [blame] | 14 | See [Blink readme](../../third_party/blink/renderer/README.md#Type-dependencies) |
| 15 | for more details on Blink directories and their type usage. |
Daniel Cheng | 01b0ec51 | 2018-03-13 20:50:42 | [diff] [blame] | 16 | |
| 17 | **Good:** |
| 18 | ```c++ |
| 19 | String title; |
| 20 | Vector<KURL> urls; |
| 21 | HashMap<int, Deque<RefPtr<SecurityOrigin>>> origins; |
| 22 | ``` |
| 23 | |
| 24 | **Bad:** |
| 25 | ```c++ |
| 26 | std::string title; |
| 27 | std::vector<GURL> urls; |
| 28 | std::unordered_map<int, std::deque<url::Origin>> origins; |
| 29 | ``` |
| 30 | |
Darwin Huang | 33266b5 | 2019-04-15 21:31:21 | [diff] [blame] | 31 | When interacting with WTF types, use `wtf_size_t` instead of `size_t`. |
| 32 | |
Darwin Huang | 2f4b7a5 | 2019-02-12 03:57:58 | [diff] [blame] | 33 | ## Do not use `new` and `delete` |
| 34 | |
Peter Kasting | 777e130 | 2020-06-02 21:44:40 | [diff] [blame] | 35 | Chromium [recommends avoiding bare new/delete](c++-dos-and-donts.md#use-and-instead-of-bare); |
| 36 | Blink bans them. In addition to the options there, Blink objects may be |
| 37 | allocated using `blink::MakeGarbageCollected` and manage their lifetimes using |
| 38 | strong Blink GC references, depending on the type. See |
| 39 | [How Blink Works](https://docs.google.com/document/d/1aitSOucL0VHZa9Z2vbRJSyAIsAz24kX8LFByQ5xQnUg/edit#heading=h.ekwf97my4bgf) |
Darwin Huang | 2f4b7a5 | 2019-02-12 03:57:58 | [diff] [blame] | 40 | for more information. |
| 41 | |
Gyuyoung Kim | 9c43d73 | 2020-01-18 12:54:26 | [diff] [blame] | 42 | ## Don't mix Create() factory methods and public constructors in one class. |
| 43 | |
| 44 | A class only can have either Create() factory functions or public constructors. |
| 45 | In case you want to call MakeGarbageCollected<> from a Create() method, a |
| 46 | PassKey pattern can be used. Note that the auto-generated binding classes keep |
| 47 | using Create() methods consistently. |
| 48 | |
| 49 | **Good:** |
| 50 | ```c++ |
| 51 | class HistoryItem { |
| 52 | public: |
| 53 | HistoryItem(); |
| 54 | ~HistoryItem(); |
| 55 | ... |
| 56 | } |
| 57 | |
| 58 | void DocumentLoader::SetHistoryItemStateForCommit() { |
| 59 | history_item_ = MakeGarbageCollected<HistoryItem>(); |
| 60 | ... |
| 61 | } |
| 62 | ``` |
| 63 | |
| 64 | **Good:** |
| 65 | ```c++ |
| 66 | class BodyStreamBuffer { |
| 67 | public: |
Peter Kasting | 796cde2 | 2020-11-18 21:06:53 | [diff] [blame] | 68 | using PassKey = base::PassKey<BodyStreamBuffer>; |
Gyuyoung Kim | 9c43d73 | 2020-01-18 12:54:26 | [diff] [blame] | 69 | static BodyStreamBuffer* Create(); |
| 70 | |
| 71 | BodyStreamBuffer(PassKey); |
| 72 | ... |
| 73 | } |
| 74 | |
| 75 | BodyStreamBuffer* BodyStreamBuffer::Create() { |
| 76 | auto* buffer = MakeGarbageCollected<BodyStreamBuffer>(PassKey()); |
| 77 | buffer->Init(); |
| 78 | return buffer; |
| 79 | } |
| 80 | |
| 81 | BodyStreamBuffer::BodyStreamBuffer(PassKey) {} |
| 82 | ``` |
| 83 | |
| 84 | **Bad:** |
| 85 | ```c++ |
| 86 | class HistoryItem { |
| 87 | public: |
| 88 | // Create() and a public constructor should not be mixed. |
| 89 | static HistoryItem* Create() { return MakeGarbageCollected<HistoryItem>(); } |
| 90 | |
| 91 | HistoryItem(); |
| 92 | ~HistoryItem(); |
| 93 | ... |
| 94 | } |
| 95 | ``` |
| 96 | |
Kentaro Hara | c7d2b2b | 2019-09-04 14:12:26 | [diff] [blame] | 97 | ## Naming |
| 98 | |
| 99 | ### Use `CamelCase` for all function names |
| 100 | |
| 101 | All function names should use `CamelCase()`-style names, beginning with an |
| 102 | uppercase letter. |
| 103 | |
| 104 | As an exception, method names for web-exposed bindings begin with a lowercase |
| 105 | letter to match JavaScript. |
| 106 | |
| 107 | **Good:** |
| 108 | ```c++ |
| 109 | class Document { |
| 110 | public: |
| 111 | // Function names should begin with an uppercase letter. |
| 112 | virtual void Shutdown(); |
| 113 | |
| 114 | // However, web-exposed function names should begin with a lowercase letter. |
| 115 | LocalDOMWindow* defaultView(); |
| 116 | |
| 117 | // ... |
| 118 | }; |
| 119 | ``` |
| 120 | |
| 121 | **Bad:** |
| 122 | ```c++ |
| 123 | class Document { |
| 124 | public: |
| 125 | // Though this is a getter, all Blink function names must use camel case. |
| 126 | LocalFrame* frame() const { return frame_; } |
| 127 | |
| 128 | // ... |
| 129 | }; |
| 130 | ``` |
| 131 | |
| 132 | ### Precede boolean values with words like “is” and “did” |
David Sanders | af70bd8f | 2022-02-19 17:58:41 | [diff] [blame] | 133 | |
| 134 | **Good:** |
Kentaro Hara | c7d2b2b | 2019-09-04 14:12:26 | [diff] [blame] | 135 | ```c++ |
| 136 | bool is_valid; |
| 137 | bool did_send_data; |
| 138 | ``` |
| 139 | |
| 140 | **Bad:** |
| 141 | ```c++ |
| 142 | bool valid; |
| 143 | bool sent_data; |
| 144 | ``` |
| 145 | |
| 146 | ### Precede setters with the word “Set”; use bare words for getters |
| 147 | Precede setters with the word “set”. Prefer bare words for getters. Setter and |
| 148 | getter names should match the names of the variable being accessed/mutated. |
| 149 | |
| 150 | If a getter’s name collides with a type name, prefix it with “Get”. |
| 151 | |
| 152 | **Good:** |
| 153 | ```c++ |
| 154 | class FrameTree { |
| 155 | public: |
| 156 | // Prefer to use the bare word for getters. |
| 157 | Frame* FirstChild() const { return first_child_; } |
| 158 | Frame* LastChild() const { return last_child_; } |
| 159 | |
| 160 | // However, if the type name and function name would conflict, prefix the |
| 161 | // function name with “Get”. |
| 162 | Frame* GetFrame() const { return frame_; } |
| 163 | |
| 164 | // ... |
| 165 | }; |
| 166 | ``` |
| 167 | |
| 168 | **Bad:** |
| 169 | ```c++ |
| 170 | class FrameTree { |
| 171 | public: |
| 172 | // Should not be prefixed with “Get” since there's no naming conflict. |
| 173 | Frame* GetFirstChild() const { return first_child_; } |
| 174 | Frame* GetLastChild() const { return last_child_; } |
| 175 | |
| 176 | // ... |
| 177 | }; |
| 178 | ``` |
| 179 | |
| 180 | ### Precede getters that return values via out-arguments with the word “Get” |
| 181 | **Good:** |
| 182 | ```c++ |
| 183 | class RootInlineBox { |
| 184 | public: |
| 185 | Node* GetLogicalStartBoxWithNode(InlineBox*&) const; |
| 186 | // ... |
| 187 | } |
| 188 | ``` |
| 189 | |
| 190 | **Bad:** |
| 191 | ```c++ |
| 192 | class RootInlineBox { |
| 193 | public: |
| 194 | Node* LogicalStartBoxWithNode(InlineBox*&) const; |
| 195 | // ... |
| 196 | } |
| 197 | ``` |
| 198 | |
| 199 | ### May leave obvious parameter names out of function declarations |
Peter Kasting | 777e130 | 2020-06-02 21:44:40 | [diff] [blame] | 200 | The |
| 201 | [Google C++ Style Guide](https://google.github.io/styleguide/cppguide.html#Function_Declarations_and_Definitions) |
| 202 | allows omitting parameter names out when they are unused. In Blink, you may |
| 203 | leave obvious parameter names out of function declarations for historical |
| 204 | reasons. A good rule of thumb is if the parameter type name contains the |
| 205 | parameter name (without trailing numbers or pluralization), then the parameter |
| 206 | name isn’t needed. |
Kentaro Hara | c7d2b2b | 2019-09-04 14:12:26 | [diff] [blame] | 207 | |
| 208 | **Good:** |
| 209 | ```c++ |
| 210 | class Node { |
| 211 | public: |
| 212 | Node(TreeScope* tree_scope, ConstructionType construction_type); |
| 213 | // You may leave them out like: |
| 214 | // Node(TreeScope*, ConstructionType); |
| 215 | |
| 216 | // The function name makes the meaning of the parameters clear. |
| 217 | void SetActive(bool); |
| 218 | void SetDragged(bool); |
| 219 | void SetHovered(bool); |
| 220 | |
| 221 | // Parameters are not obvious. |
| 222 | DispatchEventResult DispatchDOMActivateEvent(int detail, |
| 223 | Event& underlying_event); |
| 224 | }; |
| 225 | ``` |
| 226 | |
| 227 | **Bad:** |
| 228 | ```c++ |
| 229 | class Node { |
| 230 | public: |
| 231 | // ... |
| 232 | |
| 233 | // Parameters are not obvious. |
| 234 | DispatchEventResult DispatchDOMActivateEvent(int, Event&); |
| 235 | }; |
| 236 | ``` |
| 237 | |
| 238 | ## Prefer enums or StrongAliases to bare bools for function parameters |
| 239 | Prefer enums to bools for function parameters if callers are likely to be |
| 240 | passing constants, since named constants are easier to read at the call site. |
Jan Wilken Dörrie | 7b78cd2 | 2020-11-23 13:11:24 | [diff] [blame] | 241 | Alternatively, you can use `base::StrongAlias<Tag, bool>`. An exception to this |
| 242 | rule is a setter function, where the name of the function already makes clear |
| 243 | what the boolean is. |
Kentaro Hara | c7d2b2b | 2019-09-04 14:12:26 | [diff] [blame] | 244 | |
| 245 | **Good:** |
| 246 | ```c++ |
| 247 | class FrameLoader { |
| 248 | public: |
| 249 | enum class CloseType { |
| 250 | kNotForReload, |
| 251 | kForReload, |
| 252 | }; |
| 253 | |
| 254 | bool ShouldClose(CloseType) { |
| 255 | if (type == CloseType::kForReload) { |
| 256 | ... |
| 257 | } else { |
| 258 | DCHECK_EQ(type, CloseType::kNotForReload); |
| 259 | ... |
| 260 | } |
| 261 | } |
| 262 | }; |
| 263 | |
| 264 | // An named enum value makes it clear what the parameter is for. |
| 265 | if (frame_->Loader().ShouldClose(FrameLoader::CloseType::kNotForReload)) { |
| 266 | // No need to use enums for boolean setters, since the meaning is clear. |
| 267 | frame_->SetIsClosing(true); |
| 268 | |
| 269 | // ... |
| 270 | ``` |
| 271 | |
| 272 | **Good:** |
| 273 | ```c++ |
| 274 | class FrameLoader { |
| 275 | public: |
Jan Wilken Dörrie | 7b78cd2 | 2020-11-23 13:11:24 | [diff] [blame] | 276 | using ForReload = base::StrongAlias<class ForReloadTag, bool>; |
Kentaro Hara | c7d2b2b | 2019-09-04 14:12:26 | [diff] [blame] | 277 | |
| 278 | bool ShouldClose(ForReload) { |
| 279 | // A StrongAlias<_, bool> can be tested like a bool. |
| 280 | if (for_reload) { |
| 281 | ... |
| 282 | } else { |
| 283 | ... |
| 284 | } |
| 285 | } |
| 286 | }; |
| 287 | |
| 288 | // Using a StrongAlias makes it clear what the parameter is for. |
| 289 | if (frame_->Loader().ShouldClose(FrameLoader::ForReload(false))) { |
| 290 | // No need to use enums for boolean setters, since the meaning is clear. |
| 291 | frame_->SetIsClosing(true); |
| 292 | |
| 293 | // ... |
| 294 | ``` |
| 295 | |
| 296 | **Bad:** |
| 297 | ```c++ |
| 298 | class FrameLoader { |
| 299 | public: |
| 300 | bool ShouldClose(bool for_reload) { |
| 301 | if (for_reload) { |
| 302 | ... |
| 303 | } else { |
| 304 | ... |
| 305 | } |
| 306 | } |
| 307 | }; |
| 308 | |
| 309 | // Not obvious what false means here. |
| 310 | if (frame_->Loader().ShouldClose(false)) { |
| 311 | frame_->SetIsClosing(ClosingState::kTrue); |
| 312 | |
| 313 | // ... |
| 314 | ``` |
| 315 | |
Kentaro Hara | c7d2b2b | 2019-09-04 14:12:26 | [diff] [blame] | 316 | ### Use `README.md` to document high-level components |
| 317 | |
| 318 | Documentation for a related-set of classes and how they interact should be done |
| 319 | with a `README.md` file in the root directory of a component. |