aboutsummaryrefslogtreecommitdiff
Commit message (Collapse)AuthorAgeFilesLines
* libdisasm: add upstream/license detailsMike Frysinger2017-02-142-0/+146
| | | | | | | | The license file comes from the upstream libdisasm tarball/repo. Change-Id: I04a4002db72f778dd67dbcd71d3b5d1205a8c21d Reviewed-on: https://chromium-review.googlesource.com/441884 Reviewed-by: Ted Mielczarek <ted@mielczarek.org>
* There is no need to use the main queue just for perform selector.George Kola2017-02-131-34/+31
| | | | | | | | | | | | | | | | | | | | | | | | | We were using the main queue to queue up a perform selector and then the code [self sendStoredCrashReports] was immediately doing a dispatch_async. This unnecessary thread switching is not needed. We simplify the above logic and use dispatch_after to queue the block on the internal queue after a delay Note that main queue is typically more loaded and it is better for non-UI code to not use the main queue. This may also help improve crash log upload. This change also switches from @synchronized to dispatch_once as that is faster Reference: http://googlemac.blogspot.com/2006/10/synchronized-swimming.html BUG= Change-Id: I81035149cbbf13a3058ca3a11e6efd23980f19ad Reviewed-on: https://chromium-review.googlesource.com/441364 Reviewed-by: Joshua Peraza <jperaza@chromium.org>
* Appveyor CI for Windows MSVS buildJon Turney2017-02-134-1/+95
| | | | | | | | | | | | | | | | | | | | | | Add a .gyp file for building all windows tools, and add hook to run gyp to create corresponding .sln files. This doesn't try to build for platform:x64. This fails due to various errors caused by the assumption that size_t can be converted to an unsigned int without loss of information, which is not true on Windows x64 (LLP64), where size_t is 64 bits, but int is only 32 bits. There are test failures. client_tests failures are as described in [1]. dump_syms_unittest are as discussed in the description of [2]. [1] https://bugs.chromium.org/p/google-breakpad/issues/detail?id=520 [2] https://codereview.chromium.org/1782453003 BUG= Change-Id: I965244eb3746f87f30160fd0577e1cc9eb7a8b08 Reviewed-on: https://chromium-review.googlesource.com/441026 Reviewed-by: Mike Frysinger <vapier@chromium.org>
* turn on -Werror generallyMike Frysinger2017-02-132-0/+2
| | | | | | | | | | This moves us to being warning free by default rather than being free of some specific warnings. This doesn't turn on any new warnings though. Change-Id: I60bb79d1790e85ec4618b3548dad6de5d9bf8ab5 Reviewed-on: https://chromium-review.googlesource.com/438565 Reviewed-by: Mark Mentovai <mark@chromium.org>
* processor: drop set-but-unused variableMike Frysinger2017-02-121-4/+2
| | | | | | Change-Id: Idf3fe363c76734caa3e6a6cc20a53fd1d661188d Reviewed-on: https://chromium-review.googlesource.com/438564 Reviewed-by: Mark Mentovai <mark@chromium.org>
* macho_reader_unittest: use EXPECT_FALSEMike Frysinger2017-02-121-2/+2
| | | | | | | | | | | | | | | | | | | This avoids compile time errors: In file included from ./src/testing/googletest/include/gtest/gtest.h:1874:0, from ./src/breakpad_googletest_includes.h:33, from src/common/mac/macho_reader_unittest.cc:39: src/common/mac/macho_reader_unittest.cc: In member function 'virtual void LoadCommand_SegmentBE32_Test::TestBody()': ./src/testing/googletest/include/gtest/internal/gtest-internal.h:133:55: error: converting 'false' to pointer type for argument 1 of 'char testing::internal::IsNullLiteralHelper(testing::internal::Secret*)' [-Werror=conversion-null] (sizeof(::testing::internal::IsNullLiteralHelper(x)) == 1) ^ ... src/common/mac/macho_reader_unittest.cc:1117:3: note: in expansion of macro 'EXPECT_EQ' EXPECT_EQ(false, actual_segment.bits_64); Change-Id: I0cf88160dbe17b0feebed3c91ad65491b81023fd Reviewed-on: https://chromium-review.googlesource.com/439004 Reviewed-by: Mark Mentovai <mark@chromium.org>
* windows: fix build on pre-Win10 systemsMike Frysinger2017-02-111-0/+5
| | | | | | | | | | The use of DBG_PRINTEXCEPTION_WIDE_C was added for Win10 support, but that define doesn't exist in older versions which means we fail to build. Put it behind an ifdef check to work everywhere. Change-Id: Ibab8bddd5c19b4b50e356f59edeb3873c3104569 Reviewed-on: https://chromium-review.googlesource.com/441525 Reviewed-by: Mark Mentovai <mark@chromium.org>
* windows: update gtest/gmock pathsMike Frysinger2017-02-116-20/+22
| | | | | | | | | The Windows build has rotted a bit with the gtest/gmock updates. Update all of the paths to fix things up again. Change-Id: Id67ce76abfd331c0543aa4bd1138e9cc13a18c75 Reviewed-on: https://chromium-review.googlesource.com/441584 Reviewed-by: Mark Mentovai <mark@chromium.org>
* aclocal: regenerate properlyMike Frysinger2017-02-085-166/+5
| | | | | | | | | Rather than manually include m4 files in configure.ac, let aclocal do its thing and manage aclocal.m4 automatically for us. Change-Id: I50689ec78a85651949aab104e7f4de46b14bca5a Reviewed-on: https://chromium-review.googlesource.com/438544 Reviewed-by: Mark Mentovai <mark@chromium.org>
* fix write() unused-result warningMike Frysinger2017-02-083-4/+6
| | | | | | | | | | | src/client/linux/microdump_writer/microdump_writer_unittest.cc:98:47: error: ignoring return value of 'ssize_t write(int, const void*, size_t)', declared with attribute warn_unused_result [-Werror=unused-result] write(STDOUT_FILENO, identifiable_string, 0); Change-Id: I3f2305fbec0dbd1464de9aeff051e7cba2ee69a2 Reviewed-on: https://chromium-review.googlesource.com/438545 Reviewed-by: Ivan Penkov <ivanpe@chromium.org>
* Fixed leak of unloaded module lists.Joshua Peraza2017-02-072-2/+6
| | | | | | | | BUG= Change-Id: I6d03820082f793a2eac3c3c2abd184b4acf66aa4 Reviewed-on: https://chromium-review.googlesource.com/438755 Reviewed-by: Ivan Penkov <ivanpe@chromium.org>
* Address post-submit review comments related to CL #430050Tobias Sargeant2017-02-072-6/+13
| | | | | | | | | | See: https://chromium-review.googlesource.com/c/430050/ BUG=664460 Change-Id: I3cbfbd5b00725bd501f06427eebd976267c4f617 Reviewed-on: https://chromium-review.googlesource.com/438444 Reviewed-by: Primiano Tucci <primiano@chromium.org>
* md5: fix strict aliasing warningsMike Frysinger2017-02-071-2/+2
| | | | | | Change-Id: I64f4570610c625b1325249fd5fa1b9edc3a89ae4 Reviewed-on: https://chromium-review.googlesource.com/438864 Reviewed-by: Ivan Penkov <ivanpe@chromium.org>
* autotools: refresh config.{sub,guess}Mike Frysinger2017-02-062-64/+100
|
* Remove debugging fprintf in unittest code that prevents rolling breakpadTobias Sargeant2017-02-061-1/+0
| | | | | | | | BUG=664460 Change-Id: I40d8567c659e97415db65cb308c0d39391c44353 Reviewed-on: https://chromium-review.googlesource.com/438364 Reviewed-by: Primiano Tucci <primiano@chromium.org>
* Wire up stack sanitization and skipping to WriteMinidumpTobias Sargeant2017-02-035-40/+232
| | | | | | | | | | | | This makes the parameters stored in the MinidumpDescriptor structure functional for minidumps, analogously to how they are applied to microdumps. BUG=664460 Change-Id: I7578e7a1638cea8f0445b18d4bbdaf5e0a32d808 Reviewed-on: https://chromium-review.googlesource.com/435380 Reviewed-by: Robert Sesek <rsesek@chromium.org>
* Revert "Several fixes for broken Mac build"Roman Margold2017-02-014-10/+11
| | | | This reverts commit 5c521532fc0a1b65f42c0d61d2da206eadf318b8.
* Revert "iOS client identifies itself via URL params"Roman Margold2017-02-011-37/+0
| | | | This reverts commit 262a3f50fe5948c2570bbce2cd696e253a88af79.
* iOS client identifies itself via URL paramsRoman Margold2017-02-011-0/+37
| | | | Recently, Crash started applying quotas for crash report uploads to protect the service and its client products from misbehaving product or product version. For the protection to be effective, products need to identify themselves during report upload via URL parameters. This new code makes iOS apps using Breakpad provide the parameters automatically.
* Several fixes for broken Mac buildRoman Margold2017-02-014-11/+10
|
* Sanitize dumped stacks to remove data that may be identifiable.Tobias Sargeant2017-01-319-62/+362
| | | | | | | | | | | | | | | In order to sanitize the stack contents we erase any pointer-aligned word that could not be interpreted as a pointer into one of the processes' memory mappings, or a small integer (+/-4096). This still retains enough information to unwind stack frames, and also to recover some register values. BUG=682278 Change-Id: I541a13b2e92a9d1aea2c06a50bd769a9e25601d3 Reviewed-on: https://chromium-review.googlesource.com/430050 Reviewed-by: Robert Sesek <rsesek@chromium.org>
* Fixed a bug where cv record size was not correctly checked.Joshua Peraza2017-01-301-4/+6
| | | | | | | | BUG= Change-Id: I6c1d78cfe344c7b90a03f6df35193d67623bfd89 Reviewed-on: https://chromium-review.googlesource.com/434094 Reviewed-by: Ivan Penkov <ivanpe@chromium.org>
* Change symbol upload message to include 'breakpad'Bruce Dawson2017-01-281-1/+1
| | | | | | | | | | | | | | | | The breakpad symbol uploader prints messages of this form: Uploaded symbols for windows-x86/eventlog_provider.dll.pdb/... This is confusing because many people see this message and assume that symbols are being uploaded to a symbol server. This changes the message to clarify what is happening. BUG=677226 Change-Id: Id6fdd8497d0cb97be43c4af010058aab9d84375c Reviewed-on: https://chromium-review.googlesource.com/434187 Reviewed-by: Mark Mentovai <mark@chromium.org>
* Populate stack frames with unloaded module info.Joshua Peraza2017-01-1915-13/+161
| | | | | | | | | | | | | | | | | | | This CL hits lots of source files because: 1. An update to the CodeModule virtual class. I added an is_loaded method to specify whether the module is loaded. There were several mocks/test classes that needed to be updated with an implementation. An alternative to this route would be to modify MinidumpUnloadedModule::code_file to prepend "Unloaded_" to the module name. 2. Added an unloaded_modules parameter to StackFrameSymbolizer::FillSourceLineInfo. BUG= Change-Id: Ic9c7f7c7b7e932a154a5d4ccf292c1527d8da09f Reviewed-on: https://chromium-review.googlesource.com/430241 Reviewed-by: Ivan Penkov <ivanpe@chromium.org>
* Add API to skip dump if crashing thread doesn't reference a given module (2)Tobias Sargeant2017-01-191-0/+8
| | | | | | | | | | | Follow-up CL to add relevant code to the copy constructor and assignment operator for MinidumpDescriptor BUG=664460 Change-Id: I71c0ad01d8686a9215a718cebc9d11a215ea342c Reviewed-on: https://chromium-review.googlesource.com/430711 Reviewed-by: Robert Sesek <rsesek@chromium.org>
* Add API to skip dump if crashing thread doesn't reference a given moduleTobias Sargeant2017-01-1810-62/+149
| | | | | | | | | | | | | | | | | | | | | | | | This CL makes it possible to skip a dump if the crashing thread doesn't have any pointers to a given module. The concrete use case is WebView where we would like to skip generating microdump output when webview is unreferenced by the stack and thus cannot be responsible for the crash in a way that would be debuggable. The range of interesting addresses is chosen by examining the process mappings to find the one that contains a pointer that is known to be in the right shared object (i.e. an appropriately chosen function pointer) passed from the client. If the extracted stack does not contain a pointer in this range, then we do not generate a microdump. If the stack extraction fails, we still generate a microdump (without a stack). BUG=664460 Change-Id: If19406a13168264f7751245fc39591bd6cdbf5df Reviewed-on: https://chromium-review.googlesource.com/419476 Reviewed-by: Robert Sesek <rsesek@chromium.org> Reviewed-by: Primiano Tucci <primiano@chromium.org>
* Added classes to support reading unloaded module lists in minidumps.Joshua Peraza2016-12-166-1/+685
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | The implementations of Module/UnloadedModule and ModuleList/UnloadedModuleList are very similar. They have been made separate classes because they operate on different structs, complicating factoring code into a base class and have sufficiently different implementation that templates would not be suitable. When unloaded modules have partially overlapping ranges, the module shrink down feature is used to move the start of the higher range to the end of the lower range. If two unloaded modules overlap identically, the second module will not be added to the range map and the failure ignored. Places where MinidumpUnloadedModule differs from MinidumpModule: code_identifier: the android/linux case is deleted since cv_records never exist. debug_file/debug_identifier/version: always return empty strings. Read: an expected size is provided as opposed to MD_MODULE_SIZE. A seek is used if there are extra, unused bytes. Places where MinidumpUnloadedModuleList differs from MinidumpModuleList: Read: entry and header size is provided in the header in addition to count. This changes the checks and handling of padding. Failures from StoreRange are ignored. GetMainModule: always returns NULL. BUG= Change-Id: I52e93d3ccc38483f50a6418fede8b506ec879aaa Reviewed-on: https://chromium-review.googlesource.com/421566 Reviewed-by: Joshua Peraza <jperaza@chromium.org>
* Fix unit tests expecting no output when a microdump is suppressed.Tobias Sargeant2016-12-121-9/+9
| | | | | | | | BUG= Change-Id: Ie4d190c68ecbd8709874a3f1ceb872b94b36914f Reviewed-on: https://chromium-review.googlesource.com/419036 Reviewed-by: Primiano Tucci <primiano@chromium.org>
* Fix sign-compare compiler warning in MicrodumpWriterTestMike Percy2016-12-101-2/+2
| | | | | | | | | | | | | | Commit 7a8980997d0e0dcf3f3a5d8ccf3c1d8c2840ea27 introduced additional tests into MicrodumpWriterTest, two of which throw warnings which break "make check" under default settings on Linux, because the Makefiles are configured with -Werror=sign-compare. This patch just makes the signedness of the assertion arguments match. Change-Id: Ib522f44205c84f91bc9b93276fad60ebbf005f60 Reviewed-on: https://chromium-review.googlesource.com/418938 Reviewed-by: Tobias Sargeant <tobiasjs@chromium.org> Reviewed-by: Mike Frysinger <vapier@chromium.org>
* crash_generation: fix bad call to closeMike Frysinger2016-12-091-1/+1
| | | | | | | | | If signal_fd is -1 still, we end up calling close(-1). Not generally a problem, but it's bad form, and coverity is upset by it. Change-Id: I46f9c7ca4be7b43af5b609dd8e3f03a0700af418 Reviewed-on: https://chromium-review.googlesource.com/414544 Reviewed-by: Primiano Tucci <primiano@chromium.org>
* Log a message when microdump output is suppressed.Tobias Sargeant2016-12-071-0/+1
| | | | | | | Change-Id: I11542ea9b702055e8f0b99c26cad2fea8681bce0 Reviewed-on: https://chromium-review.googlesource.com/417824 Reviewed-by: Mike Frysinger <vapier@chromium.org> Reviewed-by: Primiano Tucci <primiano@chromium.org>
* Remove comparison of this with nullptrOrgad Shaneh2016-12-062-13/+9
| | | | | | | | GCC6 optimizes it out, leading to crash. Change-Id: I8425d456c1364929d135ce3860121b8098bab1f7 Reviewed-on: https://chromium-review.googlesource.com/413120 Reviewed-by: Mike Frysinger <vapier@chromium.org>
* Do not generate a microdump if there are no webview pointers on the stack.Tobias Sargeant2016-12-013-108/+208
| | | | | | | | | | | | | | The stack interest range is passed in MicrodumpExtraInfo from the client. If the extracted stack does not contain a pointer in this range, then we assume that this is not a WebView crash, and do not generate a microdump. If the stack extraction fails, we still generate a microdump (without a stack). BUG=664460 Change-Id: Ic762497f76f074a3621c7ec88a8c20ed768b9211 Reviewed-on: https://chromium-review.googlesource.com/412781 Reviewed-by: Primiano Tucci <primiano@chromium.org>
* Comment out an unused function argumentOrgad Shaneh2016-11-291-1/+1
| | | | | | Change-Id: I09c90d496edc67d4cad3e2b99f4347dc04713bdb Reviewed-on: https://chromium-review.googlesource.com/414357 Reviewed-by: Mike Frysinger <vapier@chromium.org>
* microdump_stackwalk_test: fix bashism in testMike Frysinger2016-11-254-2/+4
| | | | | | | | | These are /bin/sh scripts, and `source` is a bash-specific command. Switch to the portable `.` command instead. Change-Id: I51d8253b26aa61c130bb5fdc4789f8d623c6d9db Reviewed-on: https://chromium-review.googlesource.com/414524 Reviewed-by: Primiano Tucci <primiano@chromium.org>
* Upgrade google test to 1.8.0Orgad Shaneh2016-11-234-240/+250
| | | | | | | | Some test fail on recent debian with 1.7.0 due to crashes. Change-Id: Ia25625c27968671e24826a3eeae70dbfa5c67c95 Reviewed-on: https://chromium-review.googlesource.com/412701 Reviewed-by: Mike Frysinger <vapier@chromium.org>
* Update linksOrgad Shaneh2016-11-1812-96/+71
| | | | | | | | | | code.google.com is obsolete. Fix all broken markdown links while at it. Change-Id: I6a337bf4b84eacd5f5c749a4ee61331553279009 Reviewed-on: https://chromium-review.googlesource.com/411800 Reviewed-by: Mike Frysinger <vapier@chromium.org>
* GitIgnore: Add dump_syms_macOrgad Shaneh2016-11-141-0/+1
| | | | | | Change-Id: I4bbeb614e7477a42f096e1ba3fa7c00344827e0c Reviewed-on: https://chromium-review.googlesource.com/411105 Reviewed-by: Mike Frysinger <vapier@chromium.org>
* Add a crash reason string for the simulated crashpad exception codePierre-Antoine Manzagol2016-11-103-3/+14
| | | | | | | | BUG= Change-Id: I19a1abf1d00f208943db1c362cc426ca8bd2068e Reviewed-on: https://chromium-review.googlesource.com/409632 Reviewed-by: Mark Mentovai <mark@chromium.org>
* Allow compiling the google-breakpad code using a global ::string class ↵Ivan Penkov2016-11-0811-15/+16
| | | | | | | | | | | | instead of std::string. For more details take a look at common/using_std_string.h BUG= Change-Id: Ifebfc57f691ef3a3bef8cfed7106c567985edffc Reviewed-on: https://chromium-review.googlesource.com/399738 Reviewed-by: Mark Mentovai <mark@chromium.org>
* Basic handling of CIE version 4 in dwarf readingScott Graham2016-11-025-8/+147
| | | | | | | | | | | | | | | | | CIE looks like it's been emitted by clang since ~May 2015 [1]. This means that we didn't have any CFI because this parse aborted, which meant that all stack walks reverted to stack scanning. Allow expected values for address size and segment descriptor size through so that dump_syms can generate at least somewhat reasonable data. [1]: http://lists.llvm.org/pipermail/llvm-commits/Week-of-Mon-20150518/277292.html R=mark@chromium.org BUG=chromium:627529 Change-Id: I6dc92f51c4afd25c2adff92c09ccb8bb03bf9112 Reviewed-on: https://chromium-review.googlesource.com/406012 Reviewed-by: Mark Mentovai <mark@chromium.org>
* minidump-2-core: add more control over filenamesMike Frysinger2016-11-011-24/+93
| | | | | | | | | | | | | | | | | The code has been rewriting the location of the shared lib lookup completely which breaks normal sysroot usage with gdb. Split out the behavior into dedicated flags so people can opt into it. You can see examples of -i/-f in the usage() text. We also change the -S behavior so that it's no longer enabled by default -- if people want /var/lib/breakpad/, they can pass the -S flag explicitly. BUG=chromium:598947 Change-Id: Ic81726c27b4ad6c271c70696f2ac62798f07ccfb Reviewed-on: https://chromium-review.googlesource.com/402909 Reviewed-by: Mark Mentovai <mark@chromium.org>
* Fix pointer arithmetic in UTF8ToUTF16CharHans Wennborg2016-10-271-3/+3
| | | | | | | | | | Found by PVS-Studio! BUG=chromium:660198 Change-Id: I2605de2b1499f85c6e01d19e87e9eeb6af8486f3 Reviewed-on: https://chromium-review.googlesource.com/404552 Reviewed-by: Mike Frysinger <vapier@chromium.org>
* Generate reason for bad function table exceptionMike Wittman2016-10-272-0/+5
| | | | | | | | | | This exception is being seen in Chrome during stack unwinding. BUG= Change-Id: Ica3f721ca605dff835ffc3814c60bab9f6f9b192 Reviewed-on: https://chromium-review.googlesource.com/404332 Reviewed-by: Mark Mentovai <mark@chromium.org>
* minidump-2-core: add an -o flag for controlling core outputMike Frysinger2016-10-261-24/+46
| | | | | | | | | | | Always writing to stdout makes it hard to debug, and hard to use in some script environments. Add an explicit -o flag to make it easier. BUG=chromium:598947 Change-Id: I79667d033c8bdc8412d3a44fe3557d65f704968f Reviewed-on: https://chromium-review.googlesource.com/403988 Reviewed-by: Mark Mentovai <mark@chromium.org>
* minidump-2-core: rewrite argument processingMike Frysinger2016-10-261-85/+134
| | | | | | | | | | | | | | | | This uses the same general framework as other minidump tools by using getopt to parse command line options, and then passing the parsed state around as a struct rather than via globals. This does change the --sobasedir flag to -S because we don't support getopt_long anywhere in the tree. Unfortunate, but better to match all the other breakpad tools which only accept short options. BUG=chromium:598947 Change-Id: I473081a29a8e3ef07a370848343f1a9e6681fd4e Reviewed-on: https://chromium-review.googlesource.com/402908 Reviewed-by: Mark Mentovai <mark@chromium.org>
* Don't demangle Rust symbols by default, but allow linking to rust-demangle.Ted Mielczarek2016-10-259-54/+551
| | | | | | | | | | | | | | | | | The Rust compiler uses GCC C++ name mangling, but it has another layer of encoding so abi::cxa_demangle doesn't produce great results. This patch changes dump_syms to dump unmangled names by default so that consumers can demangle them after-the-fact. It also adds a tiny bit of support for linking against a Rust library I wrote that can demangle Rust symbols nicely: https://github.com/luser/rust-demangle-capi BUG= Change-Id: I63a425035ebb7ac516f067fed2aa782849ea9604 Reviewed-on: https://chromium-review.googlesource.com/402308 Reviewed-by: Mark Mentovai <mark@chromium.org>
* Fix iterating over the MDXStateFeature entries on 32-bit hostsGabriele Svelto2016-10-181-1/+1
| | | | | | | | | | | On 32-bit hosts the new code for dumping version 5 of the MDRawMiscInfo structure uses a 32-bit left shift to select flags corresponding to the entries in the MDXStateFeature array. Since the array is made of 64 element this automatically skipped half of it. Change-Id: Ic4e3beaf6c56083524b33da9a396c14eec0d2bd2 Reviewed-on: https://chromium-review.googlesource.com/396107 Reviewed-by: Ted Mielczarek <ted@mielczarek.org>
* Also treat DBG_PRINTEXCEPTION* as debug exceptionsTim Angus2016-10-181-1/+3
| | | | | | | | | | | | | Windows 10 now raises an exception when OutputDebugString* are called: (https://ntquery.wordpress.com/2015/09/07/windows-10-new-anti-debug-outputdebugstringw/) This change ignores these exception types such that they're not falsely identified as a crash. BUG= Change-Id: I1326212662d46e16407681d5ea6377f63ee188ce Reviewed-on: https://chromium-review.googlesource.com/398998 Reviewed-by: Mark Mentovai <mark@chromium.org>
* Provide initial EBX value to FPO frame data evaluatorScott Graham2016-10-142-6/+153
| | | | | | | | | | | | | EBX is sometimes used in "WIN FRAME 4" programs. Not providing the initial value was causing the evaluation in some frames of ntdll, resulting in a fallback to scanning and a failed stack walk. R=mark@chromium.org BUG=chromium:651453 Change-Id: I94a8184e1eed72b0d0e3212fe323fbdd10d56da5 Reviewed-on: https://chromium-review.googlesource.com/398059 Reviewed-by: Mark Mentovai <mark@chromium.org>