dartfuzz has been taking ~35 min to finish the build and upload steps even with 100% RBE cache hit rate, causing it to fail to finish before the first wave of nightly builders trigger.
Change-Id: I932facd77bc08f4d7236ae8c760013406a69fb0e
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/507267
Commit-Queue: Ryan Macnak <rmacnak@google.com>
Reviewed-by: Alexander Aprelev <aam@google.com>
These two groups were the same (except for Kallen, who was
accidentally not included in OWNERS_FOUNDATION and should have been),
so we decided to merge them. Internally, the group is called the "Dart
Language and Core Libraries" team, so `OWNERS_LANGUAGE` seems like the
one to keep.
Change-Id: I606ba5828f482c04841c2e995a0066ad6a6a6964
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/508166
Commit-Queue: Paul Berry <paulberry@google.com>
Reviewed-by: Alexander Thomas <athom@google.com>
Extend --check-clean (enabled on bots) to verify build commands don't include absolute paths.
The non-RBE GCC and MSVC builds still have absolute paths. GCC doesn't have -ffile-compilation-dir so it uses -fdebug-prefix-map, which is not fixable. The MSVC build has absolute path in the toolchain wrappers, which might be fixable.
TEST=ci
Change-Id: I3b984aaab7aefa7ff527f0a039ca42281224a09d
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/506505
Reviewed-by: Alexander Aprelev <aam@google.com>
Commit-Queue: Ryan Macnak <rmacnak@google.com>
Reviewed in https://dart-review.googlesource.com/c/sdk/+/497583
[dartpad] Part 1: Protocol and Scaffolding
This sets up folders for `package:dartpad` and `package:dartpad_worker`,
R=athom@google.com, sigurdm@google.com
including:
* `OWNERS` files,
* Protocol definition,
* Exceptions, and,
* Virtual file system utilities.
[dartpad] Part 2: HotReloadCompiler
Adds the hot reload compiler logic to dartpad_worker.
This is models on dartdevc with `--reload-last-accepted-kernel` and
`--reload-delta-kernel` options. I don't think this does an incremental
compilation, instead it does a modular compilation. Meaning, we can have
a huge collection of precompiled DDC modules and the compiler just gets
a DDC outline/summary dill files for these modules. But on the other hand
the actual code that is compiled will be recompiled on every compile()
call.
This is very important for flutter_web.js / flutter_web.dill where the
precompiled DDC modules is around 60 MB in total. The fact that
package:flutter is precompiled into flutter_web.js is critical for
compilation performance for small dartpad-style example apps.
The downside is that unlike incremental compilation with _frontendserver_,
if the user were to install package:http and use it in the app they are
compiling then every `compile()` call will recompile `package:http`.
Where as with _frontendserver_ we'd be able to invalidate specific files,
and only those would be recompiled.
This leaves future work to either:
* (A) Add support for modular compilation to _frontendserver_, such that
we can do incremental compilation without having to first compile all
of `package:flutter`.
* (B) Refactor compilation strategy implemented here, to leverage
modular further. We could for example compile all dependencies from
`PUB_CACHE` into a `pub_cache_outline.dill` inside the dartpad worker.
Then recompilation would only touch the user-code. Granted this is
still less than ideal.
At the moment the lack of incremental compilation is not a blocker,
current dartpad doesn't have incremental compilation, but granted it also
only supported a single file. This dartpad environment will support
multiple files, but adding many files will result in poor performance
until we refactor and figure out how to do incremental compilation.
Hence, why we shouldn't block progress waiting for incremental compilation
to work.
R=nshahan@google.com
[dartpad] Part 3: Pub execution wrapper
Adds the internal wrapper for executing pub commands inside the
worker's virtual file system.
R=sigurdm@google.com
[dartpad] Part 4: Language Server wrapper
Adds the internal wrapper for spawning and communicating with the
Dart LSP inside the DartPad worker.
R=scheglov@google.com
[dartpad] Part 5: DartPad Worker
Implements the main worker entrypoint, RPC message handling. The
`bin/worker.dart` is what will be compiled to WASM and will run as a
_Web Worker_.
R=sigurdm@google.com
[dartpad] Part 6: DartPad Client library
The public-facing `dartpad` library that developers use to embed
the compilation environment in their web applications.
This finishes `package:dartpad`featuring:
* `DartPad.create()`, which creates a _Web Worker_ running the compiled
`pkg/dartpad_worker/bin/worker.dart`, and returns a `DartPad` instance
wrapping RPC calls into the _Web Worker_.
* `Sandbox.createIFrame()` which creates a _sandboxed iframe_ containing
precompiled DDC modules from the SDK assets and `sandbox.js` for
wrapping `ddc_module_loader.js` with an RPC interface. This returns a
`Sandbox` instance wrapping RPC calls into the _sandboxed iframe_,
making it easy to load modules, run library entrypoints (main()),
initiate hot-reload, or launch a flutter app.
This is an initial API design that works. There are minor inconsistencies
and improvements that we should do. But I propose that we do so in
follow-up PRs.
R=sigurdm@google.com
[dartpad] Part 7: Build Targets
Adds a `dartpad` target to the Dart SDK, which produces:
```
out/ReleaseX64/dartpad/
├── dart
│ ├── dart_sdk.js.map
│ ├── sdk.js
│ └── sdk.tar
├── ddc_module_loader.js
├── sandbox.js
├── worker.loader.js
├── worker.mjs
├── worker.support.js
├── worker.wasm
└── worker.wasm.map
```
The `dartpad/` output folder is intended to be used as `assetBaseUrl` in
client libraries provided in `package:dartpad`. And the `dartpad/dart/`
folder is intended to be used as `sdkLocation`. These files are not
intended to be distributed along side the normal Dart SDK, instead we'll
publish them on a CDN and let people use `package:dartpad` to point at
these files. If they want to self-host everything, they can copy from our
CDN or rebuild the files using a local Dart SDK checkout.
These files are also necessary for testing, to ensure that we have proper
integration tests for `pkg/dartpad_worker/`. As we will want to publish
these files on a CDN the actual compiled size in release-mode matters,
thus, we have enabled wasm optimizations steps in this mode.
R=rmacnak@google.com
[dartpad] Part 8: Tests for `package:dartpad` and `pkg/dartpad_worker/`.
We have 3 kinds of tests:
* Unit tests (vm, browser or both),
* Worker tests (vm and browser),
* Integration tests (browser-only).
As the Dart SDK test runner simply executes `*_test.dart` files with
`dart` and doesn't support compiling tests to the browser, launching and
running tests in the browser, we employ `dart test` to run tests. We do
this by having `pkg/dartpad_worker/dart_test.yaml` specify that
`dart test` should look for files names `test_*.dart`, and we then create
`pkg/dartpad_worker/test/ci_test.dart´ to be a single test that simply
runs `dart test` as a subprocess. Thus, the only test file triggered by
Dart SDK test runner is `ci_test.dart`, and if you locally run `dart test`
this will run the individual `test_*.dart` files.
While this hack to use `dart test` isn't ideal it gives everyone a decent
work flow. And saves us from having to invent complex test harness for
`pkg/dartpad_worker/`.
**Worker tests** defined in `test/dart/worker/` and `test/flutter/worker/`
are all imported into `test_dart_worker.dart` and
`test_flutter_worker.dart` to reduce test compilation time. These have a
non-trivial compilation time because the worker is running in the test
process. This allows testing on VM, which can be easier to debug.
**Integration tests** unlike _worker tests_ launch the worker compiled by
the `dartpad` build target in a _Web Worker_. This has less compilation
overhead for the individual tests, but involves more setup, and can be
a bit harder to debug.
Both worker and integration tests employ `test/asset_server/` which serves
assets built by the `dartpad` build target, and serves as a pub-server for
tests. It also serves assets built by `tool/setup_local_flutter.dart`
which creates flutter assets, though this is not intended to run as part
of CI, since we do not have a Flutter checkout available. Thus, for the
time being flutter testing is local only. Maybe, later we'll move this
script to another repository.
R=sigurdm@google.com
Cq-Include-Trybots: luci.dart.try:pkg-linux-release-try,pkg-win-release-try
Change-Id: I3a99939ec5217b9f3a855fc5b9ad9699047d02cc
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/507321
Commit-Queue: Jonas Jensen <jonasfj@google.com>
Reviewed-by: Alexander Thomas <athom@google.com>
If the version is unchanged, don't write to the file. Ninja will notice the modification time is unchanged and avoid rebuilding the target's dependents.
In particular, this means merely adding or amending a commit will no longer make the VM and all SDK snapshots dirty.
Change-Id: I25617c6c584d1d1094a339fe14716d18b28c688f
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/506101
Reviewed-by: Alexander Aprelev <aam@google.com>
Commit-Queue: Ryan Macnak <rmacnak@google.com>
This reverts commit 2a93a26d91, which a change to use `defined` so that the GN script does not crash in
Flutter.
Fixesflutter/devtools#9786
See go/moving-devtools-to-dart-sdk-2025.
In this change, we make the source of devtools be configurable, in
actions like build_sdk.
If `build_devtools_from_sources` is true, we build local devtools,
and if false, we continue to use the prebuilt sources.
This may be an intermediate step, while we test out building devtools
from source. Or it may be permanently be a choice, if we keep building
with CIPD.
Change-Id: I9d509b07971942f2c4884224a6efda9001522bd4
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/502120
Reviewed-by: Alexander Thomas <athom@google.com>
Commit-Queue: Samuel Rawlins <srawlins@google.com>
dpkg-buildpackage now wants a timestamp in the changelog.
Also remove --git-revision-file and --git-timestamp-file, which were part of the previous way to build Debian packages.
Change-Id: Idbcf418571889ad7588424e6d8ae6e81ec79942b
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/504280
Reviewed-by: Ivan Inozemtsev <iinozemtsev@google.com>
Commit-Queue: Ryan Macnak <rmacnak@google.com>
The former contents of the VM isolate are now included into each isolate group. This makes each isolate group's heap independent, and in particular allows each heap to be allocated to a separate pointer cage (not done in this CL).
The duplicated stubs that allowed PC relative calls are removed, since the originals can now be the target of PC relative calls.
The bootstrapping needing to load an AppJIT or AppAOT snapshot is reduced to allocating the oddballs. The code is entirely dropped in the AOT runtime, but the JIT runtime still has it to allow for flags to affect the compilation of the stub code. Further refactoring might be able to remove this for the JIT runtime too, with only gen_snapshot knowing how to bootstrap.
Class serialization no longer distinguishes predefined classes.
The page containing null is marked as never-evacuate. null, false and true must not move because the compiler relies on their low bits having certain patterns for some optimizations. (Previously, the entire VM isolate heap never moved.)
Compaction is disabled for IA32. Due to register pressure, some stub calls must not use a scratch register and embed the address of Code.
The page containing the call-through-safepoint stub is frozen when running with --write-protect-code and the stub is created at runtime (instead of loaded from an AppJIT or AppAOT snapshot). This stub must remain executable even during a safepoint, as a foreign call might during return during a safepoint and only block after the stub directs it to the runtime.
The snapshot symbols are renamed to kDartSnapshotData and kDartSnapshotText. There is no need to distinguish the VM isolate's snapshot, and snaphots are per isolate group not per isolate. Aliases with the old names are added to ease migration.
Some global flags that were automatically set based on the VM isolate's snapshot are now isolate group flags and automatically set by the isolate group's snapshot.
TEST=ci
Change-Id: Iee82016057d609112e9b021d178fc3d4d18b5044
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/500621
Reviewed-by: Alexander Markov <alexmarkov@google.com>
Reviewed-by: Tess Strickland <sstrickl@google.com>
SLSA-Policy-Verified: SLSA Policy Verification Service <devtools-gerritcodereview-exitgate@google.com>
Commit-Queue: Ryan Macnak <rmacnak@google.com>
This CL enables the primary constructors feature by default in Dart 3.13.
The primary constructors feature is a brevity feature. There are no new semantics, but it allows us to express declarations in a less verbose way.
This feature allows one constructor and a set of instance variables to be specified in the header of a declaration.
Currently a declaration with a constructor and some fields is written as:
```dart
// Current syntax.
class Point {
int x;
int y;
Point(this.x, this.y);
}
```
With a primary constructor, we would write the above as:
```
class Point(var int x, var int y);
```
If a primary constructor needs an initializer list or a body, they can be
specified inside the class using the `this` body syntax:
```dart
class Point(var int x, var int y) {
this : assert(x >= 0) {
print('Point created at $x, $y');
}
}
```
As part of this feature, you can also use the `new` and `factory` keywords to
declare constructors in the class body without repeating the class name:
```dart
class Point {
int x, y;
// Equivalent to Point(this.x, this.y)
new(this.x, this.y);
// Equivalent to Point.origin()
new origin() : x = 0, y = 0;
// Equivalent to factory Point.clone(Point other)
factory clone(Point other) => Point(other.x, other.y);
}
```
To learn more about the feature, check out the feature specification located here: https://github.com/dart-lang/language/blob/main/accepted/future-releases/primary-constructors/feature-specification.md
Tested: Has existing language, CFE, analyzer, analysis server tests.
Bug: https://github.com/dart-lang/sdk/issues/61524
Change-Id: I296f2fcd918b87bf2a1dd00256340759866c2423
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/489241
Reviewed-by: Bob Nystrom <rnystrom@google.com>
Reviewed-by: Brian Wilkerson <brianwilkerson@google.com>
Reviewed-by: Nicholas Shahan <nshahan@google.com>
Reviewed-by: Michael Thomsen <mit@google.com>
Reviewed-by: Johnni Winther <johnniwinther@google.com>
Commit-Queue: Kallen Tu <kallentu@google.com>
Reviewed-by: Ben Konyi <bkonyi@google.com>
- Added the required `wasm-opt` target for dart2wasm compiler
- Using diamonds instead of stars to be less obnoxious
- Made emoji lines the same length
- A bit louder about success cases
Change-Id: Iaa9e964d3384f5282721f1e3e5bfca2d2f84e5a9
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/499340
Reviewed-by: Nate Bosch <nbosch@google.com>
Auto-Submit: Kevin Moore <kevmoo@google.com>
Commit-Queue: Kevin Moore <kevmoo@google.com>
This change introduces `tools/build_test_fast.py`, a developer-focused
wrapper script that automatically infers and builds the *minimal* required
GN targets needed to run a specific test, avoiding the expensive overhead
of building the full SDK (e.g. `create_sdk`) during local iteration.
Features:
- Infers the correct `test.py` compiler (`-c`) from the provided test
paths (e.g., `tests/web/wasm` -> `dart2wasm`).
- Infers the correct minimal GN targets for the selected compiler.
For example, DDC tests now only build `ddc_stable_test_local` and
`create_common_sdk`, taking ~4 seconds instead of 5 minutes.
- Automatically defaults web compilers (`dart2js`, `dart2wasm`, `ddc`)
to use the `d8` runtime to prevent noisy browser popups during testing,
unless explicitly overridden.
- Synchronizes the default `mode` between `build.py` (debug) and
`test.py` (release) to prevent silent snapshot resolution failures.
- Prints clear, copy-pasteable execution commands for both the build
and test phases.
- Displays a prominent warning if a test fails, reminding the developer
to verify the minimal target mapping against `tools/bots/test_matrix.json`.
- Adds documentation for the script to `docs/Testing.md`.
Change-Id: I75473cdacca13f78dd271d9a31d5dcb15df57f6f
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/499043
Commit-Queue: Kevin Moore <kevmoo@google.com>
Reviewed-by: Jake Macdonald <jakemac@google.com>
Fixes https://github.com/flutter/devtools/issues/9786
See go/moving-devtools-to-dart-sdk-2025.
In this change, we make the source of devtools be configurable, in
actions like build_sdk.
If `build_devtools_from_sources` is true, we build local devtools,
and if false, we continue to use the prebuilt sources.
This may be an intermediate step, while we test out building devtools
from source. Or it may be permanently be a choice, if we keep building
with CIPD.
Change-Id: I7b46d6359c69b34f316e59dccc475e211a18f965
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/498640
Reviewed-by: Alexander Aprelev <aam@google.com>
Commit-Queue: Samuel Rawlins <srawlins@google.com>
This CL migrates the `dwds` and `dwds_test_common` packages into the Dart SDK
repository.
Key Changes:
- Monorepo Compliance: Updated the pubspecs to align with the SDK pub workspace setup.
- Excluded `pkg/dwds_test_common/fixtures/` from `package_deps.dart`.
- Updated pkg to status to skip `dwds/test/integration/*` & `dwds_test_common/fixtures/*` until DWDS migration is complete.
- Remove package `build_daemon` from DWDS' `pubspec.yaml` as it's not approved for SDK env.
- Added `@skip_package_deps_validation` to the following files to ignore import checks for package:build_daemon: `server.dart`, `utilities.dart`, `context.dart`.
- Created `pkg/dwds/lib/src/utilities/test_path_utils.dart` to fix path resolution failures in tests (ie. `build_script_test.dart` and `ensure_version_test.dart`).
Testing:
- All tests passing locally.
- CI try bots are green.
Design Doc: http://goto.google.com/migrating-webdev and http://goto.google.com/migrating-dwds
Fixes https://github.com/dart-lang/sdk/issues/62100
Fixes https://github.com/dart-lang/sdk/issues/62101
Fixes https://github.com/dart-lang/sdk/issues/62102
Fixes https://github.com/dart-lang/sdk/issues/62103
Cq-Include-Trybots: luci.dart.try:pkg-win-release-try,pkg-win-release-arm64-try,pkg-mac-release-try,pkg-mac-release-arm64-try,pkg-linux-release-try,pkg-linux-release-arm64-try,pkg-linux-debug-try
Change-Id: I6130be8b7e0b42fbbf81b26a4950a2c4282e3a48
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/494660
Commit-Queue: Jessy Yameogo <yjessy@google.com>
Reviewed-by: Nate Biggs <natebiggs@google.com>
Reviewed-by: Ben Konyi <bkonyi@google.com>
I could not get the Dart script to successfully run from GN, and I
also could not find examples of the same. Everything is Python, so
Python it is.
Change-Id: I767aaf42d1584ae331d028ef073a534e5d36b77b
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/496402
Reviewed-by: Ivan Inozemtsev <iinozemtsev@google.com>
Commit-Queue: Samuel Rawlins <srawlins@google.com>
The `Match.operator[]` does the same thing and is
generally recommended (and shorter).
(I want to deprecate `group` and `groups`)
Tested: Refactoring.
CoreLibraryReviewExempt: Calling equivalent function.
Change-Id: I4c758968ae622fe16b7322be1b29b05b91e7fcd9
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/489021
Reviewed-by: Paul Berry <paulberry@google.com>
Reviewed-by: Brian Wilkerson <brianwilkerson@google.com>
Reviewed-by: Martin Kustermann <kustermann@google.com>
Commit-Queue: Lasse Nielsen <lrn@google.com>
When compiled to WebAssembly, the Dart SDK needs access to external
host functions to implement regular expressions, stack traces, timers
and more. Currently, `dart2wasm` relies on `js_interop` definition to
implement these functions in JavaScript.
As discussed in https://github.com/dart-lang/sdk/issues/53884, an
alternative is to use `wasm:import` annotations to let an arbitrary
embedder that doesn't necessarily run in a JavaScript context inject
implementations for these host functions.
This would allow running `dart2wasm` apps by e.g.
- using a runtime like wasmtime and defining host functions in Rust.
- defining a wrapper module implementing required functions by
delegating to WASI definitions, and then using say `wasm-merge` to
run the app in any WASI-compatible runtime.
This prepares the `--standalone` flag on `dart2wasm` to do just that.
When enabled, the compiler uses a different SDK platform to use imports
instead of JS interop. For now, these platforms are almost identical:
I've ported the timer logic to use wasm imports as a demo, but the rest
is still based on existing patch files. We can revisit in subsequent
CLs to incrementally reduce `js_interop` dependencies before removing
that library from the `dart2wasm_standalone` target entirely.
Change-Id: I3f406afbf2dab65506094de5c3f4067f4db66f3e
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/486380
Reviewed-by: Slava Egorov <vegorov@google.com>
Reviewed-by: Martin Kustermann <kustermann@google.com>
Commit-Queue: Martin Kustermann <kustermann@google.com>
third_party/pkg/protobuf (and possibly others) uses a compressed reference file (.git/packed-refs) rather than the .git/refs/heads/(master|main) file expected by rev_sdk_deps.dart.
modify rev_sdk_deps.dart to use "git show-ref --verify refs/remotes/origin/main" instead.
TEST: I ran the script and it produced the expected commit hashes for both a package that uses a "main" default branch and a "master" default branch.
Change-Id: Ibe2ec2cf368d859ca78d1873f287722eb73e481b
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/488262
Reviewed-by: Ryan Macnak <rmacnak@google.com>
Commit-Queue: Brian Quinlan <bquinlan@google.com>
This change includes an initial implementation of the new VM service
implementation based on `package:dart_runtime_service`, along with the
necessary plumbing to start it in place of the legacy VM service
implementation.
The entrypoint for the new VM service implementation is located in
dart_runtime_service_vm/bin/vm_service_entrypoint.dart, which is
compiled into AppJIT and AOT snapshots when the
`--include-experimental-vm-service` flag is provided to `build.py`. To run
the VM with the new VM service implementation, the
`--experimental-vm-service` flag must be provided.
Currently, the experimental VM service implementation supports:
- User specified ports
- Authentication code flags
- Enabling the HTTP server via SIGQUIT
- Some service protocol RPCs that don't require an isolate ID (e.g.,
`getVM`)
See go/dart-runtime-services-unification for more details.
TEST=Manual
CoreLibraryReviewExempt: dart:_vmservice is private
Change-Id: I4a58cd1fa0a386313baa3d5c5345720231279123
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/484820
Reviewed-by: Nicholas Shahan <nshahan@google.com>
Reviewed-by: Alexander Thomas <athom@google.com>
Reviewed-by: Ryan Macnak <rmacnak@google.com>
Commit-Queue: Ben Konyi <bkonyi@google.com>
This CL adds a grammar rule to Dart.g and Dart.g4 such that a
variable declaration can be `static abstract`. It also changes the rule
for `static final` and `static const` variable declarations such that
it is a syntax error if they do not have an initializing expression.
This restores the approach which was used before augmentations were
added. It is possible to do this because the added flexibility
would only allow cases which are intended to be an error, because it
is no longer possible to "override" or "extend" an implementation
in an augmenting declaration.
Change-Id: Id7696880a948697678eaf368c69c390b9c8f4e7f
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/485440
Commit-Queue: Erik Ernst <eernst@google.com>
Reviewed-by: Chloe Stefantsova <cstefantsova@google.com>
This CL aligns Dart.g/Dart.g4 with the feature specs for
augmentations and primary constructors such that they allow for all
kinds of membered declaration bodies to be expressed as `;`.
Change-Id: I61fe4b4fe6541fd0962f8ca39590611827c1d127
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/483340
Reviewed-by: Chloe Stefantsova <cstefantsova@google.com>
Commit-Queue: Erik Ernst <eernst@google.com>
This sets the `experimentReleaseVersion` of `primary-constructors` to `3.12.0`. This enables tests, outside the SDK in particular, to reliably pin tests that prepare for the feature to language version 3.12.
Change-Id: I85565b9434c052398be5a7839ce5731d71162416
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/482784
Reviewed-by: Konstantin Shcheglov <scheglov@google.com>
Commit-Queue: Johnni Winther <johnniwinther@google.com>
Reviewed-by: Kallen Tu <kallentu@google.com>
Reviewed-by: Sarah Zakarias <zarah@google.com>
Ryan Macnak