Bug: dawn:447 dawn:667
Change-Id: I66d5a33febeee6a5ea4efd7c4e97461accce22ee
Reviewed-on: https://dawn-review.googlesource.com/c/dawn/+/43580
Reviewed-by: Austin Eng <enga@chromium.org>
Commit-Queue: Stephen White <senorblanco@chromium.org>
Caught by the bots running SwiftShader
BUG=tint:516
Change-Id: I4e93c2f73cf48242b49c8aea1f5f29a21ac6b8e7
Reviewed-on: https://dawn-review.googlesource.com/c/dawn/+/43600
Commit-Queue: Ryan Harrison <rharrison@chromium.org>
Commit-Queue: Ben Clayton <bclayton@google.com>
Auto-Submit: Ryan Harrison <rharrison@chromium.org>
Reviewed-by: Ben Clayton <bclayton@google.com>
Reviewed-by: Austin Eng <enga@chromium.org>
This reverts commit cc4c22ebaa72a2365dc196c4a1496e5a36452cdc.
Reason for revert: Broke Dawn e2e tests w/ SwiftShader
Original change's description:
> Remove deprecated textureLoad overloads with no level param
>
> BUG=tint:516
>
> Change-Id: I7004e0dbd44d703c684118136b05b84cf609c6ba
> Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/42703
> Reviewed-by: Ben Clayton <bclayton@google.com>
> Commit-Queue: Ryan Harrison <rharrison@chromium.org>
Bug: tint:516
Change-Id: I6b7857304872fd0048c23999ac223ce9dcaf7fe1
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/43540
Reviewed-by: Ryan Harrison <rharrison@chromium.org>
Commit-Queue: Ryan Harrison <rharrison@chromium.org>
Auto-Submit: Ryan Harrison <rharrison@chromium.org>
CopyTextureForBrowserTests will cover incoming color format conversion
cases. While it is OK for |unorm| formats to compare their pixel values
on the CPU side, we cannot do such comparisons for the |float| formats
because we may meet the precision issues when comparing a value
generated at the CPU side to the one from the GPU side.
Refactor this test suites by using compute shader and do bit-by-bit
comparison from source texture and destination texture.
BUG=dawn:465
Change-Id: I979fcf1a1d96bbe9f8a4cf2f1a305d488e88b257
Reviewed-on: https://dawn-review.googlesource.com/c/dawn/+/36140
Commit-Queue: Shaobo Yan <shaobo.yan@intel.com>
Reviewed-by: Austin Eng <enga@chromium.org>
This patch uses RecordCopyBufferToTextureFromTextureCopySplit() to
record the D3D12 commands in Texture::ClearTexture() so that we can
remove some redundant code in that function.
BUG=dawn:145, dawn:693
Change-Id: Ifd0ba319c335fdf13e60bf748163451d71dad962
Reviewed-on: https://dawn-review.googlesource.com/c/dawn/+/42760
Reviewed-by: Bryan Bernhart <bryan.bernhart@intel.com>
Commit-Queue: Jiawei Shao <jiawei.shao@intel.com>
Replaces use of EXPECT_EQ with ASSERT_EQ and allows
ASSERT_EQ to propagate to caller so the test returns
early.
BUG=dawn:625
Change-Id: I6c12aad201f1821fa8c7c1f9a9b735910049cdcf
Reviewed-on: https://dawn-review.googlesource.com/c/dawn/+/42940
Reviewed-by: Austin Eng <enga@chromium.org>
Reviewed-by: Rafael Cintron <rafael.cintron@microsoft.com>
Reviewed-by: Bryan Bernhart <bryan.bernhart@intel.com>
Commit-Queue: Bryan Bernhart <bryan.bernhart@intel.com>
We use _test.cc for everything else, so this makes globbing for test
sources simpler.
Change-Id: I6d1a3ad0f3bbeaabe981fbce66967de7e2f9e4d5
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/43481
Auto-Submit: James Price <jrprice@google.com>
Reviewed-by: Ben Clayton <bclayton@google.com>
Commit-Queue: Ben Clayton <bclayton@google.com>
BUG=tint:516
Change-Id: I7004e0dbd44d703c684118136b05b84cf609c6ba
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/42703
Reviewed-by: Ben Clayton <bclayton@google.com>
Commit-Queue: Ryan Harrison <rharrison@chromium.org>
The level-less variants have been removed from the WGSL spec, so will
be removed from Tint soon.
BUG=tint:516
Change-Id: Ic09ffadc2902ca9cdb3198b91f8ad694887258d4
Reviewed-on: https://dawn-review.googlesource.com/c/dawn/+/42704
Auto-Submit: Ryan Harrison <rharrison@chromium.org>
Reviewed-by: Ben Clayton <bclayton@google.com>
Reviewed-by: Austin Eng <enga@chromium.org>
Commit-Queue: Austin Eng <enga@chromium.org>
This patch adds a regression test to dawn_end2end_tests for an
Intel D3D12 driver bug about the T2T copies with specific texture
formats when the source mipmap level is greater than 0.
This patch also does several clean-ups in CopyTests.cpp to support
testing arbitrary texture formats:
1. Initialize the source textures with device.WriteTexture().
2. Set default values for TextureSpec.copyLevel (0),
TextureSpec.levelCount (1) and TextureSpec.copyOrigin ({0, 0, 0}).
3. Add TextureSpec.format and set the default value for it (RGBA8Unorm).
4. Support initializing and comparing the texture data with arbitrary
texture format instead of RGBA8Unorm only in CopyTests_T2T.
5. Check all the texels in the related destination texture subresources
instead of only the copied texels.
BUG=chromium:1161355
TEST=dawn_end2end_tests
Change-Id: Ib32aac57dc7bdc9a4c2edc96093afa945aa08a5c
Reviewed-on: https://dawn-review.googlesource.com/c/dawn/+/42240
Commit-Queue: Jiawei Shao <jiawei.shao@intel.com>
Reviewed-by: Austin Eng <enga@chromium.org>
With this change, the base IntLiteral class now stores a u32, and
derived UintLiteral and SintLiteral are implemented in terms of it. This
will allow us to improve the current pattern of casting down to each
derived type to retrieve the value.
Change-Id: I0c1e56c5e04333a0d3d5f30a3fb28e784f785843
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/42900
Commit-Queue: Ben Clayton <bclayton@google.com>
Reviewed-by: Ben Clayton <bclayton@google.com>
This change allows multiple Dawn textures to be created from the same
D3D11 resource. This avoids re-opening the shared handle by caching the
D3D12 resource outside of the Dawn texture.
Re-opening the handle costs 5-10% of CPU cycles per frame, which far
exceeded syncronization costs.
In a future change, WrapSharedHandle will be depreciated.
BUG=dawn:625
Change-Id: If0d2dc9b7445ec3ae718bc5305164db88057c4ea
Reviewed-on: https://dawn-review.googlesource.com/c/dawn/+/42140
Commit-Queue: Bryan Bernhart <bryan.bernhart@intel.com>
Reviewed-by: Austin Eng <enga@chromium.org>
Reviewed-by: Corentin Wallez <cwallez@chromium.org>
tint::transform::Transform::Output::diagnostics is deprecated.
Diagnostics are now all in the Output::program.
Use the VertexPulling(const Config&) constructor instead of the setters. These are deprecated.
Change-Id: Icefb84719d4915550c1e554e6d0c6e06ebb05f5c
Reviewed-on: https://dawn-review.googlesource.com/c/dawn/+/42266
Auto-Submit: Ben Clayton <bclayton@google.com>
Commit-Queue: Austin Eng <enga@chromium.org>
Reviewed-by: Austin Eng <enga@chromium.org>
You have to have the CloneContext in order to call ReplaceAll() in the first place. The overhead of capturing the pointer in the closure is negligible.
Cleans up the callsites.
Change-Id: I3a0fd808517d69f19756f590f3426e5ba226c57e
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/42840
Reviewed-by: James Price <jrprice@google.com>
Commit-Queue: Ben Clayton <bclayton@google.com>
Statements like `var u : u32 = 0;` should fail because '0' is a signed
integer being used to initialize an unsigned variable.
Added test.
Bug: tint:79
Change-Id: I34f6d21b4355167f49f9a8b5d4ed34a808823185
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/42702
Reviewed-by: Ben Clayton <bclayton@google.com>
Reviewed-by: Antonio Maiorano <amaiorano@google.com>
Commit-Queue: Antonio Maiorano <amaiorano@google.com>
Although VkPhysicalDeviceVulkan11Features::storageBuffer16BitAccess is a subset
of VkPhysicalDeviceVulkan11Features::uniformAndStorageBuffer16BitAccess, when
the SPIR-V Capability (StorageBuffer16BitAccess) was declared, we must enable
the VkPhysicalDeviceVulkan11Features::storageBuffer16BitAccess feature
according to the requirements of the spec.
BUG=dawn:675
Change-Id: I99f9eb4c5f2fd6c6565a51063817efb8bc88216f
Reviewed-on: https://dawn-review.googlesource.com/c/dawn/+/42620
Reviewed-by: Corentin Wallez <cwallez@chromium.org>
Reviewed-by: Hao Li <hao.x.li@intel.com>
Reviewed-by: Jiawei Shao <jiawei.shao@intel.com>
Commit-Queue: Jiawei Shao <jiawei.shao@intel.com>
Shifts the older enum values up by 30, but if anyone was using values
rather than the enums themselves they'd land on the right formats
anyway.
Bug: dawn:695
Change-Id: I92a177b427fb1bb14b60d9280f89d030c5941a38
Reviewed-on: https://dawn-review.googlesource.com/c/dawn/+/42561
Commit-Queue: Brandon Jones <bajones@chromium.org>
Commit-Queue: Kai Ninomiya <kainino@chromium.org>
Auto-Submit: Brandon Jones <bajones@chromium.org>
Reviewed-by: Kai Ninomiya <kainino@chromium.org>
BUG=tint:516
Change-Id: Ic075dc52618bc6d2492e82538f0f02d45f925ddf
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/42700
Auto-Submit: Ryan Harrison <rharrison@chromium.org>
Reviewed-by: dan sinclair <dsinclair@chromium.org>
Reviewed-by: Ben Clayton <bclayton@google.com>
Commit-Queue: Ryan Harrison <rharrison@chromium.org>
Async functions should fail by rejecting the callback instead of
generating a device error. This fixes a leak in the wire where
the allocation for the callback was never delete since it was
never called.
Fixed: chromium:1181627
Change-Id: I840073c1d1b5f1401aa8ed29d3c8f0e1e4fefd35
Reviewed-on: https://dawn-review.googlesource.com/c/dawn/+/42540
Reviewed-by: Austin Eng <enga@chromium.org>
Reviewed-by: Corentin Wallez <cwallez@chromium.org>
Commit-Queue: Austin Eng <enga@chromium.org>
Currently on D3D12 backend a device lost will occur when we do a T2T
copy under the following situations:
1. with Depth32Float
2. only copy one row
3. bufferCopyOffset == 256
This is because in current implementation it is possible for us to do
a copy with D3D12_SUBRESOURCE_FOOTPRINT.Depth > 1, which is not allowed
with DXGI_FORMAT_D32_FLOAT because this format is not supported to be
used as 3D textures.
This patch adds a regression test for this bug and we will fix it
later.
BUG=dawn:693
TEST=dawn_end2end_tests
Change-Id: Ib6fe70988b5b217d5f14d3f32999b3841e5d23b9
Reviewed-on: https://dawn-review.googlesource.com/c/dawn/+/42600
Commit-Queue: Austin Eng <enga@chromium.org>
Reviewed-by: Corentin Wallez <cwallez@chromium.org>
Reviewed-by: Brandon Jones <brandon1.jones@intel.com>
Reviewed-by: Austin Eng <enga@chromium.org>
The are replaced by Queue::OnSubmittedWorkDone. Only Queue::CreateFence
is deprecated since all other fence-related calls require a fence to
work.
Also ports a number of uses of fences in test harness to use
Queue::OnSubmittedWorkDone instead.
Bug: chromium:1177476
Change-Id: I479415f72b08158a3484013e00db8facd11e6f33
Reviewed-on: https://dawn-review.googlesource.com/c/dawn/+/42660
Auto-Submit: Corentin Wallez <cwallez@chromium.org>
Commit-Queue: Brandon Jones <bajones@chromium.org>
Reviewed-by: Austin Eng <enga@chromium.org>
Reviewed-by: Brandon Jones <bajones@chromium.org>
This CL updates the MSL backend to emit a using namespace for metal
instead of using the `metal::` prefix.
Bug: tint:463
Change-Id: I63d3ea5b5a56e61d71cd6d17a51a5120363ea007
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/42741
Auto-Submit: dan sinclair <dsinclair@chromium.org>
Reviewed-by: Ryan Harrison <rharrison@chromium.org>
Reviewed-by: Ben Clayton <bclayton@google.com>
Commit-Queue: dan sinclair <dsinclair@chromium.org>
Bug: none
Change-Id: Ic437dc7656613d9a8088dd51f1ed0713f1992415
Reviewed-on: https://dawn-review.googlesource.com/c/dawn/+/42720
Auto-Submit: Austin Eng <enga@chromium.org>
Reviewed-by: Stephen White <senorblanco@chromium.org>
Reviewed-by: Brandon Jones <bajones@chromium.org>
Commit-Queue: Austin Eng <enga@chromium.org>
Bug: dawn:680
Change-Id: I6d57280ab11381649deef51ee7babf5ca73f359b
Reviewed-on: https://dawn-review.googlesource.com/c/dawn/+/42340
Commit-Queue: Austin Eng <enga@chromium.org>
Reviewed-by: Stephen White <senorblanco@chromium.org>
This is using the Tint -> SPIRV-Cross path, but Tint supports
the SPIR-V part now, so it works correctly in Dawn's tests.
Bug: tint:400
Change-Id: I6efd9447a9b454fb866467d65fda9350ec772f18
Reviewed-on: https://dawn-review.googlesource.com/c/dawn/+/42560
Reviewed-by: Corentin Wallez <cwallez@chromium.org>
Reviewed-by: Ben Clayton <bclayton@google.com>
Commit-Queue: Austin Eng <enga@chromium.org>
This works for the SPIR-V tint backend.
Other backends need to be enabled seperately.
Bug: tint:463
Bug: dawn:571
Change-Id: I4c937eec381a05609b267d36d202844758b13547
Reviewed-on: https://dawn-review.googlesource.com/c/dawn/+/42641
Commit-Queue: Ben Clayton <bclayton@google.com>
Reviewed-by: Corentin Wallez <cwallez@chromium.org>
It was being taken from the pre-transformed program, not the post-transformed program.
If we did get a transform error, the string would be empty.
Also change the signature of RunTransforms() to take a Transform* instead of a Manager*. There's nothing special about Manager anymore - it is just a transform that acts as a group of other transforms.
Change-Id: I4ea6cb022b5967b3c6b8c628517727dc3da3be8e
Reviewed-on: https://dawn-review.googlesource.com/c/dawn/+/42640
Auto-Submit: Ben Clayton <bclayton@google.com>
Reviewed-by: Corentin Wallez <cwallez@chromium.org>
Commit-Queue: Ben Clayton <bclayton@google.com>
The chromium tests for static initializers get upset about the call to
ClassId::New() in a global variable initializer.
Instead, take the address of a unique 'token' variable to generate the
unique identifier.
This is similar to how things were before
https://dawn-review.googlesource.com/c/tint/+/42460
but of taking the address-of the token in the header, we're instead
taking the address-of inside the cpp file, avoiding the DLL issues
that 42460 addressed.
Change-Id: Id18c5b90b365b0960fac347729a4927528d34c07
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/42462
Auto-Submit: Ben Clayton <bclayton@google.com>
Reviewed-by: Corentin Wallez <cwallez@chromium.org>
Commit-Queue: Ben Clayton <bclayton@google.com>
This is the replacement for Fence in the single-queue WebGPU world. To
keep this CL focused, it doesn't deprecate the fences yet.
Bug: chromium:1177476
Change-Id: I09d60732ec67bc1deb49f7a9d57699c049475acf
Reviewed-on: https://dawn-review.googlesource.com/c/dawn/+/41723
Auto-Submit: Corentin Wallez <cwallez@chromium.org>
Reviewed-by: Corentin Wallez <cwallez@chromium.org>
Commit-Queue: Corentin Wallez <cwallez@chromium.org>
This CL removes the registration of functions to after the function body
has been processed. This allows us to catch recursive calls where the
function calls itself. Prior to this we'd find the called function and,
if the function was an entry point, end up in a loop trying to walk all
the callers. With this change, the function is not found when we do
the lookup and we exit as expected.
Bug: tint:258
Change-Id: Ie0173207b788e87de39867a5aa41e8cc13ec33de
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/42520
Commit-Queue: dan sinclair <dsinclair@chromium.org>
Commit-Queue: Ben Clayton <bclayton@google.com>
Auto-Submit: dan sinclair <dsinclair@chromium.org>
Reviewed-by: Ben Clayton <bclayton@google.com>
Reviewed-by: Ryan Harrison <rharrison@chromium.org>
Adds dawn_native::ResetDeviceInternal, which allows us to destroy and
create a new ID3D12Device. The device should be reset after every test
when GPU-based validation is enabled in order to prevent GBV objects
from using a significant amount of memory over time.
Bug: dawn:623
Change-Id: I654d093d993ab0198c6c240bd0f3f843d2762680
Reviewed-on: https://dawn-review.googlesource.com/c/dawn/+/41340
Reviewed-by: Austin Eng <enga@chromium.org>
Reviewed-by: Rafael Cintron <rafael.cintron@microsoft.com>
Commit-Queue: Brandon Jones <brandon1.jones@intel.com>
If ClassID::Of<T>() is used inside tint and used outside tint for the same type, and tint is built as a DLL, then the address of the Unique<T>::token can resolve to different addresses, entirely breaking Castable.
Replace address-of for a unique symbol with a single static counter that's incremented for each use of TINT_INSTANTIATE_CLASS_ID().
Change-Id: I40dc81b1273110291d90a1d5ec05428f7e703c6a
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/42460
Reviewed-by: Antonio Maiorano <amaiorano@google.com>
Commit-Queue: Ben Clayton <bclayton@google.com>
The StringLiteral token was removed from the spec when all usage was
removed. This CL removes the remaining parsing bits from Tint.
Change-Id: I02f5dbdbad649c62c22c69a55616e0087a0f56d4
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/42440
Auto-Submit: dan sinclair <dsinclair@chromium.org>
Commit-Queue: Ryan Harrison <rharrison@chromium.org>
Reviewed-by: Ryan Harrison <rharrison@chromium.org>
This reverts commit 9fdbb74072a084e3085aaaba18276c53fa3463de.
Reason for revert: The SPIRV-Cross GN files have a bug which is fixed upstream, but Skia is on an old version of SPIRV-Cross so they don't have the fix. So, the build fails when rolling Dawn into Skia complaining about unknown `-fno-exceptions` on the windows bots. Need Skia to either move to vulkan-deps, or roll their spirv-cross forward before this can re-land.
Original change's description:
> Use upstream SPIRV-Cross GN files.
>
> Previously spirv-cross didn't have its own sources files but this
> changed recently so we should use them. This will ultimately allow
> sharing targets between multiple components in Chromium.
>
> Bug: chromium:1179277
> Change-Id: Ib4bb1884b9ba9c4c3804e96d8adbb8905c60c9a3
> Reviewed-on: https://dawn-review.googlesource.com/c/dawn/+/41725
> Auto-Submit: Corentin Wallez <cwallez@chromium.org>
> Reviewed-by: Peng Huang <penghuang@chromium.org>
> Reviewed-by: Austin Eng <enga@chromium.org>
> Commit-Queue: Corentin Wallez <cwallez@chromium.org>
Bug: chromium:1179277
Change-Id: If1003bafa7b35f502c08b2dab91dd3d416aab077
Reviewed-on: https://dawn-review.googlesource.com/c/dawn/+/42420
Commit-Queue: dan sinclair <dsinclair@chromium.org>
Commit-Queue: Austin Eng <enga@chromium.org>
Auto-Submit: dan sinclair <dsinclair@chromium.org>
Reviewed-by: Austin Eng <enga@chromium.org>
The spec was clarified that a value of .5 rounds towards the even value.
This matches the HLSL `round` method. This CL updates SPIR-V to use the
`RoundEven` GLSL450 call and the `rint` MSL call so they match the spec
requirements.
Change-Id: I97ce2e29b9639ad72b6846b32dbe3abb8d9f6cff
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/42421
Auto-Submit: dan sinclair <dsinclair@chromium.org>
Commit-Queue: Ryan Harrison <rharrison@chromium.org>
Reviewed-by: Ryan Harrison <rharrison@chromium.org>
Transforms are meant to be reusable. Those that hold state cannot be used concurrently.
State leakage between runs is dangerous.
To fix this:
* Add transform::Data - A new base class for extra information emitted by transforms.
* Add transform::DataMap - A container of Data, keyed by type.
* Add a transform::DataMap field to Transform::Output.
* Have FirstIndexOffset emit a FirstIndexOffset::Data.
* Deprecate the getters on the transform.
Mutability of the transform config is also dangerous as setters can be called while a transform is actively running on another thread.
To fix:
* Expose a VertexPulling::Config structure and add a constructor that accepts this.
* Deprecate the setters on VertexPulling.
Also deprecate Transform::Output::diagnostics.
Put all the transform diagnostics into the returned Program. Reduces error handling of the client.
Change-Id: Ibd228dc2fbf004ede4720e2d6019c024bc5934d1
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/42264
Reviewed-by: dan sinclair <dsinclair@chromium.org>
Reviewed-by: Ryan Harrison <rharrison@chromium.org>
Commit-Queue: Ben Clayton <bclayton@google.com>
The transform::Manager no longer does anything special. It is now a container of transforms that acts like a single transform.
Change-Id: I5e51e250cfa610b651445b7cd5efd29811c56d2e
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/42261
Commit-Queue: Ben Clayton <bclayton@google.com>
Reviewed-by: Ryan Harrison <rharrison@chromium.org>
postfix_expr() handles `[]` array accessors and call expressions `()`.
Have postfix_expr() use sync to parse these:
* It will use the end bracket token to attempt to resynchronize the parser on error
* It also considers maximum parser recursion depth, avoiding stack overflows
Fixed: chromium:1180573
Change-Id: I8c1c62c68e24a564e0e4e7d0de9f5a3fa7032369
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/42222
Commit-Queue: Ben Clayton <bclayton@google.com>
Reviewed-by: dan sinclair <dsinclair@chromium.org>
Have the TD resolve swizzles down to indices, erroring out if they're not valid.
Resolving these at TD time removes swizzle parsing in the HLSL writer, and is generally useful information.
If we don't sanitize in the TD, we can end up trying to construct a resulting vector of an invalid size (> 4) triggering an assert in the type::Vector constructor.
Fixed: chromium:1180634
Bug: tint:79
Change-Id: If1282c933d65eb02d26a8dc7e190f27801ef9dc5
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/42221
Commit-Queue: Ben Clayton <bclayton@google.com>
Reviewed-by: dan sinclair <dsinclair@chromium.org>
Change-Id: Ia6c21dfc33445ba828b2f244d8a3479fb3328805
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/42263
Commit-Queue: Ben Clayton <bclayton@google.com>
Reviewed-by: Ryan Harrison <rharrison@chromium.org>
Add test.
Change-Id: Ide47df8b3a946818ed9a8ffd62411284f568609b
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/42262
Commit-Queue: Ben Clayton <bclayton@google.com>
Reviewed-by: Ryan Harrison <rharrison@chromium.org>
