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