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 | |
Kentaro Hara | c7d2b2b | 2019-09-04 14:12:26 | [diff] [blame^] | 57 | ## Naming |
| 58 | |
| 59 | ### Use `CamelCase` for all function names |
| 60 | |
| 61 | All function names should use `CamelCase()`-style names, beginning with an |
| 62 | uppercase letter. |
| 63 | |
| 64 | As an exception, method names for web-exposed bindings begin with a lowercase |
| 65 | letter to match JavaScript. |
| 66 | |
| 67 | **Good:** |
| 68 | ```c++ |
| 69 | class Document { |
| 70 | public: |
| 71 | // Function names should begin with an uppercase letter. |
| 72 | virtual void Shutdown(); |
| 73 | |
| 74 | // However, web-exposed function names should begin with a lowercase letter. |
| 75 | LocalDOMWindow* defaultView(); |
| 76 | |
| 77 | // ... |
| 78 | }; |
| 79 | ``` |
| 80 | |
| 81 | **Bad:** |
| 82 | ```c++ |
| 83 | class Document { |
| 84 | public: |
| 85 | // Though this is a getter, all Blink function names must use camel case. |
| 86 | LocalFrame* frame() const { return frame_; } |
| 87 | |
| 88 | // ... |
| 89 | }; |
| 90 | ``` |
| 91 | |
| 92 | ### Precede boolean values with words like “is” and “did” |
| 93 | ```c++ |
| 94 | bool is_valid; |
| 95 | bool did_send_data; |
| 96 | ``` |
| 97 | |
| 98 | **Bad:** |
| 99 | ```c++ |
| 100 | bool valid; |
| 101 | bool sent_data; |
| 102 | ``` |
| 103 | |
| 104 | ### Precede setters with the word “Set”; use bare words for getters |
| 105 | Precede setters with the word “set”. Prefer bare words for getters. Setter and |
| 106 | getter names should match the names of the variable being accessed/mutated. |
| 107 | |
| 108 | If a getter’s name collides with a type name, prefix it with “Get”. |
| 109 | |
| 110 | **Good:** |
| 111 | ```c++ |
| 112 | class FrameTree { |
| 113 | public: |
| 114 | // Prefer to use the bare word for getters. |
| 115 | Frame* FirstChild() const { return first_child_; } |
| 116 | Frame* LastChild() const { return last_child_; } |
| 117 | |
| 118 | // However, if the type name and function name would conflict, prefix the |
| 119 | // function name with “Get”. |
| 120 | Frame* GetFrame() const { return frame_; } |
| 121 | |
| 122 | // ... |
| 123 | }; |
| 124 | ``` |
| 125 | |
| 126 | **Bad:** |
| 127 | ```c++ |
| 128 | class FrameTree { |
| 129 | public: |
| 130 | // Should not be prefixed with “Get” since there's no naming conflict. |
| 131 | Frame* GetFirstChild() const { return first_child_; } |
| 132 | Frame* GetLastChild() const { return last_child_; } |
| 133 | |
| 134 | // ... |
| 135 | }; |
| 136 | ``` |
| 137 | |
| 138 | ### Precede getters that return values via out-arguments with the word “Get” |
| 139 | **Good:** |
| 140 | ```c++ |
| 141 | class RootInlineBox { |
| 142 | public: |
| 143 | Node* GetLogicalStartBoxWithNode(InlineBox*&) const; |
| 144 | // ... |
| 145 | } |
| 146 | ``` |
| 147 | |
| 148 | **Bad:** |
| 149 | ```c++ |
| 150 | class RootInlineBox { |
| 151 | public: |
| 152 | Node* LogicalStartBoxWithNode(InlineBox*&) const; |
| 153 | // ... |
| 154 | } |
| 155 | ``` |
| 156 | |
| 157 | ### May leave obvious parameter names out of function declarations |
| 158 | [Google C++ Style Guide] allows us to leave parameter names out only if |
| 159 | the parameter is not used. In Blink, you may leave obvious parameter |
| 160 | names out of function declarations for historical reason. A good rule of |
| 161 | thumb is if the parameter type name contains the parameter name (without |
| 162 | trailing numbers or pluralization), then the parameter name isn’t needed. |
| 163 | |
| 164 | **Good:** |
| 165 | ```c++ |
| 166 | class Node { |
| 167 | public: |
| 168 | Node(TreeScope* tree_scope, ConstructionType construction_type); |
| 169 | // You may leave them out like: |
| 170 | // Node(TreeScope*, ConstructionType); |
| 171 | |
| 172 | // The function name makes the meaning of the parameters clear. |
| 173 | void SetActive(bool); |
| 174 | void SetDragged(bool); |
| 175 | void SetHovered(bool); |
| 176 | |
| 177 | // Parameters are not obvious. |
| 178 | DispatchEventResult DispatchDOMActivateEvent(int detail, |
| 179 | Event& underlying_event); |
| 180 | }; |
| 181 | ``` |
| 182 | |
| 183 | **Bad:** |
| 184 | ```c++ |
| 185 | class Node { |
| 186 | public: |
| 187 | // ... |
| 188 | |
| 189 | // Parameters are not obvious. |
| 190 | DispatchEventResult DispatchDOMActivateEvent(int, Event&); |
| 191 | }; |
| 192 | ``` |
| 193 | |
| 194 | ## Prefer enums or StrongAliases to bare bools for function parameters |
| 195 | Prefer enums to bools for function parameters if callers are likely to be |
| 196 | passing constants, since named constants are easier to read at the call site. |
| 197 | Alternatively, you can use base::util::StrongAlias<Tag, bool>. An exception to |
| 198 | this rule is a setter function, where the name of the function already makes |
| 199 | clear what the boolean is. |
| 200 | |
| 201 | **Good:** |
| 202 | ```c++ |
| 203 | class FrameLoader { |
| 204 | public: |
| 205 | enum class CloseType { |
| 206 | kNotForReload, |
| 207 | kForReload, |
| 208 | }; |
| 209 | |
| 210 | bool ShouldClose(CloseType) { |
| 211 | if (type == CloseType::kForReload) { |
| 212 | ... |
| 213 | } else { |
| 214 | DCHECK_EQ(type, CloseType::kNotForReload); |
| 215 | ... |
| 216 | } |
| 217 | } |
| 218 | }; |
| 219 | |
| 220 | // An named enum value makes it clear what the parameter is for. |
| 221 | if (frame_->Loader().ShouldClose(FrameLoader::CloseType::kNotForReload)) { |
| 222 | // No need to use enums for boolean setters, since the meaning is clear. |
| 223 | frame_->SetIsClosing(true); |
| 224 | |
| 225 | // ... |
| 226 | ``` |
| 227 | |
| 228 | **Good:** |
| 229 | ```c++ |
| 230 | class FrameLoader { |
| 231 | public: |
| 232 | using ForReload = base::util::StrongAlias<class ForReloadTag, bool>; |
| 233 | |
| 234 | bool ShouldClose(ForReload) { |
| 235 | // A StrongAlias<_, bool> can be tested like a bool. |
| 236 | if (for_reload) { |
| 237 | ... |
| 238 | } else { |
| 239 | ... |
| 240 | } |
| 241 | } |
| 242 | }; |
| 243 | |
| 244 | // Using a StrongAlias makes it clear what the parameter is for. |
| 245 | if (frame_->Loader().ShouldClose(FrameLoader::ForReload(false))) { |
| 246 | // No need to use enums for boolean setters, since the meaning is clear. |
| 247 | frame_->SetIsClosing(true); |
| 248 | |
| 249 | // ... |
| 250 | ``` |
| 251 | |
| 252 | **Bad:** |
| 253 | ```c++ |
| 254 | class FrameLoader { |
| 255 | public: |
| 256 | bool ShouldClose(bool for_reload) { |
| 257 | if (for_reload) { |
| 258 | ... |
| 259 | } else { |
| 260 | ... |
| 261 | } |
| 262 | } |
| 263 | }; |
| 264 | |
| 265 | // Not obvious what false means here. |
| 266 | if (frame_->Loader().ShouldClose(false)) { |
| 267 | frame_->SetIsClosing(ClosingState::kTrue); |
| 268 | |
| 269 | // ... |
| 270 | ``` |
| 271 | |
| 272 | ## Comments |
| 273 | Please follow the standard [Chromium Documentation Guidelines]. In particular, |
| 274 | most classes should have a class-level comment describing the purpose, while |
| 275 | non-trivial code should have comments describing why the code is written the |
| 276 | way it is. Often, what is apparent when the code is written is not so obvious |
| 277 | a year later. |
| 278 | |
| 279 | From [Google C++ Style Guide: Comments]: |
| 280 | > Giving sensible names to types and variables is much better than obscure |
| 281 | > names that must be explained through comments. |
| 282 | |
| 283 | ### Use `README.md` to document high-level components |
| 284 | |
| 285 | Documentation for a related-set of classes and how they interact should be done |
| 286 | with a `README.md` file in the root directory of a component. |
| 287 | |
| 288 | ### TODO style |
| 289 | |
| 290 | Comments for future work should use `TODO` and have a name or bug attached. |
| 291 | |
| 292 | From [Google C++ Style Guide: TODO Comments]: |
| 293 | |
| 294 | > The person named is not necessarily the person who must fix it. |
| 295 | |
| 296 | **Good:** |
| 297 | ```c++ |
| 298 | // TODO(dcheng): Clean up after the Blink rename is done. |
| 299 | // TODO(https://crbug.com/675877): Clean up after the Blink rename is done. |
| 300 | ``` |
| 301 | |
| 302 | **Bad:** |
| 303 | ```c++ |
| 304 | // FIXME(dcheng): Clean up after the Blink rename is done. |
| 305 | // FIXME: Clean up after the Blink rename is done. |
| 306 | // TODO: Clean up after the Blink rename is done. |
| 307 | ``` |
| 308 | |
Daniel Cheng | 7ecc1d6 | 2017-06-12 20:24:30 | [diff] [blame] | 309 | [Chromium Style Guide]: c++.md |
| 310 | [Google C++ Style Guide]: https://google.github.io/styleguide/cppguide.html |
| 311 | [Chromium Documentation Guidelines]: ../../docs/documentation_guidelines.md |
| 312 | [Google C++ Style Guide: Comments]: https://google.github.io/styleguide/cppguide.html#Comments |
| 313 | [Google C++ Style Guide: TODO Comments]:https://google.github.io/styleguide/cppguide.html#TODO_Comments |