Coding style for the Chromium projects generally follows the Google C++ Style Guide. The notes below are usually just extensions beyond what the Google style guide already says. If this document doesn't mention a rule, follow the Google C++ style. If Google style is flexible on a particular topic, and this document doesn't clarify below, then Chromium style is also flexible on that topic. Objective-C/C++ code should follow the Google Objective-C Style Guide. If you use emacs, you may find Google's style definition for c-mode helpful.
Try to follow any
per-file conventions you notice when modifying existing code.
When working on WebKit code, you should use the WebKit coding style
instead.
The remainder of this page covers rules that are specific to the Chromium projects.
Naming
- Unit tests and performance tests should be named with the suffixes _unittest and _perftest, respectively, and placed alongside (in the same directory as) the file containing the functionality they're testing.
- Win32 window class name strings should begin with Chrome_ (note the capitalization).
- Variables representing quantities should express the units of
measurement in the name unless they're extremely obvious in context,
e.g. kUpdateTimerMsec or num_downloaded_bytes
- Use URL instead of URI when referring to web addresses.
- Projects should be named using unix_hacker style, not CamelCase style.
- Google style strongly encourages (but does not mandate) unix_hacker
style on member functions that do so little work they're effectively
free, such as simple, non-virtual getters and setters. This is also the
team position. Some code uses CamelCase for everything; avoid writing patches just to convert these functions to unix_hacker style, but try to use it in new code you write unless there are encapsulation reasons why CamelCase would be much more appropriate.
- "Chromium" is the name of the project, not the product, and should never appear in code in variable names, API names etc. Use "chrome" instead.
- Though the style guide says to use kConstantNaming for enums now, we still use MACRO_STYLE naming for enums for consistency with existing code.
Code formatting
- For conditional return statements, the return should be indented on its own line (for easier tracing).
- When expressions are wrapped, the operator should be on the end of the broken line, not the start of the new line. For example:
foo = bar +
baz;
not:
foo = bar
+ baz;
An exception is log messages. When you have a log line that is longer than 80 characters, subsequent lines should start with the << operator and should be aligned based on the first << from the original line:
LOG(INFO) << "I have a very long log message here, with multiple things to"
<< " say and some variables to print out, like var1: "
<< var1 << " and var2: " << var2;
- Use NULL for null pointers rather than 0 or defining null (exception: code that needs to follow WebKit style conventions should use 0).
- Use true and false for bool values. In some places where we've been forced to use BOOL, we use TRUE and FALSE.
- Some of us have OCD and like to make sure the order of the
function implementations in our .cc files matches the order in the .h
files. While this isn't a rule in the Google C++ style guide, if you
find a file in this condition please try to leave it in this state
after modifying it.
- As with header includes, we try and make sure message map macros are
in alphabetical order for ease of scanning. Please follow this
convention when adding new message handlers.
- When you implement an interface, group those functions in your header file in one section, clearly labeled as implementing that interface. Be sure to preserve the order of the virtual base class, and do not add comments or whitespace between them, as the reader can refer to the virtual base class for more information.
Platform-specific code
Platform-specific code should use the system specific defines in build/build_config.h and not other things such as WIN32. Current platforms are OS_WIN, OS_MACOSX, and OS_LINUX. Don't use an #else clause to define platform behavior, it should opt-in to everything explicitly. Use OS_POSIX to define something applicable for Mac and Linux (for example, pthreads).
Prefer #if defined(...) rather than #ifdef for platform-specific code. This lets somebody else add an #elif defined(...) later on which will match.
Bad:
#ifdef OS_WIN
<Windows code>
#else
<Mac & Linux code>
#endif
Good:
#if defined(OS_WIN)
<Windows code>
#elif defined(OS_POSIX)
<Mac & Linux code>
#endif
If you need to do something based on the type of wchar_t, don't use platform defines. Use WCHAR_T_IS_UTF16 or WCHAR_T_IS_UTF32 instead.
Project settings
- Don't check-in .vcproj, .sln, .xcodeproj or .scons files. Use GYP instead. Most our common build settings are in common.gypi.
- The only reason we may accept non-gyp project files is that they are from third parties and aren't used. This is to keep forks as close as possible to upstream.
- Try to use #pragma lib where needed instead of explicitly adding library dependencies in project files.
- Compile with warnings set to fatal, unless you are compiling
third-party code which you cannot easily fix. (And even then, it might
be worth seeing if you can use #pragma warning(disable: [warning #]).
Static variables
- Don't use static variables (especially globals) that have constructors or destructors, even trivial. In general, just avoid them
altogether as they can lead to mysterious crashes (that our crash
reporter will not be able to catch). Use Singleton instead.
Note that Google style bans
global-scope class objects of most types for this reason. This includes string objects like std::string! Use const char* const or const wchar_t* const when you need static strings. Note the second const.
Method signatures
- Based on some empirical testing, it seems just as fast to return a std::[w]string
from a function as a return value as it is to pass it out as an out
param. So we encourage passing strings out as return values when
possible for the sake of readability.
File headers
- All source files in the Chromium project must start with the following header. Note that you don't need to update the copyright date on an old file unless you're already editing it for some other reason.
// Copyright (c) 2009 The Chromium Authors. All rights reserved.
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.
- The Chromium OS project's source files start with a slightly different header (Chromium -> Chromium OS), like this:
// Copyright (c) 2009 The Chromium OS Authors. All rights reserved.
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.
- We don't add Author: lines to our files. Instead, if you contribute changes and your name or your organization's name isn't already in trunk/src/AUTHORS, please add your name and contact information.
WebKit glue style
You should only be including WebKit files in our WebKit port and glue directories. We always use Google style for WebKit glue files with the exception of when WebKit files need to be included. Glue code should include WebKit headers the WebKit way (with no extra path information) and wrap WebKit headers with warning guards so that warnings in those files are ignored. Be sure to include config.h first when you are using WebKit includes, or you will get weird errors! Example:
#include "webkit/glue/me.h" // Begin with the corresponding header for this .cc file.
#include <stdio.h> // System includes are second.
#include <stdlib.h>
#pragma warning(push, 0) // This will make Visual Studio ignore warnings for these files.
#include "config.h" // WebKit section, note alphabetical order.
#include "Document.h"
#include "Frame.h"
#include "Page.h"
#pragma warning(pop)
#include "base/file_util.h" // Google-style includes for everything else.
#include "webkit/glue/webkit_glue.h"
Subversion properties
For Chromium development, you should configure your ~/.subversion/config (On Windows: %USERPROFILE%\AppData\Roaming\Subversion\config) file to automatically set properties on new files:
[miscellany]
global-ignores = *.pyc *.user *.suo *.bak *~ #*# *.ncb *.o *.lo *.la .*~ .#* .DS_Store .*.swp *.scons *.mk *.sln *.vcproj *.rules SConstruct
enable-auto-props = yes
[auto-props]
*.c = svn:eol-style=LF
*.h = svn:eol-style=LF
*.cc = svn:eol-style=LF
*.cpp = svn:eol-style=LF
*.def = svn:eol-style=LF
*.obsolete = svn:eol-style=LF
*.m = svn:eol-style=LF
*.mm = svn:eol-style=LF
*.idl = svn:eol-style=LF
*.html = svn:eol-style=LF
*.htm = svn:eol-style=LF
*.xml = svn:eol-style=LF
*.js = svn:eol-style=LF
*.css = svn:eol-style=LF
*.mock-http-headers = svn:eol-style=LF
*.afm = svn:eol-style=LF
*.txt = svn:eol-style=LF
*.gyp = svn:eol-style=LF
*.gypi = svn:eol-style=LF
*.grd = svn:eol-style=LF
*.xtb = svn:eol-style=LF
*.py = svn:eol-style=LF
*.pl = svn:eol-style=LF
*.pm = svn:eol-style=LF
*.sh = svn:eol-style=LF;svn:executable
*.make = svn:eol-style=LF
Makefile = svn:eol-style=LF
*.bat = svn:eol-style=CRLF
*.txt = svn:eol-style=LF
*.png = svn:mime-type=image/png
*.jpg = svn:mime-type=image/jpeg
We do not use svn:eol-style=native. It is a pain to move diff files across platforms, especially for the try server.
CHECK() vs DCHECK() and NOTREACHED()
The CHECK() macro is used for checking assertions, and will cause an immediate crash if its assertion is not met. DCHECK() is like CHECK() but is only compiled in on debug builds. NOTREACHED() is a debug-only break, equivalent to DCHECK(false). These macros are defined in base/logging.h.
Use DCHECK or NOTREACHED to inform developers when they are misusing a function or when a bad condition arises that the developer should be made aware of. In general--and this may seem counter-intuitive — it is preferable to crash instead of handling a DCHECK failure. Attempting to handle a DCHECK failure in a graceful manner is a statement that the DCHECK can fail, which contradicts the point of writing the DCHECK. (If you find yourself writing such code, then perhaps what you really want is to use DLOG or LOG.) In some cases the consequences of a failed DCHECK may result in a crash. This is not necessarily a terrible thing. The crash will be logged, and that will result in visibility to the fact that the DCHECK fails in the wild. It is far better to receive detailed crash reports than vague anecdotes about unreproducible hangs, dead clicks, or other misbehaving non-crash conditions.
Sometimes it is preferable to force a crash to happen via CHECK. Those cases include situations where you are dealing with untrusted input (provided there is no reasonable recovery path) and the consequences of continuing execution could result in a security vulnerability. It is also common practice to use CHECK to help root out the source of an observed, but obtuse, crash report. Developers may sprinkle CHECKs around the crash site to reveal more information about the crash.
Sometimes there are better options besides CHECK when dealing with malformed input from an untrusted source. For example, if a renderer sends the browser process a malformed IPC, it is better to just kill the offending renderer than to crash the browser process.
|
