All currently compiled-in/enabled tests should be reliably passing now,
so add Fuchsia bots to the CQ. (Of course, there's a lot of
functionality still compiled out).
Bug: crashpad:196
Change-Id: I1544798afefd6a505200c9ae38253d73eb6497ef
Reviewed-on: https://chromium-review.googlesource.com/923425
Commit-Queue: Scott Graham <scottmg@chromium.org>
Reviewed-by: Mark Mentovai <mark@chromium.org>
This ensures the symbol is not exposed in the binaries final symbol
table. .globl needs to be kept so that it can still be linked against
(in this case, by crashpad_info.cc.).
(Tested on Fuchsia, hopefully functional elsewhere...)
Bug: crashpad:196
Change-Id: I8c6b26cdd742a1c040779884fd97a8a34068dbdc
Reviewed-on: https://chromium-review.googlesource.com/924337
Reviewed-by: Mark Mentovai <mark@chromium.org>
Commit-Queue: Scott Graham <scottmg@chromium.org>
Fuchsia does not currently support any sort of file locking. Until a
lock server can be implemented, compile out the calls to flock(). In the
one current non-test user of locking (Settings) add a
pseudo-implementation that will DCHECK if there is ever contention on
the lock.
Bug: crashpad:217, crashpad:196
Change-Id: Ifdf7e00886ad7e7778745f1ae8f0ce2a86f0ae3b
Reviewed-on: https://chromium-review.googlesource.com/924312
Commit-Queue: Scott Graham <scottmg@chromium.org>
Reviewed-by: Mark Mentovai <mark@chromium.org>
These tests needed to be updated to expose CrashpadInfo in the same way
as the main CrashpadInfo g_crashpad_info is found on
Linux/Android/Fuchsia.
Unfortunately, while the tests pass on Fuchsia when run in isolation,
the implementation of dlclose() on Fuchsia currently does nothing. So,
if the full test suite is run, there's interference between the test
modules (i.e. the values in _small vs. the values in _large), so the
tests fail.
I filed ZX-1728 upstream about this to see if it might be implemented,
or if the test will need to spawn a clean child to do the module load
tests in.
Bug: crashpad:196
Change-Id: I9ee01b142a29c508c6967dc83da824afa254d379
Reviewed-on: https://chromium-review.googlesource.com/923182
Reviewed-by: Mark Mentovai <mark@chromium.org>
Commit-Queue: Scott Graham <scottmg@chromium.org>
Fuchsia errors out in rename() when source == dest. I believe this is
incorrect according to
http://pubs.opengroup.org/onlinepubs/9699919799/functions/rename.html,
but it's also relatively easy to work around in our code, and this fixes
CrashReportDatabaseTest.RequestUpload.
This is ZX-1729 upstream.
Bug: crashpad:196
Change-Id: I27473183b04484e146a7bd9e87e60be3aeff1932
Reviewed-on: https://chromium-review.googlesource.com/923708
Reviewed-by: Mark Mentovai <mark@chromium.org>
Commit-Queue: Scott Graham <scottmg@chromium.org>
In preference to (the reverted)
https://chromium-review.googlesource.com/c/crashpad/crashpad/+/923178
this does not share implementation with the tests in
snapshot/crashpad_info_client_options_test.cc. This is not done because
those tests use faked CrashpadInfo structures that are intentionally
differently sized than the current defintion of CrashpadInfo, meaning
that the scoped reset could overwrite past the end of the structure.
Not resetting these was causing CrashpadInfoClientOptions tests to fail
on Fuchsia, because dlclose() [legally] doesn't do anything, so
modifying the current binaries CrashpadInfo caused the expected values
from child .sos to be ignored. That could be worked around in that test
too, but it's probably better to clean up the global state in this test
anyway.
Bug: crashpad:196
Change-Id: Ia3f81f1d5872b5ef7d543fcc68b56af4c0b6ca0a
Reviewed-on: https://chromium-review.googlesource.com/923561
Reviewed-by: Joshua Peraza <jperaza@chromium.org>
Commit-Queue: Scott Graham <scottmg@chromium.org>
This reverts commit 4717300fa4cefadeabef64346ba65aa8759d43b8.
Reason for revert: When used in with the size-testing fake CrashpadInfo's, this can overwrite past the end of them.
Original change's description:
> Reset CrashpadInfo after CrashpadInfoReader tests
>
> Not resetting these was causing CrashpadInfoClientOptions tests to fail
> on Fuchsia, because dlclose() [legally] doesn't do anything, so
> modifying the current binaries CrashpadInfo caused the expected values
> from child .sos to be ignored. That could be worked around in that test
> too, but it's probably better to clean up the global state in this test
> anyway.
>
> Bug: crashpad:196
> Change-Id: Ia8119ac7c554bea81e8373e2547faf192c629122
> Reviewed-on: https://chromium-review.googlesource.com/923178
> Commit-Queue: Scott Graham <scottmg@chromium.org>
> Reviewed-by: Joshua Peraza <jperaza@chromium.org>
TBR=scottmg@chromium.org,jperaza@chromium.org
Change-Id: Ia6d8db1ba24c82bb9346210ac8b66d80f42a6925
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: crashpad:196
Reviewed-on: https://chromium-review.googlesource.com/923541
Reviewed-by: Scott Graham <scottmg@chromium.org>
Commit-Queue: Scott Graham <scottmg@chromium.org>
Not resetting these was causing CrashpadInfoClientOptions tests to fail
on Fuchsia, because dlclose() [legally] doesn't do anything, so
modifying the current binaries CrashpadInfo caused the expected values
from child .sos to be ignored. That could be worked around in that test
too, but it's probably better to clean up the global state in this test
anyway.
Bug: crashpad:196
Change-Id: Ia8119ac7c554bea81e8373e2547faf192c629122
Reviewed-on: https://chromium-review.googlesource.com/923178
Commit-Queue: Scott Graham <scottmg@chromium.org>
Reviewed-by: Joshua Peraza <jperaza@chromium.org>
In trying to clear out the end of info when the alleged size is smaller
than the current structure size, we didn't handle the opposite case. We
need to continue the rest of Read() to initialize members, but need to
make sure not to pass a very large (negative -> size_t) length to
memset().
Additionally, I believe it meant to memset from the end of the alleged
size, to the end of the local structure, rather than from the beginning
of the structure.
This repro'd on Fuchsia, but would affect all platforms that use it.
Bug: crashpad:196, crashpad:30
Change-Id: I9c35c834010b5cb26d54156ce8f9bc538dcbf96c
Reviewed-on: https://chromium-review.googlesource.com/923094
Commit-Queue: Scott Graham <scottmg@chromium.org>
Reviewed-by: Joshua Peraza <jperaza@chromium.org>
Placing a 32-bit pointer directly into a .quad results in either an
unsupported relocation error at link time (ARM) or an inability to
load the executable (x86).
Also, only attempt to read a module's CrashpadInfo if an info address
note was found.
Change-Id: I053af3d77eed70af66248be88547656d2b29878a
Reviewed-on: https://chromium-review.googlesource.com/922397
Reviewed-by: Scott Graham <scottmg@chromium.org>
Reviewed-by: Mark Mentovai <mark@chromium.org>
Commit-Queue: Joshua Peraza <jperaza@chromium.org>
Crash report upload is currently the responsibility of the embedding
client (e.g. Chrome) on Android.
Bug: crashpad:30
Change-Id: Ia658ec327783bd6d2ea6d7e279e942f458dd12ef
Reviewed-on: https://chromium-review.googlesource.com/922877
Commit-Queue: Joshua Peraza <jperaza@chromium.org>
Reviewed-by: Mark Mentovai <mark@chromium.org>
- Don't specify a directory in .so load, this allows
CrashpadInfoClientOptions.TwoModules to pass, as it is able to find
its .so.
- Set expected exe name to "app" on Fuchsia, as that's what all binaries
are called when packaged. This fixes Paths.Executable.
Bug: crashpad:196
Change-Id: I6b0a663734e93b76412a5bdba045e73dcfe7e4cf
Reviewed-on: https://chromium-review.googlesource.com/922871
Reviewed-by: Mark Mentovai <mark@chromium.org>
Commit-Queue: Scott Graham <scottmg@chromium.org>
Adds beginning ProcessReader implementation for Fuchsia which currently
only reads modules from the target process. ModuleSnapshotFuchsia
implemented enough to pull out CrashpadInfo, which in turn is passed
through ProcessSnapshotFuchsia, which is enough to get
CrashpadInfoClientOptions.OneModule to pass.
Bug: crashpad:196
Change-Id: I92b82696c464a5ba2e0db2c75aa46fd74b0fa364
Reviewed-on: https://chromium-review.googlesource.com/910324
Commit-Queue: Scott Graham <scottmg@chromium.org>
Reviewed-by: Mark Mentovai <mark@chromium.org>
This CL, based on
https://chromium-review.googlesource.com/c/crashpad/crashpad/+/689745
adds a cross-platform database implementation side-by-side with the
existing macOS and Windows implementations. The generic implementation
is used for Linux, Android and Fuchsia.
The database uses the directory structure from the macOS
implementation, but stores report metadata in companion files for each
report, rather than using filesystem attributes. The database uses
lockfiles (companion files opened with O_EXCL) to protect report access
because they are widely supported across filesystems. Lost lockfiles
are removed after 3 days, along with any reports or metadata they were
protecting.
Bug: crashpad:206
Change-Id: I086e9001350e4446dd2f8c12fd3817377f509d3e
Reviewed-on: https://chromium-review.googlesource.com/919527
Commit-Queue: Joshua Peraza <jperaza@chromium.org>
Reviewed-by: Mark Mentovai <mark@chromium.org>
Embeds the address of g_crashpad_info into a .note section (which is
readable by the generic code to read notes in ElfImageReader).
Unfortunately because the note section is in libclient.a, it would
normally be dropped at link time. To avoid that, GetCrashpadInfo() has
a reference *back* to that section, which in turn forces the linker to
include it, allowing the note reader to find it at runtime.
Previously, it was necessary to have the embedder of "client" figure out
how to cause `g_crashpad_info` to appear in the final module's dynamic
symbol table. With this new approach, there's no manual configuration
necessary, as it's not necessary for the symbol to be exported.
This is currently only implemented in the Linux module reader (and I
believe the current set of enabled tests aren't exercising it?) but it
will also be done this way for the Fuchsia implementation of
ModuleSnapshot.
Bug: crashpad:196
Change-Id: I599db5903bc98303130d11ad850ba9ceed3b801a
Reviewed-on: https://chromium-review.googlesource.com/912284
Commit-Queue: Scott Graham <scottmg@chromium.org>
Reviewed-by: Mark Mentovai <mark@chromium.org>
Reviewed-by: Joshua Peraza <jperaza@chromium.org>
This change adds CrashReportDatabase::UploadReport which owns the
report's file handle during upload. An upload is recorded as a success
by calling RecordUploadComplete(). If RecordUploadComplete() is not
called, the operation is recorded as a failure when the UploadReport is
destroyed.
Bug: crashpad:206
Change-Id: I8385d08d52185ad30b06a3ed054de9812ae006a2
Reviewed-on: https://chromium-review.googlesource.com/917983
Commit-Queue: Joshua Peraza <jperaza@chromium.org>
Reviewed-by: Mark Mentovai <mark@chromium.org>
Reviewed-by: Robert Sesek <rsesek@chromium.org>
This change updates CrashReportDatbase::NewReport objects to own the
file handle associated with the new report, now accessible via a
FileWriter. NewReport's destructor closes its file handle and removes
its new report unless disarmed with FinishedWritingCrashReport,
eliminating the need for CallErrorWritingCrashReport.
Bug: crashpad:206
Change-Id: Iccb5bbc0ebadb07a237ff8eb938389afcfeae2a5
Reviewed-on: https://chromium-review.googlesource.com/916941
Commit-Queue: Joshua Peraza <jperaza@chromium.org>
Reviewed-by: Mark Mentovai <mark@chromium.org>
Reviewed-by: Scott Graham <scottmg@chromium.org>
Pulled out of jperaza's https://crrev.com/c/689745.
Future updates to the CrashReportDatabase would like to be decide on the
Settings location later than the constructor, but still keep the Settings
object embedded inline. To allow this, pass the location FilePath in
Initialize() rather than to the constructor.
Bug: crashpad:206
Change-Id: I8792188314541f6fd0bd04b168d22f8e445bc187
Reviewed-on: https://chromium-review.googlesource.com/916533
Commit-Queue: Scott Graham <scottmg@chromium.org>
Reviewed-by: Mark Mentovai <mark@chromium.org>
Split out of crrev.com/c/689745 by jperaza, with a simple test added.
It is useful for this to be an overload instead of a separate signature
so that code that extracts a UUID string out of a filename can treat it
generically between Windows and non-Windows.
Bug: crashpad:196, crashpad:206
Change-Id: I0d7d84a93d9526d1aae8839179dfe903acca091b
Reviewed-on: https://chromium-review.googlesource.com/916885
Commit-Queue: Scott Graham <scottmg@chromium.org>
Reviewed-by: Mark Mentovai <mark@chromium.org>
Pending a definition of NativeCPUContext, and an implementation of
CaptureContext().
Bug: crashpad:196
Change-Id: Ibd7721cb740d7662379bb6b22e7804738e16c724
Reviewed-on: https://chromium-review.googlesource.com/916902
Reviewed-by: Joshua Peraza <jperaza@chromium.org>
Reviewed-by: Mark Mentovai <mark@chromium.org>
Commit-Queue: Scott Graham <scottmg@chromium.org>
Previously, an error would have been logged twice.
Bug: crashpad:30
Change-Id: I9445c022550ad14497186c6878863fbf72d8cd59
Reviewed-on: https://chromium-review.googlesource.com/911822
Reviewed-by: Mark Mentovai <mark@chromium.org>
Commit-Queue: Joshua Peraza <jperaza@chromium.org>
Previously, the mac version was under client/ and win under util/win/.
This cl brings them all together under util/misc/ and combines common
test code.
Bug: crashpad:30
Change-Id: Idf0d0158b969d5aa9802dfc8c21f73041b2bcc6c
Reviewed-on: https://chromium-review.googlesource.com/907755
Reviewed-by: Mark Mentovai <mark@chromium.org>
Commit-Queue: Joshua Peraza <jperaza@chromium.org>
Bug: crashpad:79
Change-Id: I1836b1e5aec8e772e9ff48a52b0ad755bb1894ab
Reviewed-on: https://chromium-review.googlesource.com/909861
Reviewed-by: Mark Mentovai <mark@chromium.org>
Commit-Queue: Scott Graham <scottmg@chromium.org>
Includes:
e7e8237 Don't run win_helper.py get-visual-studio-data on non-Windows
387f001 gn win: Support detection of VS2017
Bug: crashpad:79
Change-Id: I709b6a7f1b64baa3b5ab37a1e5b90c2c529c14de
Reviewed-on: https://chromium-review.googlesource.com/906971
Reviewed-by: Mark Mentovai <mark@chromium.org>
Rolls mini_chromium to include:
75b5697 gn win: Template toolchain to support x86 and x64
428a555 gn win: Automatically find location of Visual Studio
Bug: crashpad:79
Change-Id: Ie7ff10f8b4853071028e3a9580e5be8e5ca5c0ec
Reviewed-on: https://chromium-review.googlesource.com/905625
Commit-Queue: Scott Graham <scottmg@chromium.org>
Reviewed-by: Mark Mentovai <mark@chromium.org>
In setting up the gn build, slightly different optimization settings
were applied for release builds. This caused a couple things to happen,
1) the sketchy noinline declspec was ignored, and 2) the distance
between reading the IP and the actual crash exceeded the tolerance of 64
bytes in the parent.
To make the test more robust to this, use CaptureContext() (I think our
improved version didn't exist at the time the tests was originally
written). Also, switch from crashpad::CheckedWriteFile to Windows'
WriteFile(), which avoids inlining a whole lot of code at that point.
The return value is not checked, but the next thing that happens is that
the function crashes unconditionally, so this does not seem like a huge
problem.
Bug: crashpad:79
Change-Id: I8193d8ce8b01e1533c16b207813c36d6d6113d89
Reviewed-on: https://chromium-review.googlesource.com/902693
Commit-Queue: Scott Graham <scottmg@chromium.org>
Reviewed-by: Mark Mentovai <mark@chromium.org>
kDoesNotObserveDaylightSavingTime can indicate only that the
standard/daylight transition is not automatic, as opposed to it not
existing at all.
Bug: crashpad:214
Change-Id: Ib7016806e79465a6dde605dd667b75a802e1b6c5
Reviewed-on: https://chromium-review.googlesource.com/904767
Commit-Queue: Scott Graham <scottmg@chromium.org>
Reviewed-by: Mark Mentovai <mark@chromium.org>
- default to subsystem:console
- don't build posix/timezone.*
- add some missing libs
This gets all the main binaries building and running. Most configs pass,
but there's some offsets that seem different in some builds; need to
investigate more. Additionally, the binaries used by end_to_end_test.py
aren't yet built, so that script fails.
Includes mini_chromium roll to 46eeaf9:
46eea49 gn win: Add debug info and pdb to cc/cxx
902a29f gn win: Various fixes towards making GN build work
Bug: crashpad:79
Change-Id: Ie56a469b84bed7b0330172cec9f1a8aeb95f702e
Reviewed-on: https://chromium-review.googlesource.com/902403
Commit-Queue: Scott Graham <scottmg@chromium.org>
Reviewed-by: Mark Mentovai <mark@chromium.org>
https://chromium-review.googlesource.com/c/chromium/mini_chromium/+/899847
turns the warning on. This adds one annotation, and fixes one bug.
Includes mini_chromium roll:
.../mini_chromium$ git log 5fcfa43c1587b94132e24782579350cb8266b990..3b953302848580cdf23b50402befc0ae09d03ff9 --oneline
3b95330 (HEAD, origin/master, origin/HEAD) Add -Wimplicit-fallthrough when building on clang
Bug: chromium:807632
Change-Id: I2f3ddca0228e52013844cb8d78d10cb359e851d0
Reviewed-on: https://chromium-review.googlesource.com/900317
Commit-Queue: Scott Graham <scottmg@chromium.org>
Reviewed-by: Mark Mentovai <mark@chromium.org>
Fix Windows- and Mac-in-Chromium GN builds.
Bug: crashpad:79
Change-Id: I952f364ed679a13f656a8db18fb4d1fbf5858c17
Reviewed-on: https://chromium-review.googlesource.com/900206
Commit-Queue: Scott Graham <scottmg@chromium.org>
Reviewed-by: Mark Mentovai <mark@chromium.org>
Follows https://chromium-review.googlesource.com/c/374019/.
Causes MinidumpMemoryListWriter to merge all overlapping ranges before
writing the MINIDUMP_MEMORY_LIST. This is:
1) Necessary for the Google internal crash processor, which in some
cases attempts to read the raw memory (displaying ASAN red zones),
and aborts if there are any overlapping ranges in the minidump on
load;
2) Necessary for new-ish versions of windbg (see bug 216 below). It is
believed that this is a change in behavior in the tool that made
dumps with overlapping ranges unreadable;
3) More efficient. The .dmp for crashy_program goes from 306K to 140K
with this enabled. In Chrome minidumps where
set_gather_indirectly_referenced_memory() is used (in practice this
means Chrome Windows Beta, Dev, and Canary), the savings are expected
to be substantial.
Bug: crashpad:61, chromium:638370, crashpad:216
Change-Id: I969e1a52da555ceba59a727d933bfeef6787c7a5
Reviewed-on: https://chromium-review.googlesource.com/374539
Commit-Queue: Scott Graham <scottmg@chromium.org>
Reviewed-by: Mark Mentovai <mark@chromium.org>
Responsibility for creating argv_c has moved to DoubleForkAndExec().
Change-Id: Id663f0597ee1749df564cdacac1d877b5545750b
Reviewed-on: https://chromium-review.googlesource.com/898024
Reviewed-by: Mark Mentovai <mark@chromium.org>
Commit-Queue: Joshua Peraza <jperaza@chromium.org>
This change includes methods to install a signal handler to launch
the handler process at crash time or to launch the handler on behalf
of another process.
Bug: crashpad:30
Change-Id: I503c788cb3648852d09e9e8c1fe5099ca07a0277
Reviewed-on: https://chromium-review.googlesource.com/759406
Commit-Queue: Joshua Peraza <jperaza@chromium.org>
Reviewed-by: Mark Mentovai <mark@chromium.org>
Avoids using pointers shared between parent/child. Explicitly builds the
test strings in the child process, and then passes both the address and
the expected value of the string to the parent process for expectation
checking. This is necessary to have the test work on Fuchsia.
Also renames ...Forked to ...Child.
Bug: crashpad:196, crashpad:215
Change-Id: I7f22c134301a2806eb39549e371414e7ec9bf225
Reviewed-on: https://chromium-review.googlesource.com/896228
Reviewed-by: Joshua Peraza <jperaza@chromium.org>
Reviewed-by: Mark Mentovai <mark@chromium.org>
Commit-Queue: Scott Graham <scottmg@chromium.org>
Avoid fork() so that the tests can work on Fuchsia. Fills out
CrashpadInfo in the child, and then sends the addresses of various
structures to the parent process to be used for expectation checking.
Bug: crashpad:196, crashpad:215
Change-Id: I9ace6671d2e9184d48fe33016a01271ccfbcbfb6
Reviewed-on: https://chromium-review.googlesource.com/894705
Reviewed-by: Joshua Peraza <jperaza@chromium.org>
Reviewed-by: Mark Mentovai <mark@chromium.org>
Commit-Queue: Scott Graham <scottmg@chromium.org>
Without the section headers for the symbol table, there's no direct way
to calculate the number of entries in the table.
DT_HASH and DT_GNU_HASH are auxiliary tables that are designed to make
symbol lookup faster. DT_HASH is the original and is theoretically
mandatory. DT_GNU_HASH is the new-and-improved, but is more complex.
In practice, however, an Android build (at least vs. API 16) has only
DT_HASH, and not DT_GNU_HASH, and a Fuchsia build has only DT_GNU_HASH
but not DT_HASH. So, both are tried.
This change does not actually use the data in these tables to improve
the speed of symbol lookup, but instead only uses them to correctly
terminate the linear search.
DT_HASH contains the total number of symbols in the symbol table fairly
directly because there is an entry for each symbol table entry in the
hash table, so the number is the same.
DT_GNU_HASH regrettably does not. Instead, it's necessary to walk the
buckets and chain structure to find the largest entry.
DT_GNU_HASH doesn't appear in any "real" documentation that I'm aware
of, other than the binutils code (at least as far as I know). Some
more-and-less-useful references:
- https://flapenguin.me/2017/04/24/elf-lookup-dt-hash/
- https://flapenguin.me/2017/05/10/elf-lookup-dt-gnu-hash/
- http://deroko.phearless.org/dt_gnu_hash.txt
- https://sourceware.org/ml/binutils/2006-10/msg00377.html
Change-Id: I7cfc4372f29efc37446f0931d22a1f790e44076f
Bug: crashpad:213, crashpad:196
Reviewed-on: https://chromium-review.googlesource.com/876879
Commit-Queue: Scott Graham <scottmg@chromium.org>
Reviewed-by: Mark Mentovai <mark@chromium.org>
Reviewed-by: Joshua Peraza <jperaza@chromium.org>
Avoids fork()ing as per previous tests in this file, necessary for
Fuchsia.
Unfortunately, I believe that mmap()/munmap() aren't actually working
correctly on Fuchsia as tested by the EXPECT_FALSE reads, and so these
tests incorrectly fail. Bug with repro filed upstream at ZX-1631.
Bug: crashpad:196, crashpad:215
Change-Id: Iec86f64fcee12097223326f2bf2d5a5348a8a610
Reviewed-on: https://chromium-review.googlesource.com/894124
Reviewed-by: Joshua Peraza <jperaza@chromium.org>
Reviewed-by: Mark Mentovai <mark@chromium.org>
Commit-Queue: Scott Graham <scottmg@chromium.org>
Bionic uses negative values of a semaphore to represent contention.
`sem_timedwait` fails to restore the value to 0 on timeout resulting in
an error (EBUSY) upon calling `sem_destroy`.
Bug: crashpad:30
Change-Id: If1c73a54a879ebd003b0792ebb8f68ceb83ac8bb
Reviewed-on: https://chromium-review.googlesource.com/894106
Reviewed-by: Mark Mentovai <mark@chromium.org>
Commit-Queue: Joshua Peraza <jperaza@chromium.org>
Instead of using pointers shared between the parent/child due to fork,
explicitly builds and passes them between processes. This is
unfortunately a bit more verbose, but seems like it tests functionality
a little better, and is required to have the test work on Fuchsia.
Also renames the ...Forked to ...Child to be correct after the change
from Multiprocess to MultiprocessExec.
Bug: crashpad:196, crashpad:215
Change-Id: I610a7f1e35b6513805c27d9e610f7a9b9820cabc
Reviewed-on: https://chromium-review.googlesource.com/892286
Commit-Queue: Scott Graham <scottmg@chromium.org>
Reviewed-by: Mark Mentovai <mark@chromium.org>
Reviewed-by: Joshua Peraza <jperaza@chromium.org>
