Otherwise the public_configs of the proxied targets don't correctly get
propagated, causing Dawn to fail compilation due to missing include
directories.
Bug: 706
Change-Id: Ib8acfb06105e77ad005c3900975e8173d85dcf32
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/44680
Auto-Submit: Corentin Wallez <cwallez@chromium.org>
Commit-Queue: Ben Clayton <bclayton@google.com>
Reviewed-by: Ben Clayton <bclayton@google.com>
This will allow Tint's dependent to depend on libtint without GN
discovering Tint's test and try to build them. In particular it will
help use Tint in Dawn in Skia's standalone build which doesn't have
//testing.
Bug: dawn:706
Change-Id: Idd28662b89aa75df7704eaae205328dce0b96fef
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/44540
Reviewed-by: David Neto <dneto@google.com>
Commit-Queue: Corentin Wallez <cwallez@chromium.org>
Auto-Submit: Corentin Wallez <cwallez@chromium.org>
Remove the decoration groupings (Array, Function, Struct,
StructMember, Type, Variable), such that all *Decoration classes now
subclass ast::Decoration directly. This allows for decorations to be
used in multiple places; for example, builtin decorations are now
valid for both variables and struct members.
Checking that decoration lists only contain decorations that are valid
for the node that they are attached to is now done inside the
validator.
Change-Id: Ie8c0e53e5730a7dedea50a1dec8f26f9e7b00e8d
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/44320
Commit-Queue: James Price <jrprice@google.com>
Reviewed-by: Ben Clayton <bclayton@chromium.org>
Add a sanitizing transform to hoist entry point parameters out as
global variables.
Bug: tint:509
Change-Id: Ic18f69386a58d82ee11571fa9ec0c54cb5bdf2cf
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/44083
Commit-Queue: James Price <jrprice@google.com>
Auto-Submit: James Price <jrprice@google.com>
Reviewed-by: Ben Clayton <bclayton@google.com>
Try and make sense of the huge number of tests we have.
Rename tests so they have a consistent naming style.
Change-Id: I0c089d5945778a8718480a1a2f854435e7b0e79a
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/44162
Reviewed-by: Antonio Maiorano <amaiorano@google.com>
Commit-Queue: Antonio Maiorano <amaiorano@google.com>
First step in splitting out resolver tests into multiple files
Change-Id: I58c66ad5e348a50b3e028dff5749cfacb273ea62
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/44161
Reviewed-by: Antonio Maiorano <amaiorano@google.com>
Commit-Queue: Antonio Maiorano <amaiorano@google.com>
Move out of the src root and into its own subdirectory
Rename methods to remove the 'Determine' prefix.
Fixed: tint:529
Change-Id: Idf89d647780f8a2e7495c1c9e6c402e00ad45b7c
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/44041
Commit-Queue: Ben Clayton <bclayton@google.com>
Reviewed-by: David Neto <dneto@google.com>
This is now entirely handled as transforms.
Bug: tint:273
Change-Id: Ib3c0db7b5ecf024b6ae2aed7788e4b582d07c4ce
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/43983
Commit-Queue: Ben Clayton <bclayton@google.com>
Reviewed-by: James Price <jrprice@google.com>
This change begins the work to move the reserved keyword remapping out of the writer and into the sanitizer transform.
If the transform::Renamer is in use, then these symbols should never have to be remapped - however for debugging purposes it is often nice to be able to emit code that isn't entirely mangled.
The logic in the msl writer will be removed as a followup change
Bug: tint:273
Change-Id: I76af03ff80388a48d9dd80a5b5fdfe21f3c8e7a0
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/43982
Reviewed-by: James Price <jrprice@google.com>
Commit-Queue: Ben Clayton <bclayton@google.com>
Replaced with tint::transform::Rename and was never used.
Bug: tint:273
Change-Id: Icf0b19b389377f3c9a6efcf3ca232ba294775f9b
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/43981
Auto-Submit: Ben Clayton <bclayton@google.com>
Commit-Queue: Ryan Harrison <rharrison@chromium.org>
Reviewed-by: Ryan Harrison <rharrison@chromium.org>
Renames symbols.
Currently only supports renaming to a symbol with a monotonic increasing suffix.
Change-Id: I7ae67a7b1740ac0e83c1821643cf7b8b1959f84d
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/42842
Commit-Queue: Ben Clayton <bclayton@google.com>
Reviewed-by: James Price <jrprice@google.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>
If the parser hits a `maximum parser recursive depth reached` situation, then we need to try and resynchronize the parser.
If we fail to do this, then the synchronized_ flag may remain true, and the parser will believe progress is still being made.
In this situation the parser may try to reparse the same token, forever.
By calling sync_to() we either find the end of the block, and forward progress can be made, or synchronized_ is set to false, and the parser can error out cleanly.
Add test case from fuzzer report.
Fixed: chromium:1180128
Change-Id: I893077677fd3dfbd4b9b400cd32db842b06db500
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/42029
Reviewed-by: David Neto <dneto@google.com>
Commit-Queue: Ben Clayton <bclayton@google.com>
Chromium has its own test main() entrypoint.
To ensure that Chromium doesn't panic about memory leaks with the tests that exercise the ICE cases, we have to explicitly call the FreeInternalCompilerErrors() functions in these tests (at least until I can add this to end of Chromium's test main() function)
Change-Id: I2ea5109fcdb5f68f56a19709a1ec35ed72c0f760
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/42025
Reviewed-by: Austin Eng <enga@chromium.org>
Reviewed-by: dan sinclair <dsinclair@chromium.org>
Commit-Queue: Ben Clayton <bclayton@google.com>
Looks for DXC on PATH.
If found, it will invoke DXC with the shader program generated by the writer to verify the shader can compile.
Change-Id: Iad8b4021bac16d01214b500ddb89b5f743927ce9
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/41943
Commit-Queue: Ben Clayton <bclayton@google.com>
Reviewed-by: dan sinclair <dsinclair@chromium.org>
Command is a helper used by tests for executing a process with a number of arguments and an optional stdin string, and then collecting and returning the process's stdout and stderr output as strings.
Will be used to invoke HLSL and MSL shader compilers to verify our test generated code actually compiles.
Change-Id: I5cd4ca63af9aaa29be7448bb4fa8422e6d42a8ce
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/41942
Commit-Queue: Ben Clayton <bclayton@google.com>
Reviewed-by: dan sinclair <dsinclair@chromium.org>
TmpFile constructs a temporary file that can be written to, and is automatically deleted on destruction.
Will be used to create a temporary source file for verifying generated HLSL and MSL against their shader compilers.
Change-Id: Ieaa6f257b93f4f2193dafe6297603816f6964928
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/41941
Commit-Queue: Ben Clayton <bclayton@google.com>
Reviewed-by: dan sinclair <dsinclair@chromium.org>
Appends an error message with the tint compiler source location to the
provided diagnositic list, and then calls the global error handler if
one is set.
Tests and the sample app now register an error handler to print the
diagnostic list to stderr and abort when NDEBUG is not defined.
All uses of assert(false) have been fixed up to use these macros.
Change-Id: I2f63e51ed86ac23883301d280070bd1a357c6cb2
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/41620
Commit-Queue: dan sinclair <dsinclair@chromium.org>
Reviewed-by: dan sinclair <dsinclair@chromium.org>
Move these into a separate const variable declaration statement just above the before the use of the array initializer.
HLSL does not allow array initializers as part of a sub-expression
Fixed: tint:406
Change-Id: I98f93f2eca172865691831011c852de5e57c5ad6
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/41484
Commit-Queue: Ben Clayton <bclayton@google.com>
Reviewed-by: dan sinclair <dsinclair@chromium.org>
Reviewed-by: James Price <jrprice@google.com>
Use a sanitizing transform to convert scalar `sample_mask_{in,out}`
variables to single element arrays.
Add the `SampleRateShading` capability if the `sample_index` builtin
is used.
Bug: tint:372
Change-Id: Id7280e3ddb21e0a098d83587d123c97e3c34fa1b
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/41662
Reviewed-by: dan sinclair <dsinclair@chromium.org>
Commit-Queue: James Price <jrprice@google.com>
Auto-Submit: James Price <jrprice@google.com>
Add Stmt() accessor on all semantic::Expressions so the owning statement can be retrieved.
Change-Id: I5d584335a6d137fdeab0b8d74a161fcae9b46080
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/41545
Reviewed-by: James Price <jrprice@google.com>
Reviewed-by: dan sinclair <dsinclair@chromium.org>
Commit-Queue: dan sinclair <dsinclair@chromium.org>
Auto-Submit: Ben Clayton <bclayton@google.com>
These files were added in dfd1714, but only added to CMakeLists.txt.
Change-Id: I775e8fef68b2f4900a6c562533512f0eda01b795
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/41700
Commit-Queue: dan sinclair <dsinclair@chromium.org>
Auto-Submit: James Price <jrprice@google.com>
Reviewed-by: dan sinclair <dsinclair@chromium.org>
And add tests for IntrinsicTable.
Drop all the type unwrapping - be precise:
* Display the actual argument types in the signature mismatch message
* Only dereference pointer arguments if the parameter does not expect a pointer
Correctly match access control on storage types
Note that I was mistaken in tint:486 - the TypeDeterminer is resolving identifiers to variables correctly as pointer types. The confustion here was probably due to all the UnwrapAll() calls, which have now all gone.
Fixed: tint:486
Change-Id: I239eabd1fedfc082566c4af616ccfc58786cae25
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/41280
Commit-Queue: Ben Clayton <bclayton@google.com>
Auto-Submit: Ben Clayton <bclayton@google.com>
Reviewed-by: dan sinclair <dsinclair@chromium.org>
These transforms will perform work to massage the Program into something consumable by the given writer.
Change-Id: I8989e8d4bc1a9cae7ce1f8764c8f3811db3bd04d
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/41483
Commit-Queue: Ben Clayton <bclayton@google.com>
Reviewed-by: dan sinclair <dsinclair@chromium.org>
Provides a centeralized table for all intrinsic overloads.
IntrinsicTable::Lookup() takes the intrinsic type and list of arguments, returning either the matched overload, or a sensible error message.
The validator has expectations that the TypeDeterminer resolves the return type of an intrinsic call, even when the signature doesn't match. To handle this, create semantic::Intrinsic nodes even when the overload fails to match. A significant portion of the Validator's logic for handling intrinsics can be removed (future change).
There are a number of benefits to migrating the TypeDeterminer and Validator over to the IntrinsicTable:
* There's far less intrininsic-bespoke code to maintain (no more duplicate `kIntrinsicData` tables in TypeDeterminer and Validator).
* Adding or adjusting an intrinsic overload involves adding or adjusting a single Register() line.
* Error messages give helpful suggestions for related overloads when given incorrect arguments.
* Error messages are consistent for all intrinsics.
* Error messages are far more understandable than those produced by the TypeDeterminer.
* Further improvements on the error messages produced by the IntrinsicTable will benefit _all_ the intrinsics and their overloads.
* The IntrinsicTable generates correct parameter information, including whether parameters are pointers or not.
* The IntrinsicTable will help with implementing autocomplete for a language server
Change-Id: I4bfa88533396b0b372aef41a62fe47b738531aed
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/40504
Commit-Queue: Ben Clayton <bclayton@google.com>
Reviewed-by: dan sinclair <dsinclair@chromium.org>
For consistency with the other .cc file names in this directory.
The reason we need the sem_ prefix is because all the tint lib code is compiled as a single target, and .GN cannot cope with .cc files with the same file name, even if they're in different directories.
This could be fixed by building each directory as a separate target, but we have circular dependencies that currently make this impossible.
Change-Id: I2f1379ff11dd863f81662449f73d67832f0a8a4e
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/40501
Commit-Queue: Ben Clayton <bclayton@google.com>
Reviewed-by: dan sinclair <dsinclair@chromium.org>
CallTarget holds parameter information. This is simple to extract from an ast::Function.
CallTarget will also be used for intrinsics, which can be overloaded. CallTarget will hold the resolved overload parameter signature.
Bug: tint:361
Change-Id: I4dadc4a99293f12ede9e9cbd9132ba5f9b9830ed
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/40284
Reviewed-by: dan sinclair <dsinclair@chromium.org>
Commit-Queue: Ben Clayton <bclayton@google.com>
Pull the mutable 'is_swizzled' semantic field from ast::MemberAccessorExpression and into a new semantic::MemberAccessorExpression node.
Have the TypeDeterminer create these semantic::MemberAccessorExpression nodes.
Bug: tint:390
Change-Id: I8fc6e36dabb417190528536a94d027af54059222
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/40142
Commit-Queue: Ben Clayton <bclayton@google.com>
Reviewed-by: David Neto <dneto@google.com>
semantic::Call derives from semantic::Expression, and Type() is the return type of the function
Pull the mutable semantic field from ast::Identifier and into a new semantic nodes.
Have the TypeDeterminer create these new semantic nodes.
Note: This change also fixes the node that holds the semantic information for a call.
Previously this was on the identifier, and this is now correctly on the CallExpression.
The identifier of the CallExpression should resolve to the target function, not the return type.
Functions can currently be represented as a type, and the identifier of a CallExpression now has no semantic information.
Bug: tint:390
Change-Id: I03521da5634815d35022f45ba521372cbbdb6bc7
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/40065
Commit-Queue: Ben Clayton <bclayton@google.com>
Reviewed-by: David Neto <dneto@google.com>
Pull the mutable semantic field from ast::Variable and into a new semantic::Variable node.
Have the TypeDeterminer create these semantic::Variable nodes.
Bug: tint:390
Change-Id: Ia13f5e7b065941ed66ea5a86c6ccb288071feff3
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/40063
Commit-Queue: Ben Clayton <bclayton@google.com>
Reviewed-by: David Neto <dneto@google.com>
Pull the mutable semantic fields from ast::Function and into a new semantic::Function node.
Have the TypeDeterminer create these semantic::Function nodes.
Bug: tint:390
Change-Id: I237b1bed8709dd9a3cfa24d85d48fc77b7e532da
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/39902
Reviewed-by: David Neto <dneto@google.com>
Commit-Queue: Ben Clayton <bclayton@google.com>
* Move unnecessary builtin checks out of type determination and into
validation
* Type determination now uses a bare minimum of information for most
builtins
* Validation now does majority of checking of builtins
* Added const qualifier to type accessors
Change-Id: Id11b739770af904a9b7afe0b1c2de50e1428a165
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/39540
Commit-Queue: Alan Baker <alanbaker@google.com>
Reviewed-by: dan sinclair <dsinclair@chromium.org>
semantic::Expression will hold the resolved expression type.
Migration to this will happen in the next change.
Bug: tint:390
Change-Id: I637eb6777d603ab0828c0e5e7126bd2ac1b0c4bc
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/39006
Commit-Queue: Ben Clayton <bclayton@google.com>
Reviewed-by: dan sinclair <dsinclair@chromium.org>
Follows the basic shallow-clone pattern of type::Manager::Wrap(),
which this new method uses.
Use ProgramBuilder::Wrap() in the writers, where we were previously
using type::Manager::Wrap(). This simplifies some of the logic
(especially around AppendVector()).
This is now required as semantic information is generated inside the
writers for newly constructed AST nodes, and we need to register
this semantic info so it can be fetched later.
Change-Id: Ia362204a1f13054e50545baeb7d269f210a36826
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/39004
Commit-Queue: Ben Clayton <bclayton@google.com>
Reviewed-by: dan sinclair <dsinclair@chromium.org>
Will hold the mutable fields that currently reside in the otherwise immutable-AST.
Change the AST string methods to accept a `const semantic::Info&`. This is required as some nodes include type-resolved information in their output strings.
Bug: tint:390
Change-Id: Iba494a9c5645ce2096da0a8cfe63a4309a9d9c3c
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/39003
Commit-Queue: Ben Clayton <bclayton@google.com>
Reviewed-by: dan sinclair <dsinclair@chromium.org>
Program is now immutable*, and remains part of the public Tint
interface.
ProgramBuilder is the mutable builder for Programs, and is not part of
the public Tint interface. ast::Builder has been folded into
ProgramBuilder.
Immutable Programs can be cloned into a mutable ProgramBuilder with
Program::CloneAsBuilder().
Mutable ProgramBuilders can be moved into immutable Programs.
* - mostly immutable. It still has a move constructor and move
assignment operator - required for practical usage - and the
semantic information on AST nodes is still mutable.
Bug: tint:390
Change-Id: Ia856c50b1880c2f95c91467a9eef5024cbc380c6
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/38240
Reviewed-by: dan sinclair <dsinclair@chromium.org>
Enforce all places where Dawn passes in or returns a ast::Module, now takes a `const Program* ` or returns a `Program`.
As the end goal of all this is to have immutable Programs, all Program inputs take a pointer instead of moving the actual object.
As consumers of a Program are now all const, we have to const_cast to work around all the places we've been incorrectly mutating a ast::Module.
These const_casts are temporary, and will be fixed in the next set of changes.
Depends on https://dawn-review.googlesource.com/c/dawn/+/38522
Bug: tint:390
Change-Id: Ie05b112b16134937d1b601e9b713ea4ec4e1c677
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/38541
Reviewed-by: dan sinclair <dsinclair@chromium.org>
`tint::Program` will become the new public API object for a parsed shader program.
For now, have Program be a simple wrapper around ast::Module so we can migrate Dawn's use of the public tint API.
Add new Program variants of public APIs for places that returned or took a Module.
Remove Reset() methods from Generators, they aren't used, and make the migration harder.
Change-Id: Ic5bee46ceb109ea591ba7fec33685220b244a1ae
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/38540
Reviewed-by: dan sinclair <dsinclair@chromium.org>
Commit-Queue: Ben Clayton <bclayton@google.com>
CloneContext clones the AST, types, symbols and in the future semantic info.
3/4 of these are non-ast, so promote these up to the root.
Bug: tint:390
Change-Id: I49619796e6f81f9ab64f79413a12c87312cb1901
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/38361
Commit-Queue: Ben Clayton <bclayton@google.com>
Reviewed-by: dan sinclair <dsinclair@chromium.org>
Despite `tint::ast::type::Type` being in the AST namespace, these classes are clearly not AST nodes:
* They don't derive from ast::Node
* They're deduplicated by the type manager
* None of the types have an Source - they have no lexical declaration point
* The fact we have `ast::Struct` and `ast::type::Struct` clearly demonstrates what is an AST node, and what is a type.
* We have code scattered in the codebase (TypeDeterminer, writers, etc) that create new types after parsing - so clearly not part of the original syntax tree.
Types in tint are closer to being semantic info, but due to the parse-time generation of types, and tight dependency of ast::Nodes to types, I'd be reluctant to class these as semantic info. Instead, put these into a separate root level `tint::type` namespace and `src/tint` directory.
The fact that types exist in the ast::Module has already caused bugs (https://dawn-review.googlesource.com/c/tint/+/37261). This is a first step in separating out types from the ast::Module.
Bug: tint:390
Change-Id: I8349bbbd1b19597b8e6d51d5cda0890de46ecaec
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/38002
Commit-Queue: Ben Clayton <bclayton@google.com>
Reviewed-by: dan sinclair <dsinclair@chromium.org>
A container and allocator of objects of (or deriving from) the template type `T`.
Objects are allocated by calling Create(), and are owned by the BlockAllocator.
When the BlockAllocator is destructed, all constructed objects are automatically destructed and freed.
Objects held by the BlockAllocator can be iterated over using a View or ConstView.
Use this to hold the ast::Nodes in the ast::Module
This is called BlockAllocator as it can be optimized to hold objects in contiguous memory blocks, which will improve cache coherencey. Currently BlockAllocator is a straight port of the vector-of-unique-ptr, taken from ast::Module.
Change-Id: I4bf4d298aec3c70d2ddf833e2f168416cbb024c0
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/38001
Reviewed-by: dan sinclair <dsinclair@chromium.org>
Commit-Queue: Ben Clayton <bclayton@google.com>
This CL changes the set decoration to a group decoration. We still parse
`set` until downstream users can be updated.
Bug: tint:338
Change-Id: I610d1ed769b3a26c117ad6d875f8a99a3d5b7754
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/37920
Auto-Submit: dan sinclair <dsinclair@chromium.org>
Commit-Queue: Ben Clayton <bclayton@google.com>
Reviewed-by: Ben Clayton <bclayton@google.com>
This reverts commit 2f7fea1f19.
Reason for revert: breaks autorollers
Original change's description:
> Extract demangler out to separate source set
>
> And add this as a dep to libtint_core_src and tint_ast_clone_fuzzer.
>
> Should fix chromium autoroller
>
> Change-Id: Ic5f43bae85e010839a1940c1ae2d570a9613d269
> Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/37708
> Commit-Queue: dan sinclair <dsinclair@chromium.org>
> Reviewed-by: Ryan Harrison <rharrison@chromium.org>
> Reviewed-by: dan sinclair <dsinclair@chromium.org>
TBR=rharrison@chromium.org,dsinclair@chromium.org,bclayton@google.com
Change-Id: If872d1655e13dd5d67f898a3d9a43c76b2866973
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/37845
Reviewed-by: Ben Clayton <bclayton@google.com>
Reviewed-by: dan sinclair <dsinclair@chromium.org>
Commit-Queue: Ben Clayton <bclayton@google.com>
And add this as a dep to libtint_core_src and tint_ast_clone_fuzzer.
Should fix chromium autoroller
Change-Id: Ic5f43bae85e010839a1940c1ae2d570a9613d269
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/37708
Commit-Queue: dan sinclair <dsinclair@chromium.org>
Reviewed-by: Ryan Harrison <rharrison@chromium.org>
Reviewed-by: dan sinclair <dsinclair@chromium.org>