| Commit message (Collapse) | Author | Age | Files | Lines |
... | |
|
|
|
|
|
|
|
| |
Properly handle microdump processing, when the system_log file contains an incomplete microdump section at the top. The processor will process the first complete microdump section.
R=primiano@chromium.org
Review URL: https://codereview.chromium.org/1742843002 .
|
|
|
|
|
|
|
| |
BUG=587536
R=primiano@chromium.org
Review URL: https://codereview.chromium.org/1704243002 .
|
|
|
|
|
|
|
|
|
|
| |
Because tools/windows/symupload/symupload.cc uses `nullptr` (which
requires VS2010), the CLSID comparison is only performed for msdia100.dll
and later. When compiling with an older (or future) CLSID_DiaSource, we
retain the existing behaviour (i.e. fail if CoCreateInstance fails).
R=ivanpe@chromium.org
BUG=https://bugzilla.mozilla.org/show_bug.cgi?id=1236343
|
| |
|
|
|
|
|
|
|
| |
BUG=https://code.google.com/p/chromium/issues/detail?id=584174
R=mmandlis@chromium.org
Review URL: https://codereview.chromium.org/1697963002 .
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
If the Linux minidump writer crashes while writing a dump, the dump
might contain some useful information, but the header will be empty
because TypedMDRVA's destructor flushes the data, and the header var
doesn't go out of scope until the end of the `Dump` method. This
fixes that problem by putting the header in a shorter block scope.
We've seen this problem in some Android dumps in the wild, like:
https://crash-stats.mozilla.com/report/index/cef5b777-02d1-43c2-bf40-133ab2160209
R=thestig@chromium.org
BUG=https://bugzilla.mozilla.org/show_bug.cgi?id=1247978
Review URL: https://codereview.chromium.org/1696573003 .
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
GPU infromation in the following format:
G GL_VERSION|GL_VENDOR|GL_RENDERER.
The GPU version, vendor and renderer are extracted during microdump parsing and populated in the appropriate fields in the SystemInfo struct.
This is to match the changes introduced in crrev.com/1343713002 and crrev.com/1334473003
BUG=chromium:536769
R=primiano@chromium.org
Review URL: https://codereview.chromium.org/1678463002 .
|
|
|
|
|
|
|
|
|
|
|
| |
This reverts commit cb936a0243c97ae9cd2d4bb19d95dde0421fed6d.
A=dyen@chromium.org
Original Review: https://codereview.chromium.org/1672773002/
R=dyen@chromium.org
Review URL: https://codereview.chromium.org/1688493003 .
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
This patch changes MDCVInfoELF (which is currently unused, apparently
a vestigal bit of code landed as part of Solaris support) into a supported
CodeView format that simply contains a build id as raw bytes.
Modern ELF toolchains support build ids nicely:
https://access.redhat.com/documentation/en-US/Red_Hat_Enterprise_Linux/6/html/Developer_Guide/compiling-build-id.html
It would be useful to have the original build ids of loaded modules in
Linux minidumps, since tools like Fedora's darkserver allow querying by build
id and the current Breakpad code truncates the build id to the size of a GUID,
which loses information:
https://darkserver.fedoraproject.org/
A follow-up patch will change the Linux minidump generation code to produce
MDCVInfoELF in minidumps instead of MDCVInfoPDB70. This patch should be landed
first to ensure that crash processors are able to handle this format before
dumps are generated containing it.
The full build id is exposed as the return value of Minidump::code_identifier(),
which currently just returns "id" for modules in Linux dumps. For
backwards-compatibility, Minidump::debug_identifier() continues to treat
the build id as a GUID, so debug identifiers for existing modules will not
change.
BUG=
R=mark@chromium.org
Review URL: https://codereview.chromium.org/1675413002 .
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
The method -[NSURLConnection sendSynchronousRequest:returningResponse:error:]
has been deprecated in 10.11 OS X SDK and 9.0 iOS SDK without replacement. So
emulate a synchronous request by using an asynchronous request and waiting on
a semaphore for the request completion.
BUG=https://bugs.chromium.org/p/google-breakpad/issues/detail?id=675
BUG=569158
R=mark@chromium.org
Review URL: https://codereview.chromium.org/1675243002 .
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
The method -[NSString stringByAddingPercentEscapesUsingEncoding:] has been
deprecated with 10.11 OS X SDK and 9.0 iOS SDK. The recommended method is
-[NSString stringByAddingPercentEncodingWithAllowedCharacters:] available
since 10.9 OS X SDK and 7.0 iOS SDK.
Use the new method when available using URLQueryAllowedCharacterSet to get
the same encoded string.
BUG=https://bugs.chromium.org/p/google-breakpad/issues/detail?id=675
BUG=569158
R=mark@chromium.org
Review URL: https://codereview.chromium.org/1680663002 .
|
|
|
|
|
|
|
|
|
|
|
|
| |
The function CFPropertyListCreateFromXMLData is deprecated in favor of
the function CFPropertyListCreateWithData that is available since the
10.6 OS X SDK and 4.0 iOS SDK.
BUG=https://bugs.chromium.org/p/google-breakpad/issues/detail?id=675
BUG=569158
R=mark@chromium.org
Review URL: https://codereview.chromium.org/1678063002 .
|
|
|
|
|
|
|
|
|
|
| |
Adding remaining mips n64 support including stackwalker.
BUG=None
TEST=manually tested on Linux/Android
R=vapier@chromium.org
Review URL: https://codereview.chromium.org/1418453011 .
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
Original change (https://codereview.chromium.org/1527363003/) was failing
in CFReadStreamGetBuffer() call, so changed to CFReadStreamRead() to be
more conservative.
Patch provided by Scott Hancher.
BUG=
R=mark@chromium.org
Review URL: https://codereview.chromium.org/1637433003 .
|
|
|
|
|
|
|
|
|
|
|
|
| |
In order to figure out what symbols we need associated to a minidump,
it is useful to be able to dump all the modules the minidump contains.
A=dyen@chromium.org
Original Review: https://codereview.chromium.org/1651593002/
BUG=563716
R=dyen@chromium.org
Review URL: https://codereview.chromium.org/1650713002 .
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
This updates the GYP build for the processor component (on windows).
- adds/removes references to files which were added or removed from the
repository
- includes build/common.gypi in the gyp files: needed to correctly
detect the OS (I think, the generated MSVC solutions were broken
without it)
- conditionally compiles code platform-specific code for the given
platform
After this minidump processor nearly compiles with VS2013: the generated
project is correct, but some files still have compilation errors.
Disclaimer: I have not tested the GYP changes on non-windows platform,
as there does not seem to be anyone using it there.
BUG=
R=mark@chromium.org
Review URL: https://codereview.chromium.org/1643633004 .
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
Newer gcc versions default to -Werror=narrowing when using newer C++
standards (which we do). This causes issues when we try to stuff a
value like 0xea into a char -- the value is out of range for signed
char bytes. That's when gcc throws an error:
.../bytereader_unittest.cc: In member function 'virtual void Reader_DW_EH_PE_absptr4_Test::TestBody()':
.../bytereader_unittest.cc:400:55: error: narrowing conversion of '234' from 'int' to 'char' inside { } [-Wnarrowing]
BUG=chromium:579384
TEST=`make check` passes
R=mark@chromium.org
Review URL: https://codereview.chromium.org/1605153004 .
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
Some systems provide prebuilt copies of gmock/gtest (such as Chromium
OS). Add a configure flag so they can take advantage of that. This
allows for a smaller checkout as they don't need to include the full
testing/ tree.
BUG=chromium:579384
TEST=`make check` passes w/--enable-system-test-libs
TEST=`make check` passes w/--disable-system-test-libs
R=thestig@chromium.org
Review URL: https://codereview.chromium.org/1638653002 .
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
This reverts CL https://codereview.chromium.org/1563223004/
This reverts commit 7cc0d8562bf8b20b88cc941ba72593cb7230ecf6.
CL 1563223004 introduces two bugs on iOS.
- Encoding the minidump name with extra percent causing crash server to fail
processing the file.
- Using a released pointer causing random crashes on upload. The
data, resp, err pointers returned in the NSURLSession completion
handler is released at the end of the block. When used later (to get
the crash ID), it causes a crash.
BUG=569158
R=blundell@chromium.org, mark@chromium.org
Review URL: https://codereview.chromium.org/1619603002 .
Patch from Olivier Robin <olivierrobin@chromium.org>.
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
The std::getline function always returns its first arg (which is an
iostream object) and cannot return anything else. Thus, testing its
value is pointless, and even leads to build errors w/at least gcc-5
due to gtest ASSERT_TRUE funcs only taking bool types:
.../exploitability_unittest.cc: In member function 'virtual void {anonymous}::ExploitabilityLinuxUtilsTest_DisassembleBytesTest_Test::TestBody()':
.../exploitability_unittest.cc:200:136: error: no matching function for call to 'testing::AssertionResult::AssertionResult(std::basic_istream<char>&)'
In file included from .../breakpad_googletest_includes.h:33:0,
from .../exploitability_unittest.cc:35:
.../gtest.h:262:12: note: candidate: testing::AssertionResult::AssertionResult(bool)
Since we know this never fails, simply drop the ASSERT_TRUE usage.
The next line already checks the content of the buffer we read.
Further on in the file, we hit some signed warnings:
In file included from .../breakpad_googletest_includes.h:33:0,
from .../exploitability_unittest.cc:35:
.../gtest.h: In instantiation of 'testing::AssertionResult testing::internal::CmpHelperEQ(const char*, const char*, const T1&, const T2&) [with T1 = long unsigned int; T2 = int]':
.../gtest.h:1484:23: required from 'static testing::AssertionResult testing::internal::EqHelper<lhs_is_null_literal>::Compare(const char*, const char*, const T1&, const T2&) [with T1 = long unsigned int; T2 = int; bool lhs_is_null_literal = false]'
.../exploitability_unittest.cc:241:289: required from here
.../gtest.h:1448:16: error: comparison between signed and unsigned integer expressions [-Werror=sign-compare]
if (expected == actual) {
This is because we compare the register value (a uint64_t) directly to
an integer constant, and those are signed by default. Stick a U suffix
on them to fix things up.
BUG=chromium:579384
TEST=`make check` passes
R=ivanpe@chromium.org
Review URL: https://codereview.chromium.org/1611763002 .
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
Older versions of MSVC don't have a snprintf functions. Some files
were already working around that, but not all of them. Instead of
copying the logic into every file, I centralize it into a new
stdio.h wrapper file and make other files include that.
BUG=
R=mark@chromium.org
Review URL: https://codereview.chromium.org/1602563003 .
Patch from Pavel Labath <labath@google.com>.
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
In C/C++, the result of signed integer overflow is undefined.
The expression "base + size - 1" is parsed as "(base + size) - 1", and
"base + size" can overflow even if "base + (size - 1)" <= INT_MAX.
See http://g/c-compiler-chrome/461JohPKakE/JI3rEBg6FwAJ for more.
BUG=None
TEST='CC=clang CXX=clang++ ./configure && make check'
R=vapier@chromium.org
Review URL: https://codereview.chromium.org/1591793002 .
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
windows.h defines exception_code as a macro, which conflicts with our
use of the identifier in exception records. It appears that this
particular include of windows.h is not needed, so instead of undefining
the macro, I simply delete the include. Build tested with MSVC 2013.
BUG=
R=mark@chromium.org
Review URL: https://codereview.chromium.org/1579623004 .
Patch from Pavel Labath <labath@google.com>.
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
MSVC does not have the __PTRDIFF_TYPE__ macro defined, so I use the
standard [u]intptr_t types instead. Compilation tested on windows, linux
and mac.
BUG=
R=mark@chromium.org
Review URL: https://codereview.chromium.org/1571293003 .
Patch from Pavel Labath <labath@google.com>.
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
Due to operator precedence, the address was first cast to void*
and then incremented, which resulted in an error on windows, as
sizeof(void) is undefined and MSVC takes this seriously. Changing
the precedence to perform the addition first.
R=mark@chromium.org
Review URL: https://codereview.chromium.org/1570843002 .
Patch from Pavel Labath <labath@google.com>.
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
This file is not present on windows, and it's causing build errors
there. As far as I can tell, nothing in this file actually uses
that include, so I just remove it.
BUG=
R=mark@chromium.org
Review URL: https://codereview.chromium.org/1475353002 .
Patch from Pavel Labath <labath@google.com>.
|
|
|
|
|
|
|
|
|
| |
Fix some copy/paste errors from commit 41440eaa.
BUG=None
R=thestig@chromium.org
Review URL: https://codereview.chromium.org/1564293002 .
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
Fixes the following compilation warning when using recent version of
the iOS or OS X SDK by using the recommended new API:
../../breakpad/src/common/mac/HTTPMultipartUpload.m:56:10: error: 'stringByAddingPercentEscapesUsingEncoding:' is deprecated: first deprecated in iOS 9.0 - Use -stringByAddingPercentEncodingWithAllowedCharacters: instead, which always uses the recommended UTF-8 encoding, and which encodes for a specific URL component or subcomponent since each URL component or subcomponent has different rules for what characters are valid. [-Werror,-Wdeprecated-declarations]
[key stringByAddingPercentEscapesUsingEncoding:NSUTF8StringEncoding];
^
CFURLCreateStringByAddingPercentEscapes
../../breakpad/src/common/mac/HTTPMultipartUpload.m:207:29: error: 'sendSynchronousRequest:returningResponse:error:' is deprecated: first deprecated in iOS 9.0 - Use [NSURLSession dataTaskWithRequest:completionHandler:] (see NSURLSession.h [-Werror,-Wdeprecated-declarations]
data = [NSURLConnection sendSynchronousRequest:req
^
../../breakpad/src/client/mac/handler/minidump_generator.cc:158:6: error: 'CFPropertyListCreateFromXMLData' is deprecated: first deprecated in iOS 8.0 - Use CFPropertyListCreateWithData instead. [-Werror,-Wdeprecated-declarations]
(CFPropertyListCreateFromXMLData(NULL, data, kCFPropertyListImmutable,
^
BUG=https://bugs.chromium.org/p/google-breakpad/issues/detail?id=675
BUG=569158
R=mark@chromium.org
Review URL: https://codereview.chromium.org/1563223004 .
|
|
|
|
|
|
|
|
| |
Also fix lint errors.
R=mark@chromium.org
Review URL: https://codereview.chromium.org/1562273002 .
|
|
|
|
|
|
|
|
|
|
|
|
| |
cpu_features_entries is empty on AArch64 and causes tautological-compare
warning when compiling with Clang.
A=dskiba@google.com
Original Review: https://codereview.chromium.org/1562223002/
BUG=chromium:539781
Review URL: https://codereview.chromium.org/1566893002 .
|
|
|
|
|
|
|
|
|
|
| |
A=thakis@chromium.org
BUG=chromium:574817
Original Review: https://codereview.chromium.org/1562983002/
R=thakis@chromium.org
Review URL: https://codereview.chromium.org/1563043002 .
|
|
|
|
|
|
|
|
| |
A=thakis@chromium.org
BUG=chromium:573250
Original Review: https://codereview.chromium.org/1551963002/
Review URL: https://codereview.chromium.org/1551983002 .
|
|
|
|
|
|
|
|
|
| |
A=thakis@chromium.org
Original Review: https://codereview.chromium.org/1550933002/
R=thakis@chromium.org
Review URL: https://codereview.chromium.org/1554613002 .
|
|
|
|
|
|
|
|
|
| |
Patch by Scott Hancher
BUG=
R=mark@chromium.org
Review URL: https://codereview.chromium.org/1527363003 .
|
|
|
|
|
|
|
|
|
| |
file ends with 0
R=ivanpe@chromium.org
BUG=https://bugs.chromium.org/p/google-breakpad/issues/detail?id=668
Review URL: https://codereview.chromium.org/1482363003 .
|
|
|
|
|
|
|
|
|
|
|
|
| |
dump_syms generates a lot of warnings.
This CL puts logging behind a command line flag
URL=https://android-review.googlesource.com/181558
BUG=b:25460551
BUG=google-breakpad:441
R=mark@chromium.org
Review URL: https://codereview.chromium.org/1437763002 .
|
|
|
|
|
|
| |
R=ted.mielczarek@gmail.com
Review URL: https://codereview.chromium.org/1137423004 .
|
|
|
|
|
|
|
| |
R=mark@chromium.org
BUG=
Review URL: https://codereview.chromium.org/1438483002 .
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
This works around a bug in M that prevents Breakpad from using
ftruncate() in the renderer process.
To do this, skip the calls to ftruncate() when allocating bigger
minidump files and strictly depends on write() to append to the end.
It might be less efficient but this is probably less of an issue on
SD cards. It is much better than not getting crash reports.
BUG=542840
Original CL: https://codereview.appspot.com/273880044/
Original CL Author: acleung@chromium.org
Review URL: https://codereview.chromium.org/1407233016 .
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
There is an issue in StackwalkerAMD64::GetCallerByFramePointerRecovery.
Occasionally it produces invalid frames (instruction pointer == 0) which
prevents the AMD64 stack walker from proceeding to do stack scanning and
instead leads to premature termination of the stack walking process.
For more details: http://crbug/537444
BUG=
R=mark@chromium.org
Review URL: https://codereview.chromium.org/1408973002 .
|
|
|
|
|
|
| |
R=mark@chromium.org
Review URL: https://codereview.chromium.org/1402453006 .
|
|
|
|
|
|
|
|
|
|
|
|
| |
Debug Gecko builds don't build with -D_DEBUG, so the ifdef in
crash_generation_server doesn't work right. The MSDN documentation for
assert says that it's enabled based on the absence of the NDEBUG define,
so using that seems sensible.
R=thestig@chromium.org
BUG=
Review URL: https://codereview.chromium.org/1398453002 .
|
|
|
|
|
|
|
|
| |
A=David Major <dmajor@mozilla.com>
BUG=https://bugzilla.mozilla.org/show_bug.cgi?id=1048091
R=ted@mielczarek.org
Review URL: https://bugzilla.mozilla.org/show_bug.cgi?id=1048091 .
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
than 2013.
The Windows client gyp files were missing proc_maps_linux.cc for the
unittest build. Adding that revealed some build errors due to it
unconditionally including <inttypes.h>. Removing the workarounds in
breakpad_types.h (and a few other places) made that build, which means
that Visual C++ 2013 is now our minimum supported version of MSVC.
Additionally I tried building with VC++ 2015 and fixed a few warnings
(which were failing the build because we have /WX enabled) to ensure
that that builds as well.
BUG=https://code.google.com/p/google-breakpad/issues/detail?id=669
R=mark@chromium.org
Review URL: https://codereview.chromium.org/1353893002 .
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
Chrome started hitting some crashes in v8 jitted code which happens to be
non ABI compliant and debuggers (including WinDBG) are unable to produce
meaningful stack traces.
The Breakpad stack walker has some builtin heuristics to deal with such cases.
More specifically, when unable to find a good parent frame, it scans the raw
stack to find a suitable parent frame. The max scan size was set at 30
pointers which was (apparently) not enough to recover in this case.
I'm increasing it to 40 pointers. I confirmed that at 34 pointers it was able
to recover however I'm setting it to 40 in order to it some slack.
I needed to update two unittests which were expecting the previous scan limit.
BUG=
R=mark@chromium.org
Review URL: https://codereview.chromium.org/1379433005 .
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
This patch allows dump_syms to handle S_THREAD_LOCAL_ZEROFILL
and S_GB_ZEROFILL section in the same way as the more common
S_ZEROFILL section. Previously, dump_syms would fail to dump
a binary containing a __DATA,__thread_bss section, because it
tried to look up its data (and failed).
R=mark@chromium.org
Review URL: https://codereview.chromium.org/1369233003 .
Patch from Pavel Labath <labath@google.com>.
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
Although strictly the GPU fingerprint is defined by the build fingerprint,
there is not currently a straightforward mapping from build fingerprint
to useful GPU / GL driver information.
In order to aid debugging of WebView crashes that occur in GL drivers,
and to better understand the range of drivers and versions for feature
blacklisting purposes, it is useful to have GPU fingerprints in breakpad
microdumps.
Landing this patch on behalf of Tobias Sargeant<tobiasjs@chromium.org>
BUG=chromium:536769
R=primiano@chromium.org, thestig@chromium.org
Review URL: https://codereview.chromium.org/1334473003 .
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
On Android the size of the alternate stack can be very small (8k).
Even if breakpad uses sigaltstack to increase the size of the alternate
stack during initialization, that call affects only the main thread.
On Android, the libc's pthread initializer reset the sigaltstack to 8k.
When entering a signal handler, the kernel typically pushes the context
on the alternate stack. On arm64, sizeof(CrashContext) is ~5k, which
leaves 3k of usable stack for breakpad.
On top of that, breakpad allocates another struct CrashContext on the
stack. In the case of Android arm64, then, breakpad ends up using
5k + 5k > 8k of stack, which causes a stack overflow.
This got unnoticed in Android L, as the alternate stack didn't have
red-zones between them, so breakpad was often happily overflowing onto
the next thread's stack. This is not the case anymore [1].
This CL moves the CrashContext into a global variable. It should be
safe as the ExceptionHandlers are serialized on a mutex.
[1] https://android.googlesource.com/platform/bionic/+/595752f623ae88f7e4193a6e531a0805f1c6c4dc
BUG=374
R=mark@chromium.org
Review URL: https://codereview.chromium.org/1354923002 .
|
| |
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
We're working on building our Firefox Mac builds as a Linux cross-compile
(https://bugzilla.mozilla.org/show_bug.cgi?id=921040) and we need symbol
dumping to work. This change ports the Mac dump_syms tool to build and work
on Linux. I've tested it and it produces identical output to running the
tool on Mac.
The bulk of the work here was converting src/common/mac/dump_syms.mm and
src/tools/mac/dump_syms/dump_syms_tool.mm from ObjC++ to C++ and removing
their use of Foundation classes in favor of standard C/C++.
This won't compile out-of-the-box on Linux, it requires some Mac system
headers that are not included in this patch. I have those tentatively in
a separate patch to land in Gecko
(http://hg.mozilla.org/users/tmielczarek_mozilla.com/mc/rev/5fb8da23c83c),
but I wasn't sure if you'd be interested in having them in the Breakpad tree.
We could almost certainly pare down the set of headers included there, I
didn't spend too much time trying to minimize them (we primarily just need
the Mach-O structs and a few associated bits).
I just realized that this patch is missing updating the XCode project files
(ugh). I'll fix that up in a bit.
R=mark@chromium.org
BUG=https://bugzilla.mozilla.org/show_bug.cgi?id=543111
Review URL: https://codereview.chromium.org/1340543002 .
|