
This reverts commit c73e1f437889d882cbf2987f7fb3a029a6150613. Reason for revert: The problem with failed deps in chrome content/renderer had already been fixed in https://webrtc-review.googlesource.com/c/src/+/38660 Original change's description: > Revert "GN rtc_* templates: Set default visibility to webrtc_root + "/*"" > > This reverts commit 588c548657b3ddf76e7b3f241263eef7f5799f16. > > Reason for revert: > > Breaks Chrome FYI: > > /b/c/b/Linux_Builder/src/buildtools/linux64/gn gen //out/Release --check > -> returned 1 > ERROR at //build/split_static_library.gni:12:5: Dependency not allowed. > static_library(target_name) { > ^---------------------------- > The item //content/renderer:renderer > can not depend on //third_party/webrtc/media:rtc_internal_video_codecs > because it is not in //third_party/webrtc/media:rtc_internal_video_codecs's visibility list: [ > //third_party/webrtc/* > //third_party/webrtc_overrides/* > ] > > https://logs.chromium.org/v/?s=chromium%2Fbb%2Fchromium.webrtc.fyi%2FLinux_Builder%2F23560%2F%2B%2Frecipes%2Fsteps%2Fgenerate_build_files%2F0%2Fstdout > > Original change's description: > > GN rtc_* templates: Set default visibility to webrtc_root + "/*" > > > > This means that by default, targets are visible to everything under > > the WebRTC root, but not visible to anything else. > > > > API targets are manually tagged with visibility "*", so that targets > > outside the WebRTC tree can see them. > > > > BUG=webrtc:8254 > > > > Change-Id: Icdbee6e0d22d93240ff2fb530c8f9dc48e351509 > > Reviewed-on: https://webrtc-review.googlesource.com/24140 > > Reviewed-by: Mirko Bonadei <mbonadei@webrtc.org> > > Commit-Queue: Karl Wiberg <kwiberg@webrtc.org> > > Cr-Commit-Position: refs/heads/master@{#21548} > > TBR=mbonadei@webrtc.org,kwiberg@webrtc.org > > Change-Id: I06620ce3d6f67482935c22efa231dd6cab91625a > No-Presubmit: true > No-Tree-Checks: true > No-Try: true > Bug: webrtc:8254 > Reviewed-on: https://webrtc-review.googlesource.com/38760 > Reviewed-by: Per Kjellander <perkj@webrtc.org> > Commit-Queue: Per Kjellander <perkj@webrtc.org> > Cr-Commit-Position: refs/heads/master@{#21555} TBR=mbonadei@webrtc.org,kwiberg@webrtc.org,perkj@webrtc.org Change-Id: I6f720078ce21bd172e0a6471bae8c4c011e4a657 No-Presubmit: true No-Tree-Checks: true No-Try: true Bug: webrtc:8254 Reviewed-on: https://webrtc-review.googlesource.com/38860 Reviewed-by: Per Kjellander <perkj@webrtc.org> Commit-Queue: Per Kjellander <perkj@webrtc.org> Cr-Commit-Position: refs/heads/master@{#21558}
5.8 KiB
WebRTC coding style guide
General advice
Some older parts of the code violate the style guide in various ways.
- If making small changes to such code, follow the style guide when it’s reasonable to do so, but in matters of formatting etc., it is often better to be consistent with the surrounding code.
- If making large changes to such code, consider first cleaning it up in a separate CL.
C++
WebRTC follows the Chromium and Google C++ style guides. In cases where they conflict, the Chromium style guide trumps the Google style guide, and the rules in this file trump them both.
ArrayView
When passing an array of values to a function, use rtc::ArrayView
whenever possible—that is, whenever you’re not passing ownership of
the array, and don’t allow the callee to change the array size.
For example,
instead of | use |
---|---|
const std::vector<T>& |
ArrayView<const T> |
const T* ptr, size_t num_elements |
ArrayView<const T> |
T* ptr, size_t num_elements |
ArrayView<T> |
See the source for more detailed docs.
sigslot
sigslot is a lightweight library that adds a signal/slot language construct to C++, making it easy to implement the observer pattern with minimal boilerplate code.
When adding a signal to a pure interface, prefer to add a pure virtual method that returns a reference to a signal:
sigslot::signal<int>& SignalFoo() = 0;
As opposed to making it a public member variable, as a lot of legacy code does:
sigslot::signal<int> SignalFoo;
The virtual method approach has the advantage that it keeps the interface stateless, and gives the subclass more flexibility in how it implements the signal. It may:
-
Have its own signal as a member variable.
-
Use a
sigslot::repeater
, to repeat a signal of another object:sigslot::repeater<int> foo_; /* ... */ foo_.repeat(bar_.SignalFoo());
-
Just return another object's signal directly, if the other object's lifetime is the same as its own.
sigslot::signal<int>& SignalFoo() { return bar_.SignalFoo(); }
C
There’s a substantial chunk of legacy C code in WebRTC, and a lot of it is old enough that it violates the parts of the C++ style guide that also applies to C (naming etc.) for the simple reason that it pre-dates the use of the current C++ style guide for this code base.
- If making small changes to C code, mimic the style of the surrounding code.
- If making large changes to C code, consider converting the whole thing to C++ first.
Java
WebRTC follows the Google Java style guide.
Objective-C and Objective-C++
WebRTC follows the Chromium Objective-C and Objective-C++ style guide.
Python
WebRTC follows Chromium’s Python style.
Build files
The WebRTC build files are written in GN, and we follow the Chromium GN style guide. Additionally, there are some WebRTC-specific rules below; in case of conflict, they trump the Chromium style guide.
WebRTC-specific GN templates
Use the following GN templates to ensure that all our targets are built with the same configuration:
instead of | use |
---|---|
executable |
rtc_executable |
shared_library |
rtc_shared_library |
source_set |
rtc_source_set |
static_library |
rtc_static_library |
test |
rtc_test |
Target visibility and the native API
The WebRTC-specific GN templates declare build
targets whose default visibility
allows all other targets in the
WebRTC tree (and no targets outside the tree) to depend on them.
Prefer to restrict the visibility if possible:
- If a target is used by only one or a tiny number of other targets,
prefer to list them explicitly:
visibility = [ ":foo", ":bar" ]
- If a target is used only by targets in the same
BUILD.gn
file:visibility = [ ":*" ]
.
Setting visibility = [ "*" ]
means that targets outside the WebRTC
tree can depend on this target; use this only for build targets whose
headers are part of the native API.
Conditional compilation with the C preprocessor
Avoid using the C preprocessor to conditionally enable or disable pieces of code. But if you can’t avoid it, introduce a GN variable, and then set a preprocessor constant to either 0 or 1 in the build targets that need it:
if (apm_debug_dump) {
defines = [ "WEBRTC_APM_DEBUG_DUMP=1" ]
} else {
defines = [ "WEBRTC_APM_DEBUG_DUMP=0" ]
}
In the C, C++, or Objective-C files, use #if
when testing the flag,
not #ifdef
or #if defined()
:
#if WEBRTC_APM_DEBUG_DUMP
// One way.
#else
// Or another.
#endif
When combined with the -Wundef
compiler option, this produces
compile time warnings if preprocessor symbols are misspelled, or used
without corresponding build rules to set them.